test: unshadow the module handles the F811 sweep left behind - #37914
Merged
yuneng-berri merged 11 commits intoAug 22, 2026
Merged
test: unshadow the module handles the F811 sweep left behind#37914yuneng-berri merged 11 commits into
yuneng-berri merged 11 commits into
Conversation
… the two live tests that went red with it The F811 sweep in #37878 removed the fixture-local `import litellm` from four conftests, but the bare `import litellm.proxy.proxy_server` a few lines below still binds `litellm` as a function local, so `importlib.reload(litellm)` runs before the name is assigned and every test in those directories errors at setup. The `hasattr` guard on the line above already proves the module is loaded, so the import only ever bound the name. Drop it, and enable F823 in ruff-tests.toml, which flags all four sites at the failing line and would have blocked the sweep The same sweep renamed the `check_non_streaming_response` parameter but left one read of `completion`, which now resolves to `litellm.completion`, and removed an import whose side effect was the only thing making `litellm.proxy.proxy_server` reachable in the moderation hook test. That test already takes `monkeypatch`, so patch the router through it and stop leaking the router into later tests `test_content_policy_exception_openai` passed vacuously until #37887 turned it into a real `pytest.raises`, and OpenAI no longer rejects a lyrics prompt with a content policy error. Inject an AsyncOpenAI client whose transport answers with OpenAI's own `content_policy_violation` rejection so the mapping to ContentPolicyViolationError is exercised every run `test_async_create_batch` hit a 409 cancelling a batch OpenAI had already marked failed. The cancel step tolerated a completed batch but not a failed one. Fold both guards into one helper that tolerates a failed batch only when OpenAI's recorded error is the org's enqueued token limit, and prints the batch's errors so the reason is in the log either way
Contributor
Greptile SummaryThe PR repairs test failures introduced by the earlier lint sweep and makes the affected tests deterministic.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| ruff-tests.toml | Enables F823 to prevent local bindings from shadowing module-level names read earlier in test fixtures. |
| tests/batches_tests/test_openai_batches_and_files.py | Centralizes terminal batch-cancellation handling and validates token-limit failures before tolerating them. |
| tests/enterprise/conftest.py | Removes the inner proxy-server import that made litellm local and broke fixture setup. |
| tests/llm_responses_api_testing/conftest.py | Removes the shadowing import while retaining guarded proxy-server reload behavior. |
| tests/local_testing/test_exceptions.py | Makes the content-policy exception test deterministic with MockTransport and closes the wrapped async HTTP client through the AsyncOpenAI context manager. |
| tests/local_testing/test_openai_moderations_hook.py | Imports the proxy-server module explicitly and uses monkeypatch to restore router state after the test. |
| tests/local_testing/test_stream_chunk_builder.py | Fixes the assertion to inspect the response argument rather than the unrelated completion symbol. |
| tests/router_unit_tests/conftest.py | Removes the function-local import responsible for fixture-wide UnboundLocalError failures. |
| tests/vector_store_tests/conftest.py | Removes the same shadowing import from vector-store test setup. |
Reviews (2): Last reviewed commit: "test: close the injected AsyncOpenAI cli..." | Re-trigger Greptile
Contributor
Author
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…itellm_/cci-red-tests-regression-8cc5e8 # Conflicts: # test-quality-budget.json
…itellm_/cci-red-tests-regression-8cc5e8 # Conflicts: # test-quality-budget.json
…itellm_/cci-red-tests-regression-8cc5e8 # Conflicts: # test-quality-budget.json
tin-berri
approved these changes
Aug 22, 2026
yucheng-berri
approved these changes
Aug 22, 2026
…itellm_/cci-red-tests-regression-8cc5e8
…itellm_/cci-red-tests-regression-8cc5e8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
litellmshadowed in four conftestslocal_testingbroke in the same sweepHow it solves it:
import litellm.proxy.proxy_serverthehasattrguard makes redundantruff-tests.toml, which flags all four sitescompletionread and patch the router viamonkeypatchUser Flow
Before: whoever promotes staging to main opens the PR and cannot trust the CircleCI result
ci/circlecichecks redllm_responses_api_testing,litellm_router_unit_testingandlitellm_mapped_enterprise_testsand every test readsUnboundLocalError: cannot access local variable 'litellm'at fixture setup, 0 tests executedlocal_testing_part2and seemodule 'litellm.proxy' has no attribute 'proxy_server'and'function' object has no attribute 'choices'local_testing_part1and seeDID NOT RAISE ContentPolicyViolationErroron a live gpt-3.5-turbo lyrics promptbatches_testingand see409 Cannot cancel a batch with status 'failed', with no record of why the batch failedAfter: the same promotion PR shows the six checks green and the three setup-blocked suites actually ran
ci/circlecichecks are greenmake linton the test tree now fails on any fixture that reads a module-level name before shadowing it, with ruff naming the lineRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
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)Screenshots / Proof of Fix
The change is test-only, so the end-to-end proof is the CircleCI workflow itself, plus the lint rule firing on the exact lines the sweep broke
Before (7481649)
CircleCI on the promotion PR at 49da936
llm_responses_api_testing,litellm_router_unit_testing,litellm_mapped_enterprise_tests: every test errors at setup withUnboundLocalError: cannot access local variable 'litellm' where it is not associated with a valuelocal_testing_part2:test_openai_moderation_error_raisingfails withAttributeError: module 'litellm.proxy' has no attribute 'proxy_server',test_stream_chunk_builder_openai_audio_output_usagefails withAttributeError: 'function' object has no attribute 'choices'local_testing_part1:test_content_policy_exception_openaifails withDID NOT RAISE <class 'litellm.exceptions.ContentPolicyViolationError'>batches_testing:test_async_create_batch[openai]fails withopenai.ConflictError: Error code: 409 - Cannot cancel a batch with status 'failed'F823 on the four conftests at the merge base
ruff check --isolated --select F823 tests/llm_responses_api_testing/conftest.py tests/router_unit_tests/conftest.py tests/enterprise/conftest.py tests/vector_store_tests/conftest.pyF823 Local variable 'litellm' referenced before assignmentat theimportlib.reload(litellm)lineAfter (8529c71)
CircleCI on this PR
llm_responses_api_testingexecuted 170 tests,litellm_router_unit_testing319,litellm_mapped_enterprise_tests244, where the promotion run executed 0 in eachlocal_testing_part1,local_testing_part2andbatches_testinggreenF823 on the test tree
ruff check --config ruff-tests.toml testsAll checks passed!Mutation check on the rewritten content policy test
codetosome_other_errorand runpytest tests/local_testing/test_exceptions.py::test_content_policy_exception_openai1 failed, withlitellm.BadRequestErrorraised in place ofContentPolicyViolationError1 passedType
✅ Test
Caveats (if any)
Final Attestation