Skip to content

test: route the cost-calc tests through the local-cost-map fixture - #37835

Closed
yuneng-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_cost_calc_test_env_isolation
Closed

test: route the cost-calc tests through the local-cost-map fixture#37835
yuneng-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_cost_calc_test_env_isolation

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • 45 cost-calc tests leave LITELLM_LOCAL_MODEL_COST_MAP set session-wide
  • they also leave litellm.model_cost swapped to the local snapshot
  • the file's own fixture already did this correctly, unused by them

How it solves it:

  • those 45 tests request the existing _local_model_cost_map fixture
  • the fixture moves to monkeypatch, so setup failures restore too
  • drops a sys.path.insert that resolved against the working directory

User Flow

Before: a proxy admin reports a cost discrepancy, and the maintainer checking it locally reads a stale price

  1. The maintainer runs the unit tier in one process, cost-calc tests included
  2. A later test in that process reads litellm.model_cost and gets the checked-in snapshot, not the live map
  3. Any price quoted from that run reflects the snapshot, so the answer sent back can disagree with what the proxy actually charged

After: the same run reads the live map, so the answer matches the proxy

  1. The maintainer runs the same unit tier in one process, cost-calc tests included
  2. The cost-calc tests put LITELLM_LOCAL_MODEL_COST_MAP and litellm.model_cost back when each one finishes
  3. A later test reads the live map, so a quoted price matches what the proxy charged

Relevant issues

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. 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
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to 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_MAP is not part of the conftest snapshot, so nothing else restores it.

# leakprobe.py
import os, pytest
os.environ.pop("LITELLM_LOCAL_MODEL_COST_MAP", None)
pytest.main(["tests/test_litellm/litellm_core_utils/llm_cost_calc/test_llm_cost_calc_utils.py", "-q", "-p", "no:randomly"])
print("LITELLM_LOCAL_MODEL_COST_MAP after:", repr(os.environ.get("LITELLM_LOCAL_MODEL_COST_MAP")))

Before (ff02d5c)

  1. python leakprobe.py
  2. 174 passed, then LITELLM_LOCAL_MODEL_COST_MAP after: 'True'

After (7c1c800)

  1. python leakprobe.py
  2. 174 passed, then LITELLM_LOCAL_MODEL_COST_MAP after: None

Type

🧹 Refactoring
✅ Test

Caveats (if any)

  • litellm.model_cost restores via the conftest snapshot either way

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

45 tests in test_llm_cost_calc_utils.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. The file already had a _local_model_cost_map fixture doing this
correctly for the data-residency tests; those 45 now request it too.

The fixture itself moves from a hand-rolled try/finally to monkeypatch, which
also restores when setup raises rather than only when the test body finishes.

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-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR routes cost-calculation tests through the existing monkeypatch-based local-cost-map fixture so environment and global pricing state are restored after each test.

  • Replaces repeated environment and litellm.model_cost assignments with _local_model_cost_map.
  • Removes a working-directory-dependent sys.path modification.
  • Ratchets the corresponding test-quality budgets downward.

Confidence Score: 5/5

The PR appears safe to merge, with the test-state cleanup correctly scoped and no actionable regressions identified.

The fixture installs a fresh local cost map for each requesting test and relies on pytest monkeypatch teardown to restore both the environment and the original module attribute; the removed path insertion was not load-bearing under supported test invocation.

Important Files Changed

Filename Overview
tests/test_litellm/litellm_core_utils/llm_cost_calc/test_llm_cost_calc_utils.py Consolidates local cost-map setup into a monkeypatch fixture that restores environment and module state without weakening the existing assertions.
test-quality-budget.json Ratchets the path-mutation, environment-mutation, and module-global-mutation budgets to reflect violations removed by the test refactor.

Reviews (1): Last reviewed commit: "test: route the cost-calc tests through ..." | Re-trigger Greptile

@yuneng-berri

Copy link
Copy Markdown
Contributor Author

Superseded by #37815, which already covers this file and more

@yuneng-berri
yuneng-berri deleted the litellm_cost_calc_test_env_isolation branch August 21, 2026 12:57
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

1 participant