Skip to content

tests: avoid racy HTTP test ports#2760

Closed
tarunag10 wants to merge 4 commits into
modelcontextprotocol:mainfrom
tarunag10:codex/fix-python-sdk-port-race
Closed

tests: avoid racy HTTP test ports#2760
tarunag10 wants to merge 4 commits into
modelcontextprotocol:mainfrom
tarunag10:codex/fix-python-sdk-port-race

Conversation

@tarunag10
Copy link
Copy Markdown

Summary

  • Replaces racy pick-free-port-then-rebind server fixtures in the streamable HTTP/SSE test files with the existing run_uvicorn_in_thread helper.
  • Updates affected tests to consume URLs yielded from the actual listening uvicorn socket instead of separately selected port integers.
  • Adds a regression assertion that the basic streamable HTTP fixture URL uses a port that is already reserved, and closes the helper socket explicitly during shutdown.

This is an alternative/focused fix for #2704 using the existing thread helper. I noticed #2705 is already open with a process-helper approach; this draft is opened for maintainer comparison rather than as a duplicate ready-to-merge submission.

Validation

  • uv run ruff check tests/test_helpers.py tests/shared/test_streamable_http.py tests/shared/test_sse.py tests/server/test_sse_security.py tests/server/test_streamable_http_security.py tests/client/test_http_unicode.py
  • uv run pytest tests/shared/test_streamable_http.py tests/shared/test_sse.py tests/server/test_sse_security.py tests/server/test_streamable_http_security.py tests/client/test_http_unicode.py -> 120 passed, 1 skipped
  • uv run pytest -n auto tests/shared/test_streamable_http.py tests/shared/test_sse.py tests/server/test_sse_security.py tests/server/test_streamable_http_security.py tests/client/test_http_unicode.py -> first run had one non-port timing failure in test_streamable_http_multiple_reconnections; isolated rerun passed; second full affected xdist run passed with 120 passed, 1 skipped

@StantonMatt

This comment was marked as spam.

@maxisbey
Copy link
Copy Markdown
Contributor

maxisbey commented Jun 3, 2026

Thanks for taking this on — the TOCTOU diagnosis was right, and the pre-bound-socket approach is a sound fix for that race.

We ended up resolving this class of flake by removing the network layer from these tests entirely rather than fixing the port allocation: #2764, #2765, and #2767 migrated all six files to run in process over the interaction suite's streaming ASGI transport (no sockets, threads, or subprocesses, per the testing guidance in AGENTS.md), and #2767 also removed the wait_for_server helper. Closing this in favour of that series; thanks again for the comparison point.

AI Disclaimer

@maxisbey maxisbey closed this Jun 3, 2026
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.

3 participants