Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
- **TUI** (`clients/tui`): the gate covers the **non-React logic** only — `logger.ts`, `components/tabsConfig.ts`, and `utils/*` (server resolution lives in `core/` and is measured by the web suite). The Ink components, `App.tsx`, and `hooks/` are an **interim exclusion** in `clients/tui/vitest.config.ts` pending the renderer-based follow-up (#1501). When adding new **non-React** logic under `clients/tui/src`, it falls under the gate automatically — add tests for it.
- Run `npm run test:integration` (also from `clients/web/`) for the InspectorClient + transport + auth integration suite. It runs under a separate `integration` vitest project in node env (no happy-dom) with 30s timeouts. The script builds `test-servers/` first via `tsc -p ../../test-servers --noCheck` so the stdio MCP test server can be spawned as a real subprocess. CI does not run `test:integration` as its own step — the integration project is covered by the CI `coverage` gate, whose web `test:coverage` runs `--project=unit --project=integration --coverage`.
- Test files live alongside the source as `<Name>.test.tsx` (or `.test.ts` for non-React modules). Integration tests live under `clients/web/src/test/integration/`, mirroring the `core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`, `auth/node/`, `storage/`). Any test file under that folder is automatically picked up by the `integration` vitest project (node env, 30s timeouts) via the folder glob in `vite.config.ts` — placement is the manifest, there is no enumeration to keep in sync. Tests outside the folder run in the `unit` project (happy-dom). When adding a new test for, e.g., `core/mcp/remote/foo.ts`, put it at `src/test/integration/mcp/remote/foo.test.ts`.
- Use `renderWithMantine` from `src/test/renderWithMantine.tsx` to render components — it wraps in `MantineProvider` with the project theme
- Use `renderWithMantine` from `src/test/renderWithMantine.tsx` to render components — it wraps in `MantineProvider` with the project theme. It sets `env="test"` so Mantine renders transitions synchronously (no internal `setTimeout`); this prevents a `Transition`/`Modal` timer from firing after happy-dom tears down `window` at end-of-run and failing the whole run with an uncaught `ReferenceError: window is not defined` (#1760). **Always render through `renderWithMantine`; do not hand-roll a bare `MantineProvider` in a test** (that reintroduces the leak class). Only when a test must assert *mid-flight* transition state (e.g. a `data-anim="out"` cell during an exit crossfade) use `renderWithMantineTransitions` (real transitions) — and that test MUST drive the transition to completion (`await waitFor`/`findBy`) so its timer resolves before teardown.

### Responding to Code Reviews
- When asked to respond to a code review of a PR,
Expand Down
19 changes: 14 additions & 5 deletions clients/web/src/components/groups/ViewHeader/ViewHeader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import userEvent from "@testing-library/user-event";
import {
act,
renderWithMantine,
renderWithMantineTransitions,
screen,
waitFor,
} from "../../../test/renderWithMantine";
Expand All @@ -12,11 +13,13 @@ import { theme } from "../../../theme/theme";
import { ViewHeader } from "./ViewHeader";

// Render under a forced-dark MantineProvider so `useComputedColorScheme`
// returns "dark" (the light/dark icon + logo branches).
// returns "dark" (the light/dark icon + logo branches). `env="test"` disables
// timer-driven transitions, matching the shared `renderWithMantine` wrapper and
// avoiding post-teardown `window is not defined` races (#1760).
function renderDark(ui: React.ReactElement) {
return render(ui, {
wrapper: ({ children }) => (
<MantineProvider theme={theme} defaultColorScheme="dark">
<MantineProvider theme={theme} defaultColorScheme="dark" env="test">
{children}
</MantineProvider>
),
Expand Down Expand Up @@ -240,7 +243,9 @@ describe("ViewHeader", () => {

it("crossfades on disconnect: tab bar exits while the title enters, then the bar unmounts (#1450)", async () => {
mediaQueryMock.value = true;
const { rerender } = renderWithMantine(
// Asserts the mid-flight exit ("out") state, so it needs real transitions;
// it drives them to completion below (waitFor unmount).
const { rerender } = renderWithMantineTransitions(
<ViewHeader {...connectedProps} />,
);
expect(screen.getAllByRole("radio").length).toBeGreaterThan(0);
Expand Down Expand Up @@ -309,7 +314,9 @@ describe("ViewHeader", () => {

it("animates the server name and disconnect controls in on connect, out on disconnect (#1450)", async () => {
mediaQueryMock.value = true;
const { rerender } = renderWithMantine(
// Asserts the mid-flight exit ("out") state, so it needs real transitions;
// it drives them to completion below (waitFor unmount).
const { rerender } = renderWithMantineTransitions(
<ViewHeader {...connectedProps} />,
);
// Connected: the server name and the Disconnect control are in their
Expand Down Expand Up @@ -368,8 +375,10 @@ describe("ViewHeader", () => {

it("crossfades the title out and the connected header in on connect (#1450)", async () => {
mediaQueryMock.value = true;
// Asserts the mid-flight exit ("out") state, so it needs real transitions;
// it drives them to completion below (findByText for the entering header).
// Start disconnected: the title cell is the entering one.
const { rerender } = renderWithMantine(
const { rerender } = renderWithMantineTransitions(
<ViewHeader
connected={false}
onToggleTheme={vi.fn()}
Expand Down
32 changes: 29 additions & 3 deletions clients/web/src/test/renderWithMantine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,28 @@ import { render, type RenderOptions } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
import { theme } from "../theme/theme";

function MantineWrapper({ children }: { children: ReactNode }) {
// `env="test"` makes Mantine render transitions synchronously (no internal
// `setTimeout`). Without it, a `Transition`/`Modal` open/close timer can fire
// after happy-dom tears down `window` at the end of the run, throwing an
// uncaught `ReferenceError: window is not defined` that fails the whole run
// even when every assertion passed (#1760). This is the right default for the
// vast majority of tests, which don't assert on mid-transition state.
function TestEnvWrapper({ children }: { children: ReactNode }) {
return (
<MantineProvider theme={theme} defaultColorScheme="light">
<MantineProvider theme={theme} defaultColorScheme="light" env="test">
{children}
</MantineProvider>
);
}

// Opt-in wrapper that keeps Mantine's timer-driven transitions enabled, for the
// few tests that assert on transition/animation state that only exists mid-flight
// (e.g. a `data-anim="out"` cell during an exit crossfade). Such a test MUST
// drive the transition to completion (`await waitFor`/`findBy`) so its timer
// resolves before teardown; otherwise it reintroduces the #1760 leak.
function TransitionsWrapper({ children }: { children: ReactNode }) {
return (
<MantineProvider theme={theme} defaultColorScheme="light" env="default">
{children}
</MantineProvider>
);
Expand All @@ -15,7 +34,14 @@ export function renderWithMantine(
ui: ReactElement,
options?: Omit<RenderOptions, "wrapper">,
) {
return render(ui, { wrapper: MantineWrapper, ...options });
return render(ui, { wrapper: TestEnvWrapper, ...options });
}

export function renderWithMantineTransitions(
ui: ReactElement,
options?: Omit<RenderOptions, "wrapper">,
) {
return render(ui, { wrapper: TransitionsWrapper, ...options });
}

export * from "@testing-library/react";
Loading