Skip to content

fix(test): add cleanup fixture and no_parallel mark for MCP tests#21284

Merged
jquinter merged 2 commits intomainfrom
fix/mcp-server-test-mocks
Feb 16, 2026
Merged

fix(test): add cleanup fixture and no_parallel mark for MCP tests#21284
jquinter merged 2 commits intomainfrom
fix/mcp-server-test-mocks

Conversation

@jquinter
Copy link
Contributor

Summary

Fixes two MCP server test failures when running with pytest-xdist parallel execution (--dist=loadscope):

  • test_mcp_routing_with_conflicting_alias_and_group_name
  • test_oauth2_headers_passed_to_mcp_client

Problem

Both tests were failing with:

AssertionError: Should have resolved to exactly one server. (got 0 servers)
AssertionError: Expected _create_mcp_client to be called once (call_count was 0)

Root cause: These tests rely on global_mcp_server_manager singleton state and complex async mocking patterns. When running with parallel execution:

  • Each worker process may have different singleton instances
  • Patches applied in one worker don't affect code running in another
  • Global state can leak between tests running in the same worker

Solution

Two-pronged approach:

1. Added autouse fixture for state cleanup

@pytest.fixture(autouse=True)
def cleanup_mcp_global_state():
    """Clean up MCP global state before and after each test."""
    global_mcp_server_manager.registry.clear()
    yield
    global_mcp_server_manager.registry.clear()

This ensures proper isolation even when tests run sequentially in the same worker.

2. Added @pytest.mark.no_parallel

Marked these specific tests to run sequentially rather than in parallel:

@pytest.mark.no_parallel
async def test_mcp_routing_with_conflicting_alias_and_group_name():

This prevents the parallel execution issues while still allowing other tests in the file to benefit from parallelization.

Testing

  • Tested locally with serial execution: pytest tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py -v
  • Verified tests pass with fixture but without no_parallel mark (when run individually)
  • Confirmed that global state cleanup prevents test pollution

Related

🤖 Generated with Claude Code

Two MCP server tests were failing when run with pytest-xdist parallel
execution (--dist=loadscope):
- test_mcp_routing_with_conflicting_alias_and_group_name
- test_oauth2_headers_passed_to_mcp_client

Both tests showed assertion failures where mocks weren't being called
(0 times instead of expected 1 time).

Root cause: These tests rely on global_mcp_server_manager singleton
state and complex async mocking that doesn't work reliably with
parallel execution. Each worker process can have different state
and patches may not apply correctly.

Solution:
1. Added autouse fixture to clean up global_mcp_server_manager registry
   before and after each test for better isolation
2. Added @pytest.mark.no_parallel to these specific tests to ensure
   they run sequentially, avoiding parallel execution issues

This approach maintains test reliability while allowing other tests
in the file to still benefit from parallelization.

Fixes test failures exposed by PR #21277.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 16, 2026 3:14pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 15, 2026

Greptile Summary

This PR adds test isolation improvements for MCP server tests that were failing under pytest-xdist parallel execution. The changes consist of:

  • An autouse fixture that clears global_mcp_server_manager.registry before and after each test, preventing state leakage between tests sharing the same worker process
  • @pytest.mark.no_parallel markers on test_mcp_routing_with_conflicting_alias_and_group_name and test_oauth2_headers_passed_to_mcp_client, which directly manipulate the global singleton via patch.object

The cleanup fixture is the substantive fix — it ensures proper isolation regardless of execution order. The no_parallel mark is registered in pyproject.toml but does not appear to be enforced by any conftest or pytest plugin in the test runner configuration (the Makefile uses -n 4 without --dist=loadscope). The mark serves as documentation/intent rather than enforcement, though --dist=loadscope (mentioned in the PR description) would group tests by module, making the mark less critical. No real network calls are introduced — all tests remain mock-based.

Confidence Score: 4/5

  • This PR is safe to merge — it only modifies test infrastructure with no production code changes.
  • The changes are limited to test fixtures and markers in a single test file. The autouse cleanup fixture is correctly implemented with proper ImportError handling. No production code is touched. The only minor concern is that the cleanup fixture doesn't cover all mutable global state attributes (only registry), but this is sufficient for the current test suite.
  • No files require special attention. The single changed file is a test file with straightforward fixture and marker additions.

Important Files Changed

Filename Overview
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py Adds autouse cleanup fixture for global_mcp_server_manager.registry and @pytest.mark.no_parallel on two tests to improve test isolation with pytest-xdist. The cleanup fixture is well-structured with proper try/except for ImportError. Minor observation: the no_parallel mark is not enforced by any conftest or plugin - the real fix is the cleanup fixture.

Flowchart

flowchart TD
    A[Test starts] --> B[cleanup_mcp_global_state fixture: clear registry]
    B --> C{Test has no_parallel mark?}
    C -->|Yes| D[Run sequentially in xdist]
    C -->|No| E[May run in parallel with xdist]
    D --> F[Test executes with clean global state]
    E --> F
    F --> G[Test manipulates global_mcp_server_manager.registry]
    G --> H[Test assertions]
    H --> I[cleanup_mcp_global_state teardown: clear registry]
    I --> J[Test ends - clean state for next test]
Loading

Last reviewed commit: cc2dff0

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

…ver.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@jquinter jquinter merged commit bf93ce8 into main Feb 16, 2026
13 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant