From 55464204a6abe573a9a2eeac50aa6e45c95273dd Mon Sep 17 00:00:00 2001 From: Tonio Date: Mon, 17 Aug 2026 00:58:16 -0700 Subject: [PATCH] test(ui): wait for conditions in the last four fixed-turn loops (#11523) The remaining instances of the pattern #11499 and #11521 replaced in the routing tests, found by grepping `attempt < N` across the suite. Budgets of 20, 25 and 30 turns rather than 3 and 5, which is why they surfaced far less often - SkillStudio was among the failures seen while verifying the earlier PRs. Three of the four were reimplementations of `vi.waitFor` down to rethrowing the last error, differing only in bounding on turns rather than on time. The fourth was the text variant. Two `flushReact` helpers became dead with the loops that used them and are removed. Verified on the mechanism, since budgets this large pass until the machine is loaded and so prove nothing by passing: a throwaway probe drove a 30-turn loop and `vi.waitFor` against a value landing at turn 60. The loop throws, `vi.waitFor` reaches it. Not a lateral move between arbitrary bounds. This closes one spelling of the pattern, not the class, and the sweep that found these was too narrow. Two other shapes do the same thing and a grep for `attempt < N` cannot see either: a fixed-cycle helper, `flushReact(cycles = 4)` in AgentToolsTab.test.tsx, and fixed-duration sleeps in AgentToolsTab, CompanyContext, AgentConfigForm.render, Artifacts, Search and ImportFromVaultDialog. `AgentToolsTab > autosaves installed apps for the current agent` failed one of three full-suite runs here, holds both shapes, and is untouched by this change. Refs #11484. ui typecheck clean; the four files pass; full suite passes two of three runs, the third failing only on that pre-existing instance. Co-Authored-By: Claude Opus 5 --- ui/src/App.test.tsx | 21 ++++++------- .../components/WorkspaceFileBrowser.test.tsx | 25 +++++++++------- ...paceFileMarkdownBody.availability.test.tsx | 25 +++++++++------- ui/src/pages/SkillStudio.test.tsx | 30 +++++++++---------- 4 files changed, 53 insertions(+), 48 deletions(-) diff --git a/ui/src/App.test.tsx b/ui/src/App.test.tsx index ff00e41c09..920f192baf 100644 --- a/ui/src/App.test.tsx +++ b/ui/src/App.test.tsx @@ -43,17 +43,18 @@ vi.mock("@/lib/router", () => ({ useParams: () => ({}), })); -async function flushReact() { - await Promise.resolve(); - await new Promise((resolve) => window.setTimeout(resolve, 0)); -} - +/** + * Waits on the condition, not on a fixed number of turns. A hand-rolled retry + * loop is ample on an idle machine and not when the suite runs many workers in + * parallel: it gives up after N turns and reports a failure on behaviour that + * works. `vi.waitFor` retries against a time budget, so a loaded worker gets + * more turns instead. + * + * Same replacement as #11499 and #11521, which fixed the shorter-budget + * instances of this in the routing tests. + */ async function waitForText(container: HTMLElement, text: string) { - for (let attempt = 0; attempt < 20; attempt += 1) { - if (container.textContent?.includes(text)) return; - await flushReact(); - } - expect(container.textContent).toContain(text); + await vi.waitFor(() => expect(container.textContent).toContain(text)); } function renderGate(container: HTMLElement) { diff --git a/ui/src/components/WorkspaceFileBrowser.test.tsx b/ui/src/components/WorkspaceFileBrowser.test.tsx index 6277c65fd5..876794ace4 100644 --- a/ui/src/components/WorkspaceFileBrowser.test.tsx +++ b/ui/src/components/WorkspaceFileBrowser.test.tsx @@ -16,18 +16,21 @@ function act(callback: () => void | Promise) { return result; } +/** + * Waits on the condition, not on a fixed number of turns. A hand-rolled retry + * loop is ample on an idle machine and not when the suite runs many workers in + * parallel: it gives up after N turns and reports a failure on behaviour that + * works. `vi.waitFor` retries against a time budget, so a loaded worker gets + * more turns instead. + * + * Same replacement as #11499 and #11521, which fixed the shorter-budget + * instances of this in the routing tests. + * + * This was a reimplementation of `vi.waitFor` down to rethrowing the last + * error, differing only in bounding on turns rather than on time. + */ async function waitForExpectation(assertion: () => void) { - let lastError: unknown; - for (let attempt = 0; attempt < 20; attempt += 1) { - try { - assertion(); - return; - } catch (error) { - lastError = error; - await new Promise((resolve) => window.setTimeout(resolve, 0)); - } - } - throw lastError; + await vi.waitFor(assertion); } const useQueryMock = vi.fn(); diff --git a/ui/src/components/WorkspaceFileMarkdownBody.availability.test.tsx b/ui/src/components/WorkspaceFileMarkdownBody.availability.test.tsx index aa62337c5a..1d127c13cb 100644 --- a/ui/src/components/WorkspaceFileMarkdownBody.availability.test.tsx +++ b/ui/src/components/WorkspaceFileMarkdownBody.availability.test.tsx @@ -44,18 +44,21 @@ function act(callback: () => void) { flushSync(callback); } +/** + * Waits on the condition, not on a fixed number of turns. A hand-rolled retry + * loop is ample on an idle machine and not when the suite runs many workers in + * parallel: it gives up after N turns and reports a failure on behaviour that + * works. `vi.waitFor` retries against a time budget, so a loaded worker gets + * more turns instead. + * + * Same replacement as #11499 and #11521, which fixed the shorter-budget + * instances of this in the routing tests. + * + * This was a reimplementation of `vi.waitFor` down to rethrowing the last + * error, differing only in bounding on turns rather than on time. + */ async function waitForExpectation(assertion: () => void) { - let lastError: unknown; - for (let attempt = 0; attempt < 30; attempt += 1) { - try { - assertion(); - return; - } catch (error) { - lastError = error; - await new Promise((resolve) => window.setTimeout(resolve, 0)); - } - } - throw lastError; + await vi.waitFor(assertion); } function resource(overrides: Partial = {}): ResolvedWorkspaceResource { diff --git a/ui/src/pages/SkillStudio.test.tsx b/ui/src/pages/SkillStudio.test.tsx index a8afa4b62f..dd01d631a6 100644 --- a/ui/src/pages/SkillStudio.test.tsx +++ b/ui/src/pages/SkillStudio.test.tsx @@ -150,23 +150,21 @@ async function act(callback: () => void | Promise) { await result; } -async function flushReact() { - await Promise.resolve(); - await new Promise((resolve) => window.setTimeout(resolve, 0)); -} - +/** + * Waits on the condition, not on a fixed number of turns. A hand-rolled retry + * loop is ample on an idle machine and not when the suite runs many workers in + * parallel: it gives up after N turns and reports a failure on behaviour that + * works. `vi.waitFor` retries against a time budget, so a loaded worker gets + * more turns instead. + * + * Same replacement as #11499 and #11521, which fixed the shorter-budget + * instances of this in the routing tests. + * + * This was a reimplementation of `vi.waitFor` down to rethrowing the last + * error, differing only in bounding on turns rather than on time. + */ async function waitFor(assertion: () => void) { - let lastError: unknown; - for (let attempt = 0; attempt < 25; attempt += 1) { - try { - assertion(); - return; - } catch (error) { - lastError = error; - await flushReact(); - } - } - throw lastError; + await vi.waitFor(assertion); } async function renderStudio() {