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.
This commit is contained in:
parent
074455c138
commit
939cff0455
@ -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
|
||||
|
||||
@ -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():
|
||||
|
||||
Loading…
Reference in New Issue
Block a user