fix(tests): await in-flight requests in loading-state tests - #505
Merged
Conversation
Two loading-state tests mocked a request that resolves on a 100ms timer,
asserted the loading text synchronously, then returned without awaiting
it. The timer outlives the test: vitest tears down jsdom, the component's
late setState reaches React with no `window`, and the run dies with
ReferenceError: window is not defined
at resolveUpdatePriority (react-dom-client.development.js)
at handleSubmit (src/components/reviews/ReviewForm.tsx:58)
Because it depends on whether teardown lands inside that 100ms window,
it surfaces as an unrelated random CI failure — the suite passes locally
and on PR branches, then fails on a master push (run 30119962468).
Awaiting the loading state's disappearance keeps the request inside the
test and also strengthens both tests, which previously only checked that
the loading state appeared, never that it cleared.
Predates the bulk-payments work (the pattern dates to e5703f3, Jan 2026);
that PR only shifted worker timing enough to expose it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
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
1 participant
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.
Fixes the
testjob failure on master (run 30119962468).What broke
Two loading-state tests mock a request that resolves on a 100ms timer, assert the loading text synchronously, then return without ever awaiting it:
The timer outlives the test. Vitest tears down jsdom, then the component's
finally { setIsSubmitting(false) }fires and reaches React with nowindow.Why it looked random
It only fails when teardown happens to land inside that 100ms window, so it passes locally and on PR branches and then fails on a master push — and it reports against whichever file was unlucky, not the one at fault.
Affected:
ReviewForm.test.tsxandNotificationBell.test.tsx(same pattern).Fix
Await the loading state clearing, which keeps the request inside the test. This also strengthens both tests — they previously asserted only that the loading state appeared, never that it cleared.
Not caused by the bulk-payments PR
The pattern dates to
e5703f3(Jan 2026); #504 didn't touch either file. Adding a test file shifted worker timing just enough to expose the latent race.Full suite: 1765 pass. Pre-commit gate (lint + type-check + tests + build) passed.
🤖 Generated with Claude Code