fix(tests): drop module-level test calls that break local_testing collection - #29519
Closed
mateo-berri wants to merge 1 commit into
Closed
fix(tests): drop module-level test calls that break local_testing collection#29519mateo-berri wants to merge 1 commit into
mateo-berri wants to merge 1 commit into
Conversation
…lection Several files in tests/local_testing invoked their test functions at module scope (e.g. test_register_model.py ran test_update_model_cost_via_completion() at the bottom of the file). Those calls execute during pytest collection, so they fire real network requests at import time. test_register_model.py's call hit an OpenAI 429 and raised, turning into a collection error. A collection error aborts the whole session for every job that globs tests/local_testing/**/test_*.py, which is why unrelated jobs like langfuse_logging_unit_tests (-k langfuse) and litellm_assistants_api_testing (-k assistants) both failed even though neither touches register_model; the -k filter only applies after collection. pytest discovers and runs these test_* functions on its own, so the top-level calls were dead and harmful. Removes them from test_register_model.py, test_wandb.py, test_lunary.py, and test_multiple_deployments.py, and adds a regression test that scans the directory for module-level test invocations.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Author
|
Superseded by #29520; same fix, branch renamed to the Generated by Claude Code |
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.
Relevant issues
CircleCI failures on pipeline 80150 in
langfuse_logging_unit_tests(job 1757575) andlitellm_assistants_api_testing(job 1757577)Linear ticket
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🐛 Bug Fix
Changes
Both failing jobs glob
tests/local_testing/**/test_*.pyand then narrow with-k "langfuse"or-k "assistants". The-kfilter only applies after pytest has collected every file in that glob, so anything that breaks collection of any file breaks the whole job regardless of the keyword.tests/local_testing/test_register_model.pyended with a baretest_update_model_cost_via_completion()at module scope. That call runs while the module is being imported during collection, and the function fires a reallitellm.completion(model="gpt-3.5-turbo", ...). On this run OpenAI returned a 429 (quota exceeded), the function's except block calledpytest.fail(...), and pytest reported it asERROR collecting tests/local_testing/test_register_model.py. With-xthe session stopped there, so both jobs failed at collection time even though neither one actually exercises register_model.The CircleCI reruns you linked are the "rerun from failed" attempt; their logs only show the rerun plugin choking on the recorded failure (
file or directory not found: pytest.py), which is why the real cause is not visible there. The original attempt (jobs 1756105 and 1756113) shows the 429 collection error attest_register_model.py:65.Three sibling files had the same leftover pattern (
test_wandb.py,test_lunary.py,test_multiple_deployments.py). Two of them swallow exceptions so they did not error collection, buttest_multiple_deployments.pyalso callspytest.fail(...)in its except block, so it was one bad network call away from taking down everylocal_testingjob the same way. pytest discovers and runs all fourtest_*functions on its own, so the top-level calls were dead and only harmful; this removes all four.To keep the class of bug from coming back,
test_no_top_level_test_invocations.pyparses every file undertests/local_testing/and fails if any test function is invoked at module scope. It flags all four call sites on the pre-fix tree and passes once they are gone. It is pure AST parsing with no imports or network, so it is fast and deterministic.Screenshots / Proof of Fix
This is a pytest collection-time bug in CI, so there is no proxy endpoint to curl; the faithful reproduction is the exact collection the jobs do.
Before (the original job logs, gh
tests/local_testingcollection):After (collection no longer imports-and-runs anything, no network call):
Regression test green, and confirmed it flags the pre-fix tree:
Generated by Claude Code