Skip to content

Commit 2f43700

Browse files
BrunoQuaresmaBrunoQuaresma
authored
chore(site): clean up mocks after each test (coder#12805)
1 parent cfb9428 commit 2f43700

10 files changed

+154
-179
lines changed

‎site/jest.polyfills.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,16 @@ Object.defineProperties(globalThis, {
2929
FormData: { value: FormData },
3030
Request: { value: Request },
3131
Response: { value: Response },
32+
matchMedia: {
33+
value: (query) => ({
34+
matches: false,
35+
media: query,
36+
onchange: null,
37+
addListener: jest.fn(),
38+
removeListener: jest.fn(),
39+
addEventListener: jest.fn(),
40+
removeEventListener: jest.fn(),
41+
dispatchEvent: jest.fn(),
42+
}),
43+
},
3244
});

‎site/jest.setup.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ beforeAll(() =>
6363
afterEach(() => {
6464
cleanup();
6565
server.resetHandlers();
66-
jest.clearAllMocks();
66+
jest.resetAllMocks();
6767
});
6868

6969
// Clean up after the tests are finished.

‎site/src/hooks/useClipboard-http.test.ts

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎site/src/hooks/useClipboard-https.test.ts

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎site/src/hooks/useClipboard.test-setup.tsx renamed to ‎site/src/hooks/useClipboard.test.tsx

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
import { act, renderHook } from "@testing-library/react";
2+
import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
3+
import { ThemeProvider } from "contexts/ThemeProvider";
4+
import {
5+
type UseClipboardInput,
6+
type UseClipboardResult,
7+
useClipboard,
8+
} from "./useClipboard";
9+
10+
describe(useClipboard.name, () => {
11+
describe("HTTP (non-secure) connections", () => {
12+
scheduleClipboardTests({ isHttps: false });
13+
});
14+
15+
describe("HTTPS (secure/default) connections", () => {
16+
scheduleClipboardTests({ isHttps: true });
17+
});
18+
});
19+
120
/**
221
* @file This is a very weird test setup.
322
*
@@ -41,25 +60,6 @@
4160
* order of operations involving closure, but you have no idea why the code
4261
* is working, and it's impossible to debug.
4362
*/
44-
import { act, renderHook } from "@testing-library/react";
45-
import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
46-
import {
47-
type UseClipboardInput,
48-
type UseClipboardResult,
49-
useClipboard,
50-
} from "./useClipboard";
51-
52-
const initialExecCommand = global.document.execCommand;
53-
beforeAll(() => {
54-
jest.useFakeTimers();
55-
});
56-
57-
afterAll(() => {
58-
jest.restoreAllMocks();
59-
jest.useRealTimers();
60-
global.document.execCommand = initialExecCommand;
61-
});
62-
6363
type MockClipboardEscapeHatches = Readonly<{
6464
getMockText: () => string;
6565
setMockText: (newText: string) => void;
@@ -124,10 +124,10 @@ function renderUseClipboard(inputs: UseClipboardInput) {
124124
{
125125
initialProps: inputs,
126126
wrapper: ({ children }) => (
127-
<>
128-
<>{children}</>
127+
<ThemeProvider>
128+
{children}
129129
<GlobalSnackbar />
130-
</>
130+
</ThemeProvider>
131131
),
132132
},
133133
);
@@ -137,9 +137,10 @@ type ScheduleConfig = Readonly<{ isHttps: boolean }>;
137137

138138
export function scheduleClipboardTests({ isHttps }: ScheduleConfig) {
139139
const mockClipboardInstance = makeMockClipboard(isHttps);
140-
141140
const originalNavigator = window.navigator;
142-
beforeAll(() => {
141+
142+
beforeEach(() => {
143+
jest.useFakeTimers();
143144
jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
144145
...originalNavigator,
145146
clipboard: mockClipboardInstance,
@@ -170,6 +171,7 @@ export function scheduleClipboardTests({ isHttps }: ScheduleConfig) {
170171
});
171172

172173
afterEach(() => {
174+
jest.useRealTimers();
173175
mockClipboardInstance.setMockText("");
174176
mockClipboardInstance.setSimulateFailure(false);
175177
});

‎site/src/pages/TerminalPage/TerminalAlerts.tsx

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,66 @@
11
import Button from "@mui/material/Button";
22
import Link from "@mui/material/Link";
3-
import { type FC, useState } from "react";
3+
import { type FC, useState, useEffect, useRef } from "react";
4+
import type { WorkspaceAgent } from "api/typesGenerated";
45
import { Alert, type AlertProps } from "components/Alert/Alert";
56
import { docs } from "utils/docs";
7+
import type { ConnectionStatus } from "./types";
8+
9+
type TerminalAlertsProps = {
10+
agent: WorkspaceAgent | undefined;
11+
status: ConnectionStatus;
12+
onAlertChange: () => void;
13+
};
14+
15+
export const TerminalAlerts = ({
16+
agent,
17+
status,
18+
onAlertChange,
19+
}: TerminalAlertsProps) => {
20+
const lifecycleState = agent?.lifecycle_state;
21+
const prevLifecycleState = useRef(lifecycleState);
22+
useEffect(() => {
23+
prevLifecycleState.current = lifecycleState;
24+
}, [lifecycleState]);
25+
26+
// We want to observe the children of the wrapper to detect when the alert
27+
// changes. So the terminal page can resize itself.
28+
//
29+
// Would it be possible to just always call fit() when this component
30+
// re-renders instead of using an observer?
31+
//
32+
// This is a good question and the why this does not work is that the .fit()
33+
// needs to run after the render so in this case, I just think the mutation
34+
// observer is more reliable. I could use some hacky setTimeout inside of
35+
// useEffect to do that, I guess, but I don't think it would be any better.
36+
const wrapperRef = useRef<HTMLDivElement>(null);
37+
useEffect(() => {
38+
if (!wrapperRef.current) {
39+
return;
40+
}
41+
const observer = new MutationObserver(onAlertChange);
42+
observer.observe(wrapperRef.current, { childList: true });
43+
44+
return () => {
45+
observer.disconnect();
46+
};
47+
}, [onAlertChange]);
48+
49+
return (
50+
<div ref={wrapperRef}>
51+
{status === "disconnected" ? (
52+
<DisconnectedAlert />
53+
) : lifecycleState === "start_error" ? (
54+
<ErrorScriptAlert />
55+
) : lifecycleState === "starting" ? (
56+
<LoadingScriptsAlert />
57+
) : lifecycleState === "ready" &&
58+
prevLifecycleState.current === "starting" ? (
59+
<LoadedScriptsAlert />
60+
) : null}
61+
</div>
62+
);
63+
};
664

765
export const ErrorScriptAlert: FC = () => {
866
return (

‎site/src/pages/TerminalPage/TerminalPage.test.tsx

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,27 @@ import {
88
MockWorkspace,
99
MockWorkspaceAgent,
1010
} from "testHelpers/entities";
11-
import {
12-
renderWithAuth,
13-
waitForLoaderToBeRemoved,
14-
} from "testHelpers/renderHelpers";
11+
import { renderWithAuth } from "testHelpers/renderHelpers";
1512
import { server } from "testHelpers/server";
1613
import TerminalPage, { Language } from "./TerminalPage";
1714

18-
Object.defineProperty(window, "matchMedia", {
19-
writable: true,
20-
value: jest.fn().mockImplementation((query) => ({
21-
matches: false,
22-
media: query,
23-
onchange: null,
24-
addListener: jest.fn(), // deprecated
25-
removeListener: jest.fn(), // deprecated
26-
addEventListener: jest.fn(),
27-
removeEventListener: jest.fn(),
28-
dispatchEvent: jest.fn(),
29-
})),
30-
});
31-
3215
const renderTerminal = async (
3316
route = `/${MockUser.username}/${MockWorkspace.name}/terminal`,
3417
) => {
3518
const utils = renderWithAuth(<TerminalPage />, {
3619
route,
3720
path: "/:username/:workspace/terminal",
3821
});
39-
await waitForLoaderToBeRemoved();
22+
await waitFor(() => {
23+
// To avoid 'act' errors during testing, we ensure the component is
24+
// completely rendered without any outstanding state updates. This is
25+
// accomplished by incorporating a 'data-status' attribute into the
26+
// component. We then observe this attribute for any changes, as we cannot
27+
// rely on other screen elements to indicate completion.
28+
const wrapper =
29+
utils.container.querySelector<HTMLDivElement>("[data-status]")!;
30+
expect(wrapper.dataset.state).not.toBe("initializing");
31+
});
4032
return utils;
4133
};
4234

@@ -58,11 +50,15 @@ const expectTerminalText = (container: HTMLElement, text: string) => {
5850
};
5951

6052
describe("TerminalPage", () => {
53+
afterEach(() => {
54+
WS.clean();
55+
});
56+
6157
it("loads the right workspace data", async () => {
62-
const spy = jest
58+
jest
6359
.spyOn(API, "getWorkspaceByOwnerAndName")
6460
.mockResolvedValue(MockWorkspace);
65-
const ws = new WS(
61+
new WS(
6662
`ws://localhost/api/v2/workspaceagents/${MockWorkspaceAgent.id}/pty`,
6763
);
6864
await renderTerminal(
@@ -75,57 +71,45 @@ describe("TerminalPage", () => {
7571
{ include_deleted: true },
7672
);
7773
});
78-
spy.mockRestore();
79-
ws.close();
8074
});
8175

8276
it("shows an error if fetching workspace fails", async () => {
83-
// Given
8477
server.use(
8578
http.get("/api/v2/users/:userId/workspace/:workspaceName", () => {
8679
return HttpResponse.json({ id: "workspace-id" }, { status: 500 });
8780
}),
8881
);
8982

90-
// When
9183
const { container } = await renderTerminal();
9284

93-
// Then
9485
await expectTerminalText(container, Language.workspaceErrorMessagePrefix);
9586
});
9687

9788
it("shows an error if the websocket fails", async () => {
98-
// Given
9989
server.use(
10090
http.get("/api/v2/workspaceagents/:agentId/pty", () => {
10191
return HttpResponse.json({}, { status: 500 });
10292
}),
10393
);
10494

105-
// When
10695
const { container } = await renderTerminal();
10796

108-
// Then
10997
await expectTerminalText(container, Language.websocketErrorMessagePrefix);
11098
});
11199

112100
it("renders data from the backend", async () => {
113-
// Given
114101
const ws = new WS(
115102
`ws://localhost/api/v2/workspaceagents/${MockWorkspaceAgent.id}/pty`,
116103
);
117104
const text = "something to render";
118105

119-
// When
120106
const { container } = await renderTerminal();
121-
122-
// Then
123107
// Ideally we could use ws.connected but that seems to pause React updates.
124108
// For now, wait for the initial resize message instead.
125109
await ws.nextMessage;
126110
ws.send(text);
111+
127112
await expectTerminalText(container, text);
128-
ws.close();
129113
});
130114

131115
// Ideally we could just pass the correct size in the web socket URL without
@@ -134,40 +118,32 @@ describe("TerminalPage", () => {
134118
// in the other tests since ws.connected appears to pause React updates. So
135119
// for now the initial resize message (and this test) are here to stay.
136120
it("resizes on connect", async () => {
137-
// Given
138121
const ws = new WS(
139122
`ws://localhost/api/v2/workspaceagents/${MockWorkspaceAgent.id}/pty`,
140123
);
141124

142-
// When
143125
await renderTerminal();
144126

145-
// Then
146127
const msg = await ws.nextMessage;
147128
const req = JSON.parse(new TextDecoder().decode(msg as Uint8Array));
148129
expect(req.height).toBeGreaterThan(0);
149130
expect(req.width).toBeGreaterThan(0);
150-
ws.close();
151131
});
152132

153133
it("supports workspace.agent syntax", async () => {
154-
// Given
155134
const ws = new WS(
156135
`ws://localhost/api/v2/workspaceagents/${MockWorkspaceAgent.id}/pty`,
157136
);
158137
const text = "something to render";
159138

160-
// When
161139
const { container } = await renderTerminal(
162140
`/some-user/${MockWorkspace.name}.${MockWorkspaceAgent.name}/terminal`,
163141
);
164142

165-
// Then
166143
// Ideally we could use ws.connected but that seems to pause React updates.
167144
// For now, wait for the initial resize message instead.
168145
await ws.nextMessage;
169146
ws.send(text);
170147
await expectTerminalText(container, text);
171-
ws.close();
172148
});
173149
});

0 commit comments

Comments
 (0)