Skip to content

test: add 24hr Redis-backed VCR cache to additional test suites - #27159

Merged
mateo-berri merged 12 commits into
litellm_internal_stagingfrom
litellm_add_24hr_caching_to_more_test_suites
May 5, 2026
Merged

test: add 24hr Redis-backed VCR cache to additional test suites#27159
mateo-berri merged 12 commits into
litellm_internal_stagingfrom
litellm_add_24hr_caching_to_more_test_suites

Conversation

@mateo-berri

@mateo-berri mateo-berri commented May 5, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Linear ticket

Resolves LIT-2787

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Type

🚄 Infrastructure
✅ Test

Changes

Extracts the existing llm_translation VCR plumbing into a reusable helper (tests/_vcr_conftest_common.py) and wires it into the conftest.py files of every non-dockerized test directory in the build_and_test workflow that makes live LLM/provider API calls:

  • audio_tests
  • batches_tests
  • guardrails_tests
  • image_gen_tests
  • litellm_utils_tests
  • local_testing
  • logging_callback_tests
  • ocr_tests
  • pass_through_unit_tests
  • router_unit_tests
  • search_tests
  • unified_google_tests

The same helper is also adopted by the pre-existing llm_translation and llm_responses_api_testing conftests to remove the copy-pasted VCR setup.

Each consuming conftest:

  • registers the Redis persister via pytest_recording_configure
  • auto-marks collected tests with pytest.mark.vcr (skipping respx-using files where applicable, since respx and vcrpy both patch httpx)
  • gates cassette writes on test success via _vcr_outcome_gate

The cache is opt-in via CASSETTE_REDIS_URL; when unset, VCR is disabled and tests hit live providers as before. LITELLM_VCR_DISABLE=1 still forces a bypass for ad-hoc local runs.

Coverage

Across the 14 directories above, this PR enables VCR caching for 5,594 tests. Of those:

  • 60 are explicitly skipped from VCR (1.07%) for reasons listed in the per-directory conftest skip lists.
  • 209 are pre-existing respx-conflict skips (3.74%) — files that use respx to patch the httpx transport, which is incompatible with vcrpy patching the same transport.
  • 5,325 (95.19%) benefit from cassette caching.

The remaining LLM-call tests in CI run inside Docker (build_and_test, proxy_logging_guardrails_model_info_tests, proxy_store_model_in_db_tests, proxy_e2e_anthropic_messages_tests, proxy_pass_through_endpoint_tests, proxy_spend_accuracy_tests, etc.). vcrpy patches the in-process httpx transport and cannot intercept calls made from inside a Docker container, so those are out of scope by design. In practice this is a tiny share of the LLM-bill-driving test traffic — most dockerized tests route their chat/completions traffic through the proxy to a mock endpoint at https://exampleopenaiendpoint-production.up.railway.app/. The only dockerized tests that hit real upstream providers are proxy_e2e_anthropic_messages_tests and a few in pass_through_tests (~7–11 tests, ~0.1–0.2% of the real-LLM-call surface).

Out of scope

  • Test directories that run the LiteLLM proxy in Docker (build_and_test, proxy_logging_guardrails_model_info_tests, proxy_store_model_in_db_tests) — see above.
  • The installing_litellm_on_python* jobs make no LLM calls.
  • agent_testing, litellm_mapped_enterprise_tests, auth_ui_unit_tests, proxy_admin_ui_tests, using_litellm_on_windows, redis_caching_unit_tests — verified to either be fully mocked or to share an already-VCR'd directory.

CI fixes folded into this PR

The first round of CI revealed two classes of failures introduced by enabling VCR auto-marking on the new directories. Both are fixed by additional commits on this branch.

1. safe_body matcher — vcrpy's body matcher crashes on JSONL

vcrpy's stock body matcher (vcr/matchers.py) inspects Content-Type and unconditionally calls json.loads on application/json bodies. JSON Lines payloads (e.g. the Bedrock batch S3 PUT body) crash that with json.JSONDecodeError: Extra data before the matcher can return "not a match". This broke tests/batches_tests/test_bedrock_files_and_batches.py::test_async_create_file.

Fix: _safe_body_matcher in tests/_vcr_conftest_common.py, registered as safe_body and used in place of "body" in the shared match_on tuple. Compares request bodies as bytes; strictly more conservative than vcrpy's default — the only equivalence it gives up is "JSON key order doesn't matter", which is irrelevant for our deterministic litellm-built payloads. Cannot produce a false positive that the default would have rejected.

2. key_fingerprint matcher — bad-key tests being silently replayed as success

Tests that deliberately call an LLM API with a bad key (e.g. to assert a failure callback fires, or check_valid_key returns False) were being silently served the prior good-key cassette: we scrub the real Authorization/x-api-key header from the cassette before storing it, so a follow-up bad-key call becomes byte-identical to the good-key call under the rest of the match_on tuple.

