Skip to content

Commit

Permalink
Fix console errors and warnings printed in tests (#1405)
Browse files Browse the repository at this point in the history
Our tests printed a lot of noisy warnings to the logs, which made it harder to
spot real test failures. This PR fixes up the code so the warnings are no
longer logged.

Each commit in the PR fixes a different warning. It's probably going to be
easiest to review it one commit at a time.

Issue: LEMS-2141

## Test plan:

Run `yarn test`, and verify that no warnings are printed.

Author: benchristel

Reviewers: benchristel, jeremywiebe, mark-fitzgerald, Myranae, nicolecomputer

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ codecov/project, ❌ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1405
  • Loading branch information
benchristel authored Jul 12, 2024
1 parent 147ab04 commit a430de4
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 48 deletions.
6 changes: 6 additions & 0 deletions .changeset/angry-plums-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

Internal: Fix console errors and warnings printed in tests
36 changes: 28 additions & 8 deletions packages/perseus-editor/src/widgets/numeric-input-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,14 @@ class NumericInputEditor extends React.Component<Props, State> {
const addAnswerButton = (
<div>
<a
href="javascript:void(0)"
href="#"
className="simple-button orange"
onClick={() => this.addAnswer()}
onClick={(e) => {
// preventDefault ensures that href="#"
// doesn't scroll to the top of the page
e.preventDefault();
this.addAnswer();
}}
onKeyDown={(e) => this.onSpace(e, this.addAnswer)}
>
<span>Add new answer</span>
Expand Down Expand Up @@ -425,31 +430,46 @@ class NumericInputEditor extends React.Component<Props, State> {
) : null}
<div className="value-divider" />
<a
href="javascript:void(0)"
href="#"
className={"answer-status " + answer.status}
onClick={() => this.onStatusChange(i)}
onClick={(e) => {
// preventDefault ensures that href="#"
// doesn't scroll to the top of the page
e.preventDefault();
this.onStatusChange(i);
}}
onKeyDown={(e) =>
this.onSpace(e, this.onStatusChange)
}
>
{answer.status}
</a>
<a
href="javascript:void(0)"
href="#"
className="answer-trash"
aria-label="Delete answer"
onClick={() => this.onTrashAnswer(i)}
onClick={(e) => {
// preventDefault ensures that href="#"
// doesn't scroll to the top of the page
e.preventDefault();
this.onTrashAnswer(i);
}}
onKeyDown={(e) =>
this.onSpace(e, this.onTrashAnswer)
}
>
<InlineIcon {...iconTrash} />
</a>
<a
href="javascript:void(0)"
href="#"
className="options-toggle"
aria-label="Toggle options"
onClick={() => this.onToggleOptions(i)}
onClick={(e) => {
// preventDefault ensures that href="#"
// doesn't scroll to the top of the page
e.preventDefault();
this.onToggleOptions(i);
}}
onKeyDown={(e) =>
this.onSpace(e, this.onToggleOptions)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ describe("locked layer", () => {
const KasParseMock = jest
.spyOn(KAS, "parse")
.mockReturnValue({expr: {eval: () => PARSED}});
const PlotMock = jest.spyOn(Plot, "OfX").mockImplementation();
const PlotMock = jest.spyOn(Plot, "OfX").mockReturnValue(null);
renderQuestion(segmentWithLockedFunction("x^2"), {
flags: {
mafs: {
Expand All @@ -741,8 +741,8 @@ describe("locked layer", () => {
expect(plotFn(0)).toEqual(PARSED);

// Arrange - equation does NOT need parsing
KasParseMock.mockReset();
PlotMock.mockReset();
KasParseMock.mockClear();
PlotMock.mockClear();
renderQuestion(
segmentWithLockedFunction("x^2", {
equationParsed: {eval: () => PREPARSED},
Expand Down Expand Up @@ -783,7 +783,7 @@ describe("locked layer", () => {
expect(PlotOfYMock).toHaveBeenCalledTimes(0);

// Arrange - reset mocks
PlotOfXMock.mockReset();
PlotOfXMock.mockClear();

// Act - Render f(y)
renderQuestion(
Expand Down
15 changes: 5 additions & 10 deletions packages/perseus/src/widgets/__tests__/radio.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {describe, beforeEach, it} from "@jest/globals";
import {act, screen, fireEvent} from "@testing-library/react";
import {screen, fireEvent} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";
import _ from "underscore";

import {clone} from "../../../../../testing/object-utils";
import {testDependencies} from "../../../../../testing/test-dependencies";
Expand Down Expand Up @@ -409,9 +408,6 @@ describe("single-choice question", () => {
name: /Open menu for Choice B/,
}),
);
await act(async () => {
await jest.runAllTimers();
});

// Assert
expect(
Expand All @@ -429,9 +425,6 @@ describe("single-choice question", () => {

// Act
await userEvent.keyboard(" ");
await act(async () => {
await jest.runAllTimers();
});

// Assert
expect(
Expand Down Expand Up @@ -834,13 +827,15 @@ describe("multi-choice question", () => {

it.each(invalid)(
"should reject an invalid answer - test #%#",
(...choices) => {
async (...choices) => {
// Arrange
const {renderer} = renderQuestion(question, apiOptions);

// Act
const option = screen.getAllByRole("checkbox");
choices.forEach(async (i) => await userEvent.click(option[i]));
for (const i of choices) {
await userEvent.click(option[i]);
}

// Assert
expect(renderer).toHaveInvalidInput();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import * as UseDraggableModule from "../use-draggable";

import {StyledMovablePoint} from "./movable-point";

import type {GraphConfig} from "../../reducer/use-graph-config";

jest.mock("@khanacademy/wonder-blocks-tooltip", () => {
const originalModule = jest.requireActual(
"@khanacademy/wonder-blocks-tooltip",
Expand All @@ -26,14 +28,20 @@ const TooltipMock = ({children}) => {
describe("StyledMovablePoint", () => {
let useGraphConfigMock: jest.SpyInstance;
let useDraggableMock: jest.SpyInstance;
const baseGraphConfigContext = {
snapStep: 1,
const baseGraphConfigContext: GraphConfig = {
range: [
[0, 0],
[1, 1],
[0, 1],
[0, 1],
],
tickStep: [1, 1],
gridStep: [1, 1],
snapStep: [1, 1],
markings: "graph",
showTooltips: false,
graphDimensionsInPixels: [200, 200],
width: 200,
height: 200,
labels: [],
};

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export type GraphConfig = {
snapStep: vec.Vector2;
markings: "graph" | "grid" | "none";
showTooltips: boolean;
// TODO(benchristel): it seems like graphDimensionsInPixels duplicates
// width and height. Can we remove one or the other?
graphDimensionsInPixels: vec.Vector2;
width: number; // pixels
height: number; // pixels
Expand Down
18 changes: 15 additions & 3 deletions packages/perseus/src/widgets/passage/passage-markdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ const rules = {
>
<span style={SQUARE_LABEL_STYLE}>{node.content}</span>
</span>,
<AltText id="questionMarker" number={node.content} />,
<AltText
key="alt-text"
id="questionMarker"
number={node.content}
/>,
node.space ? "\u00A0" : null,
];
},
Expand Down Expand Up @@ -322,7 +326,11 @@ const rules = {
>
<span style={CIRCLE_LABEL_STYLE}>{node.content}</span>
</span>,
<AltText id="circleMarker" number={node.content} />,
<AltText
key="alt-text"
id="circleMarker"
number={node.content}
/>,
node.space ? "\u00A0" : null,
];
},
Expand Down Expand Up @@ -352,7 +360,11 @@ const rules = {
>
[{node.content}]
</span>,
<AltText id="sentenceMarker" number={node.content} />,
<AltText
key="alt-text"
id="sentenceMarker"
number={node.content}
/>,
node.space ? "\u00A0" : null,
];
},
Expand Down
23 changes: 4 additions & 19 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6034,25 +6034,10 @@ caniuse-api@^3.0.0:
lodash.memoize "^4.1.2"
lodash.uniq "^4.5.0"

caniuse-lite@^1.0.0:
version "1.0.30001335"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001335.tgz#899254a0b70579e5a957c32dced79f0727c61f2a"
integrity sha512-ddP1Tgm7z2iIxu6QTtbZUv6HJxSaV/PZeSrWFZtbY4JZ69tOeNhBCl3HyRQgeNZKE5AOn1kpV7fhljigy0Ty3w==

caniuse-lite@^1.0.30001332:
version "1.0.30001332"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001332.tgz#39476d3aa8d83ea76359c70302eafdd4a1d727dd"
integrity sha512-10T30NYOEQtN6C11YGg411yebhvpnC6Z102+B95eAsN0oB6KUs01ivE8u+G6FMIRtIrVlYXhL+LUwQ3/hXwDWw==

caniuse-lite@^1.0.30001541:
version "1.0.30001559"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001559.tgz#95a982440d3d314c471db68d02664fb7536c5a30"
integrity sha512-cPiMKZgqgkg5LY3/ntGeLFUpi6tzddBNS58A4tnTgQw1zON7u2sZMU7SzOeVH4tj20++9ggL+V6FDOFMTaFFYA==

caniuse-lite@^1.0.30001587:
version "1.0.30001587"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001587.tgz#a0bce920155fa56a1885a69c74e1163fc34b4881"
integrity sha512-HMFNotUmLXn71BQxg8cijvqxnIAofforZOwGsxyXJ0qugTdspUF4sPSJ2vhgprHCB996tIDzEq1ubumPDV8ULA==
caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001332, caniuse-lite@^1.0.30001541, caniuse-lite@^1.0.30001587:
version "1.0.30001641"
resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001641.tgz"
integrity sha512-Phv5thgl67bHYo1TtMY/MurjkHhV4EDaCosezRXgZ8jzA/Ub+wjxAvbGvjoFENStinwi5kCyOYV3mi5tOGykwA==

caseless@~0.12.0:
version "0.12.0"
Expand Down

0 comments on commit a430de4

Please sign in to comment.