Fix random failures with parallel CLI tests #831
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
The addition of capsys.disabled calls to the CLI tests was done to work around an issue with files being opened and closed in the CLI setup fixtures. When running under high parallelism the CLI tests would occasionally be split up across worker instances, and so the file system operations - which were effectively creating and removing the same file - would cause failures due to race conditions between the different test runner fixture executions. Adding capsys.disabled didn't solve this problem, it merely made it somewhat less likely to occur, and ultimately replaced the error with a different one (file not found). This commit removes the capsys.disabled calls in favor of a follow-up to stop doing all of this filesystem manipulation.
We use pytest-xdist for parallel test execution, which means multiple worker threads fire up tests which are loosely grouped by class/module. Unfortunately, there are no guarantees around any of this, so the CLI tests, which are all in one module, may execute on different workers. We had a bad fixture layout on the CLI tests which would touch a file in the CLI directory and then unlink it after the test was done. If every test ran on the same worker this was fine - only one test would ever be accessing this path location at a time. But when the tests get split across workers bad things happen with FileNotFound and things of that nature. This change moves us off of having multiple tests creating and destroying the same file in the same file path location, instead relying on a fixed file that already exists in a static location in the repository.
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
WilliamDee
approved these changes
Oct 31, 2023
courtneyholcomb
approved these changes
Oct 31, 2023
plypaul
approved these changes
Oct 31, 2023
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.
We use pytest-xdist for parallel test execution, which means multiple
worker threads fire up tests which are loosely grouped by class/module.
Unfortunately, there are no guarantees around any of this, so the CLI
tests, which are all in one module, may execute on different workers.
We had a bad fixture layout on the CLI tests which would touch a file
in the CLI directory and then unlink it after the test was done. If
every test ran on the same worker this was fine - only one test would
ever be accessing this path location at a time. But when the tests get
split across workers bad things happen with FileNotFound and things
of that nature.
This change moves us off of having multiple tests creating and
destroying the same file in the same file path location, instead
relying on a fixed file that already exists in a static location in
the repository.