Repository navigation
fix(tests): silence expected disconnect noise from MCP http fixtures - #387
Merged
Merged
Conversation
test_http_timeout abandons a 30s-sleeping handler on purpose (1s client timeout); when the handler thread wakes and writes to the dead socket, the BrokenPipeError traceback reaches process stderr, where under a full-suite run it lands inside a later test's CliRunner capture window (output mixes stderr) and corrupts stdout-purity assertions — test_run_json saw 'Extra data: line 2' on macOS CI three times, in three different tests. Note the override must live on a ThreadingHTTPServer subclass: process_request_thread invokes handle_error on the SERVER (BaseServer.handle_error), so a handler-level override is dead code. _QuietHTTPServer swallows BrokenPipeError / ConnectionResetError only; real handler bugs still print. Fixtures also server_close() after shutdown() so the listen socket can't reach GC as stderr noise. Adds a regression test, and the mechanism is proven end-to-end: an abandoned sleep-handler prints 1104 stderr bytes unpatched, 0 patched.
Faisal-Fayaz
force-pushed
the
fix/mcp-fixture-stderr-noise
branch
from
October 9, 2026 09:14
c80cd64 to
b04e623
Compare
Owner
Author
|
Update: the first version of this fix was itself broken — the override was placed on the handler class, but process_request_thread invokes handle_error on the SERVER (BaseServer.handle_error), so it was dead code. That's why CI still failed with the same signature. Corrected: _QuietHTTPServer(ThreadingHTTPServer) subclass in both fixtures, dead handler overrides removed, regression test now exercises the server method. Proven end-to-end, not just by reasoning: abandoning a real 30s-sleep handler prints 1104 stderr bytes with plain ThreadingHTTPServer vs 0 bytes with _QuietHTTPServer. 37 passed locally (mcp_remote + mcp_tool_cache + run_json), ruff + format clean. |
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
1 participant
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.
Root-causes the two macOS CI failures on #384 (test_run_json 'Extra data: line 2', twice in different tests).
Chain: test_http_timeout abandons a 30s-sleeping handler on purpose (1s client timeout). The handler thread wakes ~29s later, writes to the dead socket, and socketserver's default handle_error prints the BrokenPipeError traceback to the process stderr. CliRunner output mixes stderr, so under a full-suite run that traceback lands inside a later test's capture window and json.loads sees a second line. Timing-dependent → different victim test each run, macOS-only so far, never reproduces locally.
Fix (test-only): swallow BrokenPipeError/ConnectionResetError in both _Handler fixtures (real handler bugs still print), server_close() after shutdown() in both fixtures, plus a regression test asserting the abandoned-connection path stays off stderr.
Verified: 37 passed (mcp_remote + mcp_tool_cache + run_json), ruff + format clean. Once merged, #384 should go green on re-run.