Skip to content

Commit

Permalink
Remove enzyme and translate any remaining tests to RTL (#812)
Browse files Browse the repository at this point in the history
## Summary:

While working on our WB deps issues, I noticed we still use Enzyme for a few tests.  Two were `xit()` (ignored) in the math-input project so I deleted them as they were testing our legacy keypad components. The rest were easily migrated to our utility `renderQuestion()` function and Testing Library!

Issue: "none"

## Test plan:

`yarn test`

Author: jeremywiebe

Reviewers: jeremywiebe, benchristel, nedredmond, handeyeco, SonicScrewdriver

Required Reviewers:

Approved By: benchristel

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

Pull Request URL: #812
  • Loading branch information
jeremywiebe authored Nov 23, 2023
1 parent 8358da8 commit 6d31e04
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 1,093 deletions.
5 changes: 5 additions & 0 deletions .changeset/three-eagles-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Fix 'focus' parameter on Renderer's setInputValue function to be optional.
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ module.exports = {
"**/*.less",
"@testing-library/jest-dom",
"@testing-library/jest-dom/extend-expect",
"jest-enzyme",
"whatwg-fetch",
],
},
Expand Down
14 changes: 3 additions & 11 deletions config/test/custom-matchers.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
// Ok here we are

type Score =
| {
type: "points";
earned: number;
}
| {
type: "invalid";
message: null | string;
};
import type {PerseusScore} from "../../packages/perseus/src/types";

type PerseusRenderer = {
guessAndScore: () => [Array<any>, Score];
guessAndScore: () => [Array<any>, PerseusScore];
};

