Skip to content

Test CI stability#2684

Closed
jlowin wants to merge 2 commits intorelease/2.xfrom
ci-test-flaky
Closed

Test CI stability#2684
jlowin wants to merge 2 commits intorelease/2.xfrom
ci-test-flaky

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 23, 2025

(due to issues with the release/2.x branch, this had to be deleted and replaced with #2696)


Uvicorn 0.39.0 introduced a fix for ContextVars pollution (encode/uvicorn#2742) that wraps ASGI tasks in empty contexts. This is a workaround for a CPython bug (python/cpython#140947) being fixed in Python 3.13+.

The context isolation breaks task cancellation propagation, causing test cleanup to hang when we task.cancel() and await task on uvicorn server tasks.

Fix: Use graceful shutdown (server.should_exit = True) for real uvicorn servers, and add timeouts for mocked server cleanup.

Files changed:

  • tests/server/test_logging.py - Add timeout to mocked server cleanup
  • tests/client/test_streamable_http.py - Use graceful shutdown for nested_server fixture
  • tests/client/test_sse.py - Use graceful shutdown for nested_sse_server fixture

@marvin-context-protocol marvin-context-protocol Bot added tests ignore in release notes Minor change for release notes. Use sparingly for meta PRs like workflow tests. labels Dec 23, 2025
@jlowin jlowin changed the base branch from main to release/2.x December 23, 2025 22:40
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: CI is failing due to pre-existing resource task test failures, not the Uvicorn 0.39+ cleanup fixes in this PR.

Root Cause: All tests in tests/client/tasks/test_client_resource_tasks.py are failing with McpError: 'document'. This failure exists on the base branch (release/2.x) and is unrelated to the changes in this PR. The 6 resource task tests account for the CI failures across all 5 workflow jobs.

Suggested Solution:

  1. Skip or fix the failing resource task tests - The error McpError: 'document' suggests a server-side issue with task-enabled resources. This needs investigation separate from this PR.

  2. Verify the PR changes work correctly - The actual changes in this PR (test cleanup for Uvicorn 0.39+) pass successfully:

    • tests/server/test_logging.py (3 tests)
    • tests/client/test_streamable_http.py (13 tests)
    • tests/client/test_sse.py (8 tests)

To proceed with this PR: Either fix the resource task tests or temporarily skip them with @pytest.mark.skip to unblock this important Uvicorn 0.39+ compatibility fix.

Detailed Analysis

Failing Tests (Pre-existing on base branch)

All 6 tests in tests/client/tasks/test_client_resource_tasks.py fail with:

mcp.shared.exceptions.McpError: 'document'

The error originates from the MCP SDK when the server returns a JSON-RPC error response. The error message is just the string 'document' (the resource function name), suggesting the server-side task execution for resources is broken.

Tests Modified by This PR (All Passing)

The PR correctly fixes test cleanup issues caused by Uvicorn 0.39.0's context isolation changes:

For real Uvicorn servers (test_sse.py, test_streamable_http.py):

  • Changed from: task.cancel() + catching CancelledError
  • Changed to: server.should_exit = True + await task
  • ✅ This is the correct approach - Uvicorn 0.39+ wraps ASGI tasks in empty contexts to work around a CPython bug, which breaks cancellation propagation

For mocked servers (test_logging.py):

  • Added: asyncio.wait_for(task, timeout=2.0) with timeout protection
  • Added: Signal to finish before canceling
  • ✅ This prevents hangs when mocks don't respond to cancellation

Local Test Results

$ uv run pytest tests/server/test_logging.py tests/client/test_streamable_http.py tests/client/test_sse.py -v
======================== 22 passed, 1 xfailed in 10.35s ========================

All tests modified by this PR pass successfully on Linux.

Related Files

Modified by PR (all passing):

  • tests/server/test_logging.py - Mocked Uvicorn server cleanup
  • tests/client/test_streamable_http.py - Real Uvicorn server cleanup for HTTP transport
  • tests/client/test_sse.py - Real Uvicorn server cleanup for SSE transport

Failing (pre-existing issue):

  • tests/client/tasks/test_client_resource_tasks.py - Resource task execution broken
  • src/fastmcp/server/resources/ - Likely source of the McpError issue
  • src/fastmcp/client/client.py - Resource task client methods

@jlowin jlowin deleted the branch release/2.x December 23, 2025 22:59
@jlowin jlowin closed this Dec 23, 2025
@jlowin jlowin deleted the ci-test-flaky branch December 23, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore in release notes Minor change for release notes. Use sparingly for meta PRs like workflow tests. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant