test(proxy/client): isolate client tests from the developer's real CLI token - #30175
test(proxy/client): isolate client tests from the developer's real CLI token#30175mateo-berri wants to merge 1 commit into
Conversation
…I token Client falls back to the key stored by lite login when no api_key is given, so test_client_without_api_key failed on any machine whose ~/.litellm/token.json was issued for the URL the test uses. An autouse conftest fixture now stubs load_cli_token for the whole client test directory; the same latent leak existed in test_models.py and test_model_groups.py. Also dedupes test functions in test_client.py that shadowed each other and adds coverage for the CLI token fallback and its base_url origin check, which had no tests at the Client level
Greptile SummaryAdds a directory-wide
Confidence Score: 5/5Safe to merge — only test files are changed, no production logic is touched. The change is entirely test-side: it fixes non-hermetic tests, removes shadowed duplicates that were never running, and adds meaningful coverage for the CLI token fallback. The monkeypatch target is the correct seam (module-level attribute patched before the function that calls it resolves the name), the autouse fixture composes correctly with per-test overrides because both use the same function-scoped monkeypatch instance, and the new assertions are accurate reflections of the production code's behavior. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/test_litellm/proxy/client/conftest.py | New autouse fixture that patches load_cli_token to return None for every test in the directory, hermetically isolating tests from the developer's real ~/.litellm/token.json. |
| tests/test_litellm/proxy/client/test_client.py | Removes silently-shadowed duplicate test definitions, expands test_client_without_api_key to cover all sub-clients, splits out test_client_custom_timeout, and adds two new tests covering the CLI token fallback and its origin-check. |
Reviews (1): Last reviewed commit: "test(proxy/client): isolate client tests..." | Re-trigger Greptile
Greptile SummaryThis PR makes the
Confidence Score: 5/5Safe to merge — the change is confined to the test directory, adds no production code, and the isolation strategy correctly targets the module-level symbol that get_litellm_gateway_api_key calls. The autouse fixture patches the right symbol (litellm.litellm_core_utils.cli_token_utils.load_cli_token), which is what get_litellm_gateway_api_key resolves at call time. The two new per-test overrides compose correctly with the fixture because both use the same function-scoped monkeypatch instance, with LIFO teardown restoring state cleanly. The duplicate-test collapse retains all assertions from the surviving definitions, and no prior assertion was weakened or removed. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/test_litellm/proxy/client/conftest.py | New autouse fixture that stubs load_cli_token to None for all tests in the directory, preventing accidental reads of the developer's real ~/.litellm/token.json. |
| tests/test_litellm/proxy/client/test_client.py | Merges shadowed duplicate test functions, extracts test_client_custom_timeout, and adds two new tests covering the CLI-token fallback and its origin-check. No coverage regressions found. |
Reviews (2): Last reviewed commit: "test(proxy/client): isolate client tests..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewNote on the
make test-unitbox: the command currently aborts onlitellm_internal_stagingitself, before this PR. #29686 addedtests/test_litellm/models/test_models.py, which collides with the long-existingtests/test_litellm/proxy/client/test_models.py; neither directory has an__init__.py, so pytest assigns both the module nametest_modelsand collection dies with "import file mismatch". That needs a separate fix (add__init__.pyfiles or unique basenames). Running the suite on this branch without-xgives 21915 passed, 113 skipped, that one pre-existing collection error, and 18 failures that are parallel-run flakes: all 18 pass when run in isolation on both this branch and a clean checkout oflitellm_internal_stagingDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Simulate a developer who ran
lite loginagainst the URL the test uses, without touching the real~/.litellm/token.json:On
litellm_internal_staging:On this branch:
Type
✅ Test
Changes
Clientfalls back toget_litellm_gateway_api_key()when constructed without anapi_key, which reads the token thatlite loginstores in~/.litellm/token.json.test_client_without_api_keyasserts the resolved key isNone, so it fails on any developer machine where the stored token was issued for the same base URL the test uses. The test suite's outcome should not depend on whether the developer has logged into a local proxyThis adds a
conftest.pyfortests/test_litellm/proxy/client/with an autouse fixture that stubsload_cli_tokento returnNone, so every test in the directory is hermetic with respect to the developer's home directory. The same latent leak existed intest_models.pyandtest_model_groups.py, which also constructClientwithout a key and assert it isNone; the directory-wide fixture covers those too. The CLI tests undercli/are unaffected:auth.pyhas its ownload_token, and the tests that patchcli_token_utils.load_cli_tokendo so per test, which composes fine with the autouse fixtureWhile auditing the file I found that
test_client.pydefinedtest_client_initializationandtest_client_without_api_keytwice each; Python silently shadows the earlier definitions, so pytest only ever ran the second of each. The duplicates are now merged into single tests, with the custom timeout assertion split out astest_client_custom_timeoutThe CLI token fallback had no coverage at the
Clientlevel, so this also adds two tests: one asserting the client picks up a stored token issued for the target server, one asserting it ignores a token issued for a different server. Besides covering the fallback and its origin check, the first one pinsload_cli_tokenas the live seam, so ifclient.pyever stops resolving the key through it, the test fails and flags that the conftest isolation needs updating