test: route the cost-calculator tests through a local-cost-map fixture - #37836
test: route the cost-calculator tests through a local-cost-map fixture#37836yuneng-berri wants to merge 1 commit into
Conversation
28 tests in test_cost_calculator.py set LITELLM_LOCAL_MODEL_COST_MAP and swapped litellm.model_cost inline and never put either back, so anything that ran after them in the same process read the checked-in snapshot instead of the live map. They now request a fixture that does both through monkeypatch, which restores on teardown including when the test fails. Two tests that already took monkeypatch for the environment variable but still assigned model_cost by hand go through setattr for the same reason. Dropping the sys.path.insert takes the last reader of os and sys with it. It resolved "../.." against the working directory, so it only ever pointed at the repo root when pytest happened to be invoked from this test's own folder.
Greptile SummaryThis PR isolates cost-calculator tests by loading the local pricing snapshot through a monkeypatch-backed fixture, ensuring environment and
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified. The local-map environment and module attribute are now restored by pytest teardown, the local-map branch avoids network access, and supported test collection does not depend on the removed per-file path modification.
|
| Filename | Overview |
|---|---|
| tests/test_litellm/test_cost_calculator.py | Consolidates repeated local cost-map setup into an automatically restored fixture without changing test assertions or introducing network access. |
| test-quality-budget.json | Lowers test-quality limits to account for removed direct environment writes, global assignments, and path manipulation. |
Reviews (1): Last reviewed commit: "test: route the cost-calculator tests th..." | Re-trigger Greptile
|
Superseded by #37815, which already covers this file and more |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TLDR
Problem this solves:
LITELLM_LOCAL_MODEL_COST_MAPset session-widelitellm.model_costswapped to the local snapshotHow it solves it:
_local_model_cost_mapfixturemonkeypatch, so a failing test still restoressys.path.insertthat resolved against the working directoryUser Flow
Before: a proxy admin reports a cost discrepancy, and the maintainer checking it locally reads a stale price
litellm.model_costand gets the checked-in snapshot, not the live mapAfter: the same run reads the live map, so the answer matches the proxy
LITELLM_LOCAL_MODEL_COST_MAPandlitellm.model_costback when each one finishesRelevant issues
Linear ticket
Pre-Submission checklist
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
The leak is only visible from outside the pytest process, so the probe runs the file in-process and reads the environment back afterwards.
LITELLM_LOCAL_MODEL_COST_MAPis not part of the conftest snapshot, so nothing else restores it.Before (ff02d5c)
python leakprobe.py89 passed, thenLITELLM_LOCAL_MODEL_COST_MAP after: 'True'After (cff2af5)
python leakprobe.py89 passed, thenLITELLM_LOCAL_MODEL_COST_MAP after: NoneType
🧹 Refactoring
✅ Test
Caveats (if any)
litellm.model_costrestores via the conftest snapshot either wayFinal Attestation