gh-138013: Move I/O test infrastructre to test_io.utils - #138475
Conversation
Sorry, something went wrong.
This moves test support code to `test_io._support` and buffered test cases to `test_io.test_bufferedio` via copy/paste code movement then adjusts imports as needed (remove unneded + add all required).
|
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 -- Unraisable exception
Exception ignored while finalizing file <_io.BufferedRWPair object at 0x000001AC6C4654F0>:
Traceback (most recent call last):
File "D:\a\cpython\cpython\Lib\re\_parser.py", line 176, in append
def append(self, code):
ValueError: flush of closed file
Warning -- Unraisable exception
Exception ignored while finalizing file <_io.BufferedWriter>:
Traceback (most recent call last):
File "D:\a\cpython\cpython\Lib\re\_parser.py", line 176, in append
def append(self, code):
ValueError: flush of closed file |
Sorry, something went wrong.
|
I ran all the test_io tests in a loop for two days without reproducing... Reading through the BufferedRWPair code I think there are some tear-down race conditions but this PR doesn't change them, just effects test timing (which may make them a little more likely). That the stack trace points to a distinct module is really weird for me / I don't understand (likely a distinct bug). I plan to make a PR for some of the things I found in manual review in BufferedRWPair, but it is cases I've been able to figure out a good test case for; just hunches. tl; dr: I think this PR splitting the tests is sound. It might make an existing flaky / race case more visible |
Sorry, something went wrong.
|
Ubuntu Github Action got a better backtrace: Exception ignored while finalizing file <_io.BufferedWriter>:
Traceback (most recent call last):
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_io/_support.py", line 41, in writable
def writable(self):
ValueError: flush of closed file
Warning -- Unraisable exception
Exception ignored while finalizing file <_io.BufferedRWPair object at 0x200017dfb50>:
Traceback (most recent call last):
File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_io/_support.py", line 41, in writable
def writable(self):
ValueError: flush of closed file |
Sorry, something went wrong.
|
Found a reproducer and filed an issue for the BufferedRWPair GC: gh-138720 |
Sorry, something went wrong.
| @@ -0,0 +1,317 @@ | |||
| import array | |||
There was a problem hiding this comment.
What do you think of test_io.utils name instead? test_asyncio, test_ast and test_interpreters packages use this name.
Sorry, something went wrong.
There was a problem hiding this comment.
Works for me; This PR needs GH-138724 (or another fix for gh-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 utils a 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 for GC of unclosed io.BuffererdRWPair after .readinto results 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 splitting test_general can be landed sooner potentially
Sorry, something went wrong.
There was a problem hiding this comment.
thought of a 4:
- Remove the test_buffered moving from this, make it just the moving to test_io.utils
Sorry, something went wrong.
There was a problem hiding this comment.
Decided to implement 4
Sorry, something went wrong.
The buffered tests are being modified, get into a parallel lane
|
Merged, thanks. |
Sorry, something went wrong.
Moves the I/O test support code to test.test_io.utils leaving 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_general to increase parallelism and logical layout. Times are runtime on my 64 bit linux debug build output by slow tests when running
./python -m test test_io -uall,walltime,largefile,extralargefile -M16G -o -j0The longest remaining test in test_general after that for me is test_daemon_threads_shutdown_*_deadlock which takes ~7s (and is marked with requires_resource('walltime')). Runtime of test_io reduces from ~34.1s -> 21.4s (SignalsTest is now longest) via increased parallelism.