| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
The file previously spun up a uvicorn subprocess + TCP port for each of
20 parametrized tests (10 tests × 2 transports). This caused flaky CI
failures from port races under pytest-xdist: the `server_port` fixture
releases its socket before the subprocess binds, so another xdist worker
can grab the same port first. `wait_for_server` only checks for TCP
connect, so it happily accepts a connection to the WRONG server.
None of these tests exercise transport-specific behavior — they test MCP
protocol features (tools, resources, prompts, progress, sampling,
elicitation, notifications, completion, structured output). The transport
parametrization doubled test time without adding coverage; transport
behavior is tested separately in tests/server/test_sse_security.py,
test_streamable_http_{manager,security}.py, and tests/shared/.
Converted all tests to use `Client(mcp)` in-memory, matching the
established pattern in test_server.py and test_url_elicitation.py.
Impact:
- Eliminates port race flakiness for this file
- 20 tests → 10 tests (no transport parametrization)
- ~20s subprocess spawn overhead → 0.5s total
- -284 lines (deleted subprocess/fixture infrastructure)
Github-Issue: #1776
This method was only exercised by the subprocess-based integration tests, where coverage wasn't tracked in the child process. Now that test_integration.py runs the server in-process via Client(mcp), the method is covered and the pragma is invalid. Caught by strict-no-cover (ubuntu-only CI step). Github-Issue: #1776
There was a problem hiding this comment.
LGTM — straightforward test refactor from subprocess+TCP to in-memory transport, following the established Client(mcp) pattern.
Extended reasoning...This PR touches two files: a one-line pragma removal in src/mcp/server/session.py (removing # pragma: no cover from send_resource_list_changed since the method is now exercised by the converted tests), and a significant simplification of tests/server/mcpserver/test_integration.py that replaces subprocess+TCP test infrastructure with in-memory Client(mcp) calls, reducing the file from 643 to 359 lines.
None. The production code change is a single pragma comment removal. All other changes are test-only, removing network-based test infrastructure in favor of in-memory transport.
Low scrutiny warranted. This is a mechanical test refactor that follows an established pattern already used in test_server.py and test_url_elicitation.py. The test assertions are preserved; only the transport mechanism changes. The motivation (port-race flakiness under pytest-xdist) is well-documented with CI evidence.
The PR removes ~100 lines of test infrastructure (fixtures, subprocess management, port allocation, transport selection helpers) while preserving all 10 test scenarios and their assertions. Transport-specific coverage is noted as remaining in dedicated test files (test_sse_security.py, test_streamable_http_*.py, tests/shared/test_{sse,streamable_http,ws}.py). No bugs were found by automated analysis.
Sorry, something went wrong.
Rebased onto MAX-157 (#2267) which landed run_uvicorn_in_thread with two improvements over the helper this branch had: - Pre-binds socket + listen() before thread start: port is known immediately, no polling loop, kernel queues early connections - interface="asgi3": skips uvicorn's asyncio.iscoroutinefunction autodetect, which Python 3.14 deprecates (cleaner than catch_warnings) Kept this branch's gc.collect() + ResourceWarning suppression in the finally — MAX-157's single WS smoke test didn't need it, but the SSE abrupt-disconnect test patterns here do. Also removed wait_for_server — MAX-155 (#2277) converted its last caller (test_integration.py) to in-memory transport.
| Back | FazBrowse Home | New Git URL |
Summary
Converts tests/server/mcpserver/test_integration.py from subprocess+TCP to the in-memory Client(mcp) pattern, eliminating port-race flakiness under pytest-xdist.
Closes #1776.
Root Cause
The server_port fixture allocates a port by binding to 0 then releasing the socket before the subprocess can bind it:
With pytest -n auto, another xdist worker's subprocess can grab the same port first. wait_for_server only checks TCP connect, so it happily accepts a connection to the wrong server. CI evidence in job 66317185239 shows a 7ms 404 on /sse — not enough time for a subprocess to start, meaning wait_for_server saw a different worker's server.
Verification this is a port race, not a uvicorn lifespan gap
mcp.sse_app() returns Starlette(routes=routes) with no lifespan — routes are synchronously constructed. Uvicorn+Starlette never serves 404 before 200 for a no-lifespan app; a 404 on /sse can only mean a different server bound that port.
Fix
None of these 20 parametrized tests (10 × 2 transports) exercise transport-specific behavior — they test MCP protocol features (tools, resources, prompts, progress, sampling, elicitation, notifications, completion, structured output). Converted all to use Client(mcp) in-memory, matching the established pattern in test_server.py (×16) and test_url_elicitation.py (×11).
Transport behavior remains covered separately in tests/server/test_sse_security.py, test_streamable_http_{manager,security}.py, and tests/shared/test_{sse,streamable_http,ws}.py.
Impact
AI Disclaimer