From b1cd42ffc258b9fda8db76009876128cd3b7c2bb Mon Sep 17 00:00:00 2001 From: James Parslow Date: Tue, 15 Oct 2024 15:32:02 +0100 Subject: [PATCH 1/2] fix: downgrade @tanstack/react-virtual to version 3.10.1 Fixes a runtime error in `SimpleSelect` jest tests where @tanstack/react-virtual attempts to update state after the component has unmounted. --- package-lock.json | 18 ++++++++++-------- package.json | 2 +- .../select-list/select-list.component.tsx | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 63b1057083..dee6239113 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,7 @@ "@floating-ui/react-dom": "~1.3.0", "@octokit/rest": "^18.12.0", "@styled-system/prop-types": "^5.1.5", - "@tanstack/react-virtual": "^3.6.0", + "@tanstack/react-virtual": "3.10.1", "@types/styled-system": "^5.1.22", "chalk": "^4.1.2", "ci-info": "^3.8.0", @@ -6928,11 +6928,12 @@ } }, "node_modules/@tanstack/react-virtual": { - "version": "3.10.8", - "resolved": "https://registry.npmjs.org/@tanstack/react-virtual/-/react-virtual-3.10.8.tgz", - "integrity": "sha512-VbzbVGSsZlQktyLrP5nxE+vE1ZR+U0NFAWPbJLoG2+DKPwd2D7dVICTVIIaYlJqX1ZCEnYDbaOpmMwbsyhBoIA==", + "version": "3.10.1", + "resolved": "https://registry.npmjs.org/@tanstack/react-virtual/-/react-virtual-3.10.1.tgz", + "integrity": "sha512-h5kNeE+yQwspjl9E3sJ3UYQu/MuspNOBT5cVdc+NA0uU9B1XSkxbzp86teV3arMDVcQ4ESExqs4JyIirYAMcuA==", + "license": "MIT", "dependencies": { - "@tanstack/virtual-core": "3.10.8" + "@tanstack/virtual-core": "3.10.1" }, "funding": { "type": "github", @@ -6944,9 +6945,10 @@ } }, "node_modules/@tanstack/virtual-core": { - "version": "3.10.8", - "resolved": "https://registry.npmjs.org/@tanstack/virtual-core/-/virtual-core-3.10.8.tgz", - "integrity": "sha512-PBu00mtt95jbKFi6Llk9aik8bnR3tR/oQP1o3TSi+iG//+Q2RTIzCEgKkHG8BB86kxMNW6O8wku+Lmi+QFR6jA==", + "version": "3.10.1", + "resolved": "https://registry.npmjs.org/@tanstack/virtual-core/-/virtual-core-3.10.1.tgz", + "integrity": "sha512-JDi3wU1HIxuxx8BgD7Ix8IXlelCKdTJIh9c0qBs+QXHdix3mjMbkXI3wOq0TuCx1w1RGgzZue34QrM/NPdp/sw==", + "license": "MIT", "funding": { "type": "github", "url": "https://github.com/sponsors/tannerlinsley" diff --git a/package.json b/package.json index 9191c5410c..a6c49e13c6 100644 --- a/package.json +++ b/package.json @@ -185,7 +185,7 @@ "@floating-ui/react-dom": "~1.3.0", "@octokit/rest": "^18.12.0", "@styled-system/prop-types": "^5.1.5", - "@tanstack/react-virtual": "^3.10.8", + "@tanstack/react-virtual": "3.10.1", "@types/styled-system": "^5.1.22", "chalk": "^4.1.2", "ci-info": "^3.8.0", diff --git a/src/components/select/__internal__/select-list/select-list.component.tsx b/src/components/select/__internal__/select-list/select-list.component.tsx index 03bb666cc0..710e125197 100644 --- a/src/components/select/__internal__/select-list/select-list.component.tsx +++ b/src/components/select/__internal__/select-list/select-list.component.tsx @@ -232,7 +232,7 @@ const SelectList = React.forwardRef( if (currentIndex > -1) { // only index property is required with the item not visible so the following type assertion, even though incorrect, // should be OK - items.push({ index: currentIndex } as VirtualItem); + items.push({ index: currentIndex } as VirtualItem); } } From f5f90f2657bff916dacd704735e6789950cd27d0 Mon Sep 17 00:00:00 2001 From: James Parslow Date: Tue, 15 Oct 2024 15:36:29 +0100 Subject: [PATCH 2/2] chore: ensure jest fails if a runtime error is thrown Use jest-fail-on-console to ensure tests fail if runtime errors are thrown, helping prevent issues from slipping into production. Fix jest tests which are throwing runtime errors. --- package-lock.json | 8 ++ package.json | 1 + .../generate_metadata/generate_metadata.mjs | 7 +- .../generate_metadata.spec.js | 118 ------------------ .../generate_metadata.test.js | 102 +++++++++++++++ src/__spec_helper__/__internal__/index.ts | 7 ++ .../__internal__/select-test-utils.ts | 2 +- .../numeral-date/numeral-date.test.tsx | 7 +- .../simple-select/simple-select.test.tsx | 2 + 9 files changed, 130 insertions(+), 124 deletions(-) delete mode 100644 scripts/generate_metadata/generate_metadata.spec.js create mode 100644 scripts/generate_metadata/generate_metadata.test.js diff --git a/package-lock.json b/package-lock.json index dee6239113..790e76006e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -132,6 +132,7 @@ "jest": "^29.5.0", "jest-canvas-mock": "^2.5.2", "jest-environment-jsdom": "^29.5.0", + "jest-fail-on-console": "^3.3.1", "jest-fetch-mock": "^3.0.3", "jest-styled-components": "^6.3.4", "jsdom": "^21.1.0", @@ -16126,6 +16127,13 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/jest-fail-on-console": { + "version": "3.3.1", + "resolved": "https://registry.npmjs.org/jest-fail-on-console/-/jest-fail-on-console-3.3.1.tgz", + "integrity": "sha512-dmq/dmh5OBgJlD1MJdpznzwFQP8S7msf3ghTGWQLGhagWwHKzGtqXza76nuJUKOK7BdwqcTK6CCE49Xxv4ckUQ==", + "dev": true, + "license": "MIT" + }, "node_modules/jest-fetch-mock": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/jest-fetch-mock/-/jest-fetch-mock-3.0.3.tgz", diff --git a/package.json b/package.json index a6c49e13c6..4361267277 100644 --- a/package.json +++ b/package.json @@ -153,6 +153,7 @@ "jest": "^29.5.0", "jest-canvas-mock": "^2.5.2", "jest-environment-jsdom": "^29.5.0", + "jest-fail-on-console": "^3.3.1", "jest-fetch-mock": "^3.0.3", "jest-styled-components": "^6.3.4", "jsdom": "^21.1.0", diff --git a/scripts/generate_metadata/generate_metadata.mjs b/scripts/generate_metadata/generate_metadata.mjs index afd66c07a2..bba9a07e4b 100644 --- a/scripts/generate_metadata/generate_metadata.mjs +++ b/scripts/generate_metadata/generate_metadata.mjs @@ -1,4 +1,3 @@ -/* eslint-disable no-console */ import fs from "fs"; import fetch from "node-fetch"; import semver from "semver"; @@ -47,7 +46,7 @@ export const writeFile = (jsonString) => { if (err) { throw err; } else { - console.log("Successfully created metadata.json file."); + global.console.log("Successfully created metadata.json file."); } }); }; @@ -58,8 +57,8 @@ export const generateMetadata = async () => { try { versions = await fetchVersions(); } catch (err) { - console.error(err); - process.exit(1); + global.console.error(err); + return; } const formattedVersions = formatVersions(versions); diff --git a/scripts/generate_metadata/generate_metadata.spec.js b/scripts/generate_metadata/generate_metadata.spec.js deleted file mode 100644 index 00d21c3d32..0000000000 --- a/scripts/generate_metadata/generate_metadata.spec.js +++ /dev/null @@ -1,118 +0,0 @@ -import { generateMetadata, writeFile } from "./generate_metadata"; - -const fs = require("fs"); - -jest.mock("fs"); - -const mockNpmVersions = { - versions: { - "98.0.0": {}, - "99.0.0": {}, - "100.1.1": {}, - "100.2.1": {}, - "101.0.0": {}, - "102.0.0": {}, - }, -}; - -const mockMetadata = { - versions: { - "v102.0.0": "https://carbon.sage.com/v/102.0.0/index.html", - "v101.0.0": "https://carbon.sage.com/v/101.0.0/index.html", - "v100.2.1": "https://carbon.sage.com/v/100.2.1/index.html", - }, -}; - -describe("generateMetadata script", () => { - let consoleErrorMock; - - beforeAll(() => { - jest.spyOn(global.console, "log").mockImplementation(() => {}); - consoleErrorMock = jest - .spyOn(global.console, "error") - .mockImplementation(() => {}); - }); - - beforeEach(() => { - fetch.resetMocks(); - fetch.mockResponse(JSON.stringify(mockNpmVersions)); - fs.mkdirSync = jest.fn((path, options, callback) => { - callback(); - }); - fs.writeFileSync = jest.fn((path, json, callback) => { - callback(); - }); - }); - - afterAll(() => { - global.console.log.mockReset(); - global.console.error.mockReset(); - }); - - it("should create a metadata.json file in a metadata directory", async () => { - await generateMetadata(); - - expect(fs.mkdirSync).toHaveBeenCalledWith( - "metadata", - {}, - expect.any(Function) - ); - - expect(fs.writeFileSync).toHaveBeenCalledWith( - "metadata/metadata.json", - JSON.stringify(mockMetadata), - expect.any(Function) - ); - }); - - describe("when there is an error creating the metadata directory", () => { - it("should throw an error", () => { - fs.mkdirSync = jest.fn((path, options, callback) => { - callback(new Error("An error occurred.")); - }); - - expect(() => { - writeFile(); - }).toThrowError("An error occurred."); - }); - }); - - describe("when there is an error creating the metadata.json file", () => { - it("should throw an error", () => { - fs.writeFileSync = jest.fn((path, json, callback) => { - callback(new Error("An error occurred.")); - }); - - expect(() => { - writeFile(); - }).toThrowError("An error occurred."); - }); - }); - - describe("when there is an error fetching the carbon data from npm", () => { - const mockExit = jest - .spyOn(process, "exit") - .mockImplementation((number) => { - throw new Error(`process.exit: ${number}`); - }); - - afterAll(() => { - mockExit.mockRestore(); - }); - - it("should throw an error with exit code 1", async () => { - fetch.mockResponse(JSON.stringify(mockNpmVersions), { - status: 500, - ok: false, - }); - - await expect(async () => { - await generateMetadata(); - }).rejects.toThrowError("process.exit: 1"); - expect(consoleErrorMock).toHaveBeenCalledWith( - new Error("Failed to fetch from npm with HTTP error code 500") - ); - expect(mockExit).toHaveBeenCalledWith(1); - }); - }); -}); diff --git a/scripts/generate_metadata/generate_metadata.test.js b/scripts/generate_metadata/generate_metadata.test.js new file mode 100644 index 0000000000..55570fc5b6 --- /dev/null +++ b/scripts/generate_metadata/generate_metadata.test.js @@ -0,0 +1,102 @@ +import fs from "fs"; +import fetch from "jest-fetch-mock"; +import { generateMetadata, writeFile } from "./generate_metadata"; + +jest.mock("fs"); +const mockedFs = jest.mocked(fs); + +const mockNpmVersions = { + versions: { + "98.0.0": {}, + "99.0.0": {}, + "100.1.1": {}, + "100.2.1": {}, + "101.0.0": {}, + "102.0.0": {}, + }, +}; + +const mockMetadata = { + versions: { + "v102.0.0": "https://carbon.sage.com/v/102.0.0/index.html", + "v101.0.0": "https://carbon.sage.com/v/101.0.0/index.html", + "v100.2.1": "https://carbon.sage.com/v/100.2.1/index.html", + }, +}; + +beforeEach(() => { + jest.spyOn(global.console, "log").mockImplementation(() => {}); + jest.spyOn(global.console, "error").mockImplementation(() => {}); + + fetch.mockResponse(JSON.stringify(mockNpmVersions)); + + mockedFs.mkdirSync = jest.fn((path, options, callback) => { + callback(); + }); + mockedFs.writeFileSync = jest.fn((path, json, callback) => { + callback(); + }); +}); + +afterEach(() => { + jest.resetAllMocks(); + fetch.resetMocks(); +}); + +test("creates a metadata.json file in a metadata directory", async () => { + await generateMetadata(); + + expect(mockedFs.mkdirSync).toHaveBeenCalledWith( + "metadata", + {}, + expect.any(Function) + ); + + expect(mockedFs.writeFileSync).toHaveBeenCalledWith( + "metadata/metadata.json", + JSON.stringify(mockMetadata), + expect.any(Function) + ); +}); + +test("throws an error, when unable to create the metadata directory", () => { + mockedFs.mkdirSync = jest.fn((path, options, callback) => { + callback(new Error("An error occurred.")); + }); + + expect(() => writeFile()).toThrow("An error occurred."); +}); + +test("throws an error, when unable to create the metadata.json file", () => { + mockedFs.writeFileSync = jest.fn((path, json, callback) => { + callback(new Error("An error occurred.")); + }); + + expect(() => writeFile()).toThrow("An error occurred."); +}); + +describe("when unable to fetch carbon data from npm", () => { + it("logs error with http status code", async () => { + fetch.mockResponse(JSON.stringify(mockNpmVersions), { + status: 500, + ok: false, + }); + + await generateMetadata(); + + expect(global.console.error).toHaveBeenCalledWith( + new Error("Failed to fetch from npm with HTTP error code 500") + ); + }); + + it("does not attempt to write metadata.json file", async () => { + fetch.mockResponse(JSON.stringify(mockNpmVersions), { + status: 500, + ok: false, + }); + + await generateMetadata(); + + expect(mockedFs.writeFileSync).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__spec_helper__/__internal__/index.ts b/src/__spec_helper__/__internal__/index.ts index 03a08d45f1..8d58d4f4a2 100644 --- a/src/__spec_helper__/__internal__/index.ts +++ b/src/__spec_helper__/__internal__/index.ts @@ -1,10 +1,17 @@ import { configure } from "@testing-library/react"; import { enableFetchMocks } from "jest-fetch-mock"; +import failOnConsole from "jest-fail-on-console"; + import { setupMatchMediaMock } from "../mock-match-media"; import setupResizeObserverMock from "../mock-resize-observer"; import setupScrollToMock from "../mock-element-scrollto"; + import "@testing-library/jest-dom"; +failOnConsole({ + shouldFailOnError: true, + shouldFailOnWarn: false, +}); enableFetchMocks(); setupResizeObserverMock(); setupMatchMediaMock(); diff --git a/src/__spec_helper__/__internal__/select-test-utils.ts b/src/__spec_helper__/__internal__/select-test-utils.ts index cabeabe834..541fda2a85 100644 --- a/src/__spec_helper__/__internal__/select-test-utils.ts +++ b/src/__spec_helper__/__internal__/select-test-utils.ts @@ -25,7 +25,7 @@ export function simulateSelectTextboxEvent( }); resizeObserver.resize(); }); - if (eventType === "focus") jest.runOnlyPendingTimers(); + if (eventType === "focus") act(() => jest.runOnlyPendingTimers()); container.update(); } diff --git a/src/components/numeral-date/numeral-date.test.tsx b/src/components/numeral-date/numeral-date.test.tsx index cf07febb23..93fef7e476 100644 --- a/src/components/numeral-date/numeral-date.test.tsx +++ b/src/components/numeral-date/numeral-date.test.tsx @@ -1563,7 +1563,12 @@ test("should submit the form when enter key is pressed", async () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const onSubmit = jest.fn(); render( -
+ { + ev.preventDefault(); + onSubmit(ev); + }} + > {}} diff --git a/src/components/select/simple-select/simple-select.test.tsx b/src/components/select/simple-select/simple-select.test.tsx index 790ac95ea9..e21bc6dcb7 100644 --- a/src/components/select/simple-select/simple-select.test.tsx +++ b/src/components/select/simple-select/simple-select.test.tsx @@ -327,6 +327,7 @@ describe("typing into the input", () => { expect(screen.getByText("green", { ignore: "li" })).toBeVisible(); + jest.runOnlyPendingTimers(); jest.useRealTimers(); }); @@ -859,6 +860,7 @@ test("does not call onOpen, when openOnFocus is true and the input is refocused expect(onOpen).not.toHaveBeenCalled(); + jest.runOnlyPendingTimers(); jest.useRealTimers(); });