Skip to content

Commit 8150cd9

Browse files
authored
Jest test correctness fixes (#2427)
## Summary: This PR fixes a bunch of Jest tests to be more reliable. It improves: * Switches to using `findByXyz()` functions when using `userEvent` functions on the result. I've found that in some environments we'll see the dreaded `An update to XyzComponent inside a test was not wrapped in act(...).` error. * Move away from using the `done` callback in Jest tests (it's more portable to [return a `Promise`](https://jestjs.io/docs/asynchronous#promises) that just resolves when the test is done). * Fixed a format option in a `test.each()` test (`%b` is invalid) * Wrapping operations that mutate the DOM in [`act()`](https://testing-library.com/docs/react-testing-library/api/#act) calls. Issue: n/a ## Test plan: `pnpm test` <-- all tests pass! Author: jeremywiebe Reviewers: jeremywiebe, Myranae Required Reviewers: Approved By: Myranae Checks: ✅ 8 checks were successful Pull Request URL: #2427
1 parent 22e7de3 commit 8150cd9

File tree

15 files changed

+106
-99
lines changed

15 files changed

+106
-99
lines changed

.changeset/six-pants-brush.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---

packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor-locked-figures.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe("InteractiveGraphEditor locked figures", () => {
8787
name: "Add locked figure",
8888
});
8989
await userEvent.click(addLockedFigureButton);
90-
const addFigureButton = screen.getByText(figureType);
90+
const addFigureButton = await screen.findByText(figureType);
9191
await userEvent.click(addFigureButton);
9292

9393
// Assert

packages/perseus/src/__tests__/renderer-api.test.tsx

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,19 @@ describe("Perseus API", function () {
9696
expect(score).toHaveInvalidInput();
9797
});
9898

99-
it("should be able to accept a callback", function (done) {
100-
const {renderer} = renderQuestion(mockWidget1Item.question);
101-
act(() =>
102-
renderer.setInputValue(["mock-widget 1"], "3", function () {
103-
const guess =
104-
renderer.getUserInput()[0] as PerseusMockWidgetUserInput;
105-
expect(guess?.currentValue).toBe("3");
106-
done();
107-
}),
108-
);
109-
jest.runAllTimers();
110-
});
99+
it("should be able to accept a callback", () =>
100+
new Promise(function (resolve) {
101+
const {renderer} = renderQuestion(mockWidget1Item.question);
102+
act(() =>
103+
renderer.setInputValue(["mock-widget 1"], "3", function () {
104+
const guess =
105+
renderer.getUserInput()[0] as PerseusMockWidgetUserInput;
106+
expect(guess?.currentValue).toBe("3");
107+
resolve(null);
108+
}),
109+
);
110+
jest.runAllTimers();
111+
}));
111112
});
112113

