-
Notifications
You must be signed in to change notification settings - Fork 10
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix test that relied on slow garbage collection #903
Conversation
Good catch @braingram! Thank you! LGTM. |
Thanks for giving the PR a look! Let me know if there are any changes you'd like me to make including:
For the last one some liberal use of |
Thank you for the explanation @braingram We will need some time to update everything for the 3.0 API changes |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #903 +/- ##
=======================================
Coverage 96.45% 96.45%
=======================================
Files 95 95
Lines 6287 6287
=======================================
Hits 6064 6064
Misses 223 223 ☔ View full report in Codecov by Sentry. |
Changes
This PR makes a small change to
TestWeldXFile.test_show_header_memory_usage
to keep the openedWeldxFile
in memory while the assert is computed.Without this PR the test includes:
weldx/weldx/tests/asdf_tests/test_weldx_file.py
Line 452 in bb213a7
Breaking apart the above line the following steps occur:
WeldxFile
object is instantiated usingfn
as the inputx
value is extracted from theWeldxFile
x
are compared againstlarge_array
If garbage collection is triggered between 2 and 3 above (after
x
is extracted but before the comparison withlarge_array
) there is no reference keepingWeldxFile
in memory (and keeping the underlying asdf file open). The garbage collector is therefore free to close the asdf file and cause the comparison to fail (because the contents ofx
have not yet been read since this is a lazy-loaded array).This PR updates the test to use a with context to keep the
WeldxFile
open.An example of where this test fails can be seen here: https://github.com/asdf-format/asdf/actions/runs/7390148220/job/20104458462?pr=1721 (Note that this is a CI run for changes to asdf tree traversal algorithms. I cannot say definitively but I have the suspicion that the current asdf tree algorithms produce difficult-to-collect structures. This might be resulting in objects surviving a few generations of collection. This would explain why the issue fixed in this PR is not yet causing test failures).
Checks
CHANGELOG.md
tests/
doc/