Run google_genai MCP tests in-memory and close pooled connections - #1836
Merged
Conversation
The MCP tests spawned the FastMCP echo server as a stdio subprocess for every test and never shut the pooled connection down: the autouse fixture only cleared the pool dict, so each test left a ~67 MB server process (and its owner task) alive for the rest of the pytest process. On macOS CI the spawn itself also stalled: two timed-out runs show the pytest main thread inside _posixsubprocess.fork_exec for ~65s while forking the large test process, freezing the worker's event loop until the workflow deadline passed (test_mcp_side_effects, test_mcp_full_schema_propagation). Connect the pooling, schema, replay and side-effect tests to the same echo server over mcp.shared.memory streams instead; they exercise the connection pool and activity scheduling, not the transport. Keep the stdio subprocess for test_mcp_tool_discovery_and_call, and have the fixture evict (close) leftover connections so no server outlives its test.
There was a problem hiding this comment.
🟢 Approval recommended
The focused test infrastructure changes correctly address subprocess overhead and connection leaks.
Pull request overview
Updates Google GenAI MCP tests to reduce resource usage and prevent leaked echo-server connections.
Changes:
- Uses in-memory MCP sessions for transport-independent tests.
- Retains stdio coverage for discovery.
- Properly evicts pooled connections during teardown.
File summaries
| File | Description |
|---|---|
tests/contrib/google_genai/test_gemini_mcp.py |
Splits session transports and closes pooled connections. |
tests/contrib/google_genai/echo_mcp_server.py |
Documents stdio and in-memory usage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JasonSteving99
approved these changes
Sep 10, 2026
tconley1428
approved these changes
Sep 10, 2026
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
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.
What was changed
The pooling, schema, replay, side-effect and test-server MCP tests connect to the echo server over in-memory MCP sessions;
test_mcp_tool_discovery_and_callkeeps the stdio subprocess. The connection fixture closes leftover pool entries instead of only clearing the dict.Why
Two CI timeouts placed the main thread inside
fork_execwhile the stdio client spawned the echo server, blocking the event loop past the workflow deadline. Fork latency scales with heap size, and the suite leaked one ~67MB echo server per test because the fixture never closed them.Testing
Live echo servers during the suite: 1 to 6 before, 0 after. MCP suite x30 under load: 240/240, 121s before, 72s after. Lint clean.