deflake imageGenQuota bus tests by draining the handler instead of sleeping 20ms - #4800
Merged
Conversation
…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.
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.
Summary
imageGenQuota's bus handlers are fire-and-forget (anEventEmittercannot 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 WindowsatomicWriteadditionally 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 bootsfailed withexpected 'No renders · 24h' to be '1 render · 24h'on the Windows runner and passed on a same-SHA re-run.__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..catchmeans the tail can never reject or go unhandled.mainfor other reasons.Test plan
npx vitest run services/imageGenQuota.test.js— 32 passed, run 10× consecutively, green every time.if (subscribed) return;guard ininitImageGenQuotaHook(breaking the "subscribe once" invariant) and confirmed the test goes red withexpected '2 renders · 24h' to be '1 render · 24h'. Restored.recordImageGenOutcome: with the new drain all 32 tests still pass; with the old 20ms sleep restored, the suite fails with exactly the CI messageexpected 'No renders · 24h' to be '1 render · 24h'. Both probes removed before committing.cd server && npm test) run twice: 32,714 / 32,752 passing. The only failures wereservices/imageTo3d/trellis2NormalBake.test.jsandroutes/settings.secretsStrip.test.jshitting the known 10s timeout under full-suite load — a different pair each run, and both pass in isolation. Neither importsimageGenQuota.Closes #4788