Repository navigation
ci(test-unit): drop dead misc shard paths and skip missing paths with a warning - #42603
Conversation
… a warning Eight directories the misc shard named moved to tests/unit on 2026-09-20, and one missing path makes pytest-xdist collect [0 items] for the whole shard, which the exit-5 tolerance turned into a green required check running nothing. The shared Run tests step now drops a path that does not exist with a ::warning:: and runs pytest over the rest, keeping option tokens verbatim.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _SHARD_ENV: Final = MappingProxyType( | ||
| {"MAX_FAILURES": "10", "RERUNS": "0", "DIST": "loadscope", "TEST_TIMEOUT_SECONDS": "60", "COVERAGE_CORE": "sysmon"} | ||
| ) |
There was a problem hiding this comment.
The _SHARD_ENV mapping exceeds the repository's 120-character limit for Python lines. This explicit repository requirement must be satisfied before merging.
| _SHARD_ENV: Final = MappingProxyType( | |
| {"MAX_FAILURES": "10", "RERUNS": "0", "DIST": "loadscope", "TEST_TIMEOUT_SECONDS": "60", "COVERAGE_CORE": "sysmon"} | |
| ) | |
| _SHARD_ENV: Final = MappingProxyType( | |
| { | |
| "MAX_FAILURES": "10", | |
| "RERUNS": "0", | |
| "DIST": "loadscope", | |
| "TEST_TIMEOUT_SECONDS": "60", | |
| "COVERAGE_CORE": "sysmon", | |
| } | |
| ) |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
That line is 119 characters, under the 120-character limit in ruff.toml, and ruff's E501 check passes on this file, so nothing exceeds the repository limit
There was a problem hiding this comment.
You're right — that line is 119 characters, and ruff.toml sets the limit to 120. My line-length finding was incorrect; no change is needed.
|
bugbot run |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8c8c6e3. Configure here.
…e failure-hook regression (#42646) * test(utils): raise the post-success hook error from a guardrail in the failure-hook regression Since #42535 a plain logger raising inside async_post_call_success_deployment_hook is logged and the completed request returns, so the regression added by #36657 for "a post-success error never reaches async_post_call_failure_deployment_hook" failed with DID NOT RAISE on every main run once #42603 revived the misc unit shard. The raising callback is now a CustomGuardrail, the one kind of callback whose post-success raise still propagates, which keeps the original assertions intact * test(utils): type the guardrail's success-hook request_data as a Mapping --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
2 participants
TLDR
Problem this solves:
miscunit shard named eight test directories that moved totests/uniton 2026-09-20[0 items]for the whole shardHow it solves it:
miscshard::warning::naming it--ignore=...in a test-path pass through untouchedUser Flow
Before: a contributor breaks a test under
tests/test_litellm/passthrough, and themiscrequired check on their PR reports green after running nothingmisc / Run testsgreen after about two minutes2 workers [0 items],no tests ran in 32.53s, thenpytest collected no tests from tests/test_litellm/batches ...; passingAfter: the same PR's
misccheck runs the shard's 6,549 tests and goes red on the broken onemisc / Run testsrunning for about ten minutes2 workers [6549 items]and the broken test listed asFAILED, so the check is redWarningannotation naming that path and the rest of the shard still runsRelevant issues
Affected release
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Both runs use the
miscshard's ownRun testsstep from.github/workflows/_test-unit-base.ymlwith the shard's inputs from.github/workflows/test-unit.yml(workers: 2,dist: loadscope, the base workflow's defaults for the rest), executed the way GitHub Actions runs it (bash --noprofile --norc -eo pipefail) from the repo rootBefore (d31e8aa)
The shard's own pytest command at the main tip
Ran the step's pytest command over the shard's 26 test-path tokens with
-n 2 --dist=loadscope:Observed:
No
file or directory not foundline anywhere in the output. The step maps exit 5 to exit 0, which is what themiscjob on main did in https://github.com/BerriAI/litellm/actions/runs/35783099007/job/106933188388 (pytest collected no tests from tests/test_litellm/batches ...; passing)The same shard over only the 18 tokens that exist
uv run --no-sync pytest <the 17 existing directories> 'tests/test_litellm/test_*.py' --co -q6542 tests collected in 26.66s, so that is what the shard should have been runningAfter (8c8c6e3)
This PR's own
miscjobOpened https://github.com/BerriAI/litellm/actions/runs/35791407344/job/106960507669 (the PR's run at its merge ref) and read the
Run testsstepObserved:
The shard now runs its tests. At 8c8c6e3 the check was red on six tests that
mainhad been failing since the shard stopped running on 2026-09-20; none of them touched a file in this diff, so each was fixed onmainby its own PR (test(utils): accept the per-size image cost keys in the price-map schema check #42612, test(cost_calculator): point image-generation deployment price test at a live gemini row #42615, fix: drop duplicate cache_read_input_token_cost_batches keys from the price maps #42623, and test(realtime): drop legacy InvalidStatusCode tests and pin websockets imports #42624). After updating this branch frommainwith a merge commit (9637a82, the diff againstmainunchanged) the check is green at https://github.com/BerriAI/litellm/actions/runs/35803562002/job/106999166103:[6548 items]and6523 passed, 25 skipped, 505 warnings in 444.33s (0:07:24)The shard's own step script at the PR tip
Ran the tip's
Run testsstep script from the repo root with the shard's inputs (WORKERS=2 DIST=loadscope MAX_FAILURES=10 RERUNS=2 TEST_TIMEOUT_SECONDS=120) andTEST_PATHset to themiscshard's test-path from this branch, underbash --noprofile --norc -eo pipefailObserved:
The same six failures as the job above plus two that only fail on this laptop without outbound network (
test_main.py::test_openai_env_base[OPENAI_API_BASE]withOpenAIException - Connection error, andtest_router_per_deployment_num_retries.py::TestNoProviderRetryAmplification::test_direct_completion_still_forwards_num_retries_to_providerwithassert 3 > (2 + 1)); both passed in the job aboveThe step script with a stale path still in TEST_PATH
Ran the same script with
TEST_PATH="tests/test_litellm/models tests/test_litellm/rerank_api"(tests/test_litellm/modelsno longer exists)Observed:
The missing path is dropped with a warning and the ten
rerank_apitests run. At the merge base the sameTEST_PATHgives2 workers [0 items]and exit 5, mapped to a green stepEvery other shard that runs through the changed step
Both callers of
_test-unit-base.yml(test-unit.ymlandtest-unit-proxy-db.yml) ran the changed step on this PR. Item counts and results per shard, the base run beingmainat the merge base (https://github.com/BerriAI/litellm/actions/runs/35788783637 and https://github.com/BerriAI/litellm/actions/runs/35788783796) and the head run this PR's (https://github.com/BerriAI/litellm/actions/runs/35791407344 and https://github.com/BerriAI/litellm/actions/runs/35791407319):The few item-count differences outside
misccome from the commitsmaingained between the two runs, not from the step: no shard collected fewer tests, none printed a::warning::, and none hit thenothing to runexitType
🚄 Infrastructure
Caveats (if any)
The Medium caveat that stood at 8c8c6e3 (the required
misc / Run testscheck red on six testsmainhad been failing since 2026-09-20) no longer holds: the fixes landed onmainas #42612, #42615, #42623, and #42624, this branch was updated frommainwith a merge commit (9637a82), and the check is green at https://github.com/BerriAI/litellm/actions/runs/35803562002/job/106999166103Low
tests/unitstays green without a.github/workflowsedit (two approvals), which is what ci(tests): wire tests/unit into CircleCI and keep draining GHA shards green #42103 set out to keep.github/scripts/assert_ci_coverage.pystill does not flag a shard entry that points nowhere, so the warning is the only signalFinal Attestation
Note
Medium Risk
Changes required CI test execution for a large shard (~6.5k tests); behavior is improved but stale paths only warn and do not fail the check.
Overview
Fixes the
miscunit-test shard silently passing while running no tests when staletest-pathentries made pytest-xdist collect zero items (exit 5, treated as success).The reusable
Run testsstep in_test-unit-base.ymlnow tokenizesTEST_PATH, passes through pytest flags (-*), drops missing directories with a::warning::annotation, and only skips the job when every path is gone. Pytest is invoked with the filtered argument list instead of the raw shard string.test-unit.ymlremoves eight directories from themiscmatrix entry that no longer exist undertests/test_litellm/(they had moved totests/unit).Adds
test_unit_shard_missing_paths.py, which loads the real workflow bash snippet and asserts missing paths are warned while remaining paths still execute (serial and xdist), and that--ignore=...is not mistaken for a path.Reviewed by Cursor Bugbot for commit 8c8c6e3. Bugbot is set up for automated code reviews on this repo. Configure here.