Fix time-bomb test fixture: dynamic expires_at in commit.test.ts - #206
Merged
Conversation
test/services/previews/v1/commit.test.ts hardcoded a fixed calendar expiry (2026-08-27T10:00:00Z) for its sample artifact. Once that date passed, ttlForArtifact() in src/services/previews/v1/routes/commit.ts computed a negative remaining-retention TTL for it, which fell below cache.ts's 60s KV floor and silently disabled caching for every test relying on it - breaking: - 'shares the metadata route's cache instead of re-listing artifacts on every download' (expected 1 artifacts-list call, got 2, since the lookup was no longer cached between the metadata and download requests) - 'caches the commit lookup for longer than the default 60s' (expected a KV put() call, got 0, since the computed TTL was negative) Replaced the fixed date with a FAR_FUTURE_EXPIRES_AT computed relative to Date.now() (matching the pattern the file's other TTL tests already use), so the fixture never again silently expires. Applied to all three artifacts in the file that represent an actual matched/cached artifact. Verified: full suite (520 tests), lint, and typecheck all pass.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
api | 748835b | Commit Preview URL Branch Preview URL |
Aug 28 2026, 12:13 AM |
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Auto-approved: Test-only fix replacing a hardcoded fixture date with a runtime-computed future date to keep caching TTL positive. No production behavior, contract, or operational tradeoffs changed; the update is confined to the test fixtures and clearly addresses the time-bomb.
Re-trigger cubic
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.
Problem
CI was failing on
main(test/services/previews/v1/commit.test.ts):shares the metadata route's cache instead of re-listing artifacts on every download— expected 1 GitHub artifacts-list call, got 2caches the commit lookup for longer than the default 60s— expected a KVput()call, got 0Root cause
The test's sample artifact fixture hardcoded a fixed calendar date,
expires_at: "2026-08-27T10:00:00Z".ttlForArtifact()insrc/services/previews/v1/routes/commit.tscomputes the KV cache TTL asartifact.expires_at - now, capped at 3600s. Once that hardcoded date passed, the computation went negative, fell belowcache.ts's 60-second Cloudflare KV minimum TTL, and caching was silently skipped entirely — breaking the two tests above, which both assume caching is active.This wasn't a production bug:
pr.tsandmain.tsuse a fixed 60s TTL and don't depend onexpires_at, so they were unaffected. Confirmed by reproducing locally and by auditing every otherDate.now()-comparison in the codebase — this was the only spot where a test fixture's absolute date fed into a real-time comparison in source.Fix
Replaced the hardcoded date with
FAR_FUTURE_EXPIRES_AT, computed asDate.now() + 365 days, matching the pattern the file's other TTL-specific tests already use (e.g.new Date(Date.now() + 500_000).toISOString()). Applied to all three fixtures in the file that represent a matched/cached artifact.Verification
test/services/previews/v1/commit.test.ts: 15/15 pass (was 13/15)npm run lint: 0 errors (2 pre-existing warnings in a generated file, unrelated)npm run typecheck: clean🤖 Generated with Claude Code
Generated by Claude Code