Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
TST: force test with shared test image to run in serial#24081
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The tests test_hexbin_empty and test_hexbin_log_empty use the same targetimage, however if they the tests ended up being run in different workers whenrunning the tests in parallel they will step on each other if one of the teststarts writing the output file while the other is trying to read the testimage.By putting them in the same test they will save and evaluate in order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think either way is good enough here. Please add a comment that they have to be together, so that nobody comes along and splits the test later. - Thinking about it, given the need for a note to prevent future breakage and related complexity/mental load, maybe copying the image is the easier solution.
Theoretically, the best solution would be to solve this on the test infrastructure level. Conceptually, there should not be a problem with multiple tests using the same reference image - the reference image should be read-only. But maybe fixing this is more effort than it's worth. It's not that common that multiple tests generate the same image.
#16735 is some prior art on this (where we discussed adding file locking). I remember we had a PR that was adding some randomness to the initial file written, but I am not able to find it. However I think even if we fixed that we would still have some issues if multiple failed because of the way we name the expected / diff files. Maybe we need some logic to audit the names and make sure we do not have the reused files between tests (I think we know enough as part of the collection phase to determine that). |
meeseeksdev backport to v3.6.x |
…081-on-v3.6.xBackport PR#24081 on branch v3.6.x (TST: force test with shared test image to run in serial)
PR Summary
The tests test_hexbin_empty and test_hexbin_log_empty use the same target image, however if they the tests ended up being run in different workers when running the tests in parallel they will step on each other if one of the test starts writing the output file while the other is trying to read the test image.
By putting them in the same test they will save and evaluate in order.
I verified I could break both tests.
The other option here is to make a copy of the file (which should be free at the git level due to content based addressing). I am happy to switch to that if the consensus that is a better approach happy to switch (I was very much on the fence).
This should fix the current rash of transient failures. We are running
-n 2on CI so we need to get unlucky that the tests go to different workers and that they run close enough in time to matter which is why we only see in 1-2 jobs per CI run and re-running seems to reliably fix it.PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).