Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-138013: Move I/O test infrastructre to test_io.utils#138475
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
This moves test support code to `test_io._support` and buffered testcases to `test_io.test_bufferedio` via copy/paste code movement thenadjusts imports as needed (remove unneded + add all required).
cmaloney commentedSep 3, 2025
Windows bot failure looks real and a weird intermittent issue I've been running into locally some. Investigating: test_uninitialized (test.test_io.test_bufferedio.CBufferedRWPairTest.test_uninitialized) ...Warning--UnraisableexceptionExceptionignoredwhilefinalizingfile<_io.BufferedRWPairobjectat0x000001AC6C4654F0>:Traceback (mostrecentcalllast):File"D:\a\cpython\cpython\Lib\re\_parser.py",line176,inappenddefappend(self,code):ValueError:flushofclosedfileWarning--UnraisableexceptionExceptionignoredwhilefinalizingfile<_io.BufferedWriter>:Traceback (mostrecentcalllast):File"D:\a\cpython\cpython\Lib\re\_parser.py",line176,inappenddefappend(self,code):ValueError:flushofclosedfile |
cmaloney commentedSep 9, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I ran all the tl; dr: I think this PR splitting the tests is sound. It might make an existing flaky / race case more visible |
cmaloney commentedSep 9, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ubuntu Github Action got a better backtrace: Exceptionignoredwhilefinalizingfile<_io.BufferedWriter>:Traceback (mostrecentcalllast):File"/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_io/_support.py",line41,inwritabledefwritable(self):ValueError:flushofclosedfileWarning--UnraisableexceptionExceptionignoredwhilefinalizingfile<_io.BufferedRWPairobjectat0x200017dfb50>:Traceback (mostrecentcalllast):File"/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_io/_support.py",line41,inwritabledefwritable(self):ValueError:flushofclosedfile |
cmaloney commentedSep 9, 2025
Found a reproducer and filed an issue for the BufferedRWPair GC:gh-138720 |
| @@ -0,0 +1,317 @@ | |||
| import array | |||
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.
What do you think oftest_io.utils name instead? test_asyncio, test_ast and test_interpreters packages use this name.
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.
Works for me; This PR needsGH-138724 (or another fix forgh-138720) as that modifies a buffered test which is moved int his PR.
I see sort of three options
- close this PR and make the "split" of
utilsa PR of its own (no test moving) with these two requested changes; then make the test moving a distinct PR - close this PR and re-make it with the new name (
utils) doing the pure copy-paste after a fix forGC of unclosed io.BuffererdRWPair after.readintoresults in unraisable exception #138720 is landed - hand-tweak inside of the copy part of this PR; I had been avoiding that to try and keep this pure-"copy paste" + "add imports"
Happy to do any of the above, my leaning is 2, but 1 would make it so more of the work of splittingtest_general can be landed sooner potentially
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.
thought of a 4:
- Remove the test_buffered moving from this, make it just the moving to
test_io.utils
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.
Decided to implement 4
Uh oh!
There was an error while loading.Please reload this page.
The buffered tests are being modified, get into a parallel lane
1011724 intopython:mainUh oh!
There was an error while loading.Please reload this page.
vstinner commentedSep 11, 2025
Merged, thanks. |
Uh oh!
There was an error while loading.Please reload this page.
Moves the I/O test support code to
test.test_io.utilsleaving all test cases as they are. The code was moved via copy/paste then adjusted imports as needed (remove unneded + add all required).I plan to do a code update + two more direct code movement to split tests from
test_generalto increase parallelism and logical layout. Times are runtime on my 64 bit linux debug build output by slow tests when running./python -mtest test_io -uall,walltime,largefile,extralargefile -M16G -o -j0test_bufferedioload_testsw/ test list fromtest_generalgh-138013: Remove load_tests in test_io.test_general #138771 (see:gh-127647: Fix and enable I/O protocol tests #138369)SignalsTesttotest_signals(21.4s)TextIOWrapperTestandIncrementalDecodertests totest_textio(1.1s)The longest remaining test in
test_generalafter that for me istest_daemon_threads_shutdown_*_deadlockwhich takes ~7s (and is marked withrequires_resource('walltime')). Runtime oftest_ioreduces from ~34.1s -> 21.4s (SignalsTest is now longest) via increased parallelism.