test: replace fixed-delay sleep-then-assert races in subscriptionReplay - #1219
Merged
Conversation
Convert the "await delay(N); assert(side effect landed)" timing races in unitTests/resources/subscriptionReplay.test.js to condition-waits via the shared waitFor() helper. These fixed waits raced loaded CI runners and are an instance of the flakiness tracked in #1138. Legitimate sleeps that model elapsed time (subscribe-then-write gaps, collect() quiet periods) are left as-is. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
github-actions
Bot
requested review from
Ethan-Arrowood,
cb1kenobi,
heskew and
kriszyp
June 9, 2026 22:02
Contributor
Author
|
Reviewed; no blockers found. |
cb1kenobi
approved these changes
Jun 10, 2026
This was referenced Jul 2, 2026
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
2 participants
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.
Summary
Converts the racy
await delay(N); assert(<async side effect happened>)patterns inunitTests/resources/subscriptionReplay.test.jsto condition-waits using the sharedwaitFor()helper (unitTests/waitFor.js). These fixed wall-clock waits race loaded CI runners — the root flakiness pattern tracked in the umbrella issue #1138 (the same shape behind #19's load-dependent failures).Six sites converted:
race conditions > FIRST-subscription-on-fresh-DB ...— wait until all 200 in-flight ids deliveredrace conditions > startTime: subscribe while writes are in flight ...— wait until all 200 in-flight ids deliveredrace conditions > !omitCurrent: subscribe-then-write on a fresh database ...— wait for the single post-subscribe event (kept thedelay(50)that models production "subscribe established, then live write" timing)edge cases > count: empty initial state, then live commits ...— wait for 5 live eventsedge cases > count: only other-table records exist ...— wait for our 3 live eventsedge cases > whenNextTransaction resolves on commit ...— wait for the waiter to resolve (textbookdelay; assert(resolved)case)Legitimate sleeps that model elapsed time (the
collect()quiet-period drain, brief cursor-advance gaps, the subscribe-then-write production-timing gap) are intentionally left as fixeddelay().No production code changed — test surface only.
Test plan
npm run lint(oxlint) cleannpm run format:check— modified file clean (only untracked.harper-skills/flagged)npm run buildpassesnpm run test:unit -- unitTests/resources/subscriptionReplay.test.js— 27 passing, 9 pending (count-branch tests are LMDB-only)HARPER_STORAGE_ENGINE=lmdbinvocation locally; the converts are structurally identical to the rocksdb-running ones that pass)Closes #1138
🤖 Generated with Claude Code