Skip to content

Commit 05e19de

Browse files
committed
Clean up test code patterns
- Remove section header comments from test files - Move all inline imports to top of files - Replace hardcoded error codes (-32600) with INVALID_REQUEST constant - Replace arbitrary sleeps with polling loops for deterministic tests - Add pragma no branch to polling conditions that always succeed on first try
1 parent e68f821 commit 05e19de

File tree

5 files changed

+62
-154
lines changed

5 files changed

+62
-154
lines changed

tests/experimental/tasks/server/test_context.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
from mcp.shared.experimental.tasks.in_memory_task_store import InMemoryTaskStore
88
from mcp.types import CallToolResult, TaskMetadata, TextContent
99

10-
# --- TaskContext tests ---
11-
1210

1311
@pytest.mark.anyio
1412
async def test_task_context_properties() -> None:
@@ -98,9 +96,6 @@ async def test_task_context_cancellation() -> None:
9896
store.cleanup()
9997

10098

101-
# --- create_task_state tests ---
102-
103-
10499
def test_create_task_state_generates_id() -> None:
105100
"""create_task_state generates a unique task ID when none provided."""
106101
task1 = create_task_state(TaskMetadata(ttl=60000))
@@ -127,9 +122,6 @@ def test_create_task_state_has_created_at() -> None:
127122
assert task.createdAt is not None
128123

129124

130-
# --- task_execution context manager tests ---
131-
132-
133125
@pytest.mark.anyio
134126
async def test_task_execution_provides_context() -> None:
135127
"""task_execution provides a TaskContext for the task."""

tests/experimental/tasks/server/test_run_task_flow.py

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,33 @@
1010
"""
1111

1212
from typing import Any
13+
from unittest.mock import Mock
1314

1415
import anyio
1516
import pytest
1617
from anyio import Event
1718

1819
from mcp.client.session import ClientSession
1920
from mcp.server import Server
21+
from mcp.server.experimental.request_context import Experimental
2022
from mcp.server.experimental.task_context import ServerTaskContext
23+
from mcp.server.experimental.task_support import TaskSupport
2124
from mcp.server.lowlevel import NotificationOptions
25+
from mcp.shared.experimental.tasks.in_memory_task_store import InMemoryTaskStore
26+
from mcp.shared.experimental.tasks.message_queue import InMemoryTaskMessageQueue
2227
from mcp.shared.message import SessionMessage
2328
from mcp.types import (
2429
TASK_REQUIRED,
2530
CallToolResult,
31+
CancelTaskRequest,
32+
CancelTaskResult,
2633
CreateTaskResult,
34+
GetTaskPayloadRequest,
35+
GetTaskPayloadResult,
36+
GetTaskRequest,
37+
GetTaskResult,
38+
ListTasksRequest,
39+
ListTasksResult,
2740
TextContent,
2841
Tool,
2942
ToolExecution,
@@ -113,12 +126,12 @@ async def run_client() -> None:
113126
with anyio.fail_after(5):
114127
await work_completed.wait()
115128

116-
# Small delay to let task state update
117-
await anyio.sleep(0.1)
118-
119-
# Poll task status
120-
task_status = await client_session.experimental.get_task(task_id)
121-
assert task_status.status == "completed"
129+
# Poll until task status is completed
130+
with anyio.fail_after(5):
131+
while True:
132+
task_status = await client_session.experimental.get_task(task_id)
133+
if task_status.status == "completed": # pragma: no branch
134+
break
122135

123136
async with anyio.create_task_group() as tg:
124137
tg.start_soon(run_server)
@@ -181,11 +194,13 @@ async def run_client() -> None:
181194
with anyio.fail_after(5):
182195
await work_failed.wait()
183196

184-
await anyio.sleep(0.1)
197+
# Poll until task status is failed
198+
with anyio.fail_after(5):
199+
while True:
200+
task_status = await client_session.experimental.get_task(task_id)
201+
if task_status.status == "failed": # pragma: no branch
202+
break
185203

186-
# Task should be failed
187-
task_status = await client_session.experimental.get_task(task_id)
188-
assert task_status.status == "failed"
189204
assert "Something went wrong" in (task_status.statusMessage or "")
190205

191206
async with anyio.create_task_group() as tg:
@@ -217,9 +232,6 @@ async def test_enable_tasks_auto_registers_handlers() -> None:
217232
@pytest.mark.anyio
218233
async def test_enable_tasks_with_custom_store_and_queue() -> None:
219234
"""Test that enable_tasks() uses provided store and queue instead of defaults."""
220-
from mcp.shared.experimental.tasks.in_memory_task_store import InMemoryTaskStore
221-
from mcp.shared.experimental.tasks.message_queue import InMemoryTaskMessageQueue
222-
223235
server = Server("test-custom-store-queue")
224236

225237
# Create custom store and queue
@@ -237,17 +249,6 @@ async def test_enable_tasks_with_custom_store_and_queue() -> None:
237249
@pytest.mark.anyio
238250
async def test_enable_tasks_skips_default_handlers_when_custom_registered() -> None:
239251
"""Test that enable_tasks() doesn't override already-registered handlers."""
240-
from mcp.types import (
241-
CancelTaskRequest,
242-
CancelTaskResult,
243-
GetTaskPayloadRequest,
244-
GetTaskPayloadResult,
245-
GetTaskRequest,
246-
GetTaskResult,
247-
ListTasksRequest,
248-
ListTasksResult,
249-
)
250-
251252
server = Server("test-custom-handlers")
252253

