Skip to content

Commit 77ee5c6

Browse files
malinskibeniaminclaudegithub-actions[bot]
authored
fix(frontend): unblock browser-mode vitest and land visual baselines (#2500)
* fix(frontend): unblock browser-mode vitest so visual baselines can generate Baseline generation has never completed ('blocks on the first cold Vitest+Playwright browser-mode launch and runs out the job timeout' per the test-browser CI comment). Root causes, in the order they unmasked: - @icons-pack/react-simple-icons was barrel-imported; dev-mode Vite pre-bundles the ~3000-icon 59MB package into one module, crashing the chromium tab ('Browser connection was closed'). Switched to deep per-icon .mjs imports (exports map is verbatim './icons/*') with an ambient wildcard declaration for types. - ai-agent-list-page.browser.test.tsx mocked state/ui-state via importOriginal, evaluating the real module's huge transitive graph and stalling module loading. Replaced with a plain factory. - The same test mocked react-query/api/remote-mcp but the page imports react-query/api/aigw/mcp-servers (stale mock target); fixed and wrapped the render in QueryClientProvider. - mockConnectQuery now spreads the actual @connectrpc/connect-query so every re-export (TransportProvider, hooks) stays available; vi.mock factories resolve helpers via dynamic import (hoisting-safe). - vitest.config.browser.mts scans browser specs via optimizeDeps.entries so Vite pre-bundles their dep graph up front instead of re-optimizing mid-run. Full browser suite now completes under CI=true: 6 files, 15 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(frontend): add visual regression baselines Generated by the frontend-update-baselines workflow on the original branch; carried over verbatim so test-browser runs strict comparison. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 8705036 commit 77ee5c6

9 files changed

Lines changed: 97 additions & 53 deletions

File tree

frontend/src/__tests__/browser-test-utils.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,15 @@ export function mockRouterForBrowserTest() {
2727
};
2828
}
2929

30-
export function mockConnectQuery() {
30+
export async function mockConnectQuery() {
31+
/** Spread the real module so every re-export (TransportProvider, hooks, ...) stays available; only the RPC entry points are stubbed. */
32+
const actual = await vi.importActual<typeof import('@connectrpc/connect-query')>('@connectrpc/connect-query');
3133
return {
34+
...actual,
3235
callUnaryMethod: vi.fn(),
3336
createConnectQueryKey: vi.fn(() => ['mock-key']),
37+
useMutation: vi.fn(() => ({ mutate: vi.fn(), mutateAsync: vi.fn(), isPending: false })),
38+
useQuery: vi.fn(() => ({ data: undefined, isLoading: false, error: null })),
3439
};
3540
}
3641

frontend/src/assets/connectors/component-logo-map.tsx

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,17 @@
1-
import {
2-
SiApachekafka,
3-
SiApacheparquet,
4-
SiApachepulsar,
5-
SiClickhouse,
6-
SiCockroachlabs,
7-
SiDgraph,
8-
SiJaeger,
9-
SiMqtt,
10-
SiNeo4j,
11-
SiOllama,
12-
SiOpenai,
13-
SiSalesforce,
14-
SiSplunk,
15-
SiTrino,
16-
} from '@icons-pack/react-simple-icons';
1+
import SiApachekafka from '@icons-pack/react-simple-icons/icons/SiApachekafka.mjs';
2+
import SiApacheparquet from '@icons-pack/react-simple-icons/icons/SiApacheparquet.mjs';
3+
import SiApachepulsar from '@icons-pack/react-simple-icons/icons/SiApachepulsar.mjs';
4+
import SiClickhouse from '@icons-pack/react-simple-icons/icons/SiClickhouse.mjs';
5+
import SiCockroachlabs from '@icons-pack/react-simple-icons/icons/SiCockroachlabs.mjs';
6+
import SiDgraph from '@icons-pack/react-simple-icons/icons/SiDgraph.mjs';
7+
import SiJaeger from '@icons-pack/react-simple-icons/icons/SiJaeger.mjs';
8+
import SiMqtt from '@icons-pack/react-simple-icons/icons/SiMqtt.mjs';
9+
import SiNeo4j from '@icons-pack/react-simple-icons/icons/SiNeo4j.mjs';
10+
import SiOllama from '@icons-pack/react-simple-icons/icons/SiOllama.mjs';
11+
import SiOpenai from '@icons-pack/react-simple-icons/icons/SiOpenai.mjs';
12+
import SiSalesforce from '@icons-pack/react-simple-icons/icons/SiSalesforce.mjs';
13+
import SiSplunk from '@icons-pack/react-simple-icons/icons/SiSplunk.mjs';
14+
import SiTrino from '@icons-pack/react-simple-icons/icons/SiTrino.mjs';
1715
import { RedpandaUILetterIcon } from 'components/redpanda-ui/components/icons';
1816
import {
1917
Activity,

frontend/src/components/icons/index.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616

1717
// Social media icons - simple-icons
1818
// biome-ignore lint/performance/noBarrelFile: Intentional central icon system as per project architecture
19-
export {
20-
SiGithub as GitHubIcon, // FaGithub
21-
SiSlack as SlackIcon, // FaSlack
22-
SiX as TwitterIcon, // FaTwitter (Twitter rebranded to X)
23-
} from '@icons-pack/react-simple-icons';
19+
// Deep per-icon imports: the package barrel re-exports ~3000 icons (59MB),
20+
// which dev-mode bundlers pre-bundle whole — crashing browser-mode vitest.
21+
export { default as GitHubIcon } from '@icons-pack/react-simple-icons/icons/SiGithub.mjs'; // FaGithub
22+
export { default as SlackIcon } from '@icons-pack/react-simple-icons/icons/SiSlack.mjs'; // FaSlack
23+
export { default as TwitterIcon } from '@icons-pack/react-simple-icons/icons/SiX.mjs'; // FaTwitter (Twitter rebranded to X)
2424

2525
// LinkedIn removed from both lucide-react (v1.x) and @icons-pack/react-simple-icons
2626
// (trademark). Inline SVG preserves the brand mark while staying dependency-free.
Loading

frontend/src/components/pages/agents/list/ai-agent-list-page.browser.test.tsx

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,14 @@
1010
*/
1111

1212
import { create } from '@bufbuild/protobuf';
13+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
1314
import type { AIAgent } from 'protogen/redpanda/api/dataplane/v1alpha3/ai_agent_pb';
1415
import { AIAgent_State, AIAgentSchema } from 'protogen/redpanda/api/dataplane/v1alpha3/ai_agent_pb';
1516
import { describe, expect, test, vi } from 'vitest';
1617
import { page } from 'vitest/browser';
1718
import { render } from 'vitest-browser-react';
1819

19-
import {
20-
getRouteComponent,
21-
mockConnectQuery,
22-
mockRouterForBrowserTest,
23-
ScreenshotFrame,
24-
} from '../../../../__tests__/browser-test-utils';
20+
import { getRouteComponent, ScreenshotFrame } from '../../../../__tests__/browser-test-utils';
2521

2622
// Hoisted mocks — `vi.mock` factories run before module imports, so any state
2723
// they reference must be declared via `vi.hoisted()` to survive hoisting.
@@ -38,8 +34,18 @@ const mocks = vi.hoisted(() => ({
3834
}),
3935
}));
4036

41-
vi.mock('@tanstack/react-router', () => mockRouterForBrowserTest());
42-
vi.mock('@connectrpc/connect-query', () => mockConnectQuery());
37+
// vi.mock factories are hoisted above imports, so they must not close over
38+
// top-level bindings — resolve the helpers via dynamic import inside the
39+
// factory instead. The closure variant crashes the browser-mode mocker under
40+
// CI ("error when mocking a module"), killing the page and hanging the run.
41+
vi.mock('@tanstack/react-router', async () => {
42+
const { mockRouterForBrowserTest } = await import('../../../../__tests__/browser-test-utils');
43+
return mockRouterForBrowserTest();
44+
});
45+
vi.mock('@connectrpc/connect-query', async () => {
46+
const { mockConnectQuery } = await import('../../../../__tests__/browser-test-utils');
47+
return mockConnectQuery();
48+
});
4349

4450
vi.mock('config', () => ({
4551
config: { jwt: 'test-jwt-token', controlplaneUrl: 'http://localhost:9090' },
@@ -48,13 +54,12 @@ vi.mock('config', () => ({
4854
addBearerTokenInterceptor: vi.fn((next) => async (request: unknown) => await next(request)),
4955
}));
5056

51-
vi.mock('state/ui-state', async (importOriginal) => {
52-
const actual = await importOriginal<typeof import('state/ui-state')>();
53-
return {
54-
...actual,
55-
uiState: { pageTitle: '', pageBreadcrumbs: [] },
56-
};
57-
});
57+
// Plain factory on purpose: importOriginal would evaluate the real ui-state
58+
// module, whose transitive import graph is large enough to stall browser-mode
59+
// module loading until the run times out.
60+
vi.mock('state/ui-state', () => ({
61+
uiState: { pageTitle: '', pageBreadcrumbs: [] },
62+
}));
5863

5964
vi.mock('react-query/api/ai-agent', () => ({
6065
useListAIAgentsQuery: () => mocks.listAIAgents(),
@@ -63,15 +68,15 @@ vi.mock('react-query/api/ai-agent', () => ({
6368
useStopAIAgentMutation: () => ({ mutate: vi.fn(), mutateAsync: vi.fn(), isPending: false }),
6469
}));
6570

66-
vi.mock('react-query/api/remote-mcp', () => ({
67-
useListMCPServersQuery: () => mocks.listMCPServers(),
71+
vi.mock('react-query/api/aigw/mcp-servers', () => ({
72+
useListAigwMCPServersQuery: () => mocks.listMCPServers(),
6873
}));
6974

7075
vi.mock('react-query/api/secret', () => ({
7176
useDeleteSecretMutation: () => ({ mutate: vi.fn(), mutateAsync: vi.fn(), isPending: false }),
7277
}));
7378

74-
const { AIAgentsListPage } = await import('./ai-agent-list-page');
79+
import { AIAgentsListPage } from './ai-agent-list-page';
7580

7681
// Route-component extraction guard — exercises getRouteComponent for parity
7782
// with the ADP UI browser-test pattern.
@@ -137,9 +142,11 @@ describe('AIAgentsListPage — browser visual regression', () => {
137142
});
138143

139144
render(
140-
<ScreenshotFrame width={1280}>
141-
<AIAgentsListPage />
142-
</ScreenshotFrame>
145+
<QueryClientProvider client={new QueryClient()}>
146+
<ScreenshotFrame width={1280}>
147+
<AIAgentsListPage />
148+
</ScreenshotFrame>
149+
</QueryClientProvider>
143150
);
144151

145152
await expect.element(page.getByText('Customer Support Agent')).toBeVisible();
Loading

frontend/src/components/pages/mcp-servers/details/remote-mcp-inspector-tab.browser.test.tsx

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { describe, expect, test, vi } from 'vitest';
1616
import { page } from 'vitest/browser';
1717
import { render } from 'vitest-browser-react';
1818

19-
import { mockConnectQuery, mockRouterForBrowserTest, ScreenshotFrame } from '../../../../__tests__/browser-test-utils';
19+
import { ScreenshotFrame } from '../../../../__tests__/browser-test-utils';
2020

2121
const RUN_TOOL_REGEX = /run tool/i;
2222

@@ -54,15 +54,25 @@ const mocks = vi.hoisted(() => ({
5454
},
5555
}));
5656

57-
vi.mock('@tanstack/react-router', () => ({
58-
...mockRouterForBrowserTest(),
59-
getRouteApi: () => ({
60-
useParams: () => ({ id: 'mcp-server-visual' }),
61-
useRouteContext: ({ select }: { select: (ctx: Record<string, unknown>) => unknown }) =>
62-
select({ gatewayUrl: 'http://localhost:8090' }),
63-
}),
64-
}));
65-
vi.mock('@connectrpc/connect-query', () => mockConnectQuery());
57+
// vi.mock factories are hoisted above imports, so they must not close over
58+
// top-level bindings — resolve the helpers via dynamic import inside the
59+
// factory instead. The closure variant crashes the browser-mode mocker under
60+
// CI ("error when mocking a module"), killing the page and hanging the run.
61+
vi.mock('@tanstack/react-router', async () => {
62+
const { mockRouterForBrowserTest } = await import('../../../../__tests__/browser-test-utils');
63+
return {
64+
...mockRouterForBrowserTest(),
65+
getRouteApi: () => ({
66+
useParams: () => ({ id: 'mcp-server-visual' }),
67+
useRouteContext: ({ select }: { select: (ctx: Record<string, unknown>) => unknown }) =>
68+
select({ gatewayUrl: 'http://localhost:8090' }),
69+
}),
70+
};
71+
});
72+
vi.mock('@connectrpc/connect-query', async () => {
73+
const { mockConnectQuery } = await import('../../../../__tests__/browser-test-utils');
74+
return mockConnectQuery();
75+
});
6676

6777
vi.mock('config', () => ({
6878
config: { jwt: 'test-jwt-token' },
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Types for deep per-icon imports from @icons-pack/react-simple-icons.
3+
*
4+
* The package's exports map is `"./icons/*": "./icons/*"` (verbatim, no
5+
* extension rewriting), so runtime imports must reference the `.mjs` file
6+
* explicitly — which TypeScript cannot pair with the adjacent `.d.ts`. This
7+
* ambient wildcard supplies the component type instead.
8+
*
9+
* Deep imports are deliberate: the package barrel re-exports ~3000 icons
10+
* (59 MB), which dev-mode bundlers pre-bundle whole and crash browser-mode
11+
* Vitest; per-icon imports load only what the app uses.
12+
*/
13+
declare module '@icons-pack/react-simple-icons/icons/*' {
14+
import type { ComponentType, SVGProps } from 'react';
15+
16+
const icon: ComponentType<SVGProps<SVGSVGElement> & { size?: number | string; color?: string; title?: string }>;
17+
export default icon;
18+
}

frontend/vitest.config.browser.mts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ export default defineConfig({
4040
],
4141
resolve: { alias: sharedAliases, preserveSymlinks: true },
4242
optimizeDeps: {
43+
// Scan every browser spec up front so Vite pre-bundles their full dep
44+
// graph at startup. Without this, the optimizer discovers new deps while
45+
// workers are mid-run, re-optimizes, invalidates in-flight imports, and
46+
// the cold launch hangs until the job timeout (the documented reason
47+
// baseline generation never completed).
48+
entries: ['vitest.browser.setup.ts', 'src/**/*.browser.test.tsx'],
4349
include: [
4450
'@bufbuild/protobuf',
4551
'@connectrpc/connect',

0 commit comments

Comments
 (0)