113114
describe("getInputPaths", function () {

packages/perseus/src/components/number-input.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,12 +226,10 @@ class NumberInput extends React.Component<any, any> {
226226
}
227227

228228
const {
229-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
230-
onFormatChange,
231-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
232-
checkValidity,
233-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
234-
useArrowKeys,
229+
onFormatChange: _,
230+
checkValidity: __,
231+
useArrowKeys: ___,
232+
allowPiTruncation: ____,
235233
...restProps
236234
} = this.props;
237235

packages/perseus/src/widgets/graded-group/graded-group.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ describe("graded-group", () => {
251251

252252
// Assert
253253
expect(
254-
screen.getByRole("button", {name: "Try again"}),
254+
await screen.findByRole("button", {name: "Try again"}),
255255
).toBeVisible();
256256
});
257257

packages/perseus/src/widgets/image/__snapshots__/image.test.ts.snap

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`image widget - isMobile %b should snapshot: first render 1`] = `
3+
exports[`image widget - isMobile(false) should snapshot: first render 1`] = `
44
<div>
55
<div
66
class="perseus-renderer perseus-renderer-responsive perseus-renderer-two-columns"
@@ -28,6 +28,24 @@ exports[`image widget - isMobile %b should snapshot: first render 1`] = `
2828
class="perseus-image-widget"
2929
style="max-width: 420px;"
3030
>
31+
<div
32+
class="perseus-image-title"
33+
>
34+
<div
35+
class="perseus-renderer perseus-renderer-responsive"
36+
>
37+
<div
38+
class="paragraph"
39+
data-perseus-paragraph-index="0"
40+
>
41+
<div
42+
class="paragraph"
43+
>
44+
Image Title
45+
</div>
46+
</div>
47+
</div>
48+
</div>
3149
<div
3250
class="fixed-to-responsive svg-image"
3351
style="max-width: 420px; max-height: 345px;"
@@ -57,25 +75,20 @@ exports[`image widget - isMobile %b should snapshot: first render 1`] = `
5775
</span>
5876
</div>
5977
<figcaption
60-
class="perseus-image-caption has-title"
78+
class="perseus-image-caption"
6179
style="max-width: 420px;"
6280
>
63-
<div>
81+
<div
82+
class="perseus-renderer perseus-renderer-responsive"
83+
>
6484
<div
65-
class="perseus-renderer perseus-renderer-responsive"
85+
class="paragraph"
86+
data-perseus-paragraph-index="0"
6687
>
6788
<div
6889
class="paragraph"
69-
data-perseus-paragraph-index="0"
7090
>
71-
<div
72-
class="paragraph"
73-
>
74-
<strong>
75-
Image Title.
76-
</strong>
77-
Image Caption
78-
</div>
91+
Image Caption
7992
</div>
8093
</div>
8194
</div>
@@ -164,7 +177,7 @@ exports[`image widget - isMobile %b should snapshot: first render 1`] = `
164177
</div>
165178
`;
166179

167-
exports[`image widget - isMobile %b should snapshot: first render 2`] = `
180+
exports[`image widget - isMobile(true) should snapshot: first render 1`] = `
168181
<div>
169182
<div
170183
class="perseus-renderer perseus-renderer-responsive perseus-renderer-two-columns"
@@ -192,24 +205,6 @@ exports[`image widget - isMobile %b should snapshot: first render 2`] = `
192205
class="perseus-image-widget"
193206
style="max-width: 420px;"
194207
>
195-
<div
196-
class="perseus-image-title"
197-
>
198-
<div
199-
class="perseus-renderer perseus-renderer-responsive"
200-
>
201-
<div
202-
class="paragraph"
203-
data-perseus-paragraph-index="0"
204-
>
205-
<div
206-
class="paragraph"
207-
>
208-
Image Title
209-
</div>
210-
</div>
211-
</div>
212-
</div>
213208
<div
214209
class="fixed-to-responsive svg-image"
215210
style="max-width: 420px; max-height: 345px;"
@@ -239,20 +234,25 @@ exports[`image widget - isMobile %b should snapshot: first render 2`] = `
239234
</span>
240235
</div>
241236
<figcaption
242-
class="perseus-image-caption"
237+
class="perseus-image-caption has-title"
243238
style="max-width: 420px;"
244239
>
245-
<div
246-
class="perseus-renderer perseus-renderer-responsive"
247-
>
240+
<div>
248241
<div
249-
class="paragraph"
250-
data-perseus-paragraph-index="0"
242+
class="perseus-renderer perseus-renderer-responsive"
251243
>
252244
<div
253245
class="paragraph"
246+
data-perseus-paragraph-index="0"
254247
>
255-
Image Caption
248+
<div
249+
class="paragraph"
250+
>
251+
<strong>
252+
Image Title.
253+
</strong>
254+
Image Caption
255+
</div>
256256
</div>
257257
</div>
258258
</div>

packages/perseus/src/widgets/image/image.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {question} from "./image.testdata";
1010

1111
import type {APIOptions} from "../../types";
1212

13-
describe.each([true, false])("image widget - isMobile %b", (isMobile) => {
13+
describe.each([[true], [false]])("image widget - isMobile(%j)", (isMobile) => {
1414
const apiOptions: APIOptions = {isMobile};
1515

1616
beforeEach(() => {

packages/perseus/src/widgets/input-number/input-number.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Disclaimer: Definitely not thorough enough
33
*/
44
import {describe, beforeEach, it} from "@jest/globals";
5-
import {act, screen} from "@testing-library/react";
5+
import {act, screen, waitFor} from "@testing-library/react";
66
import {userEvent as userEventLib} from "@testing-library/user-event";
77
import _ from "underscore";
88

@@ -37,7 +37,7 @@ describe("input-number", function () {
3737
const {renderer} = renderQuestion(question);
3838

3939
// Act
40-
const textbox = screen.getByRole("textbox");
40+
const textbox = await screen.findByRole("textbox");
4141
await userEvent.click(textbox);
4242
await userEvent.type(textbox, "1/2");
4343
const score = scorePerseusItemTesting(
@@ -54,7 +54,7 @@ describe("input-number", function () {
5454
const {renderer} = renderQuestion(question);
5555

5656
// Act
57-
const textbox = screen.getByRole("textbox");
57+
const textbox = await screen.findByRole("textbox");
5858
await userEvent.click(textbox);
5959
await userEvent.type(textbox, "0.7");
6060
const score = scorePerseusItemTesting(
@@ -71,7 +71,7 @@ describe("input-number", function () {
7171
const {renderer} = renderQuestion(question);
7272

7373
// Act
74-
const textbox = screen.getByRole("textbox");
74+
const textbox = await screen.findByRole("textbox");
7575
await userEvent.click(textbox);
7676
await userEvent.type(textbox, "0..7");
7777
const score = scorePerseusItemTesting(
@@ -205,7 +205,7 @@ describe("input-number", function () {
205205
const {renderer} = renderQuestion(question);
206206

207207
// Act
208-
const textbox = screen.getByRole("textbox");
208+
const textbox = await screen.findByRole("textbox");
209209
await userEvent.click(textbox);
210210
await userEvent.type(textbox, correct);
211211
const score = scorePerseusItemTesting(
@@ -222,7 +222,7 @@ describe("input-number", function () {
222222
const {renderer} = renderQuestion(question);
223223

224224
// Act
225-
const textbox = screen.getByRole("textbox");
225+
const textbox = await screen.findByRole("textbox");
226226
await userEvent.click(textbox);
227227
await userEvent.type(textbox, incorrect);
228228
const score = scorePerseusItemTesting(
@@ -335,18 +335,22 @@ describe("focus state", () => {
335335
const gotFocus = await act(() => renderer.focus());
336336

337337
// Assert
338+
expect(screen.getByRole("textbox")).toHaveFocus();
338339
expect(gotFocus).toBe(true);
339340
});
340341

341342
it("supports blurring", async () => {
342343
// Arrange
343344
const {renderer} = renderQuestion(question);
345+
await act(() => renderer.focus());
346+
expect(screen.getByRole("textbox")).toHaveFocus();
344347

345348
// Act
346-
const gotFocus = await act(() => renderer.focus());
347-
act(() => renderer.blur());
349+
await act(() => renderer.blur());
348350

349351
// Assert
350-
expect(gotFocus).toBe(true);
352+
await waitFor(() => {
353+
expect(screen.getByRole("textbox")).not.toHaveFocus();
354+
});
351355
});
352356
});

packages/perseus/src/widgets/interactive-graphs/graphs/circle.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {render, screen} from "@testing-library/react";
1+
import {act, render, screen} from "@testing-library/react";
22
import {userEvent as userEventLib} from "@testing-library/user-event";
33
import {Mafs} from "mafs";
44
import * as React from "react";
@@ -315,7 +315,7 @@ describe("Circle graph screen reader", () => {
315315

316316
// Act
317317
// move the radius point
318-
radiusPoint.focus();
318+
act(() => radiusPoint.focus());
319319
await userEvent.keyboard("{arrowright}");
320320

321321
// Assert
@@ -333,12 +333,12 @@ describe("Circle graph screen reader", () => {
333333

334334
// Act
335335
// move the radius point so that its aria-live is set to polite
336-
radiusPoint.focus();
336+
act(() => radiusPoint.focus());
337337
await userEvent.keyboard("{arrowright}");
338338
expect(radiusPoint).toHaveAttribute("aria-live", "polite");
339339

340340
// move the circle
341-
circleGraph.focus();
341+
act(() => circleGraph.focus());
342342
await userEvent.keyboard("{arrowright}");
343343

344344
// Assert

packages/perseus/src/widgets/interactive-graphs/graphs/linear-system.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {render, screen} from "@testing-library/react";
1+
import {act, render, screen} from "@testing-library/react";
22
import {userEvent as userEventLib} from "@testing-library/user-event";
33
import * as React from "react";
44

@@ -276,7 +276,7 @@ describe("Linear System graph screen reader", () => {
276276
const movingElement = interactiveElements[index];
277277

278278
// Act - Move the element
279-
movingElement.focus();
279+
act(() => movingElement.focus());
280280
await userEvent.keyboard("{ArrowRight}");
281281

282282
const expectedAriaLive = ["off", "off", "off"];

0 commit comments

Comments
 (0)