Catch violations in times(...) expectations #2766
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a new quit() method to universe that causes all spawned actors to quit. It also collects actors exit codes that can be used to verify that all actors quit gracefully, which in turn allows to fail the test if mock objects expectations are not met. Relates to quickwit-oss#2754
|
I opened #2768 to fix the license header checker. |
fulmicoton
reviewed
Feb 8, 2023
fulmicoton
reviewed
Feb 8, 2023
fulmicoton
reviewed
Feb 8, 2023
fulmicoton
reviewed
Feb 8, 2023
Improve panic handling Co-authored-by: Paul Masurel <paul@quickwit.io>
Also adds a check to the universe.drop() to make sure that universe.assert_quit() was indeed called at the end of the test.
Seems like rust-lang/cargo#8379 is the issue here.
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
reviewed
Feb 15, 2023
fulmicoton
requested changes
Feb 15, 2023
Co-authored-by: Paul Masurel <paul@quickwit.io>
|
One thing that still annoys me with the current implementation is that when a test that uses universe fails for some unrelated reason the logic in universe drop is still triggered. So, you see two panics - first is your test's panic, which is useful, but then you also get the universe's drop panic with irrelevant message "Did you call universe.assert_quit()?". I just implemented it and it confuses me sometimes, I imagine it might also confuse others. Not really sure how to detect it though. |
|
@imotov Yes, if it is too confusing we may have to remove this Drop impl. Anyway, LGTM |
fulmicoton
approved these changes
Feb 15, 2023
fulmicoton
approved these changes
Feb 15, 2023
imotov
added a commit
to imotov/quickwit
that referenced
this pull request
Feb 16, 2023
If test fails with panic and universe drop is called it kills the test runner instead of displaying the first panic. Relates to quickwit-oss#2766
fulmicoton
pushed a commit
that referenced
this pull request
Feb 16, 2023
* Universe shouldn't panic if thread is panicking If test fails with panic and universe drop is called it kills the test runner instead of displaying the first panic. Relates to #2766 * Fix format
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a new quit() method to universe that causes all spawned actors to quit. It also collects actors exit codes that can be used to verify that all actors quit gracefully, which in turn allows to fail the test if mock objects expectations are not met.
Relates to #2754
How was this PR tested?
Added several tests to universe.rs. I also had to modify several
times(...)parameters to make tests pass. Somebody who is more familiar with these tests should probably check if these changes make sense or if I inadvertently found and buried actual bugs.