Skip to content

test(caching): drive the redis stall burst off the clock, not asyncio.wait_for - #39773

Merged
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/pr-39770-test-failure-c4aacd
Sep 4, 2026
Merged

test(caching): drive the redis stall burst off the clock, not asyncio.wait_for#39773
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/pr-39770-test-failure-c4aacd

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • A redis breaker test fails on Python 3.10 and 3.11
  • Its timeout burst never happens there, so the assertion trips
  • Test-only defect; the breaker itself behaves the same on every version

How it solves it:

  • The fake redis call now checks its own deadline against the clock
  • No more relying on asyncio.wait_for's pre-3.12 cancellation race
  • Verified green on 3.10, 3.11, 3.12, 3.13 and 3.14

User Flow

This PR changes no product code, so the person it affects is a contributor watching CI on their PR

Before: any PR touching the caching shard goes red on two matrix legs for a reason that has nothing to do with it

  1. They open a PR and the Unit Tests workflow runs
  2. responses-caching-types / Run tests (Python 3.10) and (Python 3.11) fail
  3. The only failure is test_event_loop_stall_timeout_burst_keeps_breaker_closed, with assert 0 >= 3
  4. The same shard is green on 3.12, 3.13 and 3.14, so the signal reads as a mystery rather than a real break

After: the shard is green on all five legs, so a red leg means the contributor's own change

  1. They open a PR and the Unit Tests workflow runs
  2. responses-caching-types / Run tests passes on 3.10, 3.11, 3.12, 3.13 and 3.14
  3. Redis breaker coverage is unchanged: the burst is still 8 concurrent timeouts behind one blocking loop stall, and the breaker still has to stay closed

Relevant issues

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

This PR edits one test function and no product code, so there is no request to send at a proxy and nothing an end user can observe. The proof is the same test file across the five interpreters CI runs, before and after

Setup, run from the repo root once per version:

uv sync --python 3.10 --frozen --group dev --extra proxy

Before (f74bc94)

tests/test_litellm/caching/test_redis_cache.py across the CI matrix

  1. for V in 3.10 3.11 3.12 3.13 3.14; do uv run -p $V pytest tests/test_litellm/caching/test_redis_cache.py -q -k event_loop_stall; done
  2. Output:
py3.10: FAILED test_event_loop_stall_timeout_burst_keeps_breaker_closed - AssertionError: the stall must time out a full burst
        assert 0 >= 3
py3.11: FAILED test_event_loop_stall_timeout_burst_keeps_breaker_closed - AssertionError: the stall must time out a full burst
        assert 0 >= 3
py3.12: 1 passed, 61 deselected
py3.13: 1 passed, 61 deselected
py3.14: 1 passed, 61 deselected
  1. The 3.10 leg on CI shows the same thing: https://github.com/BerriAI/litellm/actions/runs/33899868059/job/101111635042

After (7d3b03d)

tests/test_litellm/caching/test_redis_cache.py across the CI matrix

  1. for V in 3.10 3.11 3.12 3.13 3.14; do uv run -p $V pytest tests/test_litellm/caching/test_redis_cache.py -q; done
  2. Output:
py3.10: 62 passed, 1 warning in 1.41s
py3.11: 62 passed, 1 warning in 1.35s
py3.12: 62 passed, 1 warning in 2.30s
py3.13: 62 passed, 1 warning in 1.48s
py3.14: 62 passed, 1 warning in 2.62s
  1. Whole caching directory on 3.10, three consecutive runs, to check the timing-sensitive test is not merely luckier now:
370 passed, 1 skipped, 2 warnings in 41.29s
370 passed, 1 skipped, 2 warnings in 40.44s
370 passed, 1 skipped, 2 warnings in 72.49s

Type

✅ Test

Caveats (if any)

Low

  • The stall is still wall-clock timing, just no longer version-dependent
  • One more 3.10-only failure exists elsewhere, tracked separately

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

….wait_for

test_event_loop_stall_timeout_burst_keeps_breaker_closed built its timeout
burst by wrapping a healthy fake call in asyncio.wait_for. Before 3.12,
wait_for returns the inner result when the inner future also completed while
the loop was blocked, so no call timed out, the burst never materialised, and
the test's own liveness guard failed with 0 >= 3.

The fake now checks its own client deadline against the clock, the way a client
library does, so the stall produces a real redis TimeoutError burst on every
interpreter. The breaker itself is unchanged: its duration gate is plain
time.time() bookkeeping and never depended on the version.
@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes the Redis circuit-breaker stall test consistent across supported Python versions

  • Replaces asyncio.wait_for timing behavior with an explicit monotonic deadline
  • Uses the production-representative Redis timeout exception
  • Preserves assertions that a short timeout burst leaves the breaker closed

Confidence Score: 5/5

The test-only change appears safe to merge

The revised fake deterministically produces the intended timeout burst, follows the same breaker classification path, and retains the existing behavioral assertions

Important Files Changed

Filename Overview
tests/test_litellm/caching/test_redis_cache.py Updates the timeout fake without weakening the breaker duration-gate regression coverage

Reviews (1): Last reviewed commit: "test(caching): drive the redis stall bur..." | Re-trigger Greptile

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri
yuneng-berri merged commit 4ad4db2 into litellm_internal_staging Sep 4, 2026
180 of 181 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_/pr-39770-test-failure-c4aacd branch September 4, 2026 19:33
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