Skip to content

fix(tests): resolve test isolation issue in http_handler tests#21388

Merged
jquinter merged 2 commits intomainfrom
fix/test-isolation-http-handler
Feb 17, 2026
Merged

fix(tests): resolve test isolation issue in http_handler tests#21388
jquinter merged 2 commits intomainfrom
fix/test-isolation-http-handler

Conversation

@jquinter
Copy link
Contributor

Problem

Tests in test_http_handler.py were failing intermittently in CI with:

assert False
 +  where False = isinstance(<AsyncHTTPHandler object>, AsyncHTTPHandler)

Root Cause

The conftest.py fixture reloads the litellm module between test modules with importlib.reload(litellm). This causes class references imported at module-level to become stale:

  1. Test file imports AsyncHTTPHandler at the top (module-level)
  2. Fixture runs and reloads litellm, creating a NEW AsyncHTTPHandler class
  3. Test creates an instance via get_async_httpx_client() - gets instance of NEW class
  4. Test checks isinstance(obj, AsyncHTTPHandler) - compares against OLD class reference
  5. Check fails because they're different class objects

Solution

Import AsyncHTTPHandler locally within each test function that uses isinstance() checks. This ensures we get the fresh class reference after the module reload.

Fixed Tests

  • test_session_reuse_integration
  • test_get_async_httpx_client_with_shared_session
  • test_get_async_httpx_client_without_shared_session

Impact

  • ✅ Resolves intermittent CI failures
  • ✅ No functional changes to the code being tested
  • ✅ All tests pass locally and should pass in CI

Related

This issue was discovered while investigating test failures in PR #21107 (unrelated to that PR's changes).

@vercel
Copy link

vercel bot commented Feb 17, 2026

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

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 17, 2026 10:02pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

This PR fixes intermittent CI test failures in test_http_handler.py caused by stale class references after importlib.reload(litellm) in the test suite's conftest.py fixture.

  • Three test functions (test_session_reuse_integration, test_get_async_httpx_client_with_shared_session, test_get_async_httpx_client_without_shared_session) now import AsyncHTTPHandler locally within the function body (aliased as AsyncHTTPHandlerReload) instead of relying on the module-level import, ensuring isinstance() checks compare against the current class reference after module reload.
  • The module-level AsyncHTTPHandler import on line 18 is retained for other tests that only use it for constructor calls and static method invocations (which are not affected by the stale reference issue).
  • Minor whitespace normalization (trailing space removal) is included in the changed functions.
  • No functional changes to the code under test; this is purely a test infrastructure fix.

Confidence Score: 4/5

  • This PR is safe to merge — it only changes test imports to fix a test isolation issue with no impact on production code.
  • Score of 4 reflects that this is a test-only change with a well-understood root cause. The fix correctly addresses the stale class reference problem caused by conftest module reloading. The only minor concern is that the module-level AsyncHTTPHandler import (line 18) is still used by other tests for constructor/static method calls, which works but could become fragile if those tests add isinstance() checks in the future. The unused module-level import could also be cleaned up for clarity.
  • No files require special attention — the change is limited to test infrastructure.

Important Files Changed

Filename Overview
tests/test_litellm/llms/custom_httpx/test_http_handler.py Moves AsyncHTTPHandler imports into test function scope (aliased as AsyncHTTPHandlerReload) for 3 tests that use isinstance() checks, preventing stale class references after importlib.reload(litellm) in conftest.py. Also includes minor whitespace normalization (trailing spaces removed). The fix is correctly scoped to only the affected tests.

Sequence Diagram

sequenceDiagram
    participant C as conftest.py (module scope)
    participant T as test_http_handler.py
    participant L as litellm module
    participant H as http_handler module

    Note over T: Module-level import at load time
    T->>H: import AsyncHTTPHandler (OLD ref)

    Note over C: Fixture runs before each test module
    C->>L: importlib.reload(litellm)
    L->>H: Re-imports http_handler (NEW class)

    Note over T: BEFORE fix (fails)
    T->>H: get_async_httpx_client() returns NEW instance
    T--xT: isinstance(obj, OLD AsyncHTTPHandler) → False ❌

    Note over T: AFTER fix (passes)
    T->>H: from http_handler import AsyncHTTPHandler (NEW ref)
    T->>H: get_async_httpx_client() returns NEW instance
    T->>T: isinstance(obj, NEW AsyncHTTPHandler) → True ✅
Loading

Last reviewed commit: 821ed87

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.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

jquinter added a commit that referenced this pull request Feb 17, 2026
Add two detailed guides for addressing CI test flakiness:

1. test-flakiness-guide.md - Developer guide with:
   - How to use @pytest.mark.no_parallel for async mocks
   - Patterns for robust async mock setup
   - Retry logic strategies
   - Module reload issues and fixes
   - Quick reference and checklist

2. ci-test-improvements.md - Implementation plan with:
   - Priority phased rollout (Quick wins → CI → Enforcement)
   - pytest-rerunfailures plugin setup
   - GitHub Actions improvements for retries
   - Makefile targets for testing
   - Pre-commit hooks for test quality
   - Test utilities module with decorators
   - Success metrics and monitoring

These guides provide actionable solutions for the CI test failures
observed in PRs #21107, #21388, and #21390.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jquinter jquinter force-pushed the fix/test-isolation-http-handler branch from 821ed87 to a4d3613 Compare February 17, 2026 18:05
@jquinter jquinter force-pushed the fix/test-isolation-http-handler branch 2 times, most recently from 8b8d520 to a4d3613 Compare February 17, 2026 18:42
@jquinter jquinter force-pushed the fix/test-isolation-http-handler branch from a4d3613 to 978a59d Compare February 17, 2026 21:22
jquinter and others added 2 commits February 17, 2026 19:01
Fix isinstance() checks failing due to module reload in conftest.py.

The conftest.py fixture reloads the litellm module between test modules,
which causes class references imported at module-level to become stale.
When AsyncHTTPHandler is imported at the top of the file and then litellm
is reloaded by the fixture, the isinstance() check fails because the
returned instance is of the NEW AsyncHTTPHandler class while the test
is checking against the OLD class reference.

Solution: Import AsyncHTTPHandler locally within each test function that
uses isinstance() checks. This ensures we get the fresh class reference
after the module reload.

Fixed tests:
- test_session_reuse_integration
- test_get_async_httpx_client_with_shared_session
- test_get_async_httpx_client_without_shared_session

This resolves intermittent CI failures where parallel test execution
triggers the module reload behavior.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
After rebasing with main, pyproject.toml contains dependency changes from
PR #21394 (removed pytest-retry, added pytest-xdist). Running `poetry lock`
to sync the lock file with the updated pyproject.toml.

This resolves the CI error:
'pyproject.toml changed significantly since poetry.lock was last generated'
@jquinter jquinter force-pushed the fix/test-isolation-http-handler branch from 1ee3ad8 to fb839d5 Compare February 17, 2026 22:01
@jquinter jquinter merged commit 9fb886d into main Feb 17, 2026
17 of 25 checks passed
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.

1 participant