Skip to content

Commit a96565a

Browse files
mgoldsboroughmgoldsborough
authored
test(web): snapshot real api/client in preload (fix #297 residual flake) (#298)
* test(web): snapshot real api/client in preload instead of racy await import PR #297's whole-module api/client mocks spread `await import("../api/client")` to stay complete. That dynamic import reads the same process-global mock registry it's defending against, so under CI's file-scheduling it can resolve to an incomplete mock and propagate the gap — the flaky "Export named 'getActiveWorkspaceId' / 'ApiClientError' not found" link errors still fired on main (intermittently; passed on re-run). Capture the real module once in the test preload (web/test/setup.ts), before any test file can register a mock.module replacement, and freeze it as a plain object. Each whole-module mock spreads that snapshot instead. Because the snapshot is taken before any mock exists and is immune to later swaps, every registered mock is complete by construction — there is no longer a code path that registers an incomplete api/client. Web suite: 385 pass / 0 fail, 10x stable on bun 1.3.14. tsc + format clean. * test(web): migrate remaining api/client mocks to preload snapshot Spread realClient (the web/test/setup.ts snapshot) into the five test/* sites that still registered incomplete or registry-derived api/client mocks: streamingState, inlineError, useWorkspaceBriefing, useShell, and WorkspaceOverviewPage (dropping its racy `import * as actualClient`). All 10 mock.module("api/client") sites now register complete mocks, so no code path can leak an incomplete replacement into the process-global registry and crash bridge.ts/format-error.ts at link time. Completes the guarantee the snapshot was introduced to provide. 385/0, tsc clean. --------- Co-authored-by: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com>
1 parent 939353f commit a96565a

11 files changed

Lines changed: 92 additions & 47 deletions

‎web/src/__tests__/InstallConnectorDialog.test.tsx‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
// ---------------------------------------------------------------------------
2929

3030
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
31+
import { realClient } from "../../test/setup";
3132

3233
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
3334

@@ -39,14 +40,12 @@ const installConnector = mock(async (_entry: unknown, _wsId: string) => ({
3940
wsId: "ws_helix",
4041
}));
4142

42-
// Spread the real module so this whole-module mock exposes every api/client
43-
// export. Bun's `mock.module` is process-global; a *partial* stub leaking into
44-
// another suite mid-run (under CI's parallelism) is what crashed bridge tests
45-
// with "Export named 'getActiveWorkspaceId' not found". A complete mock is inert
46-
// when it leaks — only `installConnector` is overridden here.
47-
const actualClient = await import("../api/client");
43+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
44+
// whole-module mock exposes every api/client export; only `installConnector`
45+
// is overridden. Keeps the process-global mock registry complete even when it
46+
// leaks into another suite mid-run.
4847
mock.module("../api/client", () => ({
49-
...actualClient,
48+
...realClient,
5049
installConnector,
5150
}));
5251

‎web/src/__tests__/ResourceLinkView.test.tsx‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
// ---------------------------------------------------------------------------
2727

2828
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
29+
import { realClient } from "../../test/setup";
2930

3031
// Tell React 19 we're inside an act-aware test environment. Without this
3132
// flag, every awaited setState inside an async useEffect logs a warning
@@ -36,15 +37,12 @@ const readResourceMock = mock(
3637
async (_server: string, _uri: string): Promise<{ contents: unknown[] }> => ({ contents: [] }),
3738
);
3839

39-
// Spread the real module so this whole-module mock exposes every api/client
40-
// export. Bun's `mock.module` is process-global; a partial stub leaking into
41-
// another suite mid-run (under CI's parallelism) is what crashed bridge tests
42-
// with "Export named 'getActiveWorkspaceId' not found". The spread also gives us
43-
// the real `ApiClientError` constructor that ResourceLinkView's catch branch
44-
// (`err instanceof ApiClientError`) needs — only `readResource` is overridden.
45-
const actualClient = await import("../api/client");
40+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
41+
// whole-module mock exposes every api/client export; only `readResource` is
42+
// overridden. The snapshot also carries the real `ApiClientError` constructor
43+
// that ResourceLinkView's catch branch (`err instanceof ApiClientError`) needs.
4644
mock.module("../api/client", () => ({
47-
...actualClient,
45+
...realClient,
4846
readResource: readResourceMock,
4947
}));
5048

‎web/src/__tests__/bridge/bridge-transport.test.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
// ---------------------------------------------------------------------------
2323

2424
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
25+
import { realClient } from "../../../test/setup";
2526

2627
// ---------------------------------------------------------------------------
2728
// Mocks — each dependency is replaced with an observable stub so tests can
@@ -78,14 +79,12 @@ const getClientCalls = { count: 0 };
7879
// dispatching (Q3 auto-prefix). Mock `getActiveWorkspaceId` to return
7980
// a stable workspace id so the wire-name assertions below are
8081
// deterministic.
81-
// Spread the real module so this whole-module mock exposes every api/client
82-
// export. Bun's `mock.module` is process-global; a partial stub leaking into
83-
// another suite mid-run (under CI's parallelism) is what crashed these bridge
84-
// tests with "Export named 'getActiveWorkspaceId' not found". A complete mock
85-
// is inert when it leaks — only the two below are overridden.
86-
const actualClient = await import("../../api/client");
82+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
83+
// whole-module mock exposes every api/client export; only the two below are
84+
// overridden. Keeps the process-global mock registry complete even when it
85+
// leaks into another suite mid-run.
8786
mock.module("../../api/client", () => ({
88-
...actualClient,
87+
...realClient,
8988
getActiveWorkspaceId: () => "ws_test",
9089
// Keep upload benign for this transport test (no upload triggered here).
9190
uploadResource: async () => {

‎web/src/__tests__/connector-sections.test.tsx‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@
2727
// ---------------------------------------------------------------------------
2828

2929
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
30+
import { realClient } from "../../test/setup";
3031

3132
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
3233

3334
// ── api/client mocks ────────────────────────────────────────────────
3435
// Every section calls into one or two helpers from api/client. We
35-
// override those helpers but spread the real module (see the mock.module
36-
// call below) so the stub stays complete.
36+
// override those helpers but spread the real-module snapshot (see the
37+
// mock.module call below) so the stub stays complete.
3738

3839
const disconnectConnector = mock(async () => ({
3940
ok: true,
@@ -60,14 +61,12 @@ const setupConnectorOperator = mock(async () => ({
6061
clientId: "cid-rotated",
6162
}));
6263

63-
// Spread the real module so this whole-module mock exposes every api/client
64-
// export. Bun's `mock.module` is process-global; this 5-export stub previously
65-
// clobbered `../api/client` for other suites loading concurrently (the
66-
// `getActiveWorkspaceId`/`setActiveWorkspaceId` "export not found" flake). A
67-
// complete mock is inert when it leaks — only these five are overridden.
68-
const actualClient = await import("../api/client");
64+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
65+
// whole-module mock exposes every api/client export; only these five are
66+
// overridden. Keeps the process-global mock registry complete even when it
67+
// leaks into another suite loading concurrently.
6968
mock.module("../api/client", () => ({
70-
...actualClient,
69+
...realClient,
7170
disconnectConnector,
7271
initiateMcpOAuth,
7372
clearBundleUserConfig,

‎web/src/__tests__/workspace-section.test.tsx‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// ---------------------------------------------------------------------------
1717

1818
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
19+
import { realClient } from "../../test/setup";
1920

2021
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
2122

@@ -29,15 +30,13 @@ const setActiveSpy = mock((id: string | null) => {
2930
});
3031
const mockedGetActiveWorkspaceId = (): string | null => mockedActiveId;
3132

32-
// Spread the real module so this whole-module mock exposes every api/client
33-
// export. Bun's `mock.module` is process-global; a partial stub leaking into
34-
// another suite mid-run (under CI's parallelism) is what crashed bridge tests
35-
// with "Export named 'getActiveWorkspaceId' not found" — and is why this file
36-
// used to need a `b-` filename to win the load order. A complete mock is inert
37-
// when it leaks; only the three below are overridden.
38-
const actualClient = await import("../api/client");
33+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
34+
// whole-module mock exposes every api/client export; only the three below are
35+
// overridden. Keeps the process-global mock registry complete even when it
36+
// leaks into another suite mid-run (this file used to need a `b-` filename to
37+
// win the load order; the snapshot makes that unnecessary).
3938
mock.module("../api/client", () => ({
40-
...actualClient,
39+
...realClient,
4140
setActiveWorkspaceId: setActiveSpy,
4241
getActiveWorkspaceId: mockedGetActiveWorkspaceId,
4342
callTool: mock(async () => ({ structuredContent: null, content: [] })),

‎web/test/WorkspaceOverviewPage.test.tsx‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,24 @@
1717
// ---------------------------------------------------------------------------
1818

1919
import { afterEach, describe, expect, mock, test } from "bun:test";
20-
import * as actualClient from "../src/api/client";
2120
import type { WorkspaceInfo } from "../src/context/WorkspaceContext";
2221
import type { PlacementEntry } from "../src/types";
22+
import { realClient } from "./setup";
2323

2424
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
2525

2626
// Stub only `callTool` so the briefing fetch hangs in its skeleton (its own
27-
// async timeline — not what this file tests). Spread the real module so every
28-
// other export stays intact: a bare `{ callTool }` mock leaks across files in
29-
// the same test process and strips functions the client/bridge suites depend
30-
// on (getAuthToken, getActiveWorkspaceId, …). WorkspaceContext skips its list
31-
// call when given bootstrap data, so the real setActiveWorkspaceId it calls is
32-
// harmless — sibling suites reset client state in their own beforeEach.
27+
// async timeline — not what this file tests). Spread the preload's real-module
28+
// snapshot (see web/test/setup.ts) so every other export stays intact: a bare
29+
// `{ callTool }` mock leaks across files in the same test process and strips
30+
// functions the client/bridge suites depend on (getAuthToken,
31+
// getActiveWorkspaceId, …). The snapshot predates every mock.module, so it
32+
// can't itself be an incomplete stub the way `import * as` from the registry
33+
// could. WorkspaceContext skips its list call when given bootstrap data, so the
34+
// real setActiveWorkspaceId it calls is harmless — sibling suites reset client
35+
// state in their own beforeEach.
3336
mock.module("../src/api/client", () => ({
34-
...actualClient,
37+
...realClient,
3538
callTool: () => new Promise(() => {}),
3639
}));
3740

‎web/test/inlineError.test.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it, mock, beforeEach } from "bun:test";
2+
import { realClient } from "./setup";
23
import { renderHook, act } from "@testing-library/react";
34
import type { ReactNode } from "react";
45
import { MemoryRouter } from "react-router-dom";
@@ -14,7 +15,13 @@ let capturedCallback: StreamCallback | null = null;
1415
let resolveStream: (() => void) | null = null;
1516
let rejectStream: ((err: Error) => void) | null = null;
1617

18+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
19+
// whole-module mock exposes every api/client export; only the two below are
20+
// overridden. Bun's mock.module registry is process-global, so an incomplete
21+
// stub leaking into another suite's module graph is what crashed bridge tests
22+
// with "Export named 'getActiveWorkspaceId' not found".
1723
mock.module("../src/api/client", () => ({
24+
...realClient,
1825
streamChat: (_req: unknown, cb: StreamCallback) => {
1926
capturedCallback = cb;
2027
return new Promise<void>((resolve, reject) => {

‎web/test/setup.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
11
import { Window } from "happy-dom";
22

3+
// Snapshot of the real `../src/api/client` module, captured here in the test
4+
// preload — guaranteed to run before any test file, so before any test can
5+
// register a `mock.module("../api/client", ...)` replacement. Several suites
6+
// whole-module-mock api/client but only stub the one or two functions they
7+
// call; spreading this snapshot into those mocks keeps every other export
8+
// present. Bun's `mock.module` registry is process-global, so an incomplete
9+
// replacement leaking into another file's module graph (a concurrency-order
10+
// race that only manifests on CI) is what produced the flaky
11+
// "Export named 'getActiveWorkspaceId' not found" link errors.
12+
//
13+
// Why a preload snapshot rather than `await import("../api/client")` inside
14+
// each test: that dynamic import reads the *same* global registry it's trying
15+
// to defend against, so under the wrong load order it can itself resolve to an
16+
// incomplete mock and propagate the gap. Capturing here, before any mock
17+
// exists, and copying into a plain object makes the snapshot immune to later
18+
// `mock.module` swaps.
19+
import * as realApiClient from "../src/api/client";
20+
21+
export const realClient = { ...realApiClient };
22+
323
const window = new Window({ url: "http://localhost" });
424

525
// Register DOM globals that React and testing-library need

‎web/test/streamingState.test.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it, mock, beforeEach } from "bun:test";
2+
import { realClient } from "./setup";
23
import { renderHook, act } from "@testing-library/react";
34
import type { ReactNode } from "react";
45
import { MemoryRouter } from "react-router-dom";
@@ -14,7 +15,13 @@ type StreamCallback = (type: string, data: unknown) => void;
1415
let capturedCallback: StreamCallback | null = null;
1516
let resolveStream: (() => void) | null = null;
1617

18+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
19+
// whole-module mock exposes every api/client export; only the two below are
20+
// overridden. Bun's mock.module registry is process-global, so an incomplete
21+
// stub leaking into another suite's module graph is what crashed bridge tests
22+
// with "Export named 'getActiveWorkspaceId' not found".
1723
mock.module("../src/api/client", () => ({
24+
...realClient,
1825
streamChat: (_req: unknown, cb: StreamCallback) => {
1926
capturedCallback = cb;
2027
return new Promise<void>((resolve) => {

‎web/test/useShell.test.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it, mock, beforeEach } from "bun:test";
22
import { renderHook, act, waitFor } from "@testing-library/react";
33
import { useShell } from "../src/hooks/useShell";
4+
import { realClient } from "./setup";
45

56
// ---------------------------------------------------------------------------
67
// Mock getShell
@@ -10,7 +11,13 @@ const mockGetShell = mock(() =>
1011
Promise.resolve({ placements: [], chatEndpoint: "", eventsEndpoint: "" }),
1112
);
1213

14+
// Spread the preload's real-module snapshot (see web/test/setup.ts) so this
15+
// whole-module mock exposes every api/client export; only `getShell` is
16+
// overridden. Bun's mock.module registry is process-global, so an incomplete
17+
// stub leaking into another suite's module graph is what crashed bridge tests
18+
// with "Export named 'getActiveWorkspaceId' not found".
1319
mock.module("../src/api/client", () => ({
20+
...realClient,
1421
getShell: (...args: unknown[]) => mockGetShell(...args),
1522
}));
1623

0 commit comments

Comments
 (0)