fix(tests): don't stomp real mcp with MagicMocks when mcp is installed#309
Open
breedx wants to merge 2 commits intompfaffenberger:mainfrom
Open
fix(tests): don't stomp real mcp with MagicMocks when mcp is installed#309breedx wants to merge 2 commits intompfaffenberger:mainfrom
breedx wants to merge 2 commits intompfaffenberger:mainfrom
Conversation
tests/plugins/conftest.py unconditionally installed MagicMocks into
sys.modules['mcp'], 'mcp.client', and 'mcp.client.session' whenever
'mcp' wasn't already imported. Real mcp is in the dev env, but pytest
starts with it not yet imported, so the mock path always won. Later,
when tests like tests/agents/test_compaction.py transitively imported
pydantic_ai.mcp, its 'from mcp.client.sse import sse_client' blew up
with:
ModuleNotFoundError: No module named 'mcp.client.sse';
'mcp.client' is not a package
because the installed MagicMock had no __path__.
Two-file pytest invocations spanning tests/plugins/* plus any module
that touches pydantic_ai.mcp would fail collection. Single-file runs
of either worked because tests/plugins/conftest.py wasn't loaded.
Fix: try importing real mcp first, only install the mocks when the
import fails. Real dev envs skip the mock entirely; bare CI images
without mcp still get the compatibility shim.
CI installs unpinned ruff, which now wraps the negated isinstance lambda differently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Running any two test files together where one is under `tests/plugins/` and another transitively imports `pydantic_ai.mcp` fails during collection:
```
ModuleNotFoundError: No module named 'mcp.client.sse'; 'mcp.client' is not a package
```
Reproducer (requires `pydantic-ai-slim[mcp]` to be installed — it is, via the main dependency):
```bash
uv run pytest tests/agents/test_compaction.py tests/plugins/test_azure_foundry.py -q
→ ERROR at collection, 1 error
```
Run either file on its own and it passes.
Root cause
`tests/plugins/conftest.py::pytest_configure` unconditionally installs `MagicMock`s into `sys.modules` for `mcp`, `mcp.client`, and `mcp.client.session` whenever `"mcp" not in sys.modules`. Pytest starts with `mcp` not yet imported, so the mock path always wins — even when the real `mcp` package is installed.
Then when collection reaches a test module that transitively imports `pydantic_ai.mcp` (e.g. via `code_puppy.agents._builder` → `pydantic_ai.durable_exec.dbos`), the line `from mcp.client.sse import sse_client` tries to look up `.sse` as a submodule of the MagicMock-`mcp.client`, which has no `path`, and Python raises "mcp.client is not a package". That gets re-raised as the helpful "please install mcp" error.
Single-file runs of either affected test worked because `tests/plugins/conftest.py` wasn't loaded for them.
Fix
Probe with a real import of `mcp` / `mcp.types` first. Only install the mocks if the real import fails. Real dev envs (including this repo's own) skip the mocks entirely; bare CI images without `mcp` still get the compatibility shim.
Test plan