Skip to content

test: roll back live router replay membership between tests - #36278

Merged
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_live_router_test_isolation
Aug 8, 2026
Merged

test: roll back live router replay membership between tests#36278
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_live_router_test_isolation

Conversation

@mateo-berri

@mateo-berri mateo-berri commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

How it solves it:

  • the autouse isolation fixture now also rolls back _live_routers membership
  • a second canary pair proves router membership no longer outlives its test

User Flow

Before: a contributor's PR touching nothing near pricing fails the proxy-infra required check on an unrelated test, and the automatic reruns fail the same way

  1. A contributor pushes a branch and opens a PR, e.g. perf(lint): schedule the lint fan-out around its long pole #36276 changing only pre-commit tooling
  2. On the PR's Checks tab, the required check "Unit Tests: Proxy Infrastructure / proxy-infra / Run tests" turns red after ~11 minutes
  3. Opening the job log at https://github.com/BerriAI/litellm/actions/runs/31251929962/job/93089535345, they see TestPriceDataReloadIntegration::test_distributed_reload_check_function failing on an assert that expected the two-field pricing entry {'input_cost_per_token': 0.001} and got a ~106-field dump of mostly None values, in a file their PR never touched
  4. The summary line shows 2 rerun, so both automatic retries hit the identical assert and the red is sticky for this run
  5. They click "Re-run failed jobs" and wait ~10 minutes; whether the fresh run goes green depends on which tests share a worker and on garbage collection timing, so they may burn several reruns before merging

After: the same PR's proxy-infra check passes, and the test no longer depends on which tests ran before it

  1. A contributor pushes a branch and opens a PR changing only pre-commit tooling
  2. On the PR's Checks tab, "Unit Tests: Proxy Infrastructure / proxy-infra / Run tests" turns green
  3. test_distributed_reload_check_function passes regardless of which other tests ran earlier in the same worker, so no reruns are needed for this failure

Relevant issues

Observed on PR #36276 (run 31251929962), whose head already contained #36039, so this is the second, previously unfixed half of the same #35491 replay pollution

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • 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

This PR changes only test isolation, no proxy runtime code, so there is no end-user request to curl: the observable behavior is the CI shard itself. The proof is a deterministic reproduction of the exact CI failure and its disappearance at the exact commits

Leg Commit Run Result
staging, polluted e24a914 polluter then flaky test FAIL
this PR, polluted 38c8e8d polluter then flaky test PASS
staging, regression pair e24a914 new canary pair FAIL
this PR, regression pair 38c8e8d new canary pair PASS

The reproduction plants a minimal polluter that does what many router tests already do, construct a Router with a gpt-3.5-turbo deployment, and holds it alive the way any module-level fixture or uncollected reference cycle would. PYTEST_XDIST_WORKER=gw0 mimics a CI xdist worker, and the polluter's function name sorts first because the conftest orders tests by bare name

cat > tests/test_litellm/proxy/test_aaa_repro_polluter.py <<'EOF'
from litellm import Router

_held_routers = []


def test_aaa_repro_polluter_creates_router():
    _held_routers.append(
        Router(
            model_list=[
                {
                    "model_name": "gpt-3.5-turbo",
                    "litellm_params": {"model": "gpt-3.5-turbo", "api_key": "sk-test"},
                }
            ]
        )
    )
EOF
PYTEST_XDIST_WORKER=gw0 .venv/bin/pytest tests/test_litellm/proxy/test_aaa_repro_polluter.py \
  "tests/test_litellm/proxy/test_proxy_server.py::TestPriceDataReloadIntegration::test_distributed_reload_check_function" \
  -q -p no:randomly

On staging (e24a914) this fails with the same assert diff as the CI logs, the sparse {"input_cost_per_token": 0.001} mock ballooned into a ~106-field mostly-None ModelInfo dict, and the captured log even shows the router's deployment re-registration firing inside the flaky test's reload call:

FAILED tests/test_litellm/proxy/test_proxy_server.py::TestPriceDataReloadIntegration::test_distributed_reload_check_function
1 failed, 1 passed, 1 warning in 1.75s

On this branch (38c8e8d) the identical sequence passes:

2 passed, 1 warning in 1.39s

The new canary pair in tests/test_litellm/test_conftest_isolation.py fails on staging at the rollback assert (1 failed, 3 passed) and passes here (4 passed). The full proxy-infra shard command from .github/workflows/test-unit-proxy-infra.yml (-n 2 --dist=loadscope --reruns 2) was also run on this branch: 6296 passed with the only failures being the 21 tests of test_semantic_tool_filter.py hitting a missing optional semantic_router package in the local venv, which fail identically on clean staging (e24a914) without this change

Type

✅ Test

Changes

tests/test_litellm/conftest.py: the autouse isolate_litellm_state fixture snapshots litellm.router._live_routers membership before each test and restores it on teardown. Since #35491 every Router joins that weak set at construction, and every cost map swap walks it to re-register each member's deployments on top of the freshly adopted map, so a router constructed by one test kept rewriting the model entries a later test swapped in for as long as the object stayed referenced or uncollected. Restoring membership rather than dropping the routers keeps fixture-provided routers working within their own test while stopping them from contributing to later tests' cost map rebuilds

tests/test_litellm/test_conftest_isolation.py: a second canary pair; the first test constructs a router, holds it in a module-level slot so it deliberately stays alive, and asserts it joined _live_routers, the second asserts the fixture removed it despite the object still being alive. On current staging the second test fails, with this PR both pass

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

Since #35491, every Router joins the module-global _live_routers weak set at
construction, and every model cost map swap replays the deployments of every
member on top of the freshly adopted map. #36039 isolated the register_model
ledger half of that replay but not this half: under pytest-xdist, a Router
created by an earlier test in the same worker that was still referenced (or
simply not yet garbage collected) re-registered its deployments during
TestPriceDataReloadIntegration::test_distributed_reload_check_function, and
register_model hydrated the sparse mocked gpt-3.5-turbo entry into a full
ModelInfo dict, failing the exact-equality assert (reruns cannot help since
the polluting router survives in the worker process)

The autouse isolate_litellm_state fixture now snapshots _live_routers before
each test and restores its membership on teardown, so a test's routers stop
contributing to cost map rebuilds once the test ends. A canary pair in
test_conftest_isolation.py asserts the rollback
@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends test-state isolation to restore live Router membership between tests and adds regression coverage for that behavior.

  • Snapshots and restores _live_routers in the autouse isolation fixture.
  • Adds canary tests covering Router membership during and after a test.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
tests/test_litellm/conftest.py Extends function-level LiteLLM state restoration to include the live Router weak-set membership.
tests/test_litellm/test_conftest_isolation.py Adds regression coverage demonstrating that Router membership is removed between tests.

Reviews (2): Last reviewed commit: "test: roll back live router replay membe..." | Re-trigger Greptile

Comment thread tests/test_litellm/test_conftest_isolation.py
@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri
mateo-berri merged commit f6df762 into litellm_internal_staging Aug 8, 2026
78 checks passed
@mateo-berri
mateo-berri deleted the litellm_live_router_test_isolation branch August 8, 2026 17:45
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.

2 participants