Skip to content

Commit efd779e

Browse files
committed
test: improve SSE empty data test to properly mock SSE events
The original test didn't actually exercise the `if not sse.data: continue` code path. This rewrite: - Mocks the SSE event stream to include an empty "message" event - Uses httpx_sse.ServerSentEvent for accurate event simulation - Uses proper type serialization instead of hardcoded JSON strings - Validates that the client skips empty data and receives the real response
1 parent 19199b3 commit efd779e

File tree

1 file changed

+67
-15
lines changed

1 file changed

+67
-15
lines changed

tests/shared/test_sse.py

Lines changed: 67 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44
import time
55
from collections.abc import AsyncGenerator, Generator
66
from typing import Any
7-
from unittest.mock import Mock
7+
from unittest.mock import AsyncMock, MagicMock, Mock, patch
88

99
import anyio
1010
import httpx
1111
import pytest
1212
import uvicorn
13+
from httpx_sse import ServerSentEvent
1314
from inline_snapshot import snapshot
1415
from pydantic import AnyUrl
1516
from starlette.applications import Starlette
@@ -28,8 +29,11 @@
2829
from mcp.types import (
2930
EmptyResult,
3031
ErrorData,
32+
Implementation,
3133
InitializeResult,
34+
JSONRPCResponse,
3235
ReadResourceResult,
36+
ServerCapabilities,
3337
TextContent,
3438
TextResourceContents,
3539
Tool,
@@ -534,19 +538,67 @@ def test_sse_server_transport_endpoint_validation(endpoint: str, expected_result
534538
assert sse._endpoint.startswith("/")
535539

536540

541+
# ResourceWarning filter: When mocking aconnect_sse, the sse_client's internal task
542+
# group doesn't receive proper cancellation signals, so the sse_reader task's finally
543+
# block (which closes read_stream_writer) doesn't execute. This is a test artifact -
544+
# the actual code path (`if not sse.data: continue`) IS exercised and works correctly.
545+
# Production code with real SSE connections cleans up properly.
546+
@pytest.mark.filterwarnings("ignore::ResourceWarning")
537547
@pytest.mark.anyio
538-
async def test_sse_client_handles_empty_keepalive_pings(server: None, server_url: str) -> None:
539-
"""Test that SSE client properly handles empty data lines (keep-alive pings)."""
540-
async with sse_client(server_url + "/sse") as streams:
541-
async with ClientSession(*streams) as session:
542-
# Initialize the session
543-
result = await session.initialize()
544-
assert isinstance(result, InitializeResult)
545-
assert result.serverInfo.name == SERVER_NAME
548+
async def test_sse_client_handles_empty_keepalive_pings() -> None:
549+
"""Test that SSE client properly handles empty data lines (keep-alive pings).
546550
547-
# Test that we can still make requests after receiving keep-alive pings
548-
# The server may send empty data lines between actual messages
549-
response = await session.read_resource(uri=AnyUrl("foobar://test"))
550-
assert len(response.contents) == 1
551-
assert isinstance(response.contents[0], TextResourceContents)
552-
assert response.contents[0].text == "Read test"
551+
Per the MCP spec (Streamable HTTP transport): "The server SHOULD immediately
552+
send an SSE event consisting of an event ID and an empty data field in order
553+
to prime the client to reconnect."
554+
555+
This test mocks the SSE event stream to include empty "message" events and
556+
verifies the client skips them without crashing.
557+
"""
558+
# Build a proper JSON-RPC response using types (not hardcoded strings)
559+
init_result = InitializeResult(
560+
protocolVersion="2024-11-05",
561+
capabilities=ServerCapabilities(),
562+
serverInfo=Implementation(name="test", version="1.0"),
563+
)
564+
response = JSONRPCResponse(
565+
jsonrpc="2.0",
566+
id=1,
567+
result=init_result.model_dump(by_alias=True, exclude_none=True),
568+
)
569+
response_json = response.model_dump_json(by_alias=True, exclude_none=True)
570+
571+
# Create mock SSE events using httpx_sse's ServerSentEvent
572+
async def mock_aiter_sse() -> AsyncGenerator[ServerSentEvent, None]:
573+
# First: endpoint event
574+
yield ServerSentEvent(event="endpoint", data="/messages/?session_id=abc123")
575+
# Empty data keep-alive ping - this is what we're testing
576+
yield ServerSentEvent(event="message", data="")
577+
# Real JSON-RPC response
578+
yield ServerSentEvent(event="message", data=response_json)
579+
580+
mock_event_source = MagicMock()
581+
mock_event_source.aiter_sse.return_value = mock_aiter_sse()
582+
mock_event_source.response = MagicMock()
583+
mock_event_source.response.raise_for_status = MagicMock()
584+
585+
mock_aconnect_sse = MagicMock()
586+
mock_aconnect_sse.__aenter__ = AsyncMock(return_value=mock_event_source)
587+
mock_aconnect_sse.__aexit__ = AsyncMock(return_value=None)
588+
589+
mock_client = MagicMock()
590+
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
591+
mock_client.__aexit__ = AsyncMock(return_value=None)
592+
mock_client.post = AsyncMock(return_value=MagicMock(status_code=200, raise_for_status=MagicMock()))
593+
594+
with (
595+
patch("mcp.client.sse.create_mcp_http_client", return_value=mock_client),
596+
patch("mcp.client.sse.aconnect_sse", return_value=mock_aconnect_sse),
597+
):
598+
async with sse_client("http://test/sse") as (read_stream, _):
599+
# Read the message - should skip the empty one and get the real response
600+
msg = await read_stream.receive()
601+
# If we get here without error, the empty message was skipped successfully
602+
assert not isinstance(msg, Exception)
603+
assert isinstance(msg.message.root, types.JSONRPCResponse)
604+
assert msg.message.root.id == 1

0 commit comments

Comments
 (0)