Skip to content

Commit 3caf4e3

Browse files
committed
🤖 chore: clean up jsdom browser test harness
- Stop modifying LoadingScreen (use visible loading text instead) - Ensure ServiceContainer shutdown/dispose during test cleanup - Wrap unmounts in act() and set IS_REACT_ACT_ENVIRONMENT - Add jsdom polyfill for ResizeObserver - Add browser-ui Jest setup to suppress act warning noise and raise EventEmitter listener limit for full-app mounts - Keep forceExit enabled to avoid Jest hangs from lingering jsdom handles _Generated with mux_
1 parent 3e7a564 commit 3caf4e3

File tree

9 files changed

+90
-16
lines changed

9 files changed

+90
-16
lines changed

jest.config.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,17 @@ module.exports = {
4848
testMatch: ["<rootDir>/tests/browser/**/*.test.ts"],
4949
setupFilesAfterEnv: ["<rootDir>/tests/setup.ts"],
5050
},
51-
// Browser UI tests (jsdom environment) - for future use
51+
// Browser UI tests (jsdom environment)
5252
{
5353
...sharedConfig,
5454
displayName: "browser-ui",
5555
testEnvironment: "jsdom",
5656
testMatch: ["<rootDir>/tests/browser/**/*.test.tsx"],
5757
setupFiles: ["<rootDir>/tests/browser/global-setup.js"],
58-
setupFilesAfterEnv: ["<rootDir>/tests/setup.ts"],
58+
setupFilesAfterEnv: [
59+
"<rootDir>/tests/setup.ts",
60+
"<rootDir>/tests/browser/jestSetup.ts",
61+
],
5962
},
6063
],
6164
collectCoverageFrom: [
@@ -66,10 +69,11 @@ module.exports = {
6669
"!src/cli/**/*",
6770
"!src/desktop/main.ts",
6871
],
72+
// Jest + jsdom integration tests can leave behind non-critical handles.
73+
// Force-exit to keep CI stable and prevent hanging.
74+
forceExit: true,
6975
// Run tests in parallel (use 50% of available cores, or 4 minimum)
7076
maxWorkers: "50%",
71-
// Force exit after tests complete to avoid hanging on lingering handles
72-
forceExit: true,
7377
// 10 minute timeout for integration tests, 10s for unit tests
7478
testTimeout: process.env.TEST_INTEGRATION === "1" ? 600000 : 10000,
7579
// Detect open handles in development (disabled by default for speed)

src/browser/components/LoadingScreen.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
export function LoadingScreen() {
22
return (
3-
<div
4-
data-testid="loading-screen"
5-
className="bg-bg-dark flex h-screen w-screen items-center justify-center"
6-
>
3+
<div className="bg-bg-dark flex h-screen w-screen items-center justify-center">
74
<div className="flex flex-col items-center gap-4">
85
<div className="border-border-light h-12 w-12 animate-spin rounded-full border-4 border-t-transparent" />
96
<p className="text-text-secondary text-sm">Loading workspaces...</p>

tests/browser/appLoader.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ const describeIntegration = shouldRunIntegrationTests() ? describe : describe.sk
2222
describeIntegration("AppLoader React Integration", () => {
2323
describe("Initial Render", () => {
2424
test("shows loading screen initially then transitions to app", async () => {
25-
const { cleanup, getByTestId, ...queries } = await renderWithBackend();
25+
const { cleanup, getByText, ...queries } = await renderWithBackend();
2626
try {
2727
// Should show loading screen while fetching initial data
28-
expect(getByTestId("loading-screen")).toBeInTheDocument();
28+
expect(getByText(/loading workspaces/i)).toBeInTheDocument();
2929

3030
// Wait for loading screen to disappear (app loaded from real backend)
3131
await waitForAppLoad(queries);

tests/browser/global-setup.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@
55

66
// Mock import.meta.env for Vite compatibility
77
// This needs to be set before any imports that use it
8+
9+
// Polyfill ResizeObserver for Radix/layout components in jsdom
10+
if (typeof globalThis.ResizeObserver === "undefined") {
11+
globalThis.ResizeObserver = class ResizeObserver {
12+
observe() {}
13+
unobserve() {}
14+
disconnect() {}
15+
};
16+
}
817
globalThis.import_meta_env = {
918
VITE_BACKEND_URL: undefined,
1019
MODE: "test",

tests/browser/jestSetup.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Jest setup for jsdom-based browser UI tests.
3+
*
4+
* These tests mount the full app, which schedules a bunch of background React
5+
* updates (Radix tooltips/selects, async effects like branch loading, etc.).
6+
* In jsdom this can produce extremely noisy act(...) warnings that drown out
7+
* real failures.
8+
*/
9+
10+
import { EventEmitter } from "events";
11+
12+
const originalConsoleError = console.error.bind(console);
13+
const originalDefaultMaxListeners = EventEmitter.defaultMaxListeners;
14+
const originalConsoleLog = console.log.bind(console);
15+
16+
const shouldSuppressActWarning = (args: unknown[]) => {
17+
return args.some(
18+
(arg) => typeof arg === "string" && arg.toLowerCase().includes("not wrapped in act")
19+
);
20+
};
21+
22+
beforeAll(() => {
23+
// The full app creates a bunch of subscriptions on a single EventEmitter
24+
// (ProviderService configChanged). That's OK for these tests, but Node warns
25+
// once the default (10) listener limit is exceeded.
26+
EventEmitter.defaultMaxListeners = 50;
27+
jest.spyOn(console, "error").mockImplementation((...args) => {
28+
if (shouldSuppressActWarning(args)) {
29+
return;
30+
}
31+
originalConsoleError(...args);
32+
});
33+
34+
// Keep the test output focused; individual tests can temporarily unmock if
35+
// they need to assert on logs.
36+
jest.spyOn(console, "log").mockImplementation(() => {});
37+
jest.spyOn(console, "warn").mockImplementation(() => {});
38+
});
39+
40+
afterAll(() => {
41+
EventEmitter.defaultMaxListeners = originalDefaultMaxListeners;
42+
(console.error as jest.Mock).mockRestore();
43+
(console.log as jest.Mock).mockRestore();
44+
(console.warn as jest.Mock).mockRestore();
45+
46+
// Ensure captured originals don't get tree-shaken / flagged as unused in some tooling.
47+
void originalConsoleLog;
48+
});

tests/browser/renderWithBackend.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import React from "react";
10-
import { render, type RenderOptions, type RenderResult } from "@testing-library/react";
10+
import { act, render, type RenderOptions, type RenderResult } from "@testing-library/react";
1111
import { AppLoader } from "@/browser/components/AppLoader";
1212
import type { APIClient } from "@/browser/contexts/API";
1313
import { createBrowserTestEnv, type BrowserTestEnv } from "./setup";
@@ -56,7 +56,10 @@ export async function renderWithBackend(
5656
});
5757

5858
const cleanup = async () => {
59-
renderResult.unmount();
59+
await act(async () => {
60+
renderResult.unmount();
61+
});
62+
6063
// Only cleanup the env if we created it (not passed in)
6164
if (!existingEnv) {
6265
await env.cleanup();
@@ -99,7 +102,9 @@ export async function renderComponentWithBackend(
99102
});
100103

101104
const cleanup = async () => {
102-
renderResult.unmount();
105+
await act(async () => {
106+
renderResult.unmount();
107+
});
103108
await env.cleanup();
104109
};
105110

tests/browser/setup.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ export async function createBrowserTestEnv(): Promise<BrowserTestEnv> {
9898
const api = orpc as unknown as APIClient;
9999

100100
const cleanup = async () => {
101+
try {
102+
await services.shutdown();
103+
} finally {
104+
await services.dispose();
105+
}
106+
101107
const maxRetries = 3;
102108
for (let i = 0; i < maxRetries; i++) {
103109
try {

tests/browser/uiHelpers.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ type Queries = Pick<
1717
| "findByText"
1818
| "findByPlaceholderText"
1919
| "queryByRole"
20-
| "queryByTestId"
20+
| "queryByText"
2121
| "getByRole"
2222
>;
2323

2424
/**
2525
* Wait for the app to finish loading (loading screen disappears).
2626
*/
27-
export async function waitForAppLoad(queries: Pick<Queries, "queryByTestId">) {
27+
export async function waitForAppLoad(queries: Pick<Queries, "queryByText">) {
2828
await waitFor(
2929
() => {
30-
expect(queries.queryByTestId("loading-screen")).not.toBeInTheDocument();
30+
expect(queries.queryByText(/loading workspaces/i)).not.toBeInTheDocument();
3131
},
3232
{ timeout: 5000 }
3333
);

tests/setup.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
import assert from "assert";
77
import "disposablestack/auto";
88

9+
// Required by React to avoid noisy act(...) warnings in tests.
10+
// @testing-library/react sets this in many setups, but being explicit keeps
11+
// our Jest environment consistent across projects.
12+
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
13+
914
assert.equal(typeof Symbol.dispose, "symbol");
1015
// Use fast approximate token counting in Jest to avoid 10s WASM cold starts
1116
// Individual tests can override with MUX_FORCE_REAL_TOKENIZER=1

0 commit comments

Comments
 (0)