test: wait on ExecProcess after expected TypeError#2509
Merged
tonyandrewmeyer merged 4 commits intoMay 28, 2026
Merged
Conversation
`test_wait_output_no_stdout` provokes a TypeError from `wait_output()` without subsequently waiting on the process, so the ExecProcess is GC'd without `_waited` being set and `ExecProcess.__del__` emits `ResourceWarning: ExecProcess instance garbage collected without call to wait() or wait_output()`. Under `-W error` that fails the test. Call `process.wait()` after the expected TypeError so the leak detector doesn't fire on a test that's exercising a misuse-error path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tonyandrewmeyer
changed the title
If `ExecProcess._wait()` calls `cancel_stdin()` before the
`_reader_to_websocket` thread has been scheduled, `select()` reports
both the cancel pipe and the stdin reader as ready. The old check broke
the loop on cancel without reading, so any pending stdin was dropped
and only the `{"command":"end"}` frame was sent. Python 3.14's thread
scheduling made this race deterministic, failing
`TestExec.test_wait_file_io`.
Break only when the reader is not also ready; when both are ready,
drain the reader first and exit on the next iteration via EOF or a
plain cancel.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dwilding
approved these changes
May 27, 2026
Contributor
dwilding
left a comment
dwilding
left a comment
There was a problem hiding this comment.
I can't confidently review the pebble.py change, as I'm not so familiar with threading - but I get the point of the change. I also get the point of the update to the test. And I checked that we already have a test test_no_wait_call that covers the ResourceWarning.
tromai
approved these changes
May 28, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
test_wait_output_no_stdoutprovokes aTypeErrorfromwait_output()without subsequently waiting on the process, so theExecProcessis GC'd without_waitedbeing set andExecProcess.__del__emits:Under
-W errorthat fails the test.The makes a small (tiny!) change to call
process.wait()after the expectedTypeErrorso the leak detector doesn't fire on a test that's intentionally exercising a misuse-error path.Context: surfaced while making operator's test suite pass under
-W error.