test(vcr): fix CI failures from PR #27159 (JSONL crash + replay-incompatible tests) - #27166
Merged
cursor[bot] merged 2 commits intoMay 5, 2026
Conversation
…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>
|
|
cursor
Bot
merged commit May 5, 2026
4de8611
into
litellm_add_24hr_caching_to_more_test_suites
109 of 112 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on top of #27159. Fixes the six CI jobs that flipped to failing after VCR auto-marking was extended to additional test directories.
Failures and root causes
I checked all six failing jobs on the latest CircleCI workflow for #27159 and compared them against the most recent baseline run on
litellm_internal_staging. The baseline itself is heavily red right now because ofRateLimitError: You exceeded your current quotafrom the OpenAI test key — the PR actually clears most of those by replaying cached responses. The remaining failures break down as follows.Genuinely caused by the PR (this PR fixes)
batches_testingtest_async_create_filejson.JSONDecodeError: Extra data) on the JSONL S3 PUT body, before it can return "no match".logging_testingtest_basic_s3_logging[*](4 params)boto3_stubsintercepts the S3 PUT done by the s3 success-callback. The test then does a real S3 LIST to verify; cached LIST replay never contains the freshly generatedresponse_id.logging_testingtest_async_embedding_azureapi_key="my-bad-key". We scrub auth headers, so the bad-key request matches the prior good-key cassette and replays a 200; failure callback never fires.local_testing_part1test_amazing_sync_embeddinglitellm_assistants_api_testingwhile run.status != "completed": ...) mints fresh thread/run/message IDs every recording session. Polled GETs against new IDs can never match cached cassettes, so every CI run effectively re-records and the suite blows past the no_output_timeout.Pre-existing on baseline (not touched here)
litellm_router_testing :: test_acompletion_caching_with_ttl_on_router— also fails on baseline.llm_translation_testing :: test_function_calling_with_tool_response— single gw5 worker crash; flake.Fix 1: safe body matcher
vcrpy's
bodymatcher invcr/matchers.pyinspectsContent-Typeand unconditionally runsjson.loadsonapplication/jsonbodies. JSON Lines payloads (the Bedrock batch S3 PUT and similar upload paths) crash that. Added_safe_body_matchertotests/_vcr_conftest_common.py, registered it assafe_body, and replaced"body"withSAFE_BODY_MATCHER_NAMEin the sharedmatch_ontuple.The matcher compares request bodies as bytes (with
str → bytesnormalization). It 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. There is no risk of cross-contamination: it can never produce a false positive that the default would have rejected. The trade-off is that bodies containing nondeterministic values (UUIDs, timestamps) produce a cache miss; the right fix for those cases is abefore_record_requestscrubber, not a smarter matcher.Confirmed via a small reproduction that vcrpy's default
bodymatcher raisesjson.JSONDecodeErroron a JSONL body, and that_safe_body_matcheraccepts it. The reproduction is encoded intests/test_litellm/test_vcr_safe_body_matcher.py(9 unit tests, all passing locally).Fix 2: skip lists for replay-incompatible tests
The existing
apply_vcr_auto_marker_to_itemshelper already supportedskip_filesandskip_nodeid_suffixes. Added entries for the four cases above:tests/logging_callback_tests/conftest.py:_VCR_INCOMPATIBLE_FILES = {"test_amazing_s3_logs.py"}_VCR_INCOMPATIBLE_NODEID_SUFFIXES = ("::test_async_embedding_azure",)tests/local_testing/conftest.py:_VCR_INCOMPATIBLE_FILES = {"test_assistants.py"}_VCR_INCOMPATIBLE_NODEID_SUFFIXES = ("::test_amazing_sync_embedding",)These tests fall back to the pre-PR behavior (live calls, no cache). The remaining tests in each directory still benefit from caching.
Tests
The 9 unit tests cover: matcher registration in
match_on, identical-bytes match,str/bytesequivalence, JSONL acceptance (the bug we're fixing), JSONL difference rejection, byte difference rejection,Nonebody equality, JSON-key-order non-normalization (documenting the conservative-vs-default trade-off), and the documentation test that confirms vcrpy's default body matcher crashes on JSONL withjson.JSONDecodeError.Slack Thread