Fix: _key_fingerprint_matcher distinguishes requests by the SHA-256 of their API-key headers. The fingerprint is stamped into a synthetic x-litellm-key-fp header by _before_record_request, which then strips the real auth headers (must be done in this hook because vcrpy's filter_headers config knob runs before before_record_request). Cassettes never contain the secret. Bad-key requests now get a different cassette bucket than good-key requests, so vcrpy will not replay a recorded 200 in place of the expected 401.

_before_record_request is also explicitly idempotent — vcrpy invokes it more than once per request (can_play_response_for and _responses both call it), and a naive recompute on the second invocation would yield "no-key" for the auth-already-stripped copy, manifesting as UnhandledHTTPRequestError in play_response.

3. Skip lists for VCR-replay-incompatible tests

A handful of tests are fundamentally incompatible with cassette replay. They were silently masked on the baseline by upstream OpenAI rate-limit failures and surfaced only once VCR replay started succeeding. Each affected conftest passes its problematic tests through apply_vcr_auto_marker_to_items's existing skip_files / skip_nodeid_suffixes knobs:

Directory Tests skipped from VCR Why
local_testing test_assistants.py (whole file) OpenAI Assistants polling mints fresh thread/run/message IDs each session and polls until status == "completed". Cached cassettes can never match a freshly generated run id, so every CI run re-records and the suite blows past the 15-minute step timeout.
local_testing test_router_caching.py (whole file) Asserts on litellm's router-level response cache by comparing response1.id to response2.id across repeat upstream calls (test bypasses litellm cache via ttl=0 and expects upstream to return a new id each time). With VCR replay both upstream calls return the same cassette body.
logging_callback_tests test_amazing_s3_logs.py (whole file) The S3 success-callback test asserts on a per-run response_id round-tripped through a real S3 PUT/LIST. vcrpy's boto3_stubs intercepts the PUT and the LIST replays stale keys.
litellm_utils_tests test_litellm_overhead.py (whole file) Measures litellm_overhead_time_ms as a percentage of total wall-clock time. With cached responses the upstream "network" time collapses to microseconds, blowing past the 40% threshold the test asserts on.

These tests fall back to the pre-PR behavior (live calls, no cache). All other tests in each directory still benefit from caching. Bad-key tests no longer need to be in this list since the key_fingerprint matcher handles them transparently.

Slack Thread

Open in Web Open in Cursor 

Extracts the existing llm_translation VCR plumbing into a reusable helper
(tests/_vcr_conftest_common.py) and wires it into the conftest.py files
of the test directories listed in LIT-2787:

  audio_tests, batches_tests, guardrails_tests, image_gen_tests,
  litellm_utils_tests, local_testing, logging_callback_tests,
  pass_through_unit_tests, router_unit_tests, unified_google_tests

The same helper is also adopted by the pre-existing llm_translation and
llm_responses_api_testing conftests to remove the copy-pasted VCR setup.

Each consuming conftest:
- registers the Redis persister via pytest_recording_configure
- auto-marks collected tests with pytest.mark.vcr (skipping respx-using
  files where applicable, since respx and vcrpy both patch httpx)
- gates cassette writes on test success via _vcr_outcome_gate

The cache is opt-in via CASSETTE_REDIS_URL; when unset, VCR is disabled
and tests hit live providers as before. LITELLM_VCR_DISABLE=1 still
forces a bypass for ad-hoc local runs.

Test directories that run LiteLLM proxy in Docker (build_and_test,
proxy_logging_guardrails_model_info_tests, proxy_store_model_in_db_tests)
are intentionally not included: VCR.py patches the in-process httpx
transport and cannot intercept calls made from inside a Docker container.
The installing_litellm_on_python* jobs make no LLM calls and don't
benefit from caching.

https://linear.app/litellm-ai/issue/LIT-2787/add-24hr-caching-to-additional-test-suites
@CLAassistant

CLAassistant commented May 5, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

✅ mateo-berri
❌ claude
❌ cursoragent
You have signed the CLA already but the status is still pending? Let us recheck it.

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@greptile-apps

greptile-apps Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts the existing llm_translation VCR/Redis-cassette plumbing into a shared helper (tests/_vcr_conftest_common.py) and wires it into 12 additional test directories, enabling 24-hour Redis-backed cassette caching for ~5,325 tests. Two important correctness fixes are bundled in: a safe_body matcher that avoids crashing on JSONL bodies, and a key_fingerprint matcher that uses a SHA-256 fingerprint of auth headers so bad-key tests land in a different cassette bucket than good-key tests.

  • tests/_vcr_conftest_common.py (new): central module containing _safe_body_matcher, _key_fingerprint_matcher, _before_record_request (idempotent auth-scrub + fingerprint stamp), register_persister_if_enabled, apply_vcr_auto_marker_to_items, and banner helpers.
  • tests/_vcr_redis_persister.py (updated): adds per-process health counters, UserWarning emission for CI visibility, and broadens the caught exception class from transient-only to all RedisError subtypes.
  • Per-directory conftest.py files (13 total): each wires in the shared helpers with directory-specific skip_files / skip_nodeid_suffixes lists; the existing llm_translation and llm_responses_api_testing conftests are refactored to remove ~300 lines of copied setup.

Confidence Score: 5/5

Safe to merge — all changes are confined to test infrastructure with no impact on production code paths.

The changes touch only test conftest files and test-only helper modules. The key design decisions (fingerprinting auth headers before scrubbing, idempotent hook invocation, best-effort Redis persistence that never fails a test, 24-hour TTL, and the MAX_EPISODES_PER_CASSETTE guard) are all correctly implemented and well-tested by the accompanying unit tests.

No files require special attention. The most complex logic in tests/_vcr_conftest_common.py and tests/_vcr_redis_persister.py is fully covered by unit tests.

Important Files Changed

Filename Overview
tests/_vcr_conftest_common.py New shared VCR plumbing module: key-fingerprint matcher, safe-body matcher, Redis persister wiring, banner helpers — well-designed with careful idempotency handling
tests/_vcr_redis_persister.py Adds per-process health counters, UserWarning integration, and broader RedisError catch for best-effort persistence; logic is correct and degrades gracefully
tests/test_litellm/test_vcr_safe_body_matcher.py Pure unit tests for safe-body and key-fingerprint matchers; no real network calls — satisfies the mock-only rule for tests/test_litellm/
tests/llm_translation/conftest.py Refactored to delegate to shared helpers; skip lists preserved; no logic regressions
tests/local_testing/conftest.py Adds VCR auto-marking with correct respx-conflict and replay-incompatible file exclusions; key-fingerprint matcher handles bad-key tests without requiring nodeid skips
tests/logging_callback_tests/conftest.py Adds VCR plumbing with S3-integration file excluded; bad-key callback tests handled transparently by key-fingerprint matcher
tests/litellm_utils_tests/conftest.py Adds VCR plumbing with overhead-timing test excluded; small cleanup of unused imports in existing fixture
tests/llm_translation/test_vcr_conftest_common_banner.py New unit tests for the session-end banner logic; uses a fake TerminalReporter and monkeypatches capacity snapshot correctly
tests/llm_translation/test_vcr_redis_persister.py Expanded test coverage for the Redis persister including health counters, warning emission, and capacity snapshot behavior
tests/audio_tests/conftest.py New conftest wiring VCR auto-marking with no special skip lists needed for this directory
tests/batches_tests/conftest.py Adds VCR plumbing; safe_body matcher specifically addresses the Bedrock JSONL batch body crash that motivated this PR
tests/router_unit_tests/conftest.py Adds VCR plumbing alongside existing setup_and_teardown fixture; no conflicts identified
tests/llm_responses_api_testing/conftest.py Refactored to use shared helpers, removing ~130 lines of duplicated VCR setup code

Reviews (4): Last reviewed commit: "test(vcr): enable 24hr cache for ocr_tes..." | Re-trigger Greptile

Comment thread tests/_vcr_conftest_common.py Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d03914b. Configure here.

cursoragent and others added 2 commits May 5, 2026 03:53
…odies

vcrpy's stock body matcher inspects Content-Type and unconditionally
runs json.loads on application/json bodies. JSON Lines payloads (used
by the Bedrock batch S3 PUT and other upload paths) crash that with
json.JSONDecodeError: Extra data, before the matcher can return
'not a match'.

This was the root cause of the batches_testing CI job failing on
test_async_create_file once VCR auto-marking was applied to the
batches_tests directory.

Add a conservative byte-equality body matcher and use it in place of
'body' in the shared match_on tuple. The matcher is strictly more
conservative than vcrpy's default — the only thing it gives up is
'different JSON key order is treated as the same body', which doesn't
apply to deterministic litellm-built request payloads. It can never
produce a false positive that the default would have rejected, so
there is no cross-contamination risk.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
A few tests are incompatible with cassette replay and were failing on
the latest CI run after VCR auto-marking was extended to local_testing
and logging_callback_tests:

- test_amazing_s3_logs.py (logging_callback_tests): the test asserts on
  a per-run response_id that should round-trip through a real S3
  PUT/LIST. vcrpy's boto3 stub intercepts the PUT and the LIST replays
  stale keys, so the freshly-generated id is never found.
- test_async_embedding_azure (logging_callback_tests) and
  test_amazing_sync_embedding (local_testing): the failure branches
  deliberately pass api_key='my-bad-key' to assert that the failure
  callback fires. We scrub auth headers from cassettes (so the bad-key
  request matches the prior good-key request), and vcrpy replays the
  recorded 200 — the failure callback never fires.
- test_assistants.py (local_testing): the OpenAI Assistants polling
  APIs mint fresh thread/run IDs every recording session and then poll
  until status=='completed'. Replays of those polled GETs can never
  match a freshly-generated run id, so every CI run effectively
  re-records and the suite blows past the 15m no_output_timeout.

Skip these from VCR auto-marking so they continue to hit live providers
as they did before this change. The remaining tests in each directory
still get cached.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Followup to the previous commit. After re-running CI on the rebuilt
branch, three more tests surfaced as VCR-replay-incompatible:

- litellm_utils_testing :: test_get_valid_models_from_dynamic_api_key
  Calls GET /v1/models with api_key='123' to assert the result is empty.
  We scrub auth headers, so the bad-key request matches the prior
  good-key cassette and replays the recorded model list.
- litellm_utils_testing :: test_litellm_overhead.py
  Measures litellm_overhead_time_ms as a percentage of total wall-clock
  time. With cached responses the upstream 'network' time collapses to
  microseconds, blowing past the 40%% threshold the test asserts on.
  Skip the whole file (every parametrization is at risk).
- local_testing_part1 :: test_async_custom_handler_completion and
  test_async_custom_handler_embedding
  Same bad-key failure-callback pattern as the already-skipped
  test_amazing_sync_embedding.
- litellm_router_testing :: test_router_caching.py
  Asserts on litellm's own router-level response cache by comparing
  response1.id to response2.id across repeat upstream calls (test
  bypasses litellm cache via ttl=0 and expects upstream to return a
  *new* id). With VCR replay both upstream calls return the same
  cassette body, so the ids are identical. Skip the whole file.
- logging_callback_tests :: test_async_chat_azure (preemptive)
  Same shape as already-skipped test_async_embedding_azure; was masked
  by upstream OpenAI rate-limit failures on baseline.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 95c6eaa. Configure here.

mateo-berri and others added 6 commits May 5, 2026 04:53
- Replace pytest's deprecated item.fspath with item.path in
  apply_vcr_auto_marker_to_items so we don't emit deprecation
  warnings under pytest 8.
- Clarify _safe_body_matcher docstring to reflect actual behavior
  (direct == first, then UTF-8 bytes comparison, no repr fallback).

Addresses Greptile review feedback on PR #27159.
Cassette persistence is strictly best-effort: any Redis-side failure
(connection blip, timeout, OutOfMemoryError when the maxmemory cap is
hit, READONLY replicas, etc.) should degrade to 'test passed but
cassette not cached' rather than fail the test on teardown.

Previously the persister only caught ConnectionError and TimeoutError,
so OutOfMemoryError — which Redis Cloud raises when the cassette cache
hits its memory cap and there are no evictable keys — propagated out of
vcrpy's autouse fixture and ERRORed otherwise-passing tests on
teardown. This caused the litellm_utils_testing CircleCI job to fail on
the latest commit's run, even though the underlying test was a unit
test that used mock_response and produced no real upstream traffic
(the cassette was dirtied by a background langfuse callback). The
rerun only succeeded because Redis evictions happened to free enough
room before the SET — i.e. it was timing-dependent flakiness.

Catch redis.exceptions.RedisError (the common base of all server- and
client-side Redis exceptions) on both save and load, and parametrize
the regression tests across ConnectionError, TimeoutError, and
OutOfMemoryError to pin the new behavior.
…nner

When the persister silently swallows a Redis OOM (or any RedisError) on
save/load there is otherwise no visible signal that the cache is
degraded — tests pass, the cassette just isn't persisted, and the next
session still hits the same Redis at the same near-cap memory.

Add three layers of observability so that failure mode is loud:

1. Per-process health counters ("save_failures", "load_failures", and
   the last error string for each), exposed via cassette_cache_health()
   and reset via reset_cassette_cache_health(). The persister
   increments these in addition to logging.

2. VCRCassetteCacheWarning (UserWarning subclass) emitted via
   warnings.warn() inside the persister's except block. Pytest's
   built-in warnings summary at session end automatically lists every
   such warning, so the failure is visible in CI logs without any
   conftest-level wiring.

3. Session-end banner via emit_cassette_cache_session_banner() and a
   stderr-fallback atexit handler registered from
   register_persister_if_enabled(). Two states:
     - red "VCR CASSETTE CACHE DEGRADED" when save_failures or
       load_failures > 0
     - yellow "VCR CASSETTE CACHE NEAR CAPACITY" (no failures, but
       used_memory >= 85% of maxmemory) so the next session knows
       the Redis is approaching OOM before any SET actually fails

Capacity comes from a best-effort INFO memory probe
(cassette_cache_capacity_snapshot) that returns None on any failure or
when maxmemory is uncapped. The atexit handler skips xdist workers so
only the controller emits.

Tests: parametrize the existing save/load swallow-error tests across
ConnectionError/TimeoutError/OutOfMemoryError, add direct tests for
the health counters and warning emission, and a new
test_vcr_conftest_common_banner.py covering banner output for every
state (silent/red/yellow/disabled/xdist-worker).
Tests that deliberately call an LLM API with a bad key (e.g. to assert
that the failure callback fires, or that check_valid_key returns False)
were being silently served the prior good-key cassette: we scrub the
real Authorization / x-api-key header from the cassette before storing
it, so a follow-up bad-key call is byte-identical to the good-key call
under the existing match_on tuple.

Add a 'key_fingerprint' custom matcher that distinguishes requests by
the SHA-256 of their API-key headers. The fingerprint is stamped into
a synthetic 'x-litellm-key-fp' header by a new before_record_request
hook, which then strips the real auth headers (we have to do the
scrubbing here instead of via vcrpy's filter_headers knob, because
filter_headers runs *first* and would erase the value we want to hash).

Bad-key requests now get a different cassette bucket than good-key
requests, so vcrpy will not replay a recorded 200 in place of the
expected 401. The fingerprint is a one-way hash of the secret, so
cassettes never contain the key.

This permanently removes the 'bad-key' category of skips:

- tests/local_testing: dropped ::test_amazing_sync_embedding,
  ::test_async_custom_handler_completion,
  ::test_async_custom_handler_embedding
- tests/logging_callback_tests: dropped ::test_async_chat_azure,
  ::test_async_embedding_azure
- tests/litellm_utils_tests: dropped
  ::test_get_valid_models_from_dynamic_api_key

Coverage: 7 new unit tests in tests/test_litellm/test_vcr_safe_body_matcher.py
covering header stripping, fingerprint determinism, no-auth bucketing,
good-vs-bad key discrimination, x-api-key (Anthropic/Azure) discrimination,
and idempotence under replay.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Trim narration of code that is already self-evident from function and
variable names. Keep the two genuinely non-obvious bits:

- ordering constraint between filter_headers and before_record_request,
  which would invite a maintainer to re-introduce the bug if removed
- the per-directory _VCR_INCOMPATIBLE_FILES rationale, since 'why
  exactly is this skipped' is not knowable from the test name alone

Also drop the 40-line commented-out drop-in conftest snippet at the
bottom of _vcr_conftest_common.py — the consuming conftests are the
canonical reference.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
vcrpy invokes before_record_request more than once per request:
can_play_response_for calls it, then __contains__ /
_responses (reached via play_response) call it again on the
result. The second invocation sees a request whose auth headers we
already stripped, so a naive recompute yields "no-key" and
overwrites the real fingerprint stored in the header.

This makes can_play_response_for and play_response disagree on
matchability — the former says "yes, we have a stored response for
this" (matching no-key to no-key) and the latter throws
UnhandledHTTPRequestError because it computes a fresh real
fingerprint that doesn't match the stored no-key.

In CI this manifested as ~30 failing tests across guardrails_testing,
audio_testing, batches_testing, image_gen_testing, llm_responses_api,
litellm_router_unit_testing, etc. Skip the recompute when the header
is already set, so re-applying the hook is a no-op.

Adds a regression test that fires the hook twice on the same dict and
asserts the fingerprint stays put.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
@mateo-berri

Copy link
Copy Markdown
Contributor Author

Manual QA:

first run (caching pass):

image

second run (using cache):

image

@mateo-berri
mateo-berri marked this pull request as ready for review May 5, 2026 18:29
@mateo-berri
mateo-berri requested a review from yuneng-berri May 5, 2026 18:29
}
)