expect.extend({
Expand All @@ -32,7 +24,7 @@ expect.extend({
message: () => `Problem was not fully answered`,
};
}
if (score.earned !== 1) {
if (score.earned !== score.total) {
return {
pass: false,
message: () =>
Expand Down
5 changes: 0 additions & 5 deletions config/test/test-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import MutationObserver from "@sheerun/mutationobserver-shim";
import {configure} from "@testing-library/dom"; // eslint-disable-line testing-library/no-dom-import, prettier/prettier
// @ts-expect-error - TS2305 - Module '"aphrodite"' has no exported member 'StyleSheetTestUtils'.
import {StyleSheetTestUtils} from "aphrodite";
import Enzyme from "enzyme"; // eslint-disable-line no-restricted-imports
import React16EnzymeAdapter from "enzyme-adapter-react-16"; // eslint-disable-line no-restricted-imports
import jestSerializerHtml from "jest-serializer-html";
import {addSerializer} from "jest-specific-snapshot";

Expand All @@ -18,9 +16,6 @@ StyleSheetTestUtils.suppressStyleInjection();
// See https://www.npmjs.com/package/jest-specific-snapshot#with-custom-serializer
addSerializer(jestSerializerHtml);

// Enzyme requires an adapater for each version of React.
Enzyme.configure({adapter: new React16EnzymeAdapter()});

// @testing-library uses "data-testId" by default but wonder-blocks uses "data-test-id"
configure({
testIdAttribute: "data-test-id",
Expand Down
4 changes: 0 additions & 4 deletions config/test/test.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ module.exports = {
"jest-extended/all",
"<rootDir>/config/test/test-setup.ts",
"<rootDir>/config/test/custom-matchers.ts",

// TODO(LP-11633) math-input uses these matchers. We can remove this
// once we transition these tests to RTL
"<rootDir>/node_modules/jest-enzyme/lib/index.js",
],
moduleNameMapper: {
...pkgMap,
Expand Down
3 changes: 0 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@
"cypress": "^12.5.1",
"cypress-jest-adapter": "^0.1.1",
"cypress-wait-until": "^1.7.2",
"enzyme": "^3.11.0",
"enzyme-adapter-react-16": "^1.15.2",
"eslint": "^8.13.0",
"eslint-config-prettier": "^8.3.0",
"eslint-import-resolver-typescript": "^3.5.3",
Expand All @@ -92,7 +90,6 @@
"fast-glob": "^3.2.11",
"jest": "^29.0.1",
"jest-environment-jsdom": "^28.0.2",
"jest-enzyme": "^7.1.2",
"jest-extended": "^2.0.0",
"jest-serializer-html": "^7.1.0",
"jest-specific-snapshot": "^5.0.0",
Expand Down

This file was deleted.

87 changes: 35 additions & 52 deletions packages/perseus/src/__tests__/renderer-api.test.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
import {describe, beforeEach, it} from "@jest/globals";
import {RenderStateRoot} from "@khanacademy/wonder-blocks-core";
import {mount} from "enzyme"; // eslint-disable-line no-restricted-imports
import $ from "jquery";
import * as React from "react";
import ReactDOM from "react-dom";
import TestUtils from "react-dom/test-utils";
import _ from "underscore";
import "jest-enzyme";

import {testDependencies} from "../../../../testing/test-dependencies";
import * as Dependencies from "../dependencies";
import {ClassNames} from "../perseus-api";
import Renderer from "../renderer";
import {registerAllWidgetsForTesting} from "../util/register-all-widgets-for-testing";
import {renderQuestion} from "../widgets/__tests__/renderQuestion";

import imageItem from "./test-items/image-item";
import inputNumber1Item from "./test-items/input-number-1-item";
import inputNumber2Item from "./test-items/input-number-2-item";
import tableItem from "./test-items/table-item";

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

const itemWidget = inputNumber1Item;

// Returns a promise that will resolve shortly after the end of this
Expand All @@ -33,26 +29,6 @@ const delayedPromise = (value: undefined) => {
return deferred.promise();
};

const renderQuestionArea = function (item, apiOptions?: APIOptions): any {
const wrapper = mount(
<RenderStateRoot>
<Dependencies.DependenciesContext.Provider
value={{analytics: {onAnalyticsEvent: async () => undefined}}}
>
<Renderer
content={item.question.content}
images={item.question.images}
widgets={item.question.widgets}
problemNum={0}
apiOptions={apiOptions}
/>
</Dependencies.DependenciesContext.Provider>
</RenderStateRoot>,
{includeDefaultTestHarness: false},
);
return wrapper.childAt(0).instance();
};

describe("Perseus API", function () {
beforeEach(() => {
jest.spyOn(Dependencies, "getDependencies").mockReturnValue(
Expand All @@ -63,34 +39,40 @@ describe("Perseus API", function () {

describe("setInputValue", function () {
it("should be able to produce a correctly graded value", function () {
const renderer = renderQuestionArea(inputNumber1Item);
renderer.setInputValue(["input-number 1"], "5");
const score = renderer.guessAndScore()[1];
expect(score.type).toBe("points");
expect(score.earned).toBe(score.total);
// Arrange
const {renderer} = renderQuestion(inputNumber1Item.question);

// Act
renderer.setInputValue(["input-number 1"], "5", () => undefined);

// Assert
expect(renderer).toHaveBeenAnsweredCorrectly();
});

it("should be able to produce a wrong value", function () {
const renderer = renderQuestionArea(inputNumber1Item);
renderer.setInputValue(["input-number 1"], "3", () => {});
const score = renderer.guessAndScore()[1];
expect(score.type).toBe("points");
expect(score.earned).toBe(0);
// Arrange
const {renderer} = renderQuestion(inputNumber1Item.question);

// Act
renderer.setInputValue(["input-number 1"], "3");

// Assert
expect(renderer).toHaveBeenAnsweredIncorrectly();
});

it("should be able to produce an empty score", function () {
const renderer = renderQuestionArea(inputNumber1Item);
// Arrange
const {renderer} = renderQuestion(inputNumber1Item.question);

renderer.setInputValue(["input-number 1"], "3");
let score = renderer.guessAndScore()[1];
expect(score.type).toBe("points");
expect(score.earned).toBe(0);
expect(renderer).toHaveBeenAnsweredIncorrectly();

renderer.setInputValue(["input-number 1"], "");
score = renderer.guessAndScore()[1];
expect(score.type).toBe("invalid");
expect(renderer).toHaveInvalidInput();
});

it("should be able to accept a callback", function (done) {
const renderer = renderQuestionArea(inputNumber1Item);
const {renderer} = renderQuestion(inputNumber1Item.question);
renderer.setInputValue(["input-number 1"], "3", function () {
const guess = renderer.getUserInput()[0];
expect(guess.currentValue).toBe("3");
Expand All @@ -102,21 +84,21 @@ describe("Perseus API", function () {

describe("getInputPaths", function () {
it("should be able to find all the input widgets", function () {
const renderer = renderQuestionArea(inputNumber2Item);
const {renderer} = renderQuestion(inputNumber2Item.question);
const numPaths = renderer.getInputPaths().length;
expect(numPaths).toBe(2);
});

it("should be able to find all inputs within widgets", function () {
const renderer = renderQuestionArea(tableItem);
const {renderer} = renderQuestion(tableItem.question);
const numPaths = renderer.getInputPaths().length;
expect(numPaths).toBe(8);
});
});

describe("getDOMNodeForPath", function () {
it("should find one DOM node per <input>", function () {
const renderer = renderQuestionArea(inputNumber2Item);
const {renderer} = renderQuestion(inputNumber2Item.question);
const inputPaths = renderer.getInputPaths();
const allInputs = TestUtils.scryRenderedDOMComponentsWithTag(
renderer,
Expand All @@ -126,13 +108,14 @@ describe("Perseus API", function () {
});

it("should find the right DOM nodes for the <input>s", function () {
const renderer = renderQuestionArea(inputNumber2Item);
const {renderer} = renderQuestion(inputNumber2Item.question);
const inputPaths = renderer.getInputPaths();
const allInputs = TestUtils.scryRenderedDOMComponentsWithTag(
renderer,
"input",
);
_.each(inputPaths, (inputPath, i) => {
// @ts-expect-error - TS2769 - No overload matches this call.
const $node = $(renderer.getDOMNodeForPath(inputPath));
// @ts-expect-error - TS2769 - No overload matches this call.
const $input = $(ReactDOM.findDOMNode(allInputs[i]));
Expand All @@ -145,13 +128,13 @@ describe("Perseus API", function () {
describe("onInputError", function () {
it("should call a callback when grading an empty input-number", function () {
let wasCalled;
const renderer = renderQuestionArea(inputNumber1Item, {
const {renderer} = renderQuestion(inputNumber1Item.question, {
onInputError: function (widgetId) {
wasCalled = true;
},
});
const score = renderer.guessAndScore()[1];
expect(score.type).toBe("invalid");

expect(renderer).toHaveInvalidInput();
expect(wasCalled).toBe(true);
});
});
Expand All @@ -164,7 +147,7 @@ describe("Perseus API", function () {
// version
expect(ClassNames.FOCUSED).toBe("perseus-focused");

const renderer = renderQuestionArea(inputNumber1Item);
const {renderer} = renderQuestion(inputNumber1Item.question);

const input =
// @ts-expect-error - TS2531 - Object is possibly 'null'. | TS2339 - Property 'querySelector' does not exist on type 'Element | Text'.
Expand All @@ -191,7 +174,7 @@ describe("Perseus API", function () {
let callCount = 0;
let newFocusResult;
let oldFocusResult;
const renderer = renderQuestionArea(inputNumber1Item, {
const {renderer} = renderQuestion(inputNumber1Item.question, {
onFocusChange: function (newFocus, oldFocus) {
callCount++;
newFocusResult = newFocus;
Expand Down Expand Up @@ -235,7 +218,7 @@ describe("Perseus API", function () {
let callCount = 0;
let newFocusResult;
let oldFocusResult;
const renderer = renderQuestionArea(inputNumber2Item, {
const {renderer} = renderQuestion(inputNumber2Item.question, {
onFocusChange: function (newFocus, oldFocus) {
callCount++;
newFocusResult = newFocus;
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export type Widget = {
path: FocusPath,
newValue: string,
// TODO(jeremy): I think this is actually a callback
focus: () => unknown,
focus?: () => unknown,
) => void;
getUserInput?: () => WidgetUserInput | null | undefined;
simpleValidate?: (
Expand Down Expand Up @@ -1701,7 +1701,7 @@ class Renderer extends React.Component<Props, State> {
setInputValue: (
path: FocusPath,
newValue: string,
focus: () => unknown,
focus?: () => unknown,
) => void = (path, newValue, focus) => {
// @ts-expect-error - TS2345 - Argument of type 'FocusPath' is not assignable to parameter of type 'List<any>'.
const widgetId = _.first(path);
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/__tests__/matcher.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {question1} from "../__testdata__/matcher.testdata";
import {renderQuestion} from "./renderQuestion";

import type {APIOptions} from "../../types";
import type {Matcher} from "enzyme-matchers/lib/types/Matcher";
import type {Matcher} from "../matcher";

describe("matcher widget", () => {
beforeEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/matcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type State = {
texRendererLoaded: boolean;
};

class Matcher extends React.Component<any, any> {
export class Matcher extends React.Component<any, any> {
static propTypes = {
apiOptions: ApiOptions.propTypes,
labels: PropTypes.array,
Expand Down
Loading

0 comments on commit 6d31e04

Please sign in to comment.