tests: eliminate port-allocation races in SSE/StreamableHTTP tests#2264
Draft
tests: eliminate port-allocation races in SSE/StreamableHTTP tests#2264
Conversation
Replace the multiprocessing.Process + socket.bind(("",0)) pattern with a
run_server_in_thread helper that uses uvicorn with port=0 in a background
thread. The kernel atomically assigns the port at bind time and we read it
back from the bound socket, eliminating the TOCTOU window that caused
test_response and others to intermittently connect to the wrong server
under pytest-xdist parallel execution.
Where possible (StreamableHTTP security tests, Unicode tests), use
httpx.ASGITransport for fully in-process testing with no real sockets.
Legacy SSE tests keep real HTTP because ASGITransport buffers responses
and cannot support the long-lived GET /sse stream pattern.
Github-Issue: #1573
With server-side test code now running in-thread (same process), coverage tracks code paths that were previously only hit in subprocesses. Several # pragma: no cover annotations now cover lines that ARE covered, failing the strict-no-cover check. Downgrade these to # pragma: lax no cover, which permits partial coverage without failing.
uvicorn polls should_exit every 0.1s in its main_loop and adds another 0.1s sleep in shutdown(), so thread.join() blocks ~200ms per fixture teardown. With 60+ fixture-using tests in test_streamable_http.py, that was ~12s of pure shutdown latency. The thread is a daemon, so signaling exit and moving on is safe — the interpreter reaps it. The socket is still closed by uvicorn's shutdown, releasing the port. Sequential runtime for affected files: 24s → 13s (old multiprocessing baseline was 16s). Parallel (-n 14) runtime unchanged at ~4.5s.
The bulk of test_streamable_http.py's runtime was fixture overhead: 46 function-scoped uvicorn starts at ~39ms each = 1.8s of pure server lifecycle for tests that are mostly stateless HTTP validation. - Make basic_server, json_response_server module-scoped. The session manager keys sessions by ID; each test uses fresh IDs so there is no cross-contamination. One server now handles ~30 validation tests. - Convert the 5 context_aware_server tests (header propagation checks) and initialized_client_session to ASGITransport via a new asgi_client helper. These test that headers flow from ASGI scope → ctx.request, which ASGITransport exercises directly. - Collapse repeated header-echo boilerplate into _echo_headers(). 53 non-reconnection tests: 3.59s → 1.53s. Setup+teardown 1.77s → 0.20s. Remaining call time is legitimate: ~15 real-TCP MCP sessions (init + request + terminate ≈ 60ms each) and one event_server test with a sleep-polling loop (out of scope, tracked separately).
ef5bfcc to
b068384
Compare
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.
Motivation and Context
Fixes #1573.
Several test files use the pattern
socket.bind(("", 0))→ get port → start uvicorn inmultiprocessing.Process→wait_for_server()→ connect. This has a TOCTOU window between port selection and bind: withpytest-xdistparallel workers, two tests can grab the same port, leading to connections landing on the wrong server (the404 != 200failure intest_response) or "address already in use" errors.Prior attempt #1528 used worker-specific port ranges but was closed unmerged.
How Has This Been Tested?
test_responseflakefinder: 20/20 passBreaking Changes
None. Test infrastructure only.
Types of changes
Checklist
Additional context
Approach
New helper
run_server_in_thread(tests/test_helpers.py): runs uvicorn in a daemon thread withport=0. The kernel atomically assigns a free port at bind time, then we read it back from the bound socket viaserver.servers[0].sockets[0].getsockname(). No window between check and use.Why not
httpx.ASGITransporteverywhere?I tested this —
httpx.ASGITransportbuffers the full response body before returning (await self.app(scope, receive, send)blocks until the app returns). This works for StreamableHTTP POST→SSE cycles where each request completes, but deadlocks for:sse_client+SseServerTransport) — the GET/ssestream stays open indefinitelySo I used a hybrid:
test_streamable_http_security.pyhttpx.ASGITransport(all POST-based, responses complete)test_http_unicode.pyhttpx.ASGITransport(POST→SSE cycles only)test_sse_security.pyrun_server_in_thread(several tests need 200 on long-lived stream)test_sse.pyrun_server_in_thread(legacy SSE requires real HTTP)test_streamable_http.pyrun_server_in_thread(fixtures only — test bodies unchanged)Net: −412 lines (495 added, 907 deleted).
Side effects
coverage.pytracks it. Removed some# pragma: no coverannotations and cleaned up unused handlers that were never actually exercised.event_storeinevent_serverfixture is now the same object the server uses (not a pickled copy), so tests can inspect server-side state if needed.tests/shared/test_ws.pyandtests/server/mcpserver/test_integration.pystill use the old pattern — out of scope for this PR.AI Disclaimer