Skip to content

fix(ci): support CircleCI rerun failed tests for local_testing jobs - #26461

Merged
mateo-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_circleci_rerun
Apr 27, 2026
Merged

fix(ci): support CircleCI rerun failed tests for local_testing jobs#26461
mateo-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_circleci_rerun

Conversation

@mateo-berri

Copy link
Copy Markdown
Contributor

Relevant issues

CircleCI's "Rerun failed tests" feature has been broken for local_testing_part1, local_testing_part2, and litellm_router_testing jobs. Reruns collect 0 items and exit with code 123.

Pre-Submission checklist

  • I have Added testing in the tests/test_litellm/ directory — N/A, this is a CI config change with no runtime code impact
  • My PR passes all unit tests — no runtime code changed
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

Before (rerun output):

[DEBU] Received: tests.local_testing.test_router
4 workers [0 items]
============================ no tests ran in 2.74s =============================
Error: exit status 123

Debug line from the plugin: "if all tests are being run instead of only failed tests, ensure your JUnit XML has a file or classname attribute."

Root cause: The circleci-tests-plugin-cli plugin, when rerunning failed tests, reads the JUnit XML classname attribute (dot notation) and pipes it into xargs pytest. pytest receives tests.local_testing.test_router instead of tests/local_testing/test_router.py, collects zero items, and fails with exit 123.

Fix verification:

$ printf 'tests/local_testing/test_router.py\ntests.local_testing.test_router\n' \
    | awk '/\.py/ {print; next} {gsub(/\./, "/"); print $0 ".py"}'
tests/local_testing/test_router.py
tests/local_testing/test_router.py

Both the normal initial-run input (file paths) and the rerun input (dot notation) are normalized to the same file path.

Type

🐛 Bug Fix
🚄 Infrastructure

Changes

Add an awk preprocessor between the circleci tests run stdin stream and xargs pytest in the three jobs that use this pattern:

  • local_testing_part1
  • local_testing_part2
  • litellm_router_testing

The preprocessor:

  • Passes through any line containing .py unchanged (normal file-path input)
  • For lines without .py, replaces . with / and appends .py (converts tests.local_testing.test_routertests/local_testing/test_router.py)

Diff:

-              --command="xargs uv run --no-sync python -m pytest \
+              --command="awk '/\\.py/ {print; next} {gsub(/\\./, \"/\"); print \$0 \".py\"}' | xargs uv run --no-sync python -m pytest \

Applied identically in all three jobs. No change to initial-run behavior; only fixes selective rerun.

…erun

CircleCI's 'Rerun failed tests' feature passes test identifiers from the
JUnit XML classname attribute (dot notation, e.g.
'tests.local_testing.test_router') via stdin. pytest receives these paths
and collects 0 items, causing the rerun to exit 123 with no tests run.

Add an awk preprocessor before xargs that detects dot-notation module
paths and converts them to file paths (tests/local_testing/test_router.py).
File paths already containing '.py' are passed through unchanged.

Applied to all three jobs using the 'circleci tests run' + 'xargs pytest'
pattern: local_testing_part1, local_testing_part2, and the router test job.
@mateo-berri
mateo-berri requested review from a team and yuneng-berri April 24, 2026 23:37
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@veria-ai

veria-ai Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Low: CI-only change to CircleCI test rerun logic

This PR adds an awk transformation in .circleci/config.yml to convert CircleCI test class identifiers back to file paths for rerunning failed tests. No runtime code, authentication, or production configuration is affected.


Status: 0 open
Risk: 1/10

Posted by Veria AI · 2026-04-24T23:43:08.636Z

@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the CircleCI "Rerun failed tests" feature for local_testing_part1, local_testing_part2, and litellm_router_testing by inserting an awk preprocessor that normalises dot-notation classnames (e.g. tests.local_testing.test_router) emitted by the circleci-tests-plugin-cli into file paths (e.g. tests/local_testing/test_router.py) before they are piped to xargs pytest. The three-job change is applied identically and is a no-op for normal first-run execution where paths already contain .py.

Confidence Score: 5/5

Safe to merge — CI-only change with no runtime code impact and a well-reasoned fix for the selective rerun regression.

No P0/P1 issues found. The awk command correctly handles both normal file-path input and dot-notation rerun input, including the uppercase-class-name edge case via the sub guard. The fix is applied consistently across all three affected jobs. No runtime code is touched.

No files require special attention.

Important Files Changed

Filename Overview
.circleci/config.yml Adds an awk preprocessor before xargs pytest in three jobs (local_testing_part1, local_testing_part2, litellm_router_testing) to normalise CircleCI's dot-notation classnames to file paths on selective reruns; normal first-run paths pass through unchanged

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["circleci tests run stdin\n(split-by=timings output)"] --> B{Contains '.py'?}
    B -- Yes --> C["Pass through unchanged\n(e.g. tests/local_testing/test_router.py)"]
    B -- No --> D["sub: strip trailing .UppercaseSegment\n(removes test class name if present)"]
    D --> E["gsub: replace '.' with '/'"]
    E --> F["Append '.py'"]
    F --> G["Normalised file path\n(e.g. tests/local_testing/test_router.py)"]
    C --> H["xargs uv run python -m pytest"]
    G --> H
    style B fill:#f0f0f0,stroke:#333
    style H fill:#d4edda,stroke:#28a745
