Skip to content

Prevent indexed cache TTL test flakes - #2184

Open
kriszyp wants to merge 3 commits into
mainfrom
fix/caching-indexed-ttl-flake
Open

Prevent indexed cache TTL test flakes#2184
kriszyp wants to merge 3 commits into
mainfrom
fix/caching-indexed-ttl-flake

Conversation

@kriszyp

@kriszyp kriszyp commented Aug 16, 2026

Copy link
Copy Markdown
Member

The indexed cache test depended on a 5 ms expiry and 10 ms sleeps, so loaded runners could observe an additional revalidation before the exact source-request assertion. This change controls the source expiry explicitly, waits on cache state and source-write locks, preserves exact request counts, and verifies secondary-index cleanup directly.

For the human reviewer

No open judgment calls — mechanics verified by cross-model review (receipt @ 0ad3ae1).

Verification

  • npm run build — passed.
  • npx mocha unitTests/resources/caching.test.js --grep "Can load cached indexed data" repeated 50 times — 50/50 passed.
  • GitHub Actions unit tests on Node.js 22, 24, and 26 — all passed.
  • Negative control: restoring the old evict(23, entry, entry.version) argument makes the direct index assertion fail with 1 == 0; the corrected entry.value call passes.
  • npx prettier --write unitTests/resources/caching.test.js and focused oxlint --deny-warnings — passed.
  • npm run test:unit:resources — 1,553 passing / 15 pending; three unrelated existing failures remain in databases.test.js, randomAccessFieldsDirective.test.js, and replayStructures.test.js. The changed caching test passed in this gate.
  • End-to-end route: not observable end-to-end because this is test-only; the resource unit gate exercises the real table, source, storage, and index implementations.

Review coverage

Authored by GPT-5 Codex. Initial full review: Claude Opus 5, Cursor Composer 2.5, and Harper-domain adjudication. Final-head delta review: Claude Opus 5 ✓; Gemini via agy ✗ (no output); Cursor and domain legs pruned by the low-risk delta policy. Receipt @ 0ad3ae1.

Human-Review-Need: 3 @ 0ad3ae1

kriszyp and others added 3 commits August 15, 2026 05:54
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the 'Can load cached indexed data' unit test in caching.test.js to use robust asynchronous assertions (waitFor) instead of fixed timeouts, and introduces dynamic cache expiration control via a sourceExpiresAt variable. The feedback suggests adhering to the repository style guide by explicitly using assert.strictEqual instead of assert.equal for asserting primitive values.

Comment thread unitTests/resources/caching.test.js
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@kriszyp
kriszyp marked this pull request as ready for review August 20, 2026 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant