From 1e0217a80d0fadab254dff882cdf251dbf6b621e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 18:58:18 +0000 Subject: [PATCH] Fix a test that had been failing for two weeks, and run the suite in CI Adding the docs job yesterday was the first time anything in this repo ran a test. Pointing it at the whole suite turned up four failures on a clean checkout, and they were not the same kind of thing. Three were tests/test_span_tree.py and tests/test_integrations.py importing agentx.integrations.google_adk at call time. That module raises ImportError by design when google-adk is absent, so the tests failed instead of skipping - while their crewai, litellm, llamaindex and autogen neighbours in the same files guard the same situation with pytest.importorskip. They now do too. With the extra installed all three run and pass, so this skips what is genuinely not installed rather than hiding anything. The fourth was real. test_trace_tool_call_emits_real_child_span asserted the root trace carries no flat tool_calls list when a tool call became a real child span. That was true when the test was written on 2026-08-07; four days later 40c6f6e deliberately added the dual-write, because the engine's built-in "Tool failure" check and the dashboard's Tool quality column read the root's flat list and a failed trace_tool_call() was invisible to both. The behaviour is right and documented in the code; the assertion was stale, and had been failing ever since - unnoticed, because no CI ran it. So the assertion is corrected rather than the code, and corrected upward: it now pins the dual-write positively and pins it at exactly one entry. Removing the dual-write fails it, and writing a third copy fails it too - neither of which the old assertion could catch, since it only checked for absence. docs.yml becomes tests.yml and runs the whole suite; the old name stopped being true once it covered more than the docs. Optional-extra tests skip themselves, so the base install is green: 73 passed, 28 skipped without extras, 76 passed, 25 skipped with google-adk. Installing the extras to actually run those is a fair next step and a slower job, not a different one. Verified on both paths with the exact CI command, and the corrected assertion verified to fail against a tree with the dual-write removed and against one that writes three times. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WYi6akpDa8qfiojRed2cAE --- .github/workflows/docs.yml | 35 ----------------------------------- .github/workflows/tests.yml | 36 ++++++++++++++++++++++++++++++++++++ tests/test_integrations.py | 3 +++ tests/test_span_tree.py | 12 +++++++++++- 4 files changed, 50 insertions(+), 36 deletions(-) delete mode 100644 .github/workflows/docs.yml create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml deleted file mode 100644 index a3fc3cc..0000000 --- a/.github/workflows/docs.yml +++ /dev/null @@ -1,35 +0,0 @@ -# The docs are the only description of the SDK most people read, and nothing checked them: -# 0.6.36 consolidated grading configs into an LLM Judge Scorer and shipped with no mention of -# it anywhere in EVALUATIONS.md, TRACING.md, README.md or CICD_EVAL.md. release.yml only -# publishes, so there was no job that could have noticed. -# -# Scope is deliberate. This runs the checks that hold the documentation against the package, -# plus the unit tests for the surface it documents - both green on the base install. It is -# not the whole suite: tests/test_integrations.py and tests/test_span_tree.py need optional -# extras (google-adk) and fail on ImportError without them, so running everything here would -# ship a red badge that says nothing about the docs. Widening this to the full suite wants -# the extras installed first, and is its own change. -name: docs - -on: - push: - branches: [main] - pull_request: - workflow_dispatch: - -jobs: - docs: - runs-on: ubuntu-latest - timeout-minutes: 10 - strategy: - fail-fast: false - matrix: - # The version release.yml builds the published wheel with, and a current one. - python: ["3.9", "3.12"] - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python }} - - run: pip install -e ".[langchain]" pytest - - run: python -m pytest tests/test_docs_match_sdk.py tests/test_judge_scorers.py -q diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..af03368 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,36 @@ +# Until this workflow existed, nothing in the repo ran a test: release.yml only publishes. +# Two things had rotted quietly as a result - the docs never mentioned the LLM Judge Scorer +# surface 0.6.36 shipped, and tests/test_span_tree.py had been failing since 40c6f6e changed +# trace_tool_call()'s dual-write behaviour four days after the test was written. +# +# It runs the whole suite. Tests for optional extras (crewai, litellm, llamaindex, autogen, +# google-adk) skip themselves via pytest.importorskip when the extra is absent, so the base +# install is green - a skip is honest about not being covered here, where a failure would +# have drowned out real ones. Installing those extras to actually run them is a fair next +# step; it is a slower job, not a different one. +name: tests + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + tests: + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + # The version release.yml builds the published wheel with, and a current one. + python: ["3.9", "3.12"] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: pip install -e ".[langchain]" pytest + # tests/test_integration.py talks to the hosted API and skips itself without a key; + # no secret is passed here on purpose, so pull requests from forks behave the same. + - run: python -m pytest tests/ -q diff --git a/tests/test_integrations.py b/tests/test_integrations.py index baa1425..fe11dc9 100644 --- a/tests/test_integrations.py +++ b/tests/test_integrations.py @@ -46,6 +46,9 @@ def make_tracer() -> Tracer: # --------------------------------------------------------------------------- def test_adk_model_error_is_captured(): + # google-adk is an optional extra; skip like the crewai/litellm/llamaindex/autogen tests + # below rather than failing on ImportError (importing the module raises when it is absent). + pytest.importorskip("google.adk") from agentx.integrations.google_adk import AgentXADKPlugin tracer = make_tracer() diff --git a/tests/test_span_tree.py b/tests/test_span_tree.py index a9d32a7..2dc3218 100644 --- a/tests/test_span_tree.py +++ b/tests/test_span_tree.py @@ -486,6 +486,9 @@ def test_google_adk_emits_real_child_spans(): import asyncio import types + # google-adk is an optional extra; skip like every other integration test here rather + # than failing on ImportError (importing the module raises when it is absent). + pytest.importorskip("google.adk") from agentx.integrations.google_adk import AgentXADKPlugin tracer = make_tracer() @@ -529,6 +532,7 @@ def test_google_adk_model_error_is_captured(): import asyncio import types + pytest.importorskip("google.adk") from agentx.integrations.google_adk import AgentXADKPlugin tracer = make_tracer() @@ -581,7 +585,13 @@ def test_trace_tool_call_emits_real_child_span(): assert child["name"] == "policy_lookup" assert child["parent_span_id"] == root["span_id"] assert child["output"] == "digital purchases are final" - assert "tool_calls" not in root or root.get("tool_calls") in (None, []) + # ...and a summary of it also lands on the ROOT's flat tool_calls list. That dual-write is + # deliberate (40c6f6e): the child span feeds the trace detail's span tree, while the + # engine's built-in "Tool failure" check and the dashboard's Tool quality column read the + # root's flat list - before it, a failed trace_tool_call() was invisible to both surfaces. + # Asserted positively, and as exactly one entry, so a third write shows up here too. + assert [tc["name"] for tc in root["tool_calls"]] == ["policy_lookup"] + assert root["tool_calls"][0]["output"] == "digital purchases are final" def test_trace_retrieval_emits_real_child_span():