Address PyPy flakiness in test_for_leaking_fds #2888
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
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2888 +/- ##
=======================================
Coverage 99.54% 99.54%
=======================================
Files 115 115
Lines 17660 17662 +2
Branches 3165 3165
=======================================
+ Hits 17580 17582 +2
Misses 52 52
Partials 28 28
|
jakkdl
approved these changes
Nov 27, 2023
There was a problem hiding this comment.
Definitely seems like something that should get fixed somewhere upstream in one way or another, but definitely doesn't hurt to fix in CI for now.
To track the issue one could run the test once before doing gc.collect(), and then rerun it after a collect() on failure - although I'm not sure how to collect data from that.
|
I think the problem is that even without that |
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.
Refs #2885
I've run the PyPy test suite a lot and this test in particular has failed.
See https://github.com/A5rocks/trio/actions/runs/6953123771/job/18951033826#step:5:5284 for an example of this happening.
I'm guessing that this is because PyPy GC is weird and isn't guaranteed to clean up previous test objects which might contain something with a file descriptor. I added this here and I didn't get this test to fail again in a bunch of runs, so I think this fixes that.
Now I'm kinda thinking of if it's possible to upstream this in pytest, because this kind of thing is insidious. https://foss.heptapod.net/pypy/pypy/-/commit/5a6a733370542b26e7c4ee756c4fb566523e53d8 introduces a new PyPy GC API that can address this more generally, I believe.