_VCR_INCOMPATIBLE_NODEID_SUFFIXES: tuple[str, ...] = ()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 VCR-incompatible node-ID suffixes not wired up

The PR description explicitly lists ::test_async_chat_azure and ::test_async_embedding_azure as "bad-key failure-callback tests" that must be excluded from VCR auto-marking, but _VCR_INCOMPATIBLE_NODEID_SUFFIXES is left empty. When CASSETTE_REDIS_URL is set, both tests will receive pytest.mark.vcr, the bad key will match a cached good-key cassette (because auth headers are scrubbed), the 200 replay will prevent the failure callback from firing, and the assertions will flip.

@mateo-berri mateo-berri May 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We removed those skips on purpose after adding key_fingerprint matcher that hashes the auth header into a synthetic x-litellm-key-fp cassette header. Bad-key calls now land in a different cassette bucket than good-key calls, so vcrpy can no longer replay a recorded 200 in place of an expected 401. The underlying reason for the skips is gone

}
)

_VCR_INCOMPATIBLE_NODEID_SUFFIXES: tuple[str, ...] = ()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 VCR-incompatible node-ID suffixes not wired up

The PR description lists three bad-key failure-callback tests — ::test_amazing_sync_embedding, ::test_async_custom_handler_completion, and ::test_async_custom_handler_embedding — that need to bypass VCR auto-marking because a scrubbed auth header means the bad-key request matches the cached good-key cassette, replaying a 200 and preventing the failure callback from firing. _VCR_INCOMPATIBLE_NODEID_SUFFIXES is empty here, so all three will be marked with pytest.mark.vcr and fail when CASSETTE_REDIS_URL is set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same here

}
)

