Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pool: Break on failed pool tests. #388

Closed

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Sep 20, 2023

Any calls to t.Fatal inside of a t.Run will only fatal that specific instance of run. This adds logic to stop running the remaining pool tests when there is a failed test.

Any calls to t.Fatal inside of a t.Run will only fatal that specific
instance of run.  This adds logic to stop running the remaining pool
tests when there is a failed test.
@jholdstock
Copy link
Member

There is already a built-in flag -failfast which you can pass to go test for this behaviour. Hard-coding it and removing the togglability seems a bit heavy handed. Personally I like to know how many tests are broken so I can review them all in one pass, rather than having to repeatedly "fix one test, run tests, fix one test, run tests, etc"

-failfast
    Do not start new tests after the first test failure.

from https://pkg.go.dev/cmd/go/internal/test

@davecgh
Copy link
Member Author

davecgh commented Sep 21, 2023

I'm aware of the flag, but I'm not sure how else we can avoid purging the database on a test failure (which #389 does) without adding these.

In general, I like to see all failing tests too, but the way these tests are written is not at all conducive to that. It cost me quite a lot of time not being able to see the data left in the database.

I'm open to another method, but after having just suffered through it, I really don't think we want the database purged on test failures. It makes it exceedingly difficult to analyze test failures when you nuke the data.

@davecgh
Copy link
Member Author

davecgh commented Sep 21, 2023

Alright, I thought of a way to allow -failfast to work and still allow #389 to retain the data but not otherwise. Closing and will update #389 accordingly.

@davecgh davecgh closed this Sep 21, 2023
@davecgh davecgh deleted the pooltest_break_on_failed_tests branch September 21, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants