Skip to content

Commit 491a522

Browse files
cliffhallclaude
andcommitted
fix(stories): wait on dialog role with longer timeout for ServerConfigModal plays
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) <noreply@anthropic.com>
1 parent f6ac075 commit 491a522

1 file changed

Lines changed: 26 additions & 11 deletions

File tree

clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.stories.tsx

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,21 +57,35 @@ const meta: Meta<typeof ServerConfigModal> = {
5757
export default meta;
5858
type Story = StoryObj<typeof ServerConfigModal>;
5959

60+
// Mantine's Modal portals to `document.body` outside the storybook
61+
// canvas, and its first paint can race against the 1000ms default that
62+
// `findBy*` queries retry against — especially in the Storybook dev UI
63+
// (slower than `test:storybook`'s headless browser, particularly with
64+
// devtools open or coverage instrumentation on). Waiting on the dialog
65+
// role with a longer ceiling is more reliable than `findByText` against
66+
// the title: it asserts the dialog actually mounted, scopes subsequent
67+
// queries to its subtree (no false-positive matches from sibling
68+
// loaders), and uses the accessible name Mantine derives from the
69+
// `title` prop.
70+
const DIALOG_MOUNT_TIMEOUT_MS = 5000;
71+
const findDialog = (body: ReturnType<typeof within>, name: RegExp | string) =>
72+
body.findByRole("dialog", { name }, { timeout: DIALOG_MOUNT_TIMEOUT_MS });
73+
6074
export const AddEmpty: Story = {
6175
args: { mode: "add" },
6276
play: async ({ canvasElement }) => {
6377
const body = within(canvasElement.ownerDocument.body);
64-
await expect(await body.findByText("Add server")).toBeInTheDocument();
65-
const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement;
78+
const dialog = within(await findDialog(body, "Add server"));
79+
const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement;
6680
await expect(idInput).toBeInTheDocument();
67-
await expect(body.getByLabelText(/Command/i)).toBeInTheDocument();
81+
await expect(dialog.getByLabelText(/Command/i)).toBeInTheDocument();
6882
// Regression guard for the synthetic-event currentTarget bug — happy-dom
6983
// doesn't null currentTarget after the handler returns, so unit tests
7084
// sail past it. Real Chromium (here) does, so any future onChange that
7185
// reads e.currentTarget inside a setState updater will throw here.
7286
await userEvent.type(idInput, "my-server");
7387
await expect(idInput.value).toBe("my-server");
74-
const cmdInput = body.getByLabelText(/Command/i) as HTMLInputElement;
88+
const cmdInput = dialog.getByLabelText(/Command/i) as HTMLInputElement;
7589
await userEvent.type(cmdInput, "node");
7690
await expect(cmdInput.value).toBe("node");
7791
},
@@ -86,8 +100,8 @@ export const EditStdio: Story = {
86100
},
87101
play: async ({ canvasElement }) => {
88102
const body = within(canvasElement.ownerDocument.body);
89-
await expect(await body.findByText("Edit server")).toBeInTheDocument();
90-
const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement;
103+
const dialog = within(await findDialog(body, "Edit server"));
104+
const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement;
91105
await expect(idInput.value).toBe("filesystem-server-default");
92106
},
93107
};
@@ -101,10 +115,10 @@ export const CloneStdio: Story = {
101115
},
102116
play: async ({ canvasElement }) => {
103117
const body = within(canvasElement.ownerDocument.body);
104-
await expect(await body.findByText("Clone server")).toBeInTheDocument();
105-
const idInput = body.getByLabelText(/Server ID/i) as HTMLInputElement;
118+
const dialog = within(await findDialog(body, "Clone server"));
119+
const idInput = dialog.getByLabelText(/Server ID/i) as HTMLInputElement;
106120
await expect(idInput.value).toBe("");
107-
const cmdInput = body.getByLabelText(/Command/i) as HTMLInputElement;
121+
const cmdInput = dialog.getByLabelText(/Command/i) as HTMLInputElement;
108122
await expect(cmdInput.value).toBe("npx");
109123
},
110124
};
@@ -118,8 +132,9 @@ export const EditSse: Story = {
118132
},
119133
play: async ({ canvasElement }) => {
120134
const body = within(canvasElement.ownerDocument.body);
121-
await expect(await body.findByLabelText(/^URL/)).toBeInTheDocument();
135+
const dialog = within(await findDialog(body, "Edit server"));
136+
await expect(await dialog.findByLabelText(/^URL/)).toBeInTheDocument();
122137
// Headers are no longer entered here — they live in ServerSettingsForm.
123-
await expect(body.queryByLabelText(/Headers/i)).toBeNull();
138+
await expect(dialog.queryByLabelText(/Headers/i)).toBeNull();
124139
},
125140
};

0 commit comments

Comments
 (0)