test(proxy): make model_info endpoint tests hermetic to kill an order/merge-skew flake - #34037
Conversation
…/merge-skew flake The model_info / get_model_info_with_id endpoint tests drove refactored endpoints with bare, unspec'd MagicMock routers and models. Because the mocks were unspec'd, any attribute or method the (refactored) endpoints newly read auto-materialized a child MagicMock, and whether that child was reached depended on process-global state (premium_user, and the real get_available_models_for_user chain reading litellm globals) that sibling tests in the same xdist worker mutate. When reached, the MagicMock either unpacked to empty (a, b = mock.method() -> 'not enough values to unpack (expected 2, got 0)') or leaked into RouterModelInfo(**model_info) and failed Pydantic str validation. Pass in isolation, fail under xdist. The original TestModelInfoEndpoint failure (#33807 CI) was the same class surfaced by merge skew: #33721 added a get_configured_token_limits unpack to create_model_info_response, and CI's merge commit ran that against the un-updated bare-mock test before the #33742 band-aid landed. Fix (test-only, no product change): - TestModelInfoEndpoint: mock the real seam (get_available_models_for_user), configure the router methods the endpoint actually calls, return a real Deployment, and drop the dead proxy_server.get_key_models/get_team_models/ get_complete_model_list patches the refactor had stranded. - TestGetModelInfoWithIdBlocked: spec the model mock so unset enterprise columns read as None instead of child MagicMocks. - test_ProxyConfig_get_model_info_with_id_missing_model_id_raises: pin premium_user so the asserted AttributeError no longer flips with the ambient license global.
Greptile SummaryThis test-only PR makes three flaky tests hermetic by fixing the underlying root cause: tests were driving refactored endpoints through bare, unspec'd
Confidence Score: 5/5Test-only change that moves patches to the correct import locations and tightens MagicMock specs; no production code is touched and all existing assertions are preserved or made more precise. All three fixes target genuine root causes (wrong patch targets, un-spec'd mocks leaking into Pydantic, global state bleed) rather than masking failures by relaxing assertions. The success-path tests add stronger real-type constraints via concrete Deployment objects, and the monkeypatch approach isolates global state correctly. No files require special attention; both changed files are test utilities only.
|
| Filename | Overview |
|---|---|
| tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py | TestModelInfoEndpoint now patches the real seam and uses concrete Deployment objects; TestGetModelInfoWithIdBlocked adds a spec to prevent MagicMock child attributes from leaking into Pydantic validation. |
| tests/test_litellm/proxy/proxy_server/test_proxy_config.py | Pins premium_user=False via monkeypatch so the expected AttributeError is reliably raised instead of a TypeError when the premium_user global is True from a sibling test. |
Reviews (1): Last reviewed commit: "test(proxy): make model_info endpoint te..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Relevant issues
Fixes the
proxy-endpointsandproxy-serverCI flake first observed on #33807 (TestModelInfoEndpointfailing withValueError: not enough values to unpack (expected 2, got 0))Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This is a test-only change (no file under
litellm/is touched), so there is no runtime/product behavior to curl against a live proxy; the meaningful proof is the deterministic before/after of the flaky tests themselves, captured at specific commitsBEFORE (the exact merge-skew window,
8536e3b80e= product refactor #33721 landed, test not yet updated) — the two tests from #33807's CI, in isolation:BEFORE (current tip, the two sibling manifestations of the same root cause) — deterministic under an enterprise license (
premium_user is True), order-dependent otherwise:AFTER (this PR,
44604620a4) — the two shards that went red, run the way CI runs them (-n 2), repeatedly:The rewritten
TestModelInfoEndpointalso passes against BOTH the pre-#33721 (get_model_group_info) and post-#33721 (get_configured_token_limits) endpoint, so a future merge-skew cannot reproduce the original failureType
✅ Test
🐛 Bug Fix
Changes
Root cause (full RCA linked below). The
model_info/get_model_info_with_idendpoint unit tests drove refactored endpoints with bare, unspec'dMagicMock()routers and models. Because the mocks were unspec'd, any attribute or method the refactored endpoints newly read auto-materialized a childMagicMock, and whether that child was reached depended on process-global state the endpoints branch on:premium_user, and the realget_available_models_for_userchain readinglitellmglobals, both of which sibling tests in the same xdist worker mutate. When reached, the childMagicMockeither unpacked to empty (a, b = mock.method()raisesnot enough values to unpack (expected 2, got 0), since a bareMagicMock's default__iter__yields nothing) or leaked intoRouterModelInfo(**model_info)and failed Pydantic'sstrvalidation. The tests pass in isolation and fail under xdist / CI merge-skewThe specific #33807 failure was this class surfaced by merge timing: #33721 (2026-07-17 20:04) added a
get_configured_token_limitsunpack tocreate_model_info_response; #33742 (21:17) band-aided the test withget_configured_token_limits.return_value = (None, None); #33807's branch was based at 19:28 (before #33721), so GitHub's PR-merge CI ran #33721's product code against the un-updated bare-mock test, deterministically hitting the unpack. Locally the branch stayed green because its pinned tree still used the oldget_model_group_infopathThe fix is test-only, no product change:
TestModelInfoEndpoint: mock the real seam the refactored endpoint uses (litellm.proxy.utils.get_available_models_for_user), return a realDeploymentfrom the router'sget_deployment_by_model_group_name, configure the exact router methods the endpoint calls, pingeneral_settings, and delete the deadproxy_server.get_key_models/get_team_models/get_complete_model_listpatches the refactor had stranded (the endpoint imports those fresh fromauth.model_checks, so the patches were silent no-ops)TestGetModelInfoWithIdBlocked:MagicMock(spec=["model_id", "model_info", "blocked"])so unset enterprise columns read asNoneinstead of child mockstest_ProxyConfig_get_model_info_with_id_missing_model_id_raises: pinpremium_user=Falseso the assertedAttributeErrorno longer flips to aTypeErrorwith the ambient license globalSame root-cause class still lives (out of scope for this isolated PR, flagged in the RCA for follow-up) in
tests/test_litellm/proxy/proxy_server/test_team_model_name_translation.py::test_model_info_v1_*andtests/test_litellm/proxy/guardrails/test_guardrail_coverage.py::test_secret_detection_*, both of which I observed failing under xdist ordering and neither of which this PR touchesRCA: https://app.notion.com/p/3a343b8acdab81e49681cf3defd58a52
Final Attestation