_VCR_INCOMPATIBLE_NODEID_SUFFIXES: tuple[str, ...] = ()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 VCR-incompatible node-ID suffixes not wired up

The PR description lists ::test_get_valid_models_from_dynamic_api_key as a test that deliberately passes an invalid key to assert an empty model list. When VCR replay is active the cassette returns the cached model list from a prior successful run, flipping the len(...) == 0 assertion. _VCR_INCOMPATIBLE_NODEID_SUFFIXES is left empty so this test will be auto-marked and fail whenever CASSETTE_REDIS_URL is set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

again

These two directories were the only non-dockerized test suites in the
build_and_test workflow that make live LLM/provider API calls but were
not VCR-enabled by this PR. Together they account for 96 tests:

- tests/ocr_tests/ (31): Mistral OCR, Azure AI OCR, Azure Document
  Intelligence, Vertex AI OCR. Pure-unit tests inside the same files
  (e.g. TestAzureDocumentIntelligencePagesParam) make no HTTP calls
  and become benign VCR NOOPs.
- tests/search_tests/ (65): Brave, DataForSEO, DuckDuckGo, Exa,
  Firecrawl, Google PSE, Linkup, Parallel.ai, Perplexity, SearchAPI,
  Searxng, Serper, Tavily.

Both directories use the canonical minimal conftest pattern from
tests/audio_tests/conftest.py with no skip lists. None of the test
files use respx, none assert on per-call upstream non-determinism
(no response1.id != response2.id, no overhead-as-fraction-of-total,
no live polling), so the default match_on tuple should cache cleanly.
If a flake surfaces during the first cassette-recording CI run, we
can add a targeted skip the same way we did for the other dirs.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 23c5d38. Configure here.

