From 939cff04550758c2a5093961b6dc337231e62ad5 Mon Sep 17 00:00:00 2001 From: Mateo Wang <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 5 Jun 2026 10:22:41 -0700 Subject: [PATCH] test(vcr): stop refreshing cassette TTL on read so cassettes lapse after 24h (#29784) The Redis cassette persister slid the 24h TTL forward on every successful read, so any cassette replayed at least once per day never expired. With CI running more than once a day that means a recorded response is replayed forever and the suite never re-hits the provider, so a changed request or response contract goes undetected indefinitely. Drop the refresh-on-read. The TTL now counts down from the last write, so a cassette lapses 24h after it was recorded and the next run past that point re-records live and catches provider drift. Per-commit runs in between still replay from cache; only the one boundary-crossing run goes live. --- tests/_vcr_redis_persister.py | 22 +++----- .../test_vcr_redis_persister.py | 50 ++++--------------- 2 files changed, 17 insertions(+), 55 deletions(-) diff --git a/tests/_vcr_redis_persister.py b/tests/_vcr_redis_persister.py index 706d2561aa..bb76d5fb1e 100644 --- a/tests/_vcr_redis_persister.py +++ b/tests/_vcr_redis_persister.py @@ -174,21 +174,13 @@ def make_redis_persister( _log.warning(msg) warnings.warn(msg, VCRCassetteCacheWarning, stacklevel=2) raise CassetteNotFoundError() from exc - # Slide the expiry forward on every successful read. A plain GET - # does not touch the key's TTL, so a cassette that is only ever - # replayed (HIT/NOOP, never re-recorded) expires exactly - # ``ttl_seconds`` after its last *write* no matter how often it is - # read — and whichever CI run happens to cross that boundary - # re-records it live, surfacing as a spurious VCR MISS that no - # amount of matcher tolerance can prevent. Refreshing the TTL on - # read keeps any cassette used at least once per TTL window alive - # indefinitely, so the second/third run of a day replays cleanly. - # Best-effort: a failed refresh must never turn a successful load - # into a miss. - try: - redis_client.expire(key, ttl_seconds) - except RedisError: - pass + # TTL is intentionally not refreshed on read. The cassette must + # lapse ``ttl_seconds`` after its last *write*, so the next run + # past that point re-records live and catches provider request or + # response contract drift instead of replaying a frozen response + # forever. Sliding the expiry forward on read would keep an + # actively-used cassette alive indefinitely and that drift check + # would never run. return result @staticmethod diff --git a/tests/llm_translation/test_vcr_redis_persister.py b/tests/llm_translation/test_vcr_redis_persister.py index 7262c09c9b..236ed77522 100644 --- a/tests/llm_translation/test_vcr_redis_persister.py +++ b/tests/llm_translation/test_vcr_redis_persister.py @@ -79,57 +79,27 @@ def test_load_missing_key_raises_cassette_not_found(): persister.load_cassette("never/recorded", yamlserializer) -def test_load_refreshes_ttl_so_replayed_cassettes_do_not_expire(): - """A successful read must slide the cassette's expiry forward. +def test_load_does_not_refresh_ttl_so_cassettes_lapse_after_write(): + """A successful read must not slide the cassette's expiry forward. - Regression: ``load_cassette`` used a plain ``GET``, which does not - touch the key's TTL. A cassette that is only ever replayed (HIT/NOOP, - never re-recorded) therefore expired exactly ``CASSETTE_TTL_SECONDS`` - after its last *write* no matter how often it was read, and whichever - CI run crossed that 24h boundary re-recorded it live — a spurious VCR - MISS on otherwise-deterministic cassettes. Reading must refresh the - TTL so an actively-used cassette never expires. + The TTL deliberately counts down from the last *write*: a cassette that + is only ever replayed must still lapse ``CASSETTE_TTL_SECONDS`` after it + was recorded, so the next run past that point re-records live and catches + provider request/response contract drift. Refreshing the TTL on read + would keep an actively-used cassette alive forever and that drift check + would never run. """ fake, persister = _persister_with_fake_redis() - cassette_id = "tests/llm_translation/test_x/test_ttl_refresh" + cassette_id = "tests/llm_translation/test_x/test_ttl_no_refresh" key = redis_key_for(cassette_id) persister.save_cassette(cassette_id, _sample_cassette_dict(), yamlserializer) # Simulate a cassette written ~most-of-a-day ago: only a little TTL left. fake.expire(key, 60) - assert fake.ttl(key) <= 60 persister.load_cassette(cassette_id, yamlserializer) - refreshed = fake.ttl(key) - assert CASSETTE_TTL_SECONDS - 5 <= refreshed <= CASSETTE_TTL_SECONDS - - -def test_load_ttl_refresh_failure_does_not_break_load(): - """A failed TTL refresh must never turn a successful load into a miss.""" - - class _RefreshFailsRedis: - def __init__(self, inner): - self._inner = inner - - def get(self, *args, **kwargs): - return self._inner.get(*args, **kwargs) - - def set(self, *args, **kwargs): - return self._inner.set(*args, **kwargs) - - def expire(self, *args, **kwargs): - raise RedisConnectionError("simulated outage") - - client = _RefreshFailsRedis(fakeredis.FakeStrictRedis()) - persister = make_redis_persister(client=client) - cassette_id = "tests/llm_translation/test_x/test_ttl_refresh_fail" - - persister.save_cassette(cassette_id, _sample_cassette_dict(), yamlserializer) - requests, responses = persister.load_cassette(cassette_id, yamlserializer) - - assert len(requests) == 1 - assert len(responses) == 1 + assert fake.ttl(key) <= 60 def test_redis_key_normalizes_path_passed_by_pytest_recording():