Skip to content

Commit 5830e3c

Browse files
committed
Add coverage tests for experimental tasks code
- Remove unused _get_event method from InMemoryTaskMessageQueue - Add Resolver error case tests (set_result/set_exception when completed) - Add message_queue tests for peek empty, double-check race condition - Add in_memory_task_store test for wait_for_update with nonexistent task - Add task_support test for accessing task_group before run() - Add task_result_handler tests for exception handling and missing original_id
1 parent 27303bc commit 5830e3c

File tree

5 files changed

+189
-9
lines changed

5 files changed

+189
-9
lines changed

src/mcp/shared/experimental/tasks/message_queue.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,6 @@ def _get_queue(self, task_id: str) -> list[QueuedMessage]:
171171
self._queues[task_id] = []
172172
return self._queues[task_id]
173173

174-
def _get_event(self, task_id: str) -> anyio.Event:
175-
"""Get or create the wait event for a task."""
176-
if task_id not in self._events:
177-
self._events[task_id] = anyio.Event()
178-
return self._events[task_id]
179-
180174
async def enqueue(self, task_id: str, message: QueuedMessage) -> None:
181175
"""Add a message to the queue."""
182176
queue = self._get_queue(task_id)

tests/experimental/tasks/server/test_run_task_flow.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,17 @@ async def work(task: ServerTaskContext) -> CallToolResult:
233233
await experimental.run_task(work)
234234

235235

236+
@pytest.mark.anyio
237+
async def test_task_support_task_group_before_run_raises() -> None:
238+
"""Test that accessing task_group before run() raises RuntimeError."""
239+
from mcp.server.experimental.task_support import TaskSupport
240+
241+
task_support = TaskSupport.in_memory()
242+
243+
with pytest.raises(RuntimeError, match="TaskSupport not running"):
244+
_ = task_support.task_group
245+
246+
236247
@pytest.mark.anyio
237248
async def test_run_task_without_session_raises() -> None:
238249
"""Test that run_task raises when session is not available."""

tests/experimental/tasks/server/test_store.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,11 @@ async def test_terminal_status_allows_same_status(store: InMemoryTaskStore) -> N
321321
assert updated.statusMessage == "Updated message"
322322

323323

324-
# =============================================================================
325-
# cancel_task helper function tests
326-
# =============================================================================
324+
@pytest.mark.anyio
325+
async def test_wait_for_update_nonexistent_raises(store: InMemoryTaskStore) -> None:
326+
"""Test that wait_for_update raises for nonexistent task."""
327+
with pytest.raises(ValueError, match="not found"):
328+
await store.wait_for_update("nonexistent-task-id")
327329

328330

329331
@pytest.mark.anyio

tests/experimental/tasks/server/test_task_result_handler.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,3 +253,94 @@ async def test_deliver_registers_resolver_for_request_messages(
253253

254254
assert "inner-req-1" in handler._pending_requests
255255
assert handler._pending_requests["inner-req-1"] is resolver
256+
257+
258+
@pytest.mark.anyio
259+
async def test_deliver_skips_resolver_registration_when_no_original_id(
260+
store: InMemoryTaskStore, queue: InMemoryTaskMessageQueue, handler: TaskResultHandler
261+
) -> None:
262+
"""Test that _deliver_queued_messages skips resolver registration when original_request_id is None."""
263+
task = await store.create_task(TaskMetadata(ttl=60000), task_id="test-task")
264+
265+
resolver: Resolver[dict[str, Any]] = Resolver()
266+
queued_msg = QueuedMessage(
267+
type="request",
268+
message=JSONRPCRequest(
269+
jsonrpc="2.0",
270+
id="inner-req-1",
271+
method="elicitation/create",
272+
params={},
273+
),
274+
resolver=resolver,
275+
original_request_id=None, # No original request ID
276+
)
277+
await queue.enqueue(task.taskId, queued_msg)
278+
279+
mock_session = Mock()
280+
mock_session.send_message = AsyncMock()
281+
282+
await handler._deliver_queued_messages(task.taskId, mock_session, "outer-req-1")
283+
284+
# Resolver should NOT be registered since original_request_id is None
285+
assert len(handler._pending_requests) == 0
286+
# But the message should still be sent
287+
mock_session.send_message.assert_called_once()
288+
289+
290+
@pytest.mark.anyio
291+
async def test_wait_for_task_update_handles_store_exception(
292+
store: InMemoryTaskStore, queue: InMemoryTaskMessageQueue, handler: TaskResultHandler
293+
) -> None:
294+
"""Test that _wait_for_task_update handles store exception gracefully."""
295+
task = await store.create_task(TaskMetadata(ttl=60000), task_id="test-task")
296+
297+
# Make wait_for_update raise an exception
298+
async def failing_wait(task_id: str) -> None:
299+
raise RuntimeError("Store error")
300+
301+
store.wait_for_update = failing_wait # type: ignore[method-assign]
302+
303+
# Queue a message to unblock the race via the queue path
304+
async def enqueue_later() -> None:
305+
await anyio.sleep(0.01)
306+
await queue.enqueue(
307+
task.taskId,
308+
QueuedMessage(
309+
type="notification",
310+
message=JSONRPCRequest(
311+
jsonrpc="2.0",
312+
id="notif-1",
313+
method="test/notification",
314+
params={},
315+
),
316+
),
317+
)
318+
319+
async with anyio.create_task_group() as tg:
320+
tg.start_soon(enqueue_later)
321+
# This should complete via the queue path even though store raises
322+
await handler._wait_for_task_update(task.taskId)
323+
324+
325+
@pytest.mark.anyio
326+
async def test_wait_for_task_update_handles_queue_exception(
327+
store: InMemoryTaskStore, queue: InMemoryTaskMessageQueue, handler: TaskResultHandler
328+
) -> None:
329+
"""Test that _wait_for_task_update handles queue exception gracefully."""
330+
task = await store.create_task(TaskMetadata(ttl=60000), task_id="test-task")
331+
332+
# Make wait_for_message raise an exception
333+
async def failing_wait(task_id: str) -> None:
334+
raise RuntimeError("Queue error")
335+
336+
queue.wait_for_message = failing_wait # type: ignore[method-assign]
337+
338+
# Update the store to unblock the race via the store path
339+
async def update_later() -> None:
340+
await anyio.sleep(0.01)
341+
await store.update_task(task.taskId, status="completed")
342+
343+
async with anyio.create_task_group() as tg:
344+
tg.start_soon(update_later)
345+
# This should complete via the store path even though queue raises
346+
await handler._wait_for_task_update(task.taskId)

tests/experimental/tasks/test_message_queue.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,3 +247,85 @@ async def wait_for_notification() -> None:
247247
tg.start_soon(notify_when_ready)
248248

249249
assert notified is True
250+
251+
@pytest.mark.anyio
252+
async def test_peek_empty_queue_returns_none(self, queue: InMemoryTaskMessageQueue) -> None:
253+
"""Peek on empty queue returns None."""
254+
result = await queue.peek("nonexistent-task")
255+
assert result is None
256+
257+
@pytest.mark.anyio
258+
async def test_wait_for_message_double_check_race_condition(self, queue: InMemoryTaskMessageQueue) -> None:
259+
"""wait_for_message returns early if message arrives after event creation but before wait."""
260+
task_id = "task-1"
261+
262+
# To test the double-check path (lines 223-225), we need a message to arrive
263+
# after the event is created (line 220) but before event.wait() (line 228).
264+
# We simulate this by injecting a message before is_empty is called the second time.
265+
266+
original_is_empty = queue.is_empty
267+
call_count = 0
268+
269+
async def is_empty_with_injection(tid: str) -> bool:
270+
nonlocal call_count
271+
call_count += 1
272+
if call_count == 2 and tid == task_id:
273+
# Before second check, inject a message - this simulates a message
274+
# arriving between event creation and the double-check
275+
queue._queues[task_id] = [QueuedMessage(type="request", message=make_request())]
276+
return await original_is_empty(tid)
277+
278+
queue.is_empty = is_empty_with_injection # type: ignore[method-assign]
279+
280+
# Should return immediately due to double-check finding the message
281+
with anyio.fail_after(1):
282+
await queue.wait_for_message(task_id)
283+
284+
285+
class TestResolver:
286+
@pytest.mark.anyio
287+
async def test_set_result_and_wait(self) -> None:
288+
"""Test basic set_result and wait flow."""
289+
resolver: Resolver[str] = Resolver()
290+
291+
resolver.set_result("hello")
292+
result = await resolver.wait()
293+
294+
assert result == "hello"
295+
assert resolver.done()
296+
297+
@pytest.mark.anyio
298+
async def test_set_exception_and_wait(self) -> None:
299+
"""Test set_exception raises on wait."""
300+
resolver: Resolver[str] = Resolver()
301+
302+
resolver.set_exception(ValueError("test error"))
303+
304+
with pytest.raises(ValueError, match="test error"):
305+
await resolver.wait()
306+
307+
assert resolver.done()
308+
309+
@pytest.mark.anyio
310+
async def test_set_result_when_already_completed_raises(self) -> None:
311+
"""Test that set_result raises if resolver already completed."""
312+
resolver: Resolver[str] = Resolver()
313+
resolver.set_result("first")
314+
315+
with pytest.raises(RuntimeError, match="already completed"):
316+
resolver.set_result("second")
317+
318+
@pytest.mark.anyio
319+
async def test_set_exception_when_already_completed_raises(self) -> None:
320+
"""Test that set_exception raises if resolver already completed."""
321+
resolver: Resolver[str] = Resolver()
322+
resolver.set_result("done")
323+
324+
with pytest.raises(RuntimeError, match="already completed"):
325+
resolver.set_exception(ValueError("too late"))
326+
327+
@pytest.mark.anyio
328+
async def test_done_returns_false_before_completion(self) -> None:
329+
"""Test done() returns False before any result is set."""
330+
resolver: Resolver[str] = Resolver()
331+
assert resolver.done() is False

0 commit comments

Comments
 (0)