Run StreamableHTTP transport tests in process instead of over sockets#2767
Draft
maxisbey wants to merge 1 commit into
Draft
Run StreamableHTTP transport tests in process instead of over sockets#2767maxisbey wants to merge 1 commit into
maxisbey wants to merge 1 commit into
Conversation
Final installment of the in-process test migration: this was the last file spawning uvicorn subprocesses on bind-then-close ports with readiness polling, which races under pytest-xdist when two workers pick the same ephemeral port. Two tests in this file have flaked exactly that way under parallel load. All four subprocess servers (basic, JSON-response, event-store, context-aware) become in-process apps served through the interaction suite's StreamingASGITransport, held open by the session manager's run() context. Raw `requests` calls become httpx calls against the bridge client; the sync request-validation tests become anyio tests. The second-GET-409 test now holds the first stream open by construction, where the subprocess version noted it "might fail if the first stream fully closed before this runs". Assertions are unchanged, with documented exceptions now that the server handlers run as traced in-process code: - The long_running_with_checkpoints tool and the slow:// resource branch had no callers and are removed, so the expected tools/list count drops from 10 to 9 in five tests. - Dead defensive arms become asserts (sampling non-text fallback, close_sse_stream truthiness checks, the context server's unknown-tool fallthrough and request checks), and the event store's replay-from-unknown-event arm becomes a lookup that requires a stored event, since unreachable branches now fail branch coverage instead of hiding in an untraced subprocess. - test_client_crash_handled no longer sleeps between crashing clients; the bridge drains each client's teardown before the next connects. Three pragmas in src/mcp/server/streamable_http.py covered only by the formerly untraced subprocess (close_standalone_sse_stream, its session message callback, and the JSON-mode Accept rejection) are now executed by traced tests and removed. With the last wait_for_server user migrated, the helper is deleted from tests/test_helpers.py; run_uvicorn_in_thread stays for the websocket smoke test.
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.
Final installment of the in-process test migration (#2764, #2765):
tests/shared/test_streamable_http.pywas the last file spawning uvicorn subprocesses on bind-then-close ports, the pattern that races under pytest-xdist when two workers pick the same ephemeral port. Two of its tests have flaked exactly this way under parallel load (test_server_validates_protocol_version_header,test_streamable_http_client_mcp_headers_override_defaults).Closes #2704.
Motivation and Context
Same mechanism and fix as the previous two PRs. All four subprocess servers (basic, JSON-response, event-store, context) become in-process apps driven through
StreamingASGITransport; with this, no test in the repo binds a port outside the one pre-bound websocket smoke test.What changed beyond the plumbing swap:
requests-library tests; they're now anyio + httpx over the bridge with the same methods, headers, bodies, and assertions (including suppressing the library-defaultAcceptheader where the old tests did).wait_for_serveris deleted fromtests/test_helpers.py— zero users remain.run_uvicorn_in_threadstays fortests/shared/test_ws.py.# pragma: no coverinsrc/mcp/server/streamable_http.pyare removed (close_standalone_sse_stream, its session-message callback, the JSON-mode Accept rejection): those lines were only ever executed inside the untraced subprocess; the migrated tests now cover them, and keeping the pragmas would failstrict-no-cover.close_standalone_sse_streamclaimed client reconnection for standalone GET streams "is NOT implemented — this is a known gap", but the note was self-contradictory from the day it landed — the same commit (Add SSE polling support (SEP-1699) #1654) implemented the auto-reconnect and the test that proves it.long_running_with_checkpointstool and theslow://resource branch had no callers anywhere; they're deleted, which moves the tools/list count assertions from 10 to 9 in five tests. Unreachable handler branches elsewhere become dispatch asserts, since they now fail branch coverage instead of hiding in an untraced subprocess.test_streamable_http_client_resumption's flag-polling loop (while not flag: sleep(0.1), unbounded) is now twoanyio.Events awaited underfail_after(5).test_get_sse_stream's 409 assertion is deterministic by construction (the first GET is held open across the second), where the old version's comment admitted it could race.Deliberately retained: the genuinely time-based tests (SSE polling driven by
retry_interval=500, theelapsed >= 0.4retry-interval assertion, tool sleeps that create the disconnect windows) keep their real-clock behaviour and byte-identical assertions — in-process execution strictly tightens their margins versus the subprocess version. A follow-up may convert the reconnection choreography to event-store-sequenced waits (retry_interval=0+ the interaction suite'swait_until_storedpattern) and shave most of the file's remaining ~6s runtime; that's a behaviour-preserving quality pass kept out of this PR so the harness swap stays auditable.How Has This Been Tested?
./scripts/testgreen: 1526 passed, 100% line+branch coverage includingtests/,strict-no-covercleanpytest-xdist -n 4, 3× — stable; the nine resumption/reconnection tests 5× soloBreaking Changes
None — test-only, plus pragma/docstring removals in src.
Types of changes
Checklist
Additional context
Several tests here overlap with
tests/interaction/transports/coverage (resumption-token replay, stream-close auto-reconnect, priming events, standalone GET delivery); deduplication is inventoried for a follow-up ticket, kept out of scope so this diff remains a pure harness swap.AI Disclaimer