253254
# Register custom handlers BEFORE enable_tasks (never called, just for registration)
@@ -281,8 +282,6 @@ async def custom_cancel_task(req: CancelTaskRequest) -> CancelTaskResult:
281282
@pytest.mark.anyio
282283
async def test_run_task_without_enable_tasks_raises() -> None:
283284
"""Test that run_task raises when enable_tasks() wasn't called."""
284-
from mcp.server.experimental.request_context import Experimental
285-
286285
experimental = Experimental(
287286
task_metadata=None,
288287
_client_capabilities=None,
@@ -300,8 +299,6 @@ async def work(task: ServerTaskContext) -> CallToolResult:
300299
@pytest.mark.anyio
301300
async def test_task_support_task_group_before_run_raises() -> None:
302301
"""Test that accessing task_group before run() raises RuntimeError."""
303-
from mcp.server.experimental.task_support import TaskSupport
304-
305302
task_support = TaskSupport.in_memory()
306303

307304
with pytest.raises(RuntimeError, match="TaskSupport not running"):
@@ -311,9 +308,6 @@ async def test_task_support_task_group_before_run_raises() -> None:
311308
@pytest.mark.anyio
312309
async def test_run_task_without_session_raises() -> None:
313310
"""Test that run_task raises when session is not available."""
314-
from mcp.server.experimental.request_context import Experimental
315-
from mcp.server.experimental.task_support import TaskSupport
316-
317311
task_support = TaskSupport.in_memory()
318312

319313
experimental = Experimental(
@@ -333,11 +327,6 @@ async def work(task: ServerTaskContext) -> CallToolResult:
333327
@pytest.mark.anyio
334328
async def test_run_task_without_task_metadata_raises() -> None:
335329
"""Test that run_task raises when request is not task-augmented."""
336-
from unittest.mock import Mock
337-
338-
from mcp.server.experimental.request_context import Experimental
339-
from mcp.server.experimental.task_support import TaskSupport
340-
341330
task_support = TaskSupport.in_memory()
342331
mock_session = Mock()
343332

@@ -469,11 +458,12 @@ async def run_client() -> None:
469458
with anyio.fail_after(5):
470459
await work_completed.wait()
471460

472-
await anyio.sleep(0.1)
473-
474-
# Task should be completed (from manual complete, not auto-complete)
475-
status = await client_session.experimental.get_task(task_id)
476-
assert status.status == "completed"
461+
# Poll until task status is completed
462+
with anyio.fail_after(5):
463+
while True:
464+
status = await client_session.experimental.get_task(task_id)
465+
if status.status == "completed": # pragma: no branch
466+
break
477467

478468
async with anyio.create_task_group() as tg:
479469
tg.start_soon(run_server)
@@ -533,11 +523,14 @@ async def run_client() -> None:
533523
with anyio.fail_after(5):
534524
await work_completed.wait()
535525

536-
await anyio.sleep(0.1)
526+
# Poll until task status is failed
527+
with anyio.fail_after(5):
528+
while True:
529+
status = await client_session.experimental.get_task(task_id)
530+
if status.status == "failed": # pragma: no branch
531+
break
537532

538533
# Task should still be failed (from manual fail, not auto-fail from exception)
539-
status = await client_session.experimental.get_task(task_id)
540-
assert status.status == "failed"
541534
assert status.statusMessage == "Manually failed" # Not "This error should not change status"
542535

543536
async with anyio.create_task_group() as tg:

tests/experimental/tasks/server/test_server.py

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,18 @@
88

99
from mcp.client.session import ClientSession
1010
from mcp.server import Server
11+
from mcp.server.experimental.task_result_handler import TaskResultHandler
1112
from mcp.server.lowlevel import NotificationOptions
1213
from mcp.server.models import InitializationOptions
1314
from mcp.server.session import ServerSession
14-
from mcp.shared.message import SessionMessage
15+
from mcp.shared.exceptions import McpError
16+
from mcp.shared.experimental.tasks.in_memory_task_store import InMemoryTaskStore
17+
from mcp.shared.experimental.tasks.message_queue import InMemoryTaskMessageQueue
18+
from mcp.shared.message import ServerMessageMetadata, SessionMessage
19+
from mcp.shared.response_router import ResponseRouter
1520
from mcp.shared.session import RequestResponder
1621
from mcp.types import (
22+
INVALID_REQUEST,
1723
TASK_FORBIDDEN,
1824
TASK_OPTIONAL,
1925
TASK_REQUIRED,
@@ -52,8 +58,6 @@
5258
ToolExecution,
5359
)
5460

55-
# --- Experimental handler tests ---
56-
5761

5862
@pytest.mark.anyio
5963
async def test_list_tasks_handler() -> None:
@@ -174,9 +178,6 @@ async def handle_cancel_task(request: CancelTaskRequest) -> CancelTaskResult:
174178
assert result.root.status == "cancelled"
175179

176180

177-
# --- Server capabilities tests ---
178-
179-
180181
@pytest.mark.anyio
181182
async def test_server_capabilities_include_tasks() -> None:
182183
"""Test that server capabilities include tasks when handlers are registered."""
@@ -223,9 +224,6 @@ async def handle_list_tasks(request: ListTasksRequest) -> ListTasksResult:
223224
assert capabilities.tasks.cancel is None # Not registered
224225

225226

226-
# --- Tool annotation tests ---
227-
228-
229227
@pytest.mark.anyio
230228
async def test_tool_with_task_execution_metadata() -> None:
231229
"""Test that tools can declare task execution mode."""
@@ -270,9 +268,6 @@ async def list_tools():
270268
assert tools[2].execution.taskSupport == TASK_OPTIONAL
271269

272270

273-
# --- Integration tests ---
274-
275-
276271
@pytest.mark.anyio
277272
async def test_task_metadata_in_call_tool_request() -> None:
278273
"""Test that task metadata is accessible via RequestContext when calling a tool."""
@@ -471,8 +466,6 @@ async def test_default_task_handlers_via_enable_tasks() -> None:
471466
- _default_list_tasks
472467
- _default_cancel_task
473468
"""
474-
from mcp.shared.exceptions import McpError
475-
476469
server = Server("test-default-handlers")
477470
# Enable tasks with default handlers (no custom handlers registered)
478471
task_support = server.experimental.enable_tasks()
@@ -569,10 +562,6 @@ async def run_server() -> None:
569562
@pytest.mark.anyio
570563
async def test_set_task_result_handler() -> None:
571564
"""Test that set_task_result_handler adds the handler as a response router."""
572-
from mcp.server.experimental.task_result_handler import TaskResultHandler
573-
from mcp.shared.experimental.tasks.in_memory_task_store import InMemoryTaskStore
574-
from mcp.shared.experimental.tasks.message_queue import InMemoryTaskMessageQueue
575-
576565
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
577566
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
578567

@@ -751,8 +740,6 @@ async def test_build_create_message_request() -> None:
751740
@pytest.mark.anyio
752741
async def test_send_message() -> None:
753742
"""Test that send_message sends a raw session message."""
754-
from mcp.shared.message import ServerMessageMetadata
755-
756743
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
757744
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
758745

@@ -790,8 +777,6 @@ async def test_send_message() -> None:
790777
@pytest.mark.anyio
791778
async def test_response_routing_success() -> None:
792779
"""Test that response routing works for success responses."""
793-
from mcp.shared.session import ResponseRouter
794-
795780
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
796781
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
797782

@@ -846,8 +831,6 @@ def route_error(self, request_id: str | int, error: ErrorData) -> bool:
846831
@pytest.mark.anyio
847832
async def test_response_routing_error() -> None:
848833
"""Test that error routing works for error responses."""
849-
from mcp.shared.session import ResponseRouter
850-
851834
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
852835
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
853836

@@ -878,7 +861,7 @@ def route_error(self, request_id: str | int, error: ErrorData) -> bool:
878861
server_session.add_response_router(router)
879862

880863
# Simulate receiving an error response from client
881-
error_data = ErrorData(code=-32600, message="Test error")
864+
error_data = ErrorData(code=INVALID_REQUEST, message="Test error")
882865
error_response = JSONRPCError(jsonrpc="2.0", id="test-req-2", error=error_data)
883866
message = SessionMessage(message=JSONRPCMessage(error_response))
884867

@@ -903,8 +886,6 @@ def route_error(self, request_id: str | int, error: ErrorData) -> bool:
903886
@pytest.mark.anyio
904887
async def test_response_routing_skips_non_matching_routers() -> None:
905888
"""Test that routing continues to next router when first doesn't match."""
906-
from mcp.shared.session import ResponseRouter
907-
908889
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
909890
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
910891

@@ -963,8 +944,6 @@ def route_error(self, request_id: str | int, error: ErrorData) -> bool:
963944
@pytest.mark.anyio
964945
async def test_error_routing_skips_non_matching_routers() -> None:
965946
"""Test that error routing continues to next router when first doesn't match."""
966-
from mcp.shared.session import ResponseRouter
967-
968947
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
969948
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage](10)
970949

@@ -1004,7 +983,7 @@ def route_error(self, request_id: str | int, error: ErrorData) -> bool:
1004983
server_session.add_response_router(MatchingRouter())
1005984

1006985
# Send an error - should skip first router and be handled by second
1007-
error_data = ErrorData(code=-32600, message="Test error")
986+
error_data = ErrorData(code=INVALID_REQUEST, message="Test error")
1008987
error_response = JSONRPCError(jsonrpc="2.0", id="test-req-2", error=error_data)
1009988
message = SessionMessage(message=JSONRPCMessage(error_response))
1010989
await client_to_server_send.send(message)

0 commit comments

Comments
 (0)