litellm_fix_mapped_tests_core: fix test isolation and mock injection issues#20208
litellm_fix_mapped_tests_core: fix test isolation and mock injection issues#20208shin-bot-litellm wants to merge 1 commit intomainfrom
Conversation
…issues ## Problem Four tests in litellm_mapped_tests_core were failing: 1. test_register_model_with_scientific_notation - KeyError due to test isolation issues 2. test_search_uses_registry_credentials - Mock not being called due to incorrect patch path 3. test_send_email_missing_api_key - Real API calls despite mocking 4. test_stream_transformation_error_sync - Mock not effective, real API called ## Solution ### test_register_model_with_scientific_notation - Use unique model name to avoid conflicts with other tests - Clear LRU caches before test to prevent stale data - Clean up model_cost entry after test ### test_search_uses_registry_credentials - Use patch.object() on the actual base_llm_http_handler instance - String-based patching for instance methods can fail; direct object patching is more reliable ### test_send_email_missing_api_key - Directly inject mock HTTP client into logger instance - This bypasses any caching issues that could cause the fixture mock to be ineffective ### test_stream_transformation_error_sync - Patch litellm.completion directly instead of the handler module's litellm reference - This ensures the mock is effective regardless of import order ## Regression These tests were affected by LRU caching added in #19606 and HTTP client caching.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Closing in favor of PR from properly named branch to trigger CircleCI tests. |
Greptile OverviewGreptile SummaryFixes test isolation and mock injection issues in four failing tests caused by LRU caching and HTTP client caching introduced in #19606. Key Changes:
All changes are test-only with no production code impact. The fixes properly address cache-related test isolation issues. Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| tests/test_litellm/enterprise/enterprise_callbacks/send_emails/test_resend_email.py | Improves mock injection by directly assigning mock client to logger instance, preventing cache-related issues |
| tests/test_litellm/google_genai/test_google_genai_handler.py | Simplifies patch path from module-level to direct litellm.completion patching |
| tests/test_litellm/test_utils.py | Adds proper test isolation with unique model name, cache clearing, and cleanup |
| tests/test_litellm/vector_stores/test_vector_store_registry.py | Fixes instance method patching by using patch.object() on imported handler instance |
Sequence Diagram
sequenceDiagram
participant Test as Test Case
participant Cache as LRU Cache/Client Cache
participant Mock as Mock Objects
participant SUT as System Under Test
Note over Test,SUT: Before Fix: Cache Issues
Test->>Cache: Initialize (cached data exists)
Test->>Mock: Setup mock (ineffective)
Cache-->>SUT: Returns cached real client
SUT->>SUT: Uses real client (test fails)
Note over Test,SUT: After Fix: Proper Isolation
Test->>Cache: Clear caches (_invalidate_model_cost_lowercase_map)
Test->>Test: Use unique identifiers (test-model-unique-12345)
Test->>Mock: Create mock client
Test->>SUT: Direct injection (logger.async_httpx_client = mock)
Test->>Mock: Setup patch.object(instance, method)
SUT->>Mock: Uses injected mock (test passes)
Test->>Cache: Cleanup (delete entries, clear caches)
| del litellm.model_cost[test_model_name] | ||
|
|
||
| # Clear LRU caches that might have stale data | ||
| from litellm.utils import get_model_info, _cached_get_model_info_helper, _invalidate_model_cost_lowercase_map |
There was a problem hiding this comment.
get_model_info and _cached_get_model_info_helper are imported but never used
| from litellm.utils import get_model_info, _cached_get_model_info_helper, _invalidate_model_cost_lowercase_map | |
| from litellm.utils import _invalidate_model_cost_lowercase_map |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_litellm/test_utils.py
Line: 2293:2293
Comment:
`get_model_info` and `_cached_get_model_info_helper` are imported but never used
```suggestion
from litellm.utils import _invalidate_model_cost_lowercase_map
```
How can I resolve this? If you propose a fix, please make it concise.| # Patch litellm.completion directly to prevent real API calls | ||
| with patch("litellm.completion", return_value=mock_stream): |
There was a problem hiding this comment.
inconsistent with the async test approach at line 215 which patches litellm.google_genai.adapters.handler.litellm. Consider using the same direct patching approach for consistency
| # Patch litellm.completion directly to prevent real API calls | |
| with patch("litellm.completion", return_value=mock_stream): | |
| with patch("litellm.google_genai.adapters.handler.litellm") as mock_litellm: | |
| mock_litellm.completion = MagicMock(return_value=mock_stream) |
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_litellm/google_genai/test_google_genai_handler.py
Line: 186:187
Comment:
inconsistent with the async test approach at line 215 which patches `litellm.google_genai.adapters.handler.litellm`. Consider using the same direct patching approach for consistency
```suggestion
with patch("litellm.google_genai.adapters.handler.litellm") as mock_litellm:
mock_litellm.completion = MagicMock(return_value=mock_stream)
```
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Problem
Four tests in
litellm_mapped_tests_corewere failing:test_register_model_with_scientific_notation- KeyError due to test isolation issuestest_search_uses_registry_credentials- Mock not being called due to incorrect patch pathtest_send_email_missing_api_key- Real API calls despite mockingtest_stream_transformation_error_sync- Mock not effective, real API calledSolution
test_register_model_with_scientific_notation
test_search_uses_registry_credentials
patch.object()on the actualbase_llm_http_handlerinstancetest_send_email_missing_api_key
test_stream_transformation_error_sync
litellm.completiondirectly instead of the handler module litellm referenceRegression
These tests were affected by LRU caching added in #19606 and HTTP client caching.
Failing Tests Link
https://app.circleci.com/pipelines/github/BerriAI/litellm/56737/workflows/9aa8a62f-0d73-4f49-a084-aa5408142a69/jobs/1140414/tests