Widen the stdio round-trip test's termination grace and overall timeout - #2880
Merged
maxisbey merged 1 commit intoJun 15, 2026
Merged
Widen the stdio round-trip test's termination grace and overall timeout#2880maxisbey merged 1 commit into
maxisbey merged 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
The test patches PROCESS_TERMINATION_TIMEOUT so the spawned server can unwind, save its subprocess coverage data, and print the clean-exit line before the kill escalation. One heavily starved Windows CI runner stalled the child past the previous 10s grace, so it was killed before printing and the stderr assertion failed with empty output. The post-stdin-close tail is normally well under a second even on loaded Windows runners, so double the grace to 20s (and the outer fail_after to 30s); the grace wait returns as soon as the child exits, so passing runs are unaffected.
maxisbey
marked this pull request as ready for review
June 15, 2026 17:05
maxisbey
enabled auto-merge (squash)
June 15, 2026 17:05
felixweinberger
approved these changes
Jun 15, 2026
Copy link
Copy Markdown
Contributor
|
This pull request is included in pre-release v2.0.0a2 |
Sorry, something went wrong.
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
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.
Widens the stdio round-trip test's termination grace (10s → 20s) and its outer fail_after (20s → 30s) so a badly starved CI runner can't kill the child server before it writes the clean-exit line.
Motivation and Context
test_tool_call_and_notification_round_trip_over_a_stdio_subprocess patches PROCESS_TERMINATION_TIMEOUT so the spawned server has time, after stdin closes, to unwind, save its subprocess coverage data, and print stdio-echo: clean exit (the ordering #2840 established so a kill can't silently lose the coverage data). On one heavily contended run (3.13, lowest-direct, windows-latest, 2026-06-15) the runner stalled the child past the 10s grace; it was killed before printing and the test failed with an empty stderr capture.
Measurements on a loaded windows-latest runner with the same configuration (Python 3.13, lowest-direct, subprocess coverage, background pytest + CPU load) put the post-stdin-close tail at ~0.2s median / ~0.4s max — dominated by the coverage save — so the failure needed an extreme machine-level stall rather than anything systematic. Doubling the grace keeps that class of runner pathology from failing the leg, and costs nothing on healthy runs because the wait returns as soon as the child exits. A genuinely hung child still fails, just after 20s instead of 10s.
How Has This Been Tested?
Breaking Changes
None — test-only change; the production PROCESS_TERMINATION_TIMEOUT default (2.0s) is untouched.
Types of changes
Checklist
Additional context
Since #2773 and #2840 this is the only stdio test failure observed in CI (once across all runs and matrix legs since June 12), so this is a belt-and-braces margin bump rather than a fix for an active problem.
AI Disclaimer