Title: Run SSE and Unicode transport tests in process instead of over sockets by maxisbey · Pull Request #2765 · modelcontextprotocol/python-sdk · GitHub
Open Graph Title: Run SSE and Unicode transport tests in process instead of over sockets by maxisbey · Pull Request #2765 · modelcontextprotocol/python-sdk
X Title: Run SSE and Unicode transport tests in process instead of over sockets by maxisbey · Pull Request #2765 · modelcontextprotocol/python-sdk
Description: Second installment of the in-process test migration that started in #2764: tests/shared/test_sse.py and tests/client/test_http_unicode.py still used the bind-then-close port pick + uvicorn subprocess + readiness-poll harness, which races under pytest-xdist when two workers grab the same ephemeral port. test_sse_session_cleanup_on_disconnect has flaked exactly this way under parallel load. Motivation and Context Same mechanism and fix as #2764. The two files now drive the same Starlette apps in process through StreamingASGITransport (via the sanctioned tests.interaction.transports re-export): tests/shared/test_sse.py: sse_client connects through an httpx_client_factory backed by the bridge — the same pattern the interaction suite's SSE leg uses — and the basic/mounted/request-context servers become plain app factories sharing one make_app(server) builder. DNS-rebinding protection is disabled in the in-process apps (it guards against a network attack that can't exist here; the protection behaviour itself is pinned by tests/server/test_sse_security.py). tests/client/test_http_unicode.py: the Unicode echo server runs in process and streamable_http_client speaks to it through the bridge with follow_redirects=True, matching the SDK's own client factory (Starlette's Mount 307-redirects the bare /mcp path; the subprocess version silently relied on redirect-following too). Assertions are unchanged, with two deliberate exceptions now that the server handlers run as traced in-process code: test_sse_client_timeout is deleted. It was @pytest.mark.skip from the day it was introduced ("highlights a possible bug in SSE read timeout exception handling") and carried pragma: no cover, so no executing coverage is lost. Its premise — a real socket read timeout firing mid-stream — cannot occur for an in-process app, so it could never be unskipped here. The slow:// resource branch and the sse_read_timeout=0.5 fixture plumbing existed only for it. Note for the record: the suspicion it encoded (an expired sse_read_timeout may not surface an error to the in-flight request — the ReadTimeout path in src/mcp/client/sse.py is still pragma: lax no cover) is not tracked anywhere; nearest existing issue is #1811, which is the same failure family with a different trigger. Happy to file a dedicated issue so deleting the test doesn't delete the suspicion. Handlers no test ever invoked are gone or restructured. The main SSE server's tools handlers had no callers; the context server's unknown-tool fallthrough and defensive if ctx.request: arms were dead. They're removed or replaced with dispatch asserts (assert params.name in (...)), since unreachable branches now fail branch coverage instead of hiding in an untraced subprocess. The migrated test_sse.py also gets the same prose pass the security files got in #2764: behaviour-sentence docstrings, full annotations, and the line-counter loop in the raw-connection test replaced by two anext() assertions, which retires the file's last pragma: no branch. tests/shared/test_streamable_http.py is the remaining subprocess-based file and will be its own (final) migration, along with pruning the then-unused wait_for_server helper. How Has This Been Tested? ./scripts/test green: 1526 passed, 100% line+branch coverage, strict-no-cover clean (the suite's skip count drops from 2 to 1 with the dead test removed) The two files stress-tested heavily: ~100 runs including pytest-xdist at -n 2/4/8/16, sustained CPU load (loadavg >100), GC-canary runs for ResourceWarning leaks under filterwarnings=error, and failure-injection runs to verify clean teardown on assertion failure — zero failures The post-disconnect 404 regression test (#1227) is now deterministic by construction: the bridge's drain-on-close means the server-side session teardown provably completes before the follow-up POST (the old socket version actually raced uvicorn's disconnect processing) Breaking Changes None — test-only. Types of changes Bug fix (non-breaking change which fixes an issue) Checklist I have read the MCP Documentation My code follows the repository's style guidelines New and existing tests pass locally I have added appropriate error handling I have added or updated documentation as needed Additional context Several migrated tests now overlap with tests/interaction/transports/test_sse.py coverage (basic connection, session-created callback, disconnect cleanup, happy/error round trips). Deduplicating is deliberately out of scope here — this PR keeps the harness swap auditable — but it's inventoried for a follow-up. AI Disclaimer
Open Graph Description: Second installment of the in-process test migration that started in #2764: tests/shared/test_sse.py and tests/client/test_http_unicode.py still used the bind-then-close port pick + uvicorn subproce...
X Description: Second installment of the in-process test migration that started in #2764: tests/shared/test_sse.py and tests/client/test_http_unicode.py still used the bind-then-close port pick + uvicorn subproce...
Opengraph URL: https://github.com/modelcontextprotocol/python-sdk/pull/2765
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:bde0fd8e-ccae-64be-4c24-058b13bd3176 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 9982:210405:178C9E7:1E7FB43:6A5C58D1 |
| html-safe-nonce | d0fb4ee17d2605b97eebc646c1cef29edddfc94e5cca34b33b78e795c960e502 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI5OTgyOjIxMDQwNToxNzhDOUU3OjFFN0ZCNDM6NkE1QzU4RDEiLCJ2aXNpdG9yX2lkIjoiMjgwNzczNDE3MDYyMTI3ODI1IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 84d7d5e5acf663d2b56d157cc0e1e17f0de9d9d1f85603f130a199618a5d990c |
| hovercard-subject-tag | pull_request:3790967877 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/modelcontextprotocol/python-sdk/pull/2765/files |
| twitter:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/224885523?s=400&v=4 |
| og:image:alt | Second installment of the in-process test migration that started in #2764: tests/shared/test_sse.py and tests/client/test_http_unicode.py still used the bind-then-close port pick + uvicorn subproce... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5290d7e14309ad1e76106a9c4237bd1041517e83ea182c8ab756752cb0c6940b |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/modelcontextprotocol/python-sdk git https://github.com/modelcontextprotocol/python-sdk.git |
| octolytics-dimension-user_id | 182288589 |
| octolytics-dimension-user_login | modelcontextprotocol |
| octolytics-dimension-repository_id | 862584018 |
| octolytics-dimension-repository_nwo | modelcontextprotocol/python-sdk |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 862584018 |
| octolytics-dimension-repository_network_root_nwo | modelcontextprotocol/python-sdk |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 9c975978430e9ad293956f2bbdaf153b1bd84a99 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width