@mateo-berri

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri
mateo-berri merged commit 7e13256 into litellm_internal_staging May 5, 2026
117 checks passed
@mateo-berri
mateo-berri deleted the litellm_add_24hr_caching_to_more_test_suites branch May 5, 2026 22:13
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…iAI#27159)

* test: add 24hr Redis-backed VCR cache to additional test suites

Extracts the existing llm_translation VCR plumbing into a reusable helper
(tests/_vcr_conftest_common.py) and wires it into the conftest.py files
of the test directories listed in LIT-2787:

  audio_tests, batches_tests, guardrails_tests, image_gen_tests,
  litellm_utils_tests, local_testing, logging_callback_tests,
  pass_through_unit_tests, router_unit_tests, unified_google_tests

The same helper is also adopted by the pre-existing llm_translation and
llm_responses_api_testing conftests to remove the copy-pasted VCR setup.

Each consuming conftest:
- registers the Redis persister via pytest_recording_configure
- auto-marks collected tests with pytest.mark.vcr (skipping respx-using
  files where applicable, since respx and vcrpy both patch httpx)
- gates cassette writes on test success via _vcr_outcome_gate

The cache is opt-in via CASSETTE_REDIS_URL; when unset, VCR is disabled
and tests hit live providers as before. LITELLM_VCR_DISABLE=1 still
forces a bypass for ad-hoc local runs.

