Skip to content

fix(mcp): update test mocks for renamed filter_server_ids_by_ip_with_info#22327

Merged
jquinter merged 1 commit intomainfrom
fix/mcp-test-mock-filter-method-name
Feb 28, 2026
Merged

fix(mcp): update test mocks for renamed filter_server_ids_by_ip_with_info#22327
jquinter merged 1 commit intomainfrom
fix/mcp-test-mock-filter-method-name

Conversation

@jquinter
Copy link
Collaborator

Summary

  • MCP tests in test_mcp_server.py were mocking the old method name filter_server_ids_by_ip but production code (server.py:774) calls filter_server_ids_by_ip_with_info which returns a (server_ids, blocked_count) tuple
  • The unmocked method on AsyncMock returned a coroutine instead of a tuple, causing TypeError: cannot unpack non-iterable coroutine object and ValueError: not enough values to unpack
  • Updated all 11 mock definitions to use the correct method name and return the expected (server_ids, 0) tuple

Fixes 3 failing tests on main:

  • test_get_tools_from_mcp_servers
  • test_filter_tools_by_disallowed_tools_integration
  • test_filter_tools_by_allowed_tools_integration

Test plan

  • All 3 previously failing tests now pass locally
  • CI MCP test suite passes

🤖 Generated with Claude Code

…th_info

Tests were mocking the old method name `filter_server_ids_by_ip` but production
code at server.py:774 calls `filter_server_ids_by_ip_with_info` which returns
a (server_ids, blocked_count) tuple. The unmocked method on AsyncMock returned
a coroutine, causing "cannot unpack non-iterable coroutine object" errors.

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

vercel bot commented Feb 28, 2026

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

Project Deployment Actions Updated (UTC)
litellm Building Building Preview, Comment Feb 28, 2026 1:37am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR fixes 3 failing MCP tests by updating mock definitions to match a renamed production method. The method filter_server_ids_by_ip was renamed to filter_server_ids_by_ip_with_info in mcp_server_manager.py, changing the return type from List[str] to Tuple[List[str], int]. The test file's 11 mock definitions were still using the old name and return format, causing TypeError and ValueError when the production code tried to unpack the tuple.

  • All 11 mock definitions updated from filter_server_ids_by_ip to filter_server_ids_by_ip_with_info
  • Mock return values updated from server_ids to (server_ids, 0) to match the new (filtered_ids, ip_blocked_count) tuple signature
  • No remaining references to the old method name in the test file
  • Changes are consistent with mocks in the newer test file at tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py

Confidence Score: 5/5

  • This PR is safe to merge — it only updates test mocks to match the already-renamed production method, with no production code changes.
  • The change is a mechanical find-and-replace of mock method names and return values in a single test file. The new mock signatures exactly match the production code's method signature. No production code is modified.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/mcp_tests/test_mcp_server.py Updates 11 mock definitions from old method name filter_server_ids_by_ip to renamed filter_server_ids_by_ip_with_info, with correct (server_ids, 0) tuple return value matching production code signature.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Test calls production code<br/>(server.py:777)"] --> B["global_mcp_server_manager<br/>.filter_server_ids_by_ip_with_info()"]
    B --> C{"Mock present?"}
    C -->|"Before fix: mocked as<br/>filter_server_ids_by_ip"| D["❌ Unmocked AsyncMock<br/>returns coroutine"]
    D --> E["TypeError: cannot unpack<br/>non-iterable coroutine object"]
    C -->|"After fix: mocked as<br/>filter_server_ids_by_ip_with_info"| F["✅ Returns (server_ids, 0)<br/>tuple as expected"]
    F --> G["Production code unpacks:<br/>ids, blocked = result"]
    G --> H["Test passes ✅"]
Loading

Last reviewed commit: cb4cfa1

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, no comments

Edit Code Review Agent Settings | Greptile

@jquinter
Copy link
Collaborator Author

The Ruff PLR0915 lint failures are not related to this PR. They are pre-existing on main. Fix PR: #22328

@jquinter
Copy link
Collaborator Author

The e2e test failure (test_independent_clients_no_shared_session) is not related to this PR. It fails on main too — the test gets "Error: User not allowed to call this tool." instead of "30", which is an MCP authorization issue in the e2e test setup, not a mock naming issue.

@jquinter
Copy link
Collaborator Author

The test_independent_clients_no_shared_session e2e failure is pre-existing on main (all 4 MCP e2e tests fail locally). Root cause: set_auth_context() uses a ContextVar set during the HTTP handler, but the MCP SessionManager spawns separate anyio.TaskGroup tasks for protocol messages like call_tool. The ContextVar doesn't propagate into these new tasks, so get_auth_context() returns empty/stale data → "User not allowed to call this tool". This needs a separate issue/fix for auth context propagation in the MCP session manager.

@jquinter
Copy link
Collaborator Author

The realtime streaming guardrail test failures (test_realtime_guardrail_blocks_prompt_injection, test_realtime_text_input_guardrail_blocks_and_returns_error) are also pre-existing on main — test assertions didn't account for the "voice the guardrail message" feature. Fix PR: #22332

@jquinter
Copy link
Collaborator Author

CI Failure: Anthropic pass-through test + Bedrock parallel_tool_calls

Both failures are pre-existing on main and unrelated to this PR's MCP test mock changes.

1. Bedrock test_parallel_tool_calls_newer_model_adds_disable_flag

Root cause: Commit 8565c70e53 (revert) removed parallel_tool_calls from map_openai_params, and the subsequent re-fix d0445e1e33 only restored the transform_request consumer but not the producer.
Fix PR: #22333

2. Anthropic test_anthropic_experimental_pass_through_messages_handler_dynamic_api_key_and_api_base_and_custom_values

Root cause: Commit 99c62ca40e removed "azure" from _RESPONSES_API_PROVIDERS, routing Azure models through litellm.completion instead of litellm.responses. The test was not updated to match.
Fix PR: #22334

@jquinter
Copy link
Collaborator Author

CI Failure: Schema migration check

The test_aaaasschema_migration_check failure is pre-existing on main and unrelated to this PR.

Root cause: PR #22271 added LiteLLM_ClaudeCodePluginTable to schema.prisma but did not include a corresponding migration file.

Fix PR: #22335

@jquinter
Copy link
Collaborator Author

@shin-bot-litellm evaluate this PR

@jquinter jquinter merged commit 8b50703 into main Feb 28, 2026
30 of 35 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