Skip to content

Commit e045ebb

Browse files
committed
Consolidate stdio tests: in-process logic tests, minimal real-process set
The stdio client's logic — framing, parse errors, the shutdown escalation decisions — is now tested in process against a fake process injected through the spawn seam, and only the properties that are OS behaviour keep real subprocesses. The in-process tests include regressions for the shutdown bugs fixed in the previous commit: cancellation must still run the full shutdown, tree kill must reach children after the leader has exited, writes racing server death must surface clean closure, exiting with a server message still undelivered must be a clean exit, and a server exiting on stdin close must never be terminated. Each was verified to fail against the previous implementation. The real-process set shrinks to: one consolidated tree-kill test driven through the public stdio_client (a parent that exits instantly on SIGTERM, a child, and a grandchild — pinning group inheritance, atomic group kill, and dead-leader robustness in one spawn), the SIGTERM-ignoring SIGKILL escalation test, a dead-group no-op test, and exec-failure ENOENT. Grace and kill timeouts are shortened via monkeypatch in the real tests: the escalation decision is pinned in process against a shortened grace, the SIGTERM-then-SIGKILL escalation itself is exercised only by the real escalation test, and the production constants' values are deliberately unpinned. The stdio surface drops from ~11s to ~3.5s. Deleted as subsumed or broken: - The two tee-based tests: the interaction suite's subprocess e2e pins the real-pipe round trip with a real server, and tee could never deterministically exercise the chunk-reassembly buffer that the new in-process framing test pins (real pipes deliver short messages as whole lines). - test_stdio_client_bad_path: its script only "failed" because non-existent-file.py happens to be a Python expression that raises NameError; the property (stdout EOF fails initialize with CONNECTION_CLOSED) is now pinned in process. - The basic/early-parent cleanup tests and universal-cleanup / graceful-stdin-exit tests, folded into the consolidated tree test and the in-process graceful/escalation tests. - tests/issues/test_1027_win_unreachable_cleanup.py: its lifespan cleanup chain is covered by the interaction e2e's clean-exit marker plus the lifespan tests, it polled marker files with fixed sleeps, and it leaked its child process when an assertion failed mid-test. Its only helper module (tests/shared/test_win32_utils.py) goes with it. The MCPServer.run("stdio") coverage its subprocess scripts provided is replaced by an in-process test that drives run() over injected stdio and asserts a real response. tests/issues/test_552_windows_hang.py now asserts initialize succeeded instead of swallowing every exception (its handcrafted response also sent the invalid protocolVersion "1.0", so it errored on every run and passed anyway), the stdio server tests are bounded by fail_after, and the elicitation tests that claimed "stdio" in their names run, and now say they run, over the in-memory transport.
1 parent ddb8ef8 commit e045ebb

7 files changed

Lines changed: 539 additions & 610 deletions

File tree

tests/client/test_stdio.py

Lines changed: 451 additions & 290 deletions
Large diffs are not rendered by default.

tests/interaction/transports/test_stdio.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,12 @@ async def collect(params: LoggingMessageNotificationParams) -> None:
6969
# stdio_client deliberately filters the inherited environment to a safe minimum,
7070
# which drops the variables coverage.py's subprocess support uses; pass them through
7171
# so the server module is measured. Empty when not running under coverage.
72-
env={key: value for key, value in os.environ.items() if key.startswith("COVERAGE_")},
72+
# SyntaxWarning is suppressed because the child compiles dependencies from source
73+
# (pytest's pyc tag doesn't match a plain python child's): at the anyio>=4.9 floor,
74+
# Python 3.14 emits a compile-time warning for anyio's return-in-finally, which
75+
# would land on the snapshot-asserted stderr below.
76+
env={key: value for key, value in os.environ.items() if key.startswith("COVERAGE_")}
77+
| {"PYTHONWARNINGS": "ignore::SyntaxWarning"},
7378
),
7479
errlog=errlog,
7580
)

tests/issues/test_1027_win_unreachable_cleanup.py

Lines changed: 0 additions & 240 deletions
This file was deleted.
Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Test for issue #552: stdio_client hangs on Windows."""
22

3+
import json
34
import sys
45
from textwrap import dedent
56

@@ -8,56 +9,48 @@
89

910
from mcp import ClientSession, StdioServerParameters
1011
from mcp.client.stdio import stdio_client
12+
from mcp.types import LATEST_PROTOCOL_VERSION, InitializeResult
1113

1214

1315
@pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific test") # pragma: no cover
1416
@pytest.mark.anyio
15-
async def test_windows_stdio_client_with_session():
16-
"""Test the exact scenario from issue #552: Using ClientSession with stdio_client.
17-
18-
This reproduces the original bug report where stdio_client hangs on Windows 11
19-
when used with ClientSession.
17+
async def test_initialize_succeeds_and_shutdown_returns_after_the_server_exits_mid_session():
18+
"""Initialize completes (and shutdown returns) against a server that responds and
19+
then exits mid-session — the proactor pipe scenario that hung on Windows 11 in
20+
issue #552. The positive assertion matters: a session that *errors* quickly would
21+
also "not hang", so finishing fast alone proves nothing.
2022
"""
21-
# Create a minimal MCP server that responds to initialization
22-
server_script = dedent("""
23+
# A minimal server: answer initialize correctly, then exit.
24+
server_script = dedent(f"""
2325
import json
2426
import sys
2527
26-
# Read initialization request
2728
line = sys.stdin.readline()
29+
request = json.loads(line)
2830
29-
# Send initialization response
30-
response = {
31+
response = {{
3132
"jsonrpc": "2.0",
32-
"id": 1,
33-
"result": {
34-
"protocolVersion": "1.0",
35-
"capabilities": {},
36-
"serverInfo": {"name": "test-server", "version": "1.0"}
37-
}
38-
}
33+
"id": request["id"],
34+
"result": {{
35+
"protocolVersion": {json.dumps(LATEST_PROTOCOL_VERSION)},
36+
"capabilities": {{}},
37+
"serverInfo": {{"name": "test-server", "version": "1.0"}}
38+
}}
39+
}}
3940
print(json.dumps(response))
4041
sys.stdout.flush()
41-
42-
# Exit after a short delay
43-
import time
44-
time.sleep(0.1)
45-
sys.exit(0)
4642
""").strip()
4743

4844
params = StdioServerParameters(
4945
command=sys.executable,
5046
args=["-c", server_script],
5147
)
5248

53-
# This is the exact pattern from the bug report
5449
with anyio.fail_after(10):
55-
try:
56-
async with stdio_client(params) as (read, write):
57-
async with ClientSession(read, write) as session:
58-
await session.initialize()
59-
# Should exit ClientSession without hanging
60-
# Should exit stdio_client without hanging
61-
except Exception:
62-
# Connection errors are expected when process exits
63-
pass
50+
async with stdio_client(params) as (read, write):
51+
async with ClientSession(read, write) as session:
52+
result = await session.initialize()
53+
assert isinstance(result, InitializeResult)
54+
assert result.server_info.name == "test-server"
55+
# Exiting ClientSession and stdio_client must not hang even though the
56+
# server process is already gone.

tests/server/mcpserver/test_elicitation.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Test the elicitation feature using stdio transport."""
1+
"""Test the elicitation feature over the in-memory client transport."""
22

33
from typing import Any
44

@@ -58,9 +58,9 @@ async def call_tool_and_assert(
5858

5959

6060
@pytest.mark.anyio
61-
async def test_stdio_elicitation():
62-
"""Test the elicitation feature using stdio transport."""
63-
mcp = MCPServer(name="StdioElicitationServer")
61+
async def test_elicitation_accept_returns_the_users_answer_to_the_tool():
62+
"""An accepted elicitation delivers the user's content back to the requesting tool."""
63+
mcp = MCPServer(name="ElicitationServer")
6464
create_ask_user_tool(mcp)
6565

6666
# Create a custom handler for elicitation requests
@@ -76,9 +76,9 @@ async def elicitation_callback(context: RequestContext[ClientSession], params: E
7676

7777

7878
@pytest.mark.anyio
79-
async def test_stdio_elicitation_decline():
80-
"""Test elicitation with user declining."""
81-
mcp = MCPServer(name="StdioElicitationDeclineServer")
79+
async def test_elicitation_decline_reaches_the_tool_without_content():
80+
"""A declined elicitation reports the decline to the tool, with no content attached."""
81+
mcp = MCPServer(name="ElicitationDeclineServer")
8282
create_ask_user_tool(mcp)
8383

8484
async def elicitation_callback(context: RequestContext[ClientSession], params: ElicitRequestParams):

0 commit comments

Comments
 (0)