Test directories that run LiteLLM proxy in Docker (build_and_test,
proxy_logging_guardrails_model_info_tests, proxy_store_model_in_db_tests)
are intentionally not included: VCR.py patches the in-process httpx
transport and cannot intercept calls made from inside a Docker container.
The installing_litellm_on_python* jobs make no LLM calls and don't
benefit from caching.

https://linear.app/litellm-ai/issue/LIT-2787/add-24hr-caching-to-additional-test-suites

* test(vcr): add safe-body matcher to handle JSONL and binary request bodies

vcrpy's stock body matcher inspects Content-Type and unconditionally
runs json.loads on application/json bodies. JSON Lines payloads (used
by the Bedrock batch S3 PUT and other upload paths) crash that with
json.JSONDecodeError: Extra data, before the matcher can return
'not a match'.

This was the root cause of the batches_testing CI job failing on
test_async_create_file once VCR auto-marking was applied to the
batches_tests directory.

Add a conservative byte-equality body matcher and use it in place of
'body' in the shared match_on tuple. The matcher is strictly more
conservative than vcrpy's default — the only thing it gives up is
'different JSON key order is treated as the same body', which doesn't
apply to deterministic litellm-built request payloads. It can never
produce a false positive that the default would have rejected, so
there is no cross-contamination risk.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): exclude tests that VCR replay actively breaks

A few tests are incompatible with cassette replay and were failing on
the latest CI run after VCR auto-marking was extended to local_testing
and logging_callback_tests:

- test_amazing_s3_logs.py (logging_callback_tests): the test asserts on
  a per-run response_id that should round-trip through a real S3
  PUT/LIST. vcrpy's boto3 stub intercepts the PUT and the LIST replays
  stale keys, so the freshly-generated id is never found.
