Litellm test cleanup - #24755
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Greptile SummaryThis PR is a broad test-suite cleanup across 92 files: it removes skipped/commented-out tests, converts real-network A2A tests to fully mocked equivalents, migrates Azure fine-tuning tests to the Key changes:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| tests/test_litellm/proxy/auth/test_user_api_key_auth.py | Renamed azure_api_key_header parameter keyword to a non-existent name at 4 call sites, causing TypeError at runtime for get_api_key and _user_api_key_auth_builder tests. |
| tests/enterprise/litellm_enterprise/proxy/auth/test_user_api_key_auth.py | Same wrong parameter keyword passed to _user_api_key_auth_builder(), causing TypeError. |
| tests/agent_tests/test_a2a_agent.py | Replaced real-network A2A tests with fully mocked versions — good change that removes env var dependency. |
| tests/test_litellm/llms/azure/test_azure_fine_tuning_api.py | New mocked fine-tuning tests added for create, list, cancel operations; replaces real-network tests from batches_tests. |
| tests/batches_tests/test_fine_tuning_api.py | Removed real-network test_azure_create_fine_tune_jobs_async and migrated test_azure_trainingtype_defaults_to_one unit test to new mock-based location. |
| tests/local_testing/test_completion.py | Removed skipped/dead tests; renamed AZURE_API_KEY/BASE env vars to AZURE_AI_API_KEY/BASE throughout. |
| tests/proxy_unit_tests/test_proxy_server.py | Monkeypatch fixture updated to use AZURE_AI_API_BASE; remainder of diff is formatting-only. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR: Test Cleanup] --> B[Remove dead/skipped tests]
A --> C[Env-var rename\nAZURE_API_KEY → AZURE_AI_API_KEY\nAZURE_API_BASE → AZURE_AI_API_BASE]
A --> D[Convert real-network tests to mocks]
A --> E[Auth test keyword rename]
B --> B1[test_router_init.py deleted\nfully commented-out]
B --> B2[test_clarifai_completion.py deleted\nall tests were skipped]
B --> B3[Skipped blocks removed\nfrom various files]
C --> C1[Test .py files updated]
C --> C2[YAML config files updated]
C --> C3[CI secrets must also be updated]
D --> D1[test_a2a_agent.py\nMockA2AClient added]
D --> D2[test_azure_fine_tuning_api.py\nnew mock-based tests]
D --> D3[batches_tests/test_fine_tuning_api.py\nreal-network test removed]
E --> E1["azure_api_key_header → wrong name\n❌ TypeError at runtime"]
E --> E2["Affected: test_user_api_key_auth.py x2"]
style E fill:#ff9999
style E1 fill:#ff9999
style E2 fill:#ff9999
style C3 fill:#ffffaa
Comments Outside Diff (2)
-
tests/enterprise/litellm_enterprise/proxy/auth/test_user_api_key_auth.py, line 72 (link)Same wrong keyword argument name
_user_api_key_auth_builder()expectsazure_api_key_headerbut receivesAZURE_AI_API_KEY_header. This will raiseTypeError: _user_api_key_auth_builder() got an unexpected keyword argument 'AZURE_AI_API_KEY_header'.Rule Used: What: Flag any modifications to existing tests and... (source)
-
tests/test_litellm/proxy/auth/test_user_api_key_auth.py, line 28-38 (link)Parameter name mismatch will raise
TypeErrorat runtimeThe call to
get_api_key()on line 28 passes a keyword argument whose name does not match the parameter declared in the production function (seelitellm/proxy/auth/user_api_key_auth.py, line 345:azure_api_key_header: Optional[str]). Python raisesTypeError: got an unexpected keyword argumentwhen calling a function with an unrecognised keyword.The same mismatch appears at lines 59–69, 374, and 845, where
_user_api_key_auth_builder()(production signature at line 522:azure_api_key_header: str) is invoked with the wrong keyword. All four call sites must use the correct parameter name that matches the production source.Rule Used: What: Flag any modifications to existing tests and... (source)
Reviews (6): Last reviewed commit: "refactor: make unit test" | Re-trigger Greptile
| @@ -731,12 +731,12 @@ def test_openai_transform_compact_response_api_request_path_without_query(self): | |||
| def test_azure_transform_list_input_items_request_minimal(self): | |||
There was a problem hiding this comment.
Uppercase local variable name is non-idiomatic Python
The variable AZURE_AI_API_BASE is a local variable but follows the ALL_CAPS convention reserved for module-level constants (PEP 8). This appears three times in this file (lines 731, 749, 768) and makes the code look like it references a global constant when it is really a local string.
Consider using azure_api_base instead, which matches the snake_case convention used elsewhere in the same file (e.g., azure_style_api_base at line 691).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @@ -40,7 +41,7 @@ def test_get_required_headers_includes_x_api_key(self): | |||
| assert headers["anthropic-version"] == "2023-06-01" | |||
There was a problem hiding this comment.
Unconventional uppercase in test method name
The method was renamed to test_get_required_headers_includes_AZURE_AI_API_KEY, embedding an ALL_CAPS identifier in the middle of a snake_case test name. Python test methods should use snake_case throughout.
| assert headers["anthropic-version"] == "2023-06-01" | |
| def test_get_required_headers_includes_azure_ai_api_key(self): |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| "model, api_base, api_key", | ||
| [ | ||
| ( | ||
| "azure_ai/Cohere-embed-v3-multilingual-jzu", | ||
| "https://Cohere-embed-v3-multilingual-jzu.eastus2.models.ai.azure.com", | ||
| os.getenv("AZURE_AI_COHERE_API_KEY_2"), | ||
| "azure_ai/Cohere-embed-v3-multilingual-2", | ||
| os.getenv("AZURE_AI_API_BASE"), | ||
| os.getenv("AZURE_AI_API_KEY"), | ||
| ) | ||
| ], |
There was a problem hiding this comment.
Azure Cohere embedding test now uses generic credentials instead of Cohere-specific ones
The parametrized embedding test was changed in two ways:
- The model changed from
azure_ai/Cohere-embed-v3-multilingual-jzutoazure_ai/Cohere-embed-v3-multilingual-2 - The
api_basewas changed from the concrete Cohere endpointhttps://Cohere-embed-v3-multilingual-jzu.eastus2.models.ai.azure.comto the genericos.getenv("AZURE_AI_API_BASE")
AZURE_AI_API_BASE is likely an Azure OpenAI endpoint rather than a Cohere endpoint. Pointing a Cohere embedding model at a generic Azure OpenAI base is likely to fail at runtime (wrong endpoint for the model) or silently test a different deployment than intended. If the old Cohere deployment is no longer available, a @pytest.mark.skip with a note would communicate that more clearly than silently substituting a different endpoint.
Rule Used: What: Flag any modifications to existing tests and... (source)
| @pytest.mark.asyncio | ||
| async def test_azure_acancel_fine_tuning_job_request_and_output_match_expected_json(): | ||
| expected_request = _load_json("azure_cancel_request.json") | ||
| raw_response = _load_json("azure_cancel_raw_response.json") |
There was a problem hiding this comment.
List response type assertion may be fragile
alist_fine_tuning_jobs ultimately calls client.fine_tuning.jobs.list(...) whose mock is set to return_value=list_payload (a plain dict). The assertion assert response == raw_list_response assumes the handler passes this dict through unchanged.
By contrast, the create and cancel operations wrap their mock returns in _MockSDKResponse (so .model_dump() is called on them). If the list handler were ever updated to normalise its return value, this assertion would silently start comparing a model instance against a raw dict.
Consider asserting specific fields instead:
assert response["object"] == "list"
assert len(response["data"]) == len(raw_list_response["data"])
assert response["data"][0]["id"] == raw_list_response["data"][0]["id"]PR #24755 renamed `azure_api_key_header` to `AZURE_AI_API_KEY_header` in the test file but did not update the actual function signatures of `get_api_key()` and `_user_api_key_auth_builder()`, causing TypeError on all affected test cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Litellm test cleanup
PR BerriAI#24755 renamed `azure_api_key_header` to `AZURE_AI_API_KEY_header` in the test file but did not update the actual function signatures of `get_api_key()` and `_user_api_key_auth_builder()`, causing TypeError on all affected test cases.
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays 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:
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes