From 83d2d1f948a118cad850ac8eb4c15624d5626999 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 25 Jul 2026 01:04:48 +0000 Subject: [PATCH] fix(tests): await in-flight requests in loading-state tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/components/notifications/NotificationBell.test.tsx | 7 +++++++ src/components/reviews/ReviewForm.test.tsx | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/components/notifications/NotificationBell.test.tsx b/src/components/notifications/NotificationBell.test.tsx index 902ba537..30589a1d 100644 --- a/src/components/notifications/NotificationBell.test.tsx +++ b/src/components/notifications/NotificationBell.test.tsx @@ -186,6 +186,13 @@ describe("NotificationBell", () => { fireEvent.click(button); expect(screen.getByText("Loading...")).toBeInTheDocument(); + + // Let the mocked 100ms request settle before the test ends — a timer that + // outlives the test fires after vitest tears down jsdom, and the resulting + // state update throws "window is not defined" from an unrelated file. + await waitFor(() => + expect(screen.queryByText("Loading...")).not.toBeInTheDocument() + ); }); it("marks notification as read when clicked", async () => { diff --git a/src/components/reviews/ReviewForm.test.tsx b/src/components/reviews/ReviewForm.test.tsx index a5fb43ec..d3844a35 100644 --- a/src/components/reviews/ReviewForm.test.tsx +++ b/src/components/reviews/ReviewForm.test.tsx @@ -315,6 +315,15 @@ describe("ReviewForm", () => { fireEvent.click(submitButton); expect(screen.getByText("Submitting...")).toBeInTheDocument(); + + // Let the mocked 100ms request settle before the test ends. Otherwise the + // timer outlives the test, vitest tears down jsdom, and the component's + // late setIsSubmitting(false) hits React with no `window` — surfacing as + // "ReferenceError: window is not defined" in an unrelated file. It only + // reproduces under CI timing, so it reads as a random CI failure. + await waitFor(() => + expect(screen.queryByText("Submitting...")).not.toBeInTheDocument() + ); }); it("trims comment before sending", async () => {