- test_async_embedding_azure (logging_callback_tests) and
  test_amazing_sync_embedding (local_testing): the failure branches
  deliberately pass api_key='my-bad-key' to assert that the failure
  callback fires. We scrub auth headers from cassettes (so the bad-key
  request matches the prior good-key request), and vcrpy replays the
  recorded 200 — the failure callback never fires.
- test_assistants.py (local_testing): the OpenAI Assistants polling
  APIs mint fresh thread/run IDs every recording session and then poll
  until status=='completed'. Replays of those polled GETs can never
  match a freshly-generated run id, so every CI run effectively
  re-records and the suite blows past the 15m no_output_timeout.

Skip these from VCR auto-marking so they continue to hit live providers
as they did before this change. The remaining tests in each directory
still get cached.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): expand skip lists for second batch of incompatible tests

Followup to the previous commit. After re-running CI on the rebuilt
branch, three more tests surfaced as VCR-replay-incompatible:

- litellm_utils_testing :: test_get_valid_models_from_dynamic_api_key
  Calls GET /v1/models with api_key='123' to assert the result is empty.
  We scrub auth headers, so the bad-key request matches the prior
  good-key cassette and replays the recorded model list.
- litellm_utils_testing :: test_litellm_overhead.py
  Measures litellm_overhead_time_ms as a percentage of total wall-clock
  time. With cached responses the upstream 'network' time collapses to
  microseconds, blowing past the 40%% threshold the test asserts on.
  Skip the whole file (every parametrization is at risk).
- local_testing_part1 :: test_async_custom_handler_completion and
  test_async_custom_handler_embedding
  Same bad-key failure-callback pattern as the already-skipped
  test_amazing_sync_embedding.
- litellm_router_testing :: test_router_caching.py
  Asserts on litellm's own router-level response cache by comparing
  response1.id to response2.id across repeat upstream calls (test
  bypasses litellm cache via ttl=0 and expects upstream to return a
  *new* id). With VCR replay both upstream calls return the same
  cassette body, so the ids are identical. Skip the whole file.
- logging_callback_tests :: test_async_chat_azure (preemptive)
  Same shape as already-skipped test_async_embedding_azure; was masked
  by upstream OpenAI rate-limit failures on baseline.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): use item.path and tighten matcher docstring

- Replace pytest's deprecated item.fspath with item.path in
  apply_vcr_auto_marker_to_items so we don't emit deprecation
  warnings under pytest 8.
- Clarify _safe_body_matcher docstring to reflect actual behavior
  (direct == first, then UTF-8 bytes comparison, no repr fallback).

Addresses Greptile review feedback on PR BerriAI#27159.

* test(vcr): swallow all RedisError on cassette save/load

Cassette persistence is strictly best-effort: any Redis-side failure
(connection blip, timeout, OutOfMemoryError when the maxmemory cap is
hit, READONLY replicas, etc.) should degrade to 'test passed but
cassette not cached' rather than fail the test on teardown.

Previously the persister only caught ConnectionError and TimeoutError,
so OutOfMemoryError — which Redis Cloud raises when the cassette cache
hits its memory cap and there are no evictable keys — propagated out of
vcrpy's autouse fixture and ERRORed otherwise-passing tests on
teardown. This caused the litellm_utils_testing CircleCI job to fail on
the latest commit's run, even though the underlying test was a unit
test that used mock_response and produced no real upstream traffic
(the cassette was dirtied by a background langfuse callback). The
rerun only succeeded because Redis evictions happened to free enough
room before the SET — i.e. it was timing-dependent flakiness.

Catch redis.exceptions.RedisError (the common base of all server- and
client-side Redis exceptions) on both save and load, and parametrize
the regression tests across ConnectionError, TimeoutError, and
OutOfMemoryError to pin the new behavior.

* test(vcr): surface cassette-cache failures with warnings + session banner

When the persister silently swallows a Redis OOM (or any RedisError) on
save/load there is otherwise no visible signal that the cache is
degraded — tests pass, the cassette just isn't persisted, and the next
session still hits the same Redis at the same near-cap memory.

Add three layers of observability so that failure mode is loud:

1. Per-process health counters ("save_failures", "load_failures", and
   the last error string for each), exposed via cassette_cache_health()
   and reset via reset_cassette_cache_health(). The persister
   increments these in addition to logging.

2. VCRCassetteCacheWarning (UserWarning subclass) emitted via
   warnings.warn() inside the persister's except block. Pytest's
   built-in warnings summary at session end automatically lists every
   such warning, so the failure is visible in CI logs without any
   conftest-level wiring.

3. Session-end banner via emit_cassette_cache_session_banner() and a
   stderr-fallback atexit handler registered from
   register_persister_if_enabled(). Two states:
     - red "VCR CASSETTE CACHE DEGRADED" when save_failures or
       load_failures > 0
     - yellow "VCR CASSETTE CACHE NEAR CAPACITY" (no failures, but
       used_memory >= 85% of maxmemory) so the next session knows
       the Redis is approaching OOM before any SET actually fails

