From 491a522e8de59b3ddc5910165bd4125aafd826d3 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 26 May 2026 14:41:00 -0400 Subject: [PATCH] fix(stories): wait on dialog role with longer timeout for ServerConfigModal plays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four ServerConfigModal play functions used `findByText("Add server")` etc. to assert the modal mounted. Mantine's Modal portals to document.body outside the storybook canvas, and its first paint can race against the 1000ms default that `findBy*` queries retry against — especially in the Storybook dev UI (slower than `test:storybook`'s headless browser, particularly with devtools open or coverage instrumentation on). When the play fired before the modal mounted, findByText timed out, Storybook flagged the story as FAIL, and left the "preparing" loader visible. Switch to `findByRole('dialog', { name })` with a 5s timeout, and scope subsequent queries (`getByLabelText`, etc.) to the dialog's subtree via `within(dialog)`. The role-based query asserts the dialog actually mounted (more semantically meaningful than a free text lookup), the longer ceiling covers slow first-paint, and scoping to the dialog avoids false-positive matches from sibling loader overlays. Verified locally: 1758 tests pass, 8/8 consecutive reloads of the Add Empty story in the Storybook dev UI now show "Pass". Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ServerConfigModal.stories.tsx | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx index 5bd4c539..c180a95b 100644 --- a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx +++ b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx @@ -57,21 +57,35 @@ const meta: Meta = { export default meta; type Story = StoryObj; +// Mantine's Modal portals to `document.body` outside the storybook +// canvas, and its first paint can race against the 1000ms default that +// `findBy*` queries retry against — especially in the Storybook dev UI +// (slower than `test:storybook`'s headless browser, particularly with +// devtools open or coverage instrumentation on). Waiting on the dialog +// role with a longer ceiling is more reliable than `findByText` against +// the title: it asserts the dialog actually mounted, scopes subsequent +// queries to its subtree (no false-positive matches from sibling +// loaders), and uses the accessible name Mantine derives from the +// `title` prop. +const DIALOG_MOUNT_TIMEOUT_MS = 5000; +const findDialog = (body: ReturnType, name: RegExp | string) => + body.findByRole("dialog", { name }, { timeout: DIALOG_MOUNT_TIMEOUT_MS }); + export const AddEmpty: Story = { args: { mode: "add" }, play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); - await expect(await body.findByText("Add server")).toBeInTheDocument(); - const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement; + const dialog = within(await findDialog(body, "Add server")); + const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement; await expect(idInput).toBeInTheDocument(); - await expect(body.getByLabelText(/Command/i)).toBeInTheDocument(); + await expect(dialog.getByLabelText(/Command/i)).toBeInTheDocument(); // Regression guard for the synthetic-event currentTarget bug — happy-dom // doesn't null currentTarget after the handler returns, so unit tests // sail past it. Real Chromium (here) does, so any future onChange that // reads e.currentTarget inside a setState updater will throw here. await userEvent.type(idInput, "my-server"); await expect(idInput.value).toBe("my-server"); - const cmdInput = body.getByLabelText(/Command/i) as HTMLInputElement; + const cmdInput = dialog.getByLabelText(/Command/i) as HTMLInputElement; await userEvent.type(cmdInput, "node"); await expect(cmdInput.value).toBe("node"); }, @@ -86,8 +100,8 @@ export const EditStdio: Story = { }, play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); - await expect(await body.findByText("Edit server")).toBeInTheDocument(); - const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement; + const dialog = within(await findDialog(body, "Edit server")); + const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement; await expect(idInput.value).toBe("filesystem-server-default"); }, }; @@ -101,10 +115,10 @@ export const CloneStdio: Story = { }, play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); - await expect(await body.findByText("Clone server")).toBeInTheDocument(); - const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement; + const dialog = within(await findDialog(body, "Clone server")); + const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement; await expect(idInput.value).toBe(""); - const cmdInput = body.getByLabelText(/Command/i) as HTMLInputElement; + const cmdInput = dialog.getByLabelText(/Command/i) as HTMLInputElement; await expect(cmdInput.value).toBe("npx"); }, }; @@ -118,8 +132,9 @@ export const EditSse: Story = { }, play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); - await expect(await body.findByLabelText(/^URL/)).toBeInTheDocument(); + const dialog = within(await findDialog(body, "Edit server")); + await expect(await dialog.findByLabelText(/^URL/)).toBeInTheDocument(); // Headers are no longer entered here — they live in ServerSettingsForm. - await expect(body.queryByLabelText(/Headers/i)).toBeNull(); + await expect(dialog.queryByLabelText(/Headers/i)).toBeNull(); }, };