Skip to content

deflake imageGenQuota bus tests by draining the handler instead of sleeping 20ms - #4800

Merged
atomantic merged 1 commit into
mainfrom
claim/issue-4788
Aug 22, 2026
Merged

deflake imageGenQuota bus tests by draining the handler instead of sleeping 20ms#4800
atomantic merged 1 commit into
mainfrom
claim/issue-4788

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

  • imageGenQuota's bus handlers are fire-and-forget (an EventEmitter cannot await its listeners), so the suite slept a fixed 20ms before asserting the persisted ledger. The recorder's write is real disk I/O, and on Windows atomicWrite additionally sleeps between rename retries when the destination is briefly locked — so the write can outlast any constant a test picks. That is the flake: subscribes only once across repeated boots failed with expected 'No renders · 24h' to be '1 render · 24h' on the Windows runner and passed on a same-SHA re-run.
  • The hook now keeps a single promise tail over every outcome its handlers dispatched, and exposes __drainImageGenQuotaHookForTests() so the suite awaits the actual write instead of guessing at a duration. The drain loops until quiescent, so work a settling handler chains on is drained too.
  • Production behaviour is unchanged: dispatch still happens immediately and is still fire-and-forget; the tail only records what was dispatched. The handler's existing .catch means the tail can never reject or go unhandled.
  • Scope: this fixes only the named case's timing dependency. It does not touch the unrelated Windows-CI failures that have been red on main for other reasons.

Test plan

  • npx vitest run services/imageGenQuota.test.js — 32 passed, run 10× consecutively, green every time.
  • Not vacuous — bypass probe. Temporarily removed the if (subscribed) return; guard in initImageGenQuotaHook (breaking the "subscribe once" invariant) and confirmed the test goes red with expected '2 renders · 24h' to be '1 render · 24h'. Restored.
  • Reproduced the reported failure. Injected a 200ms artificial delay into recordImageGenOutcome: with the new drain all 32 tests still pass; with the old 20ms sleep restored, the suite fails with exactly the CI message expected 'No renders · 24h' to be '1 render · 24h'. Both probes removed before committing.
  • Full server suite (cd server && npm test) run twice: 32,714 / 32,752 passing. The only failures were services/imageTo3d/trellis2NormalBake.test.js and routes/settings.secretsStrip.test.js hitting the known 10s timeout under full-suite load — a different pair each run, and both pass in isolation. Neither imports imageGenQuota.

Closes #4788

…ng 20ms (#4788)

The imageGenQuota bus handlers are fire-and-forget (an emitter cannot await
its listeners), so the suite waited on a fixed 20ms sleep before asserting the
ledger. The recorder's write is real disk I/O, and on Windows atomicWrite also
sleeps between rename retries when the destination is briefly locked, so the
write can easily outlast that constant — which is why 'subscribes only once
across repeated boots' failed with "expected 'No renders - 24h' to be
'1 render - 24h'" on the Windows runner and passed on a same-SHA re-run.

The hook now keeps a single tail over every outcome its handlers dispatched,
and exposes __drainImageGenQuotaHookForTests() so the suite awaits the actual
write instead of guessing at a duration. Production behaviour is unchanged:
dispatch still happens immediately and is still fire-and-forget.
@atomantic
atomantic merged commit 064704d into main Aug 22, 2026
7 checks passed
@atomantic
atomantic deleted the claim/issue-4788 branch August 22, 2026 02:01
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.

deflake imageGenQuota 'subscribes only once across repeated boots' on Windows CI

1 participant