From 4bad110942be842c3f3aae660e3e95bfacf42059 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 21:11:11 -0400 Subject: [PATCH 1/2] test(web): eliminate timeout flakiness in the web test suite (closes #1596) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-cause and deterministically fix the recurring 5s-timeout flakes that surfaced under v8-instrumented, concurrent load. All fixes are test-only; no product source changed. No test was skipped or disabled, and the per-file coverage gate (>=90 on all four dims) still holds. Common root cause across the modal/form/screen suites: userEvent.setup() schedules a real setTimeout between keystrokes. Under CPU contention those yields balloon, so multi-field interactions blew past the 5s per-test timeout. Fix: userEvent.setup({ delay: null }) removes the per-keystroke real-timer dependence (typing dispatches synchronously), making the suites load-independent. Per file: - ServerConfigModal / ServerImportConfigModal / ServerImportJsonModal / ResourcesScreen / InspectorView: userEvent.setup({ delay: null }). - PromptArgumentsForm (completions): replaced wall-clock sleeps sized just past the 300ms completion debounce (await setTimeout(400)) — which race the debounce timer under load — with awaited conditions (findBy/waitFor) on the real rendered outcome, plus delay:null. Negative assertions now lean on the component's synchronous state/timer teardown instead of a timed window, so they are deterministic regardless of machine speed. - inspectorClient integration "tracks stderr logs": the child's stderr is piped out-of-band from the tool's JSON-RPC response, so reading the log synchronously after callTool raced the stderr chunk. Wrapped the assertion in vi.waitFor so it polls until the line lands. Verified: web validate + 6x test:coverage (4 sequential + 2 concurrent under load) + standalone test:integration, all green (229 files / 3237 tests per coverage run; 799 integration tests), zero intermittent failures. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- .../PromptArgumentsForm.test.tsx | 170 ++++++++++-------- .../ServerConfigModal.test.tsx | 42 ++--- .../ServerImportConfigModal.test.tsx | 44 ++--- .../ServerImportJsonModal.test.tsx | 16 +- .../ResourcesScreen/ResourcesScreen.test.tsx | 34 ++-- .../InspectorView/InspectorView.test.tsx | 24 +-- .../integration/mcp/inspectorClient.test.ts | 22 ++- 7 files changed, 192 insertions(+), 160 deletions(-) diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index 679bd02e..88871639 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -2,9 +2,23 @@ import { useState } from "react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; -import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { + renderWithMantine, + screen, + waitFor, +} from "../../../test/renderWithMantine"; import { PromptArgumentsForm } from "./PromptArgumentsForm"; +// Completion requests are debounced (COMPLETION_DEBOUNCE_MS = 300ms). Rather +// than sleeping past that window on the wall clock — which races the debounce +// timer under instrumented/concurrent load (#1596) — every completion test +// below uses `userEvent.setup({ delay: null })` (no per-keystroke real-timer +// scheduling) and waits on the *real* rendered outcome via `findBy`/`waitFor`. +// Those poll until the debounce fires and the response settles, so they +// resolve as soon as the work is done and never depend on machine speed. A +// scoped 3s timeout covers the debounce + async settle under heavy load. +const SETTLE = { timeout: 3000 } as const; + /** * Wrapper that owns `argumentValues` state so completion tests can * type multi-character input naturally — the production parent @@ -130,7 +144,7 @@ describe("PromptArgumentsForm", () => { }); it("invokes onArgumentChange when typing in an argument input", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onArgumentChange = vi.fn(); renderWithMantine( { }); it("clears an argument via its Clear button (onArgumentChange with empty value)", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onArgumentChange = vi.fn(); renderWithMantine( { }); it("invokes onGetPrompt when Get Prompt is clicked", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onGetPrompt = vi.fn(); renderWithMantine( { }); it("disables Get Prompt until every required argument has a value", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( , ); @@ -219,7 +233,7 @@ describe("PromptArgumentsForm", () => { describe("completions", () => { it("fires a completion immediately on focus before any keystroke", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -240,17 +254,16 @@ describe("PromptArgumentsForm", () => { ); await user.click(screen.getByRole("textbox", { name: /^text/ })); - // No debounce on focus — the call fires synchronously off the - // focus handler. A microtask is enough for the response to settle. - await new Promise((r) => setTimeout(r, 0)); + // No debounce on focus — the call fires synchronously off the focus + // handler; wait on the rendered option rather than a fixed sleep. + expect(await screen.findByText("alpha")).toBeInTheDocument(); expect(onCompleteArgument).toHaveBeenCalledWith("text", "", { targetLanguage: "", }); - expect(await screen.findByText("alpha")).toBeInTheDocument(); }); it("calls onCompleteArgument (debounced) and surfaces values when supported", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -271,18 +284,19 @@ describe("PromptArgumentsForm", () => { ); await user.type(screen.getByRole("textbox", { name: /^text/ }), "al"); - await new Promise((r) => setTimeout(r, 400)); - // The last call carries the typed value and the context for the - // other (still empty) sibling. + // Wait for the debounced response to render, then assert the last call + // carries the typed value and the (still empty) sibling context. + expect( + await screen.findByText("alpha", undefined, SETTLE), + ).toBeInTheDocument(); + expect(screen.getByText("alphabet")).toBeInTheDocument(); expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "al", { targetLanguage: "", }); - expect(await screen.findByText("alpha")).toBeInTheDocument(); - expect(screen.getByText("alphabet")).toBeInTheDocument(); }); it("sends every sibling argument in context, including the unset ones", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -304,16 +318,20 @@ describe("PromptArgumentsForm", () => { ); await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); - await new Promise((r) => setTimeout(r, 400)); - // The completing arg ("text") is excluded from context; every - // other declared argument rides along — even ones still empty. - expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "h", { - targetLanguage: "es", - }); + // The completing arg ("text") is excluded from context; every other + // declared argument rides along — even ones still empty. Poll until the + // debounced "text" call lands rather than sleeping past the window. + await waitFor( + () => + expect(onCompleteArgument).toHaveBeenLastCalledWith("text", "h", { + targetLanguage: "es", + }), + SETTLE, + ); }); it("captures sibling values at fire time, not at schedule time", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -335,34 +353,30 @@ describe("PromptArgumentsForm", () => { // Type into "text" — this schedules a debounced completion call. await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); - // Before the 300ms debounce fires, update the sibling. The - // text-arg fire that lands at t=300 must see the latest sibling - // value, not the empty one captured at schedule time. + // Before the 300ms debounce fires, update the sibling. Because typing is + // synchronous (delay: null), the sibling holds its value well before the + // debounce window elapses. The text-arg fire must read the sibling at + // *fire* time, not the empty value captured at schedule time. await user.type( screen.getByRole("textbox", { name: /targetLanguage/ }), "es", ); - await new Promise((r) => setTimeout(r, 400)); - // The most recent call for "text" carries the up-to-date sibling - // value, even though it was scheduled before the sibling was typed. - // (There's also a focus-fire call when the second input gained focus — - // separate stream, not asserted here.) The exact sibling string depends - // on where the 300ms debounce lands relative to the two real-timer - // keystrokes ("e" then "es"), so assert the load-bearing fact: the fire - // captured a NON-EMPTY sibling, i.e. read at fire time rather than the - // empty value present when the call was scheduled. - const textCalls = onCompleteArgument.mock.calls.filter( - ([n]) => n === "text", - ); - const lastTextCall = textCalls.at(-1); - expect(lastTextCall?.[0]).toBe("text"); - expect(lastTextCall?.[1]).toBe("h"); - expect(lastTextCall?.[2].targetLanguage).toMatch(/^es?$/); + // Poll until the debounced "text" call fires, then assert it captured a + // non-empty sibling (read at fire time). The exact string may be "e" or + // "es" depending on where the fire lands relative to the keystrokes. + await waitFor(() => { + const textCalls = onCompleteArgument.mock.calls.filter( + ([n]) => n === "text", + ); + const lastTextCall = textCalls.at(-1); + expect(lastTextCall?.[1]).toBe("h"); + expect(lastTextCall?.[2].targetLanguage).toMatch(/^es?$/); + }, SETTLE); }); it("clears stale dropdown options the instant a new keystroke arrives", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const deferred: Array<{ value: string; resolve: (values: string[]) => void; @@ -386,22 +400,21 @@ describe("PromptArgumentsForm", () => { // Focus → first call (value=""). Resolve so the dropdown has // something to show. await user.click(screen.getByRole("textbox", { name: /^text/ })); - await new Promise((r) => setTimeout(r, 0)); - expect(deferred.length).toBe(1); + await waitFor(() => expect(deferred.length).toBe(1)); deferred[0].resolve(["alpha", "alphabet"]); expect(await screen.findByText("alpha")).toBeInTheDocument(); - // Type a new character — the keystroke handler must drop the - // stale options immediately so the dropdown doesn't show - // "alpha" / "alphabet" while the next request is in flight - // (300ms debounce + network latency). + // Type a new character — the keystroke handler must drop the stale + // options synchronously so the dropdown doesn't show "alpha" / + // "alphabet" while the next request is in flight (300ms debounce + + // network latency). await user.type(screen.getByRole("textbox", { name: /^text/ }), "z"); expect(screen.queryByText("alpha")).not.toBeInTheDocument(); expect(screen.queryByText("alphabet")).not.toBeInTheDocument(); }); it("aborts an in-flight request when a faster keystroke arrives", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const calls: Array<{ value: string; resolve: (values: string[]) => void; @@ -422,22 +435,25 @@ describe("PromptArgumentsForm", () => { />, ); - // Focus fires the first call (value=""). Type "h" → second call - // after debounce. Type "i" → third call after debounce. + // Focus fires the first call (value=""). Type "h" → second call after + // the debounce; wait for it to actually be in flight before typing more. await user.type(screen.getByRole("textbox", { name: /^text/ }), "h"); - await new Promise((r) => setTimeout(r, 350)); + await waitFor( + () => expect(calls.some((c) => c.value === "h")).toBe(true), + SETTLE, + ); await user.type(screen.getByRole("textbox", { name: /^text/ }), "i"); - await new Promise((r) => setTimeout(r, 350)); + await waitFor( + () => expect(calls.some((c) => c.value === "hi")).toBe(true), + SETTLE, + ); // Resolve the late "h" response — it should be dropped because // the form aborted that controller when the "hi" request started. const hi = calls.find((c) => c.value === "hi"); const h = calls.find((c) => c.value === "h"); - expect(hi).toBeDefined(); - expect(h).toBeDefined(); h?.resolve(["from-stale-h"]); hi?.resolve(["from-fresh-hi"]); - await new Promise((r) => setTimeout(r, 0)); // The dropdown shows the fresh response, not the stale one. expect(await screen.findByText("from-fresh-hi")).toBeInTheDocument(); @@ -445,7 +461,7 @@ describe("PromptArgumentsForm", () => { }); it("surfaces an empty dropdown when the completion request rejects", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); // The first (focus) call resolves with options; the debounced // keystroke call rejects. The rejection (not aborted) must clear the // dropdown to [] rather than leave the stale options showing. @@ -471,18 +487,24 @@ describe("PromptArgumentsForm", () => { const input = screen.getByRole("textbox", { name: /^text/ }); await user.click(input); - await new Promise((r) => setTimeout(r, 0)); expect(await screen.findByText("alpha")).toBeInTheDocument(); - // Type a char → debounced call rejects → options reset to []. + // Type a char → the stale options drop synchronously, and the debounced + // call then rejects, leaving the dropdown empty. Wait for the second + // (rejecting) call to fire so the catch → reset-to-[] path runs. await user.type(input, "z"); - await new Promise((r) => setTimeout(r, 400)); - expect(screen.queryByText("alpha")).not.toBeInTheDocument(); - expect(screen.queryByText("alphabet")).not.toBeInTheDocument(); + await waitFor( + () => expect(onCompleteArgument).toHaveBeenCalledTimes(2), + SETTLE, + ); + await waitFor(() => { + expect(screen.queryByText("alpha")).not.toBeInTheDocument(); + expect(screen.queryByText("alphabet")).not.toBeInTheDocument(); + }); }); it("cancels a pending debounce timer when the input is re-focused", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -508,23 +530,23 @@ describe("PromptArgumentsForm", () => { }); // Type into text → schedules a debounce timer. Immediately move focus - // away and back before the 300ms fires, so handleFocus sees a pending - // timer for "text" and clears it. + // away and back (synchronous under delay: null, so well within the 300ms + // window), so handleFocus sees the pending timer for "text" and clears + // it. Clearing is synchronous, so the keystroke completion can never + // fire afterward regardless of machine speed. await user.type(textInput, "h"); await user.click(siblingInput); await user.click(textInput); onCompleteArgument.mockClear(); - // The cancelled debounce must not fire a stale keystroke completion. - await new Promise((r) => setTimeout(r, 400)); const textCalls = onCompleteArgument.mock.calls.filter( ([n]) => n === "text", ); - // Only the focus-fire call (value "h"), never the cancelled debounce. - expect(textCalls.every(([, v]) => v === "h")).toBe(true); + // The cancelled debounce produced no post-clear "text" completion. + expect(textCalls.length).toBe(0); }); it("does not call onCompleteArgument when completions are unsupported", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi.fn(); renderWithMantine( { onCompleteArgument={onCompleteArgument} />, ); - // Focus the input first, then type — neither should trigger a call. + // Focus the input first, then type — neither path is wired to + // completions, so no request is ever scheduled or fired. await user.click(screen.getByPlaceholderText("Enter text...")); await user.type(screen.getByPlaceholderText("Enter text..."), "ab"); - await new Promise((r) => setTimeout(r, 400)); expect(onCompleteArgument).not.toHaveBeenCalled(); }); }); diff --git a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx index 14fe826e..884a180b 100644 --- a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx +++ b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx @@ -38,7 +38,7 @@ describe("ServerConfigModal", () => { }); it("rejects an id containing illegal characters", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine(); await user.type(screen.getByLabelText(/Server ID/i), "bad id!"); expect( @@ -47,7 +47,7 @@ describe("ServerConfigModal", () => { }); it("flags duplicate ids against existingIds", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( , ); @@ -56,7 +56,7 @@ describe("ServerConfigModal", () => { }); it("calls onSubmit with a valid stdio config", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -74,7 +74,7 @@ describe("ServerConfigModal", () => { }); it("requires a command for stdio submission", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -86,7 +86,7 @@ describe("ServerConfigModal", () => { }); it("rejects malformed env lines", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -118,7 +118,7 @@ describe("ServerConfigModal", () => { }); it("submits an sse config with just the url (headers move to the settings form)", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine( { }); it("submits a streamable-http config", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine( { }); it("rejects an empty URL on sse submission", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine( { }); it("shows the error message and stays open when onSubmit rejects", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = { ...base(), onSubmit: vi.fn().mockRejectedValue(new Error("server full")), @@ -252,7 +252,7 @@ describe("ServerConfigModal", () => { }); it("blocks submit and shows the id error message on bad-id submit click", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base({ existingIds: ["alpha"] }); renderWithMantine(); @@ -265,7 +265,7 @@ describe("ServerConfigModal", () => { }); it("supports editing the working directory field", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -283,7 +283,7 @@ describe("ServerConfigModal", () => { }); it("clears the Server ID field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("clears the Command field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("clears the URL field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("clears the Arguments field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("clears the Environment field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("clears the Working directory field via its Clear button", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("switches transport via the Transport select, revealing the URL field", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine(); // Starts on stdio — Command is shown, URL is not. @@ -411,7 +411,7 @@ describe("ServerConfigModal", () => { }); it("requires a server id before submitting", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -427,7 +427,7 @@ describe("ServerConfigModal", () => { }); it("rejects an env line whose '=' is at the start (no key)", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); @@ -442,7 +442,7 @@ describe("ServerConfigModal", () => { }); it("calls onClose when Cancel is clicked", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const props = base(); renderWithMantine(); await user.click(screen.getByRole("button", { name: /Cancel/ })); diff --git a/clients/web/src/components/groups/ServerImportConfigModal/ServerImportConfigModal.test.tsx b/clients/web/src/components/groups/ServerImportConfigModal/ServerImportConfigModal.test.tsx index 072f0edb..cd038c57 100644 --- a/clients/web/src/components/groups/ServerImportConfigModal/ServerImportConfigModal.test.tsx +++ b/clients/web/src/components/groups/ServerImportConfigModal/ServerImportConfigModal.test.tsx @@ -94,14 +94,14 @@ describe("ServerImportConfigModal", () => { }); it("enables Import once a client is selected", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); await user.selectOptions(screen.getByLabelText("Client"), "Cursor"); expect(screen.getByRole("button", { name: "Import" })).toBeEnabled(); }); it("fetches a source and shows additions + conflicts in review", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => @@ -114,7 +114,7 @@ describe("ServerImportConfigModal", () => { }); it("imports additions and skips conflicts by default", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("New servers (1)")); @@ -136,7 +136,7 @@ describe("ServerImportConfigModal", () => { }); it("lets the user skip an individual new server", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("New servers (1)")); @@ -150,7 +150,7 @@ describe("ServerImportConfigModal", () => { }); it("skips a deselected new server while still importing others", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("New servers (1)")); @@ -171,7 +171,7 @@ describe("ServerImportConfigModal", () => { }); it("overwrites a conflict when chosen", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("Already exists (1)")); @@ -185,7 +185,7 @@ describe("ServerImportConfigModal", () => { }); it("renames a conflict to the supplied id", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("Already exists (1)")); @@ -204,7 +204,7 @@ describe("ServerImportConfigModal", () => { }); it("flags an invalid/colliding rename target and blocks import", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("Already exists (1)")); @@ -240,7 +240,7 @@ describe("ServerImportConfigModal", () => { }); it("reports a per-server failure in the summary", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup(); h.onAddServer.mockRejectedValueOnce(new Error("boom")); await pickClientAndImport(user, "Cursor"); @@ -253,7 +253,7 @@ describe("ServerImportConfigModal", () => { }); it("shows the searched-paths notice when no config is found", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup({ type: "cursor", found: false, @@ -266,7 +266,7 @@ describe("ServerImportConfigModal", () => { }); it("shows an error when the source has a parse error", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup({ type: "cursor", found: true, @@ -280,7 +280,7 @@ describe("ServerImportConfigModal", () => { }); it("shows an error when the fetch rejects", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(new Error("network down")); await pickClientAndImport(user, "Cursor"); await waitFor(() => @@ -289,7 +289,7 @@ describe("ServerImportConfigModal", () => { }); it("parses an uploaded file (mcpServers) into review", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); const file = new File( [JSON.stringify({ mcpServers: { fromfile: { command: "x" } } })], @@ -306,7 +306,7 @@ describe("ServerImportConfigModal", () => { }); it("parses an uploaded VS Code file (servers) into review", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); const file = new File( [JSON.stringify({ servers: { vscodesrv: { command: "x" } } })], @@ -323,7 +323,7 @@ describe("ServerImportConfigModal", () => { }); it("shows an error for an unparseable uploaded file", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); const file = new File(["{not json"], "mcp.json", { type: "application/json", @@ -338,7 +338,7 @@ describe("ServerImportConfigModal", () => { }); it("reports when the selected source has no servers", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup({ type: "cursor", found: true, @@ -352,7 +352,7 @@ describe("ServerImportConfigModal", () => { }); it("reflects the chosen client as the dropdown value", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); // Selecting a client makes the dropdown's value non-empty (the truthy // branch of `vm.selectedType ?? ""`). @@ -361,7 +361,7 @@ describe("ServerImportConfigModal", () => { }); it("resets the dropdown to no selection when the placeholder is re-chosen", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); const dropdown = screen.getByLabelText("Client"); await user.selectOptions(dropdown, "Cursor"); @@ -374,7 +374,7 @@ describe("ServerImportConfigModal", () => { }); it("renders a conflicts-only review without a new-servers section", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); // Every incoming server already exists → no additions, only conflicts (the // false branch of `plan.additions.length > 0`). setup( @@ -395,7 +395,7 @@ describe("ServerImportConfigModal", () => { }); it("renders an additions-only review without a conflicts section", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); // No existing ids → every incoming server is a brand-new addition, so the // "Already exists" conflicts section never renders (the false branch of // `plan.conflicts.length > 0`). @@ -415,7 +415,7 @@ describe("ServerImportConfigModal", () => { }); it("goes back from review to the source picker", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); setup(); await pickClientAndImport(user, "Cursor"); await waitFor(() => screen.getByText("New servers (1)")); @@ -426,7 +426,7 @@ describe("ServerImportConfigModal", () => { }); it("closes from the summary via Done", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const h = setup({ type: "cursor", found: true, diff --git a/clients/web/src/components/groups/ServerImportJsonModal/ServerImportJsonModal.test.tsx b/clients/web/src/components/groups/ServerImportJsonModal/ServerImportJsonModal.test.tsx index baedb668..1627e371 100644 --- a/clients/web/src/components/groups/ServerImportJsonModal/ServerImportJsonModal.test.tsx +++ b/clients/web/src/components/groups/ServerImportJsonModal/ServerImportJsonModal.test.tsx @@ -114,7 +114,7 @@ describe("ServerImportJsonModal", () => { }); it("builds the config with env overrides and calls onAddServer", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onAddServer = vi.fn().mockResolvedValue(undefined); const onClose = vi.fn(); renderWithMantine( @@ -142,7 +142,7 @@ describe("ServerImportJsonModal", () => { }); it("honors a server name override", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onAddServer = vi.fn().mockResolvedValue(undefined); renderWithMantine( { }); it("lets the user pick among multiple packages", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onAddServer = vi.fn().mockResolvedValue(undefined); renderWithMantine( { }); it("surfaces an onAddServer rejection instead of closing", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onAddServer = vi.fn().mockRejectedValue(new Error("disk full")); const onClose = vi.fn(); renderWithMantine( @@ -241,7 +241,7 @@ describe("ServerImportJsonModal", () => { }); it("loads server.json from a chosen file", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("re-opens File Contents when the textarea is cleared", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("rejects an invalid id override and blocks Add Server", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onAddServer = vi.fn(); renderWithMantine( { }); it("closes via the Escape key (no Cancel button)", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onClose = vi.fn(); renderWithMantine( { }); it("shows the read error alert when error and a resource is selected", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("falls back to default error when error message is missing", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( , ); @@ -98,7 +98,7 @@ describe("ResourcesScreen", () => { }); it("shows the loading state when reading", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( , ); @@ -107,7 +107,7 @@ describe("ResourcesScreen", () => { }); it("renders the preview panel when readState has a result", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("toggles a section's open state through the lifted openSections handler", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine(); const urisHeader = screen.getByRole("button", { name: /URIs \(2\)/ }); // Open by default (compact=false); clicking routes through ResourcesScreen's @@ -150,7 +150,7 @@ describe("ResourcesScreen", () => { }); it("renders the template panel when a template is selected", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine(); // Templates section is open by default; select the template to open its form. await user.click(screen.getByText("files")); @@ -160,7 +160,7 @@ describe("ResourcesScreen", () => { }); it("hides the template panel once the user reads the resource", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onReadResource = vi.fn(); renderWithMantine( , @@ -176,7 +176,7 @@ describe("ResourcesScreen", () => { }); it("auto-reads when a resource is clicked in the sidebar", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onReadResource = vi.fn(); renderWithMantine( , @@ -186,7 +186,7 @@ describe("ResourcesScreen", () => { }); it("forwards refresh and subscribe events from the preview panel", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onReadResource = vi.fn(); const onSubscribeResource = vi.fn(); renderWithMantine( @@ -209,7 +209,7 @@ describe("ResourcesScreen", () => { }); it("closing the preview returns to the originating template form", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onReadResource = vi.fn(); const templates: ResourceTemplate[] = [ { uriTemplate: "file:///{path}", name: "files" }, @@ -251,7 +251,7 @@ describe("ResourcesScreen", () => { }); it("closing the preview from the error state returns to the template form", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const templates: ResourceTemplate[] = [ { uriTemplate: "demo://resource/dynamic/text/{id}", name: "Dynamic" }, ]; @@ -282,7 +282,7 @@ describe("ResourcesScreen", () => { }); it("closing the preview for a plain resource returns to the empty state", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("invokes onUnsubscribeResource when already subscribed", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onUnsubscribeResource = vi.fn(); renderWithMantine( { }); it("routes sidebar search text through onUiChange", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onUiChange = vi.fn(); renderWithMantine(); await user.type(screen.getByPlaceholderText("Search..."), "y.txt"); @@ -329,7 +329,7 @@ describe("ResourcesScreen", () => { }); it("renders nothing in the preview when an ok state carries no result", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("threads onCompleteArgument with a ref/resource envelope from the template form", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const onCompleteArgument = vi .fn< ( @@ -379,7 +379,7 @@ describe("ResourcesScreen", () => { }); it("hides the Subscriptions section and Subscribe button when subscriptionsSupported is false", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { it("dispatches onToggleConnection with the server id when the card toggle is clicked", async () => { const onToggleConnection = vi.fn(); - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { })} />, ); - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const tabSelect = await screen.findByDisplayValue("Servers"); await user.click(tabSelect); await user.click(await screen.findByText("Tools")); @@ -514,7 +514,7 @@ describe("InspectorView", () => { }); it("filters tools to apps and auto-launches a no-fields app on the Apps tab", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); // Plain (non-app) tool plus a tool with a malformed UI resource URI // exercise both branches of the appTools filter: the non-app drop and // the try/catch around `isAppTool` for malformed metadata. @@ -630,7 +630,7 @@ describe("InspectorView", () => { }); it("snaps activeTab back to Servers when the Apps tab disappears after a refresh", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const { rerender } = renderWithMantine( { it("dispatches onSetLogLevel through to the Logs screen", async () => { const onSetLogLevel = vi.fn(); - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( { }); it("persists Logs sort direction to localStorage and restores it on remount", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const { unmount } = renderWithMantine( { }); it("falls back to newest-first when a corrupted sort value is stored", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); window.localStorage.setItem("inspector.sortDirection.history", "garbage"); renderWithMantine( { }); it("persists History list compact state to localStorage and restores it on remount", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const historyEntry = { id: "req-1", timestamp: new Date("2026-03-17T10:00:00Z"), @@ -981,7 +981,7 @@ describe("InspectorView", () => { }); it("falls back to collapsed when a corrupted compact value is stored", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); window.localStorage.setItem("inspector.listCompact.history", "garbage"); const historyEntry = { id: "req-1", @@ -1084,7 +1084,7 @@ describe("InspectorView", () => { }); it("toggles the Servers list compact state from the list toggle", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); renderWithMantine( , ); @@ -1098,7 +1098,7 @@ describe("InspectorView", () => { }); it("toggles the Network list compact state from the list toggle", async () => { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const httpServer: ServerEntry = { id: "beta", name: "Beta", @@ -1167,7 +1167,7 @@ describe("InspectorView", () => { // The indicator only mounts on the active screen, so each case connects, // navigates to the target tab, and asserts the "List updated" affordance. async function gotoTab(tab: string) { - const user = userEvent.setup(); + const user = userEvent.setup({ delay: null }); const tabSelect = await screen.findByDisplayValue("Servers"); await user.click(tabSelect); await user.click(await screen.findByText(tab)); diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 97c291c3..5cb82ff5 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import * as z from "zod/v4"; import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; import { @@ -2452,11 +2452,21 @@ describe("InspectorClient", () => { const writeToStderrTool = await getTool(client, "write_to_stderr"); await client.callTool(writeToStderrTool, { message: testMessage }); - const logs = stderrLogState.getStderrLogs(); - expect(Array.isArray(logs)).toBe(true); - const matching = logs.filter((l) => l.message.includes(testMessage)); - expect(matching.length).toBeGreaterThan(0); - expect(matching[0]!.message).toContain(testMessage); + // The child's stderr is piped out-of-band from the tool's JSON-RPC + // response, so the "data" chunk carrying `testMessage` can still be in + // flight when `callTool` resolves. Reading the log synchronously here + // races that chunk (the historical flake). Poll until the line lands + // instead of asserting on a single sample. + await vi.waitFor( + () => { + const logs = stderrLogState.getStderrLogs(); + expect(Array.isArray(logs)).toBe(true); + const matching = logs.filter((l) => l.message.includes(testMessage)); + expect(matching.length).toBeGreaterThan(0); + expect(matching[0]!.message).toContain(testMessage); + }, + { timeout: 5000, interval: 25 }, + ); stderrLogState.destroy(); }); }); From dd7812063c94c411d07f88a40978300659a1de31 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 1 Jul 2026 21:22:04 -0400 Subject: [PATCH 2/2] test(web): make the debounce-cancellation test load-bearing (review round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "cancels a pending debounce timer when the input is re-focused" test passed trivially after the delay:null change: synchronous typing meant the assertion ran at t≈0, before the 300ms debounce could ever fire, so it passed whether or not handleFocus's clearTimeout worked. Rewrote it with fake timers so the debounce window elapses deterministically (advance the clock by 400ms directly). The interaction is driven with fireEvent rather than userEvent because userEvent's async internals deadlock under vitest fake timers with the Mantine/happy-dom stack. Verified load-bearing: removing the clearTimeout in handleFocus makes it fail (expected 1 to be 0), and it passes once restored. Also tightened the sibling-context comment (with delay:null the sibling is reliably "es"); the tolerant /^es?$/ regex is left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX --- .../PromptArgumentsForm.test.tsx | 101 +++++++++++------- 1 file changed, 63 insertions(+), 38 deletions(-) diff --git a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx index 88871639..f00afc6c 100644 --- a/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx +++ b/clients/web/src/components/groups/PromptArgumentsForm/PromptArgumentsForm.test.tsx @@ -6,6 +6,8 @@ import { renderWithMantine, screen, waitFor, + fireEvent, + act, } from "../../../test/renderWithMantine"; import { PromptArgumentsForm } from "./PromptArgumentsForm"; @@ -363,8 +365,9 @@ describe("PromptArgumentsForm", () => { ); // Poll until the debounced "text" call fires, then assert it captured a - // non-empty sibling (read at fire time). The exact string may be "e" or - // "es" depending on where the fire lands relative to the keystrokes. + // non-empty sibling (read at fire time, not the empty value present when + // the call was scheduled). With delay:null the sibling is fully typed + // ("es") before the debounce fires; the /^es?$/ regex stays tolerant. await waitFor(() => { const textCalls = onCompleteArgument.mock.calls.filter( ([n]) => n === "text", @@ -504,45 +507,67 @@ describe("PromptArgumentsForm", () => { }); it("cancels a pending debounce timer when the input is re-focused", async () => { - const user = userEvent.setup({ delay: null }); - const onCompleteArgument = vi - .fn< - ( - argName: string, - value: string, - context: Record, - ) => Promise - >() - .mockResolvedValue([]); + // This is the one completion test that must let the 300ms debounce + // window fully elapse to be meaningful — asserting at t≈0 (as the + // sibling negative tests do) would pass whether or not handleFocus's + // clearTimeout actually cancels the pending timer. Fake timers make that + // window deterministic: we advance the clock directly. The interaction + // is driven with fireEvent rather than userEvent, because userEvent's + // async internals deadlock under vitest fake timers with this + // Mantine/happy-dom stack. This test is load-bearing: if the + // clearTimeout in handleFocus were removed, the debounce would fire + // during the advance and the final assertion would fail (verified). + vi.useFakeTimers(); + try { + const onCompleteArgument = vi + .fn< + ( + argName: string, + value: string, + context: Record, + ) => Promise + >() + .mockResolvedValue([]); - renderWithMantine( - , - ); + renderWithMantine( + , + ); - const textInput = screen.getByRole("textbox", { name: /^text/ }); - const siblingInput = screen.getByRole("textbox", { - name: /targetLanguage/, - }); + const textInput = screen.getByRole("textbox", { name: /^text/ }); + const siblingInput = screen.getByRole("textbox", { + name: /targetLanguage/, + }); - // Type into text → schedules a debounce timer. Immediately move focus - // away and back (synchronous under delay: null, so well within the 300ms - // window), so handleFocus sees the pending timer for "text" and clears - // it. Clearing is synchronous, so the keystroke completion can never - // fire afterward regardless of machine speed. - await user.type(textInput, "h"); - await user.click(siblingInput); - await user.click(textInput); - onCompleteArgument.mockClear(); - const textCalls = onCompleteArgument.mock.calls.filter( - ([n]) => n === "text", - ); - // The cancelled debounce produced no post-clear "text" completion. - expect(textCalls.length).toBe(0); + // Type into text → schedules the 300ms debounce timer for "text". + fireEvent.focus(textInput); + fireEvent.change(textInput, { target: { value: "h" } }); + // Move focus to the sibling and back — the re-focus makes + // handleFocus("text") see the pending debounce timer and clear it. + fireEvent.focus(siblingInput); + fireEvent.focus(textInput); + + // Drop the focus-fire calls made above; only a *debounce* fire after + // this point would indicate the cancellation failed. + onCompleteArgument.mockClear(); + + // Elapse well past the debounce window. The cancelled timer must not + // fire a stale keystroke completion. + await act(async () => { + await vi.advanceTimersByTimeAsync(400); + }); + + const textCalls = onCompleteArgument.mock.calls.filter( + ([n]) => n === "text", + ); + expect(textCalls.length).toBe(0); + } finally { + vi.useRealTimers(); + } }); it("does not call onCompleteArgument when completions are unsupported", async () => {