tests(vcr): drop redundant comments and docstrings

Remove explanatory comments that restated what the code already says.
Kept only those that document non-obvious external contracts (the aiohttp
record-path patch's reason for re-feeding the body, and the warning
messages inside save_cassette that reach the user).
This commit is contained in:
mateo-berri 2026-05-01 14:36:48 -07:00
parent 53f71fbf4d
commit 80415b472e
5 changed files with 9 additions and 180 deletions

View File

@ -9,35 +9,15 @@ from vcr.serialize import deserialize, serialize
CASSETTE_TTL_SECONDS = 24 * 60 * 60
REDIS_KEY_PREFIX = "litellm:vcr:cassette:"
# Healthy cassettes hold 15 episodes (a single test rarely makes more than a
# handful of distinct HTTP calls). When a cassette balloons past this, it
# usually means a test produces non-deterministic request bodies (e.g. uuid)
# under record_mode=new_episodes, and every CI run is appending fresh
# unmatched episodes instead of replaying. That growth is unbounded over
# time and silently inflates Redis. Refuse to persist past this threshold so
# the pathology surfaces loudly instead.
CASSETTE_REDIS_URL_ENV = "CASSETTE_REDIS_URL"
VCR_VERBOSE_ENV = "LITELLM_VCR_VERBOSE"
MAX_EPISODES_PER_CASSETTE = 50
_log = logging.getLogger(__name__)
# Per-process map: cassette key -> "did the test that produced this cassette
# pass?". The conftest's pytest_runtest_makereport hook sets True when the test
# body succeeds; save_cassette consults it to avoid persisting recordings from
# failed runs. We key by the redis cache key so retries (which may produce a
# fresh cassette object each time but write to the same key) interleave
# correctly.
_passed_by_cassette_key: dict[str, bool] = {}
def mark_test_outcome_for_cassette(cassette_path: str, passed: bool) -> None:
"""Record whether the test that owns ``cassette_path`` passed.
Called from a pytest hook in conftest. The recorded value is consulted by
``save_cassette`` so failed-attempt recordings (e.g. a flaky test that
asserts on provider state) don't poison the cache for future runs.
"""
_passed_by_cassette_key[redis_key_for(cassette_path)] = passed
@ -49,14 +29,7 @@ def redis_key_for(cassette_path: str) -> str:
return f"{REDIS_KEY_PREFIX}{rel}"
CASSETTE_REDIS_URL_ENV = "CASSETTE_REDIS_URL"
def _redis_url_from_env() -> Optional[str]:
# Use a dedicated cassette Redis URL so the VCR cache is isolated from any
# application Redis used by tests (which may be flushed by other suites).
# Intentionally do NOT fall back to REDIS_URL/REDIS_HOST — sharing a Redis
# with the app cache risks cassettes being wiped by flushdb/flushall.
return os.environ.get(CASSETTE_REDIS_URL_ENV) or None
@ -74,8 +47,6 @@ def _build_default_client():
"Cassette Redis is intentionally separate from the application "
"Redis (REDIS_URL/REDIS_HOST) to avoid being flushed by tests."
)
# Managed Redis providers (e.g. Upstash) drop idle TLS connections; retry on
# connection/timeout errors so a single dropped socket doesn't fail teardown.
return redis.Redis.from_url(
url,
socket_timeout=5,
@ -92,8 +63,6 @@ def make_redis_persister(
):
redis_client = client if client is not None else _build_default_client()
# Lazily resolve the redis exception classes so callers can pass any
# client (incl. fakeredis) without importing the real `redis` package.
try:
from redis.exceptions import ConnectionError as RedisConnectionError
from redis.exceptions import TimeoutError as RedisTimeoutError
@ -108,8 +77,6 @@ def make_redis_persister(
try:
data = redis_client.get(redis_key_for(cassette_path))
except _transient_errors as exc:
# Treat a Redis outage on read as a cassette miss so tests fall
# through to a live call instead of erroring in setup.
_log.warning(
"VCR redis load failed for %s; treating as cache miss: %s",
cassette_path,
@ -125,20 +92,9 @@ def make_redis_persister(
@staticmethod
def save_cassette(cassette_path, cassette_dict, serializer):
key = redis_key_for(cassette_path)
# Only persist successful runs. A failed test (incl. all the failed
# retries before a passing one) would otherwise poison the cache —
# e.g. a flaky test that observes provider state across two calls
# could capture a "bad luck" response that deterministically fails
# every future replay. We default to True if the hook didn't run
# (e.g. cassette saved outside a test context) so non-test usage
# still works.
passed = _passed_by_cassette_key.pop(key, True)
episode_count = len(cassette_dict.get("requests", []) or [])
if episode_count > MAX_EPISODES_PER_CASSETTE:
# Pathology: the test is producing non-deterministic request
# bodies and accumulating unbounded episodes. Refuse the save
# so the cassette can't keep ballooning, and surface a loud
# warning so someone investigates / opts the test out.
_log.warning(
"VCR redis save refused for %s; cassette has %d episodes "
"(> MAX_EPISODES_PER_CASSETTE=%d). The test likely produces "
@ -162,9 +118,6 @@ def make_redis_persister(
try:
redis_client.set(key, payload, ex=ttl_seconds)
except _transient_errors as exc:
# Cassette persistence is a cache, not test correctness. A Redis
# outage on save should not fail an otherwise-passing test —
# the next run will simply re-record.
_log.warning(
"VCR redis save failed for %s; cassette not persisted: %s",
cassette_path,
@ -175,7 +128,6 @@ def make_redis_persister(
def filter_non_2xx_response(response):
# Returning None tells vcrpy to skip persisting; see Cassette.append.
if not isinstance(response, dict):
return response
status = response.get("status")
@ -209,32 +161,14 @@ def patch_vcrpy_aiohttp_record_path() -> None:
_PATCHED_AIOHTTP_RECORD = True
VCR_VERBOSE_ENV = "LITELLM_VCR_VERBOSE"
def vcr_verbose_enabled() -> bool:
return os.environ.get(VCR_VERBOSE_ENV) == "1"
def format_vcr_verdict(cassette: Any) -> str:
"""Build a one-line hit/miss verdict for a vcrpy Cassette.
HIT at least one request was served from cache and nothing new was
recorded. (Pure replay.)
MISS nothing from cache; one or more requests went live and were
recorded. (Cold cache.)
PARTIAL mix of replay and new recordings. Usually means the cassette
matches some but not all requests for this test (e.g. retries,
new branches, or vcrpy match_on too strict).
NOOP test made no HTTP calls (or VCR not engaged for it).
"""
if cassette is None:
return "[VCR NOOP]"
played = getattr(cassette, "play_count", 0) or 0
# cassette.data is the recorded request/response list; len(cassette) counts
# recorded episodes. New recordings during this test = len - prior_len, but
# we don't have prior_len here, so we use cassette.dirty (set when an append
# happened during this run) as the "new recording" signal.
dirty = getattr(cassette, "dirty", False)
total = len(cassette) if hasattr(cassette, "__len__") else 0
if played == 0 and not dirty:

View File

@ -23,8 +23,6 @@ from tests._vcr_redis_persister import ( # noqa: E402
)
# Controller-side handles for writing per-test VCR verdicts to the live
# terminal. See the matching comment in tests/llm_translation/conftest.py.
_controller_pluginmanager = None
_controller_terminal_reporter = None
@ -98,9 +96,6 @@ def vcr_config():
def _vcr_disabled() -> bool:
if os.environ.get("LITELLM_VCR_DISABLE") == "1":
return True
# Cassettes live on a dedicated Redis (CASSETTE_REDIS_URL) so the cache
# isn't shared with — and accidentally flushed by — tests that exercise
# the application Redis via REDIS_URL/REDIS_HOST.
return not os.environ.get("CASSETTE_REDIS_URL")
@ -113,12 +108,6 @@ def pytest_recording_configure(config, vcr):
@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_makereport(item, call):
"""Attach each phase's report to the item so fixture teardown can read it.
Used by ``_vcr_outcome_gate`` below to skip persisting cassettes for
failed test runs (incl. failed retries that pytest-rerunfailures will
re-attempt) so a "bad luck" recording can't poison future replays.
"""
outcome = yield
rep = outcome.get_result()
setattr(item, f"rep_{rep.when}", rep)
@ -126,17 +115,6 @@ def pytest_runtest_makereport(item, call):
@pytest.fixture(autouse=True)
def _vcr_outcome_gate(request, vcr):
"""Tell the persister whether the test that owns this cassette passed.
Runs after ``vcr`` (which yields the active Cassette). At teardown time
the call-phase report is attached to the item by the makereport hook
above, so we can mark the cassette key passed/failed before vcrpy's
Cassette.__exit__ triggers persister.save_cassette.
Stashes a per-test hit/miss verdict on ``user_properties`` so the
controller-side ``pytest_runtest_logreport`` hook can surface it to the
live terminal under xdist.
"""
yield
cassette = vcr
rep_call = getattr(request.node, "rep_call", None)
@ -152,7 +130,6 @@ def _vcr_outcome_gate(request, vcr):
def pytest_configure(config):
"""Stash the pluginmanager so the logreport hook can find TerminalReporter."""
global _controller_pluginmanager
if os.environ.get("PYTEST_XDIST_WORKER"):
return
@ -172,7 +149,6 @@ def _resolve_terminal_reporter():
def pytest_runtest_logreport(report):
"""Emit per-test VCR verdicts on the controller's live terminal."""
if report.when != "teardown":
return
if os.environ.get("PYTEST_XDIST_WORKER"):

View File

@ -401,16 +401,9 @@ class BaseLLMChatTest(ABC):
{
"type": "file",
"file": {
# jsDelivr serves the repo's tests/llm_translation/fixtures/dummy.pdf
# with `Content-Type: application/pdf`. Two reasons we don't
# use raw.githubusercontent.com or upload.wikimedia.org:
# - raw GitHub returns Content-Type: application/octet-stream,
# which OpenAI/Gemini reject when LiteLLM fetches the URL
# client-side and forwards the bytes.
# - Wikimedia URLs intermittently return 400 from Anthropic's
# server-side URL fetcher.
# The URL is pinned to a specific commit SHA so jsDelivr can
# serve it as immutable (cache-control: immutable, max-age=1y).
# SHA-pinned jsDelivr mirror of tests/llm_translation/fixtures/dummy.pdf;
# raw.githubusercontent.com serves PDFs as application/octet-stream
# which OpenAI/Gemini reject when LiteLLM client-fetches the URL.
"file_id": "https://cdn.jsdelivr.net/gh/BerriAI/litellm@aab3ef8988b12d166b20356a81c53127480f1125/tests/llm_translation/fixtures/dummy.pdf"
},
},

View File

@ -28,19 +28,12 @@ from tests._vcr_redis_persister import ( # noqa: E402
)
# Controller-side handles for writing per-test VCR verdicts to the live
# terminal. ``pytest_configure`` stashes the pluginmanager (workers don't get
# a TerminalReporter — their output is captured and aggregated by the
# controller), and ``pytest_runtest_logreport`` resolves the TerminalReporter
# lazily on first use because it isn't registered yet at conftest configure
# time.
_controller_pluginmanager = None
_controller_terminal_reporter = None
# vcrpy and respx both patch the httpx transport — applying both makes one
# silently win. Files in this set use respx and are skipped by the
# auto-marker below.
# silently win, so respx-using files opt out of the auto-marker.
_RESPX_CONFLICTING_FILES = frozenset(
{
"test_azure_o_series.py",
@ -58,26 +51,11 @@ _VCR_AUTO_MARKER_SKIP_FILES = _RESPX_CONFLICTING_FILES | frozenset(
)
# Tests that observe live cross-call provider state (e.g. prompt-cache
# warm-up between two consecutive calls) cannot benefit from cassette
# replay: the second call's "expected" state depends on what the *live*
# provider does between the two calls, not on what was recorded earlier.
# Auto-marking them with @pytest.mark.vcr just wastes cycles and (before
# the outcome gate) used to poison the cache. They go live with their
# existing @pytest.mark.flaky retry logic.
#
# Match by suffix on the pytest nodeid so subclassed/parametrized variants
# are covered: e.g. "::test_prompt_caching" matches all subclasses that
# inherit the base test.
# warm-up between two consecutive calls); replay can't reproduce that state.
_VCR_INCOMPATIBLE_NODEID_SUFFIXES = frozenset(
{
# Provider prompt-cache propagation isn't deterministic between two
# back-to-back calls; the test is flaky against the live provider.
"::test_prompt_caching",
# Bedrock Nova returns tool_call vs JSON nondeterministically; the
# base assertion expects JSON. Other providers' versions of this
# test are healthy, so we narrow with a class-name guard below.
"TestBedrockInvokeNovaJson::test_json_response_pydantic_obj",
# Bedrock streaming response_cost calc returns None intermittently.
"::test_bedrock_converse__streaming_passthrough",
}
)
@ -156,9 +134,6 @@ def vcr_config():
def _vcr_disabled() -> bool:
if os.environ.get("LITELLM_VCR_DISABLE") == "1":
return True
# Cassettes live on a dedicated Redis (CASSETTE_REDIS_URL) so the cache
# isn't shared with — and accidentally flushed by — tests that exercise
# the application Redis via REDIS_URL/REDIS_HOST.
return not os.environ.get("CASSETTE_REDIS_URL")
@ -171,12 +146,6 @@ def pytest_recording_configure(config, vcr):
@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_makereport(item, call):
"""Attach each phase's report to the item so fixture teardown can read it.
Used by ``_vcr_outcome_gate`` below to skip persisting cassettes for
failed test runs (incl. failed retries that pytest-rerunfailures will
re-attempt) so a "bad luck" recording can't poison future replays.
"""
outcome = yield
rep = outcome.get_result()
setattr(item, f"rep_{rep.when}", rep)
@ -184,19 +153,6 @@ def pytest_runtest_makereport(item, call):
@pytest.fixture(autouse=True)
def _vcr_outcome_gate(request, vcr):
"""Tell the persister whether the test that owns this cassette passed.
Runs after ``vcr`` (which yields the active Cassette). At teardown time
the call-phase report is attached to the item by the makereport hook
above, so we can mark the cassette key passed/failed before vcrpy's
Cassette.__exit__ triggers persister.save_cassette.
Stashes a per-test hit/miss verdict on ``user_properties`` so the
controller-side ``pytest_runtest_logreport`` hook can surface it to the
live terminal. xdist serializes ``user_properties`` on each phase's
report back to the controller, which is the only process that has a
TerminalReporter wired to CI's live log.
"""
yield
cassette = vcr
rep_call = getattr(request.node, "rep_call", None)
@ -212,20 +168,13 @@ def _vcr_outcome_gate(request, vcr):
def pytest_configure(config):
"""Stash the pluginmanager so the logreport hook can find TerminalReporter.
We can't grab TerminalReporter directly here — it's not registered until
pytest's own ``pytest_configure`` runs, and conftest hooks may run first.
Stashing the config is enough; the hook resolves on first use.
"""
global _controller_pluginmanager
if os.environ.get("PYTEST_XDIST_WORKER"):
return # workers don't have a live-log TerminalReporter
return
_controller_pluginmanager = config.pluginmanager
def _resolve_terminal_reporter():
"""Lazy-resolve the TerminalReporter once it's been registered."""
global _controller_terminal_reporter
if _controller_terminal_reporter is not None:
return _controller_terminal_reporter
@ -238,15 +187,10 @@ def _resolve_terminal_reporter():
def pytest_runtest_logreport(report):
"""Print VCR verdicts on the controller, alongside PASSED/FAILED markers.
Runs once per phase per test. We pick teardown so the verdict (appended
in ``_vcr_outcome_gate`` teardown) is present in ``report.user_properties``.
"""
if report.when != "teardown":
return
if os.environ.get("PYTEST_XDIST_WORKER"):
return # only the controller has a live-log TerminalReporter
return
if not vcr_verbose_enabled():
return
reporter = _resolve_terminal_reporter()

View File

@ -74,10 +74,6 @@ def test_load_missing_key_raises_cassette_not_found():
def test_redis_key_normalizes_path_passed_by_pytest_recording():
# pytest-recording passes paths shaped like
# ``<test_dir>/cassettes/<module>/<test>.yaml``. The persister stores them
# under a clean test-identifier key — no extension, no ``cassettes/``
# directory segment — so ``redis-cli keys`` reads as test IDs.
raw = "tests/llm_translation/cassettes/test_anthropic/test_streaming.yaml"
assert (
redis_key_for(raw)
@ -86,8 +82,6 @@ def test_redis_key_normalizes_path_passed_by_pytest_recording():
class _FlakyRedis:
"""Wraps a fake redis but raises ConnectionError on the chosen op."""
def __init__(self, inner, fail_on: str):
self._inner = inner
self._fail_on = fail_on
@ -104,7 +98,6 @@ class _FlakyRedis:
def test_save_swallows_connection_errors_so_teardown_does_not_fail():
# Persistence is a cache; an outage shouldn't fail an otherwise-passing test.
flaky = _FlakyRedis(fakeredis.FakeStrictRedis(), fail_on="set")
persister = make_redis_persister(client=flaky)
@ -116,18 +109,15 @@ def test_save_swallows_connection_errors_so_teardown_does_not_fail():
def test_save_skipped_when_test_marked_failed_and_prior_cassette_preserved():
# A flaky test that fails should NOT overwrite a previously-good cassette.
fake, persister = _persister_with_fake_redis()
cassette_id = "tests/llm_translation/test_x/test_flaky"
key = redis_key_for(cassette_id)
# Seed a "known-good" recording from a prior successful run.
good = _sample_cassette_dict()
persister.save_cassette(cassette_id, good, yamlserializer)
good_payload = fake.get(key)
assert good_payload is not None
# Simulate a failed run: the hook records "did not pass" before save.
mark_test_outcome_for_cassette(cassette_id, passed=False)
bad_response = {
"status": {"code": 200, "message": "OK"},
@ -137,7 +127,6 @@ def test_save_skipped_when_test_marked_failed_and_prior_cassette_preserved():
bad = {"requests": good["requests"], "responses": [bad_response]}
persister.save_cassette(cassette_id, bad, yamlserializer)
# Prior good payload is still there — the bad save was suppressed.
assert fake.get(key) == good_payload
@ -153,8 +142,6 @@ def test_save_proceeds_when_test_marked_passed():
def test_save_refused_when_cassette_exceeds_max_episodes():
# Pathological cassettes (non-deterministic body → unbounded episode growth)
# should be refused. Any prior good payload stays intact.
fake, persister = _persister_with_fake_redis()
cassette_id = "tests/llm_translation/test_x/test_runaway"
key = redis_key_for(cassette_id)
@ -179,7 +166,6 @@ def test_save_refused_when_cassette_exceeds_max_episodes():
}
persister.save_cassette(cassette_id, bloated, yamlserializer)
# Refused — the seed payload is unchanged.
assert fake.get(key) == seed_payload
@ -209,8 +195,6 @@ def test_save_proceeds_at_max_episodes_threshold():
def test_save_proceeds_when_outcome_unknown():
# Used outside a pytest run (e.g. ad-hoc scripts), the outcome gate is
# bypassed so the persister still works.
fake, persister = _persister_with_fake_redis()
cassette_id = "tests/llm_translation/test_x/test_no_marker"
key = redis_key_for(cassette_id)
@ -221,8 +205,6 @@ def test_save_proceeds_when_outcome_unknown():
def test_load_treats_connection_errors_as_cassette_miss():
# An outage on read should fall through to a live call (CassetteNotFound),
# not surface a redis exception in the test setup.
flaky = _FlakyRedis(fakeredis.FakeStrictRedis(), fail_on="get")
persister = make_redis_persister(client=flaky)