Capacity comes from a best-effort INFO memory probe
(cassette_cache_capacity_snapshot) that returns None on any failure or
when maxmemory is uncapped. The atexit handler skips xdist workers so
only the controller emits.

Tests: parametrize the existing save/load swallow-error tests across
ConnectionError/TimeoutError/OutOfMemoryError, add direct tests for
the health counters and warning emission, and a new
test_vcr_conftest_common_banner.py covering banner output for every
state (silent/red/yellow/disabled/xdist-worker).

* test(vcr): bucket cassettes by API key fingerprint, drop bad-key skips

Tests that deliberately call an LLM API with a bad key (e.g. to assert
that the failure callback fires, or that check_valid_key returns False)
were being silently served the prior good-key cassette: we scrub the
real Authorization / x-api-key header from the cassette before storing
it, so a follow-up bad-key call is byte-identical to the good-key call
under the existing match_on tuple.

Add a 'key_fingerprint' custom matcher that distinguishes requests by
the SHA-256 of their API-key headers. The fingerprint is stamped into
a synthetic 'x-litellm-key-fp' header by a new before_record_request
hook, which then strips the real auth headers (we have to do the
scrubbing here instead of via vcrpy's filter_headers knob, because
filter_headers runs *first* and would erase the value we want to hash).

Bad-key requests now get a different cassette bucket than good-key
requests, so vcrpy will not replay a recorded 200 in place of the
expected 401. The fingerprint is a one-way hash of the secret, so
cassettes never contain the key.

This permanently removes the 'bad-key' category of skips:

- tests/local_testing: dropped ::test_amazing_sync_embedding,
  ::test_async_custom_handler_completion,
  ::test_async_custom_handler_embedding
- tests/logging_callback_tests: dropped ::test_async_chat_azure,
  ::test_async_embedding_azure
- tests/litellm_utils_tests: dropped
  ::test_get_valid_models_from_dynamic_api_key

Coverage: 7 new unit tests in tests/test_litellm/test_vcr_safe_body_matcher.py
covering header stripping, fingerprint determinism, no-auth bucketing,
good-vs-bad key discrimination, x-api-key (Anthropic/Azure) discrimination,
and idempotence under replay.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): drop redundant comments and docstrings

Trim narration of code that is already self-evident from function and
variable names. Keep the two genuinely non-obvious bits:

- ordering constraint between filter_headers and before_record_request,
  which would invite a maintainer to re-introduce the bug if removed
- the per-directory _VCR_INCOMPATIBLE_FILES rationale, since 'why
  exactly is this skipped' is not knowable from the test name alone

Also drop the 40-line commented-out drop-in conftest snippet at the
bottom of _vcr_conftest_common.py — the consuming conftests are the
canonical reference.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): make _before_record_request idempotent

vcrpy invokes before_record_request more than once per request:
can_play_response_for calls it, then __contains__ /
_responses (reached via play_response) call it again on the
result. The second invocation sees a request whose auth headers we
already stripped, so a naive recompute yields "no-key" and
overwrites the real fingerprint stored in the header.

This makes can_play_response_for and play_response disagree on
matchability — the former says "yes, we have a stored response for
this" (matching no-key to no-key) and the latter throws
UnhandledHTTPRequestError because it computes a fresh real
fingerprint that doesn't match the stored no-key.

In CI this manifested as ~30 failing tests across guardrails_testing,
audio_testing, batches_testing, image_gen_testing, llm_responses_api,
litellm_router_unit_testing, etc. Skip the recompute when the header
is already set, so re-applying the hook is a no-op.

Adds a regression test that fires the hook twice on the same dict and
asserts the fingerprint stays put.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

* test(vcr): drop more redundant docstrings and headers

* test(vcr): enable 24hr cache for ocr_tests and search_tests

These two directories were the only non-dockerized test suites in the
build_and_test workflow that make live LLM/provider API calls but were
not VCR-enabled by this PR. Together they account for 96 tests:

- tests/ocr_tests/ (31): Mistral OCR, Azure AI OCR, Azure Document
  Intelligence, Vertex AI OCR. Pure-unit tests inside the same files
  (e.g. TestAzureDocumentIntelligencePagesParam) make no HTTP calls
  and become benign VCR NOOPs.
- tests/search_tests/ (65): Brave, DataForSEO, DuckDuckGo, Exa,
  Firecrawl, Google PSE, Linkup, Parallel.ai, Perplexity, SearchAPI,
  Searxng, Serper, Tavily.

Both directories use the canonical minimal conftest pattern from
tests/audio_tests/conftest.py with no skip lists. None of the test
files use respx, none assert on per-call upstream non-determinism
(no response1.id != response2.id, no overhead-as-fraction-of-total,
no live polling), so the default match_on tuple should cache cleanly.
If a flake surfaces during the first cassette-recording CI run, we
can add a targeted skip the same way we did for the other dirs.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
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.

5 participants