Fix under-specified sample dependencies and stale test/CI wiring - #349
Open
brianstrauch wants to merge 4 commits into
Open
Fix under-specified sample dependencies and stale test/CI wiring#349brianstrauch wants to merge 4 commits into
brianstrauch wants to merge 4 commits into
Conversation
Both groups relied on sibling groups to supply or constrain mcp, so each was broken when resolved on its own. google-adk never declared mcp at all, even though google_adk_agents imports McpToolset. google.adk.tools.mcp_tool imports it inside a try/except ImportError, so a missing mcp does not raise -- the symbol is simply never exported and the sample's toolsets silently disappear. google-genai declared "mcp>=1.0.0" while the sample imports mcp.shared.session, which mcp 2.0 removed. Nothing in that group caps mcp, so on its own the declaration selects 2.0.0. Also upgrades the lock from mcp 1.27.1 to 1.29.0. It sat at 1.27.1 only because uv does not upgrade already-locked packages; no constraint required it. Verified each group in a pruned environment (uv run --exact --no-default-groups --group ...), and on mcp 1.29.0 the full suite collects 150 tests as before, with 148 passed, 4 skipped and mypy clean. The uv.lock diff also picks up pre-existing drift: pyproject declares temporalio[langsmith]>=1.31.0 for the deepagents group while the committed lock recorded >=1.30.0, so every uv run produced a spurious lock diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lock sat at 2.6.2 only because uv does not upgrade already-locked packages. 2.6.3 is compatible: google_adk_agents tests pass (6 passed), the full suite collects 150 tests with 148 passed and 4 skipped, and mypy is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The step was gated on `matrix.python == '3.12'`, which was correct when written but stopped matching in a5ee599 when the matrix moved to 3.10/3.13. It has not run since July 2025, across two matrix bumps. Rather than hardcode another version, drop the condition so the step runs on every matrix entry and cannot silently die on the next bump. Verified locally on both ends of the matrix, 3.10 and 3.14: the workflow completes and returns "Hello, Temporal!". The test is self-contained -- it starts its own WorkflowEnvironment -- so it needs no extra CI setup. Also moves the step above the pydantic downgrade, which is documented as needing to remain last: `uv sync --group gevent` re-syncs the environment from the lock and would undo the pinned pydantic 1.10. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four unrelated papercuts: - Add the llm-stream dependency group. workflow_streams/README.md tells readers to run `uv sync --group llm-stream` for scenario 5, but the group was never created, so the command fails. The sample's openai import resolves today only because the langfuse-tracing and langsmith-tracing groups happen to install openai. - Rename tests/activity_sticky_queues to tests/worker_specific_task_queues, and drop the stale prefix from the two test filenames. The sample was renamed but its test directory was not, so the tests sat under a directory naming a sample that no longer exists. - Locate google_genai's echo MCP server through the installed package rather than Path(__file__).parents[2], which hardcodes the distance from the test file to the sample and silently resolves to a nonexistent path if either moves. A missing server script makes the test hang rather than fail, since the client waits on a subprocess that never started. - Add __init__.py to the five test directories that lacked it. Test filenames collide across samples by design (workflow_test.py, activity_test.py and mcp_test.py each appear in several), and package markers are what keep pytest from treating them as the same module. The five worked only because they had not collided yet. Full suite unchanged at 150 collected, 148 passed, 4 skipped; mypy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
brianstrauch
enabled auto-merge (squash)
August 13, 2026 20:51
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.
Summary
Four independent fixes. Three share a root cause: a sample's dependencies are satisfied only because another group happens to install them, so the sample breaks the moment its own group is used in isolation. The repo's single shared resolution hides this.
google-adknever declaredmcp.google_adk_agentsimportsMcpToolset, butgoogle.adk.tools.mcp_toolimports it inside atry/except ImportError. A missingmcptherefore doesn't raise — the symbol is simply never exported and the sample's toolsets silently disappear. It works today only becausegoogle-genaiandstrands-agentspullmcpin.google-genaideclaredmcp>=1.0.0while the sample importsmcp.shared.session, which mcp 2.0 removed. Nothing in that group caps mcp, so on its own the declaration selects 2.0.0. Demonstrated with a scratch project holding only that group's dependencies:Added the
llm-streamgroup.workflow_streams/README.mdtells readers to runuv sync --group llm-streamfor scenario 5, but the group was never created, so the command fails. That sample'sopenaiimport resolves today only via the langfuse/langsmith groups.The gevent test hasn't run in CI since July 2025. It was gated on
matrix.python == '3.12', which stopped matching whena5ee599moved the matrix to 3.10/3.13 — two matrix bumps ago. The gate was originally!= '3.7'(gevent didn't support 3.7); once 3.7 was dropped the guard was obsolete, but it was rewritten in positive form as== '3.12'and relabelled "on latest", which is what let it rot. Dropping the condition restores the original intent and can't go stale again.Also included
uv lockdoesn't upgrade already-locked packages; nothing constrained them.tests/activity_sticky_queues/→tests/worker_specific_task_queues/. The sample was renamed long ago; its test directory named a sample that no longer exists.tests/google_genai/mcp_test.pylocates its echo MCP server through the installed package instead ofPath(__file__).parents[2], which hardcodes the distance from test to sample. A wrong path here makes the test hang rather than fail, since the client waits on a subprocess that never started.__init__.pyadded to the 10 test directories that lacked one. Test filenames collide across samples by design (workflow_test.py,activity_test.pyandmcp_test.pyeach appear in several), and package markers are what keep pytest from treating them as the same module. The ones without a marker worked only because they hadn't collided yet — and the rename above adds exactly such a filename.Test plan
poe test: 150 collected, 148 passed, 4 skipped — collected count identical to beforepoe lint: cleanuv run --exact --no-default-groups --group <g>), which is what surfaced the missing/loose declarationsgoogle_genai,google_adk_agents,strands_plugin,openai_agents) run together on mcp 1.29.0: 24 passedgevent_async/test/run_combined.pyrun on both ends of the matrix, 3.10 and 3.14One thing CI will exercise that I couldn't check locally: the gevent step now runs on all 8 matrix jobs including
windows-latest, where it hasn't actually executed since July 2025. If it's unhappy there, gating onrunner.os != 'Windows'is the obvious fallback.🤖 Generated with Claude Code