Fix order-dependent flaky tests related to UTF-8 support - #1093
Merged
csmarchbanks merged 2 commits intoMar 3, 2025
Merged
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Signed-off-by: Dennis Gaebler <dennis.gaebler@uni-ulm.de>
csmarchbanks
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
Thanks!
The downside to this approach is that if one of the assertions raise then the next test will also fail since disable_legacy_validation() will no longer be called. A couple options would be to:
- Wrap the validation enablement/disabling with a With Statement Context Manager
- Create a test fixture to be used when we need to use legacy validation.
Sorry, something went wrong.
Signed-off-by: Dennis Gaebler <dennis.gaebler@uni-ulm.de>
Copy link
Copy Markdown
Contributor
Author
|
Thanks for your feedback, I didn't think of that. I added a context manager that enables and disables legacy validation and changed all uses of these functions to with statements |
Sorry, something went wrong.
csmarchbanks
approved these changes
Mar 3, 2025
csmarchbanks
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
Looks great, thanks!
Sorry, something went wrong.
This file contains hidden or 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
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.
For a university course, I tested this project for flaky tests, which are tests that can both pass or fail without changes to their code. I noticed that the tests changed in #1070 fail when their execution order is changed because the legacy validation is enabled in some tests but disabled in the test that follows. Since the execution order is not ensured, it's safer to disable them in the same test.
With these changes applied, the whole test suit passes every time even when executed in random order.