test_nanny_worker_port_range hangs on Windows#5956
Conversation
Unit Test Results 12 files + 4 12 suites +4 7h 16m 40s ⏱️ + 4h 7m 35s For more details on these failures, see this check. Results for commit 1b3e6c3. ± Comparison against base commit 2d3fddc. ♻️ This comment has been updated with latest results. |
8ad80e8 to
6a9abbe
Compare
6a9abbe to
68fbbcc
Compare
|
To clarify: this PR does not make the tests with |
| print("\n\nPrint from stdout\n=================\n") | ||
| print(out.decode()) | ||
| if dump_stdout: | ||
| print("\n" + "-" * 27 + " Subprocess stdout/stderr" + "-" * 27) |
There was a problem hiding this comment.
| print("\n" + "-" * 27 + " Subprocess stdout/stderr" + "-" * 27) | |
| print("\n" + "-" * 27 + " Subprocess stdout" + "-" * 27) |
Are you not printing err because kwargs["stderr"] = subprocess.STDOUT means it's already been printed?
There was a problem hiding this comment.
Yes, they are merged together because I was afraid of the subprocess getting stuck on stdout while a test is reading from stderr and vice versa.
There was a problem hiding this comment.
Oh I see yes, stderr is redirected into stdout, which is then captured.
| "--no-dashboard", | ||
| ] | ||
| ], | ||
| flush_output=False, |
There was a problem hiding this comment.
Why not flush_output=True here and remove the await to_thread(worker.communicate, timeout=5)? Maybe there's a subtle difference in timing I'm missing.
There was a problem hiding this comment.
This and another test were abusing Proc.communicate to just wait for the subprocess to finish. I changed them now.
|
Following up here @crusaderky . Thank you for doing this PR. I'm curious, a lot of the tests that I've run into that fail unexpectedly use popen. Do you have thoughts on any additional cleanup here that might make sense? I don't have any ideas in particular, this comment is just a shot in the dark. |
|
(I found this PR through git blame) |
Uh oh!
There was an error while loading. Please reload this page.