Loading

Reviews (2): Last reviewed commit: "fix(ci): strip trailing class segment fr..." | Re-trigger Greptile

Comment thread .circleci/config.yml Outdated
--split-by=timings \
--verbose \
--command="xargs uv run --no-sync python -m pytest \
--command="awk '/\\.py/ {print; next} {gsub(/\\./, \"/\"); print \$0 \".py\"}' | xargs uv run --no-sync python -m pytest \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Classnames with test class names will mis-convert

When a pytest test lives inside a class, pytest sets the JUnit XML classname to tests.local_testing.test_router.TestRouterClass (module + class). The awk preprocessor has no .py in that string, so it hits the gsub branch and produces tests/local_testing/test_router/TestRouterClass.py — a path that doesn't exist. If none of the three jobs use test classes this is a non-issue, but it's worth confirming. Consider stripping a trailing uppercase/class-name segment if classnames ever include one.

@mateo-berri mateo-berri Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been fixed

…test

Pytest tests inside a class produce JUnit XML classnames like
'tests.local_testing.test_file_types.TestFileConsts' (module + class).
The previous awk preprocessor would convert this to
'tests/local_testing/test_file_types/TestFileConsts.py', which doesn't
exist, causing pytest to collect 0 items on rerun.

Strip a trailing '.<UppercaseSegment>' before the dot-to-slash
conversion. Module path segments are lowercase (test files start with
'test_'), and the class name is the only segment beginning with an
uppercase letter, so this is unambiguous.

Verified affected files in tests/local_testing/: test_file_types.py
(TestFileConsts), test_gcs_cache_unit_tests.py, test_disk_cache_unit_tests.py,
test_docker_no_network_on_deploy.py, test_sagemaker_nova_integration.py,
test_cache_preset_key.py.
@mateo-berri
mateo-berri temporarily deployed to integration-postgres April 24, 2026 23:42 — with GitHub Actions Inactive
@mateo-berri
mateo-berri temporarily deployed to integration-postgres April 24, 2026 23:43 — with GitHub Actions Inactive
@mateo-berri
mateo-berri temporarily deployed to integration-postgres April 24, 2026 23:43 — with GitHub Actions Inactive
@mateo-berri
mateo-berri temporarily deployed to integration-postgres April 24, 2026 23:43 — with GitHub Actions Inactive
@mateo-berri
mateo-berri temporarily deployed to integration-postgres April 24, 2026 23:43 — with GitHub Actions Inactive

@ryan-crabbe-berri ryan-crabbe-berri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mateo-berri
mateo-berri merged commit 82dacfb into litellm_internal_staging Apr 27, 2026
98 of 100 checks passed
@mateo-berri
mateo-berri deleted the litellm_fix_circleci_rerun branch April 27, 2026 20:26
krrish-berri-2 added a commit to BerriAI/pr-review-agent-skills that referenced this pull request Apr 28, 2026
…ECI_TOKEN

Replace the script's direct CircleCI v1.1 HTTP fetch with a call to the
LiteLLM proxy's `circle_ci_mcp-get_build_failure_logs` MCP tool. The
proxy holds the CircleCI credential server-side, so deployments no
longer need a `CIRCLECI_TOKEN` in their environment — `LITELLM_API_BASE`
+ `LITELLM_API_KEY` (already required by the host bot for the LLM call)
are sufficient.

What changed in the script:

- New `_LiteLLMMcp` class (~50 lines) that POSTs `tools/call` to
  `{LITELLM_API_BASE}/mcp/` and parses the SSE-framed JSON-RPC reply.
  Stateless, no session id, returns None on any HTTP/parse error so
  callers degrade identically to the pre-MCP path.
- `_fetch_circleci_failure_log` now hands the GitHub status `target_url`
  straight to the MCP tool — the upstream CircleCI MCP server already
  parses all five CircleCI URL shapes (project / pipeline / workflow /
  job / legacy), so the local URL regexes and v1.1 fetcher are gone.
- `_MCP_TRUNCATION_PRELUDE_RE` strips the `<MCPTruncationWarning>...
  </MCPTruncationWarning>` block the upstream server prepends when its
  internal log slice cap fires, so the warning text doesn't crowd out
  actual log content in our `MAX_LOG_CHARS` tail window.
- `gather()` and `main()` now take `mcp` instead of `circleci_token`,
  and `main()` reads `LITELLM_API_BASE` + `LITELLM_API_KEY` instead of
  `CIRCLECI_TOKEN`. Missing vars print a friendly warning and the
  script exits 0 with no log tails spliced — same shape as today's
  CIRCLECI_TOKEN-unset path.

End-to-end verified on BerriAI/litellm#26461 (a real merged PR with
two failing CircleCI jobs at HEAD): both `ci/circleci:
litellm_router_testing` and `ci/circleci: llm_translation_testing`
get full 3KB log tails with the same shape the v1.1 path used to
produce — `test_router_caching_ttl` traceback for the first, the
OpenAI org-verification `model_not_found` error for the second.
Output JSON keys are byte-identical to the pre-MCP shape (no new
fields, no removed fields).

Docs (SKILL.md, README.md) updated to point at the new env vars
and explain that the proxy holds the CircleCI credential.

Made-with: Cursor
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
fix(ci): support CircleCI rerun failed tests for local_testing jobs
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.

4 participants