Skip to content

litellm_fix_mapped_tests_core: clear client cache and fix isinstance checks#20195

Closed
shin-bot-litellm wants to merge 1 commit intomainfrom
shin/fix-mapped-tests-core-v2
Closed

litellm_fix_mapped_tests_core: clear client cache and fix isinstance checks#20195
shin-bot-litellm wants to merge 1 commit intomainfrom
shin/fix-mapped-tests-core-v2

Conversation

@shin-bot-litellm
Copy link
Contributor

Problem

Tests using mocked HTTP clients were hitting real APIs because:

  1. HTTP client cache was returning previously cached real clients
  2. isinstance checks failed due to module identity issues from sys.path

Tests affected:

  • test_send_email_missing_api_key
  • test_send_email_multiple_recipients (resend & sendgrid)
  • test_search_uses_registry_credentials
  • test_vector_store_create_with_simple_provider_name
  • test_image_edit_merges_headers_and_extra_headers

Solution

  1. Add clear_client_cache fixture (autouse=True) to clear litellm.in_memory_llm_clients_cache before each test
  2. Fix isinstance checks to use type name comparison (avoids module identity issues from sys.path.insert)

Why not disable_aiohttp_transport

The default transport is aiohttp, so tests should work with it. Clearing the cache ensures mocks are used instead of cached real clients.

Regression

PR #19829 (commit f95572e3ed) added @respx.mock but cached clients from earlier tests were being reused, bypassing the mocks.

…checks

## Problem
Tests using mocked HTTP clients were hitting real APIs because:
1. HTTP client cache was returning previously cached real clients
2. isinstance checks failed due to module identity issues from sys.path

### Tests affected:
- test_send_email_missing_api_key
- test_send_email_multiple_recipients (resend & sendgrid)
- test_search_uses_registry_credentials
- test_vector_store_create_with_simple_provider_name
- test_image_edit_merges_headers_and_extra_headers

## Solution
1. Add clear_client_cache fixture (autouse=True) to clear
   litellm.in_memory_llm_clients_cache before each test
2. Fix isinstance checks to use type name comparison
   (avoids module identity issues from sys.path.insert)

## Why not disable_aiohttp_transport
The default transport is aiohttp, so tests should work with it.
Clearing the cache ensures mocks are used instead of cached real clients.

## Regression
PR #19829 (commit f95572e) added @respx.mock but cached clients
from earlier tests were being reused, bypassing the mocks.
@vercel
Copy link

vercel bot commented Jan 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Error Error Jan 31, 2026 11:07pm

Request Review

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 31, 2026

Greptile Overview

Greptile Summary

This PR fixes test isolation issues where cached HTTP clients bypassed mocks, causing tests to make real API calls instead of using mocked responses.

Changes:

  • Added clear_client_cache fixture (autouse=True) to 4 test files to flush litellm.in_memory_llm_clients_cache before and after each test
  • Replaced isinstance() checks with type name comparisons in vector store tests to avoid module identity issues from sys.path.insert()

Root Cause:
After PR #19829 added @respx.mock decorators, tests were still hitting real APIs because get_async_httpx_client() was returning cached clients from previous tests, bypassing the mocks. The function-scoped cache clearing ensures each test gets a fresh client that respects the mock configuration.

Impact:

  • Tests are now properly isolated and won't make unintended external API calls
  • The isinstance fix prevents test failures when the same class is imported through different module paths
  • No changes to production code, only test improvements

Confidence Score: 5/5

  • This PR is safe to merge - it only modifies test files to improve test isolation
  • The changes are well-targeted test improvements with clear benefits: (1) the clear_client_cache fixture correctly uses flush_cache() on the existing cache object, (2) the type name comparison is a valid workaround for module identity issues, (3) no production code is modified, (4) the fixes address real test flakiness issues identified in the PR description
  • No files require special attention

Important Files Changed

Filename Overview
tests/test_litellm/enterprise/enterprise_callbacks/send_emails/test_resend_email.py Added clear_client_cache fixture to flush HTTP client cache before/after each test, preventing cached real clients from bypassing mocks
tests/test_litellm/enterprise/enterprise_callbacks/send_emails/test_sendgrid_email.py Added clear_client_cache fixture to flush HTTP client cache before/after each test, ensuring test isolation with mocked clients
tests/test_litellm/test_main.py Added clear_client_cache fixture to flush HTTP client cache before/after each test, ensuring mocks are used instead of cached clients
tests/test_litellm/vector_stores/test_vector_store_create_provider_logic.py Replaced isinstance checks with type name comparisons to avoid module identity issues caused by sys.path manipulation in test setup
tests/test_litellm/vector_stores/test_vector_store_registry.py Added clear_client_cache fixture to flush HTTP client cache before/after each test, preventing real API calls when mocks should be used

Sequence Diagram

sequenceDiagram
    participant Test as Test Function
    participant Fixture as clear_client_cache Fixture
    participant Cache as in_memory_llm_clients_cache
    participant HTTPHandler as get_async_httpx_client()
    participant Mock as @respx.mock / Mock Client

    Note over Fixture,Cache: Before Test Execution
    Fixture->>Cache: flush_cache()
    Cache-->>Fixture: Cache cleared
    
    Note over Test,Mock: Test Execution
    Test->>HTTPHandler: Request HTTP client
    HTTPHandler->>Cache: get_cache(key)
    Cache-->>HTTPHandler: None (no cached client)
    HTTPHandler->>Mock: Create new client (uses mock)
    Mock-->>HTTPHandler: Mocked client
    HTTPHandler->>Cache: set_cache(key, client)
    HTTPHandler-->>Test: Return mocked client
    
    Test->>Mock: Make HTTP request
    Mock-->>Test: Mocked response (no real API call)
    
    Note over Fixture,Cache: After Test Execution
    Fixture->>Cache: flush_cache()
    Cache-->>Fixture: Cache cleared
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@shin-bot-litellm shin-bot-litellm deleted the shin/fix-mapped-tests-core-v2 branch January 31, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants