Skip to content

test: convert test_integration.py to in-memory transport (fix flaky)#2277

Draft
maxisbey wants to merge 1 commit intomainfrom
maxisbey/max-155-flaky-convert-test_integrationpy-to-in-process-remove
Draft

test: convert test_integration.py to in-memory transport (fix flaky)#2277
maxisbey wants to merge 1 commit intomainfrom
maxisbey/max-155-flaky-convert-test_integrationpy-to-in-process-remove

Conversation

@maxisbey
Copy link
Contributor

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:

@pytest.fixture
def server_port() -> int:
    with socket.socket() as s:
        s.bind(("127.0.0.1", 0))
        return s.getsockname()[1]  # socket closed → port released → TOCTOU window

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

Before After
Tests 20 (parametrized over 2 transports) 10
Runtime ~20s (subprocess spawn + uvicorn startup) ~0.5s
Lines 643 359 (−284)
Flakiness Port race under xdist None (in-process, deterministic)

AI Disclaimer

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix flaky test test_tool_progress[server_transport1] in test_integration.py

1 participant