From 7379ff36492a83162baa8d903a60c314521e9b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 25 Oct 2023 17:52:11 +0100 Subject: [PATCH] [web] Fix tests mocking window.location Beause it's no longer possible to mock some window properties since jsdom 21.0.0 and it was updated to version 22.1.0 in previous commits. See https://github.com/jsdom/jsdom/blob/master/Changelog.md#2100 and https://github.com/jsdom/jsdom/issues/3492 --- web/src/components/core/DBusError.jsx | 3 +- web/src/components/core/DBusError.test.jsx | 14 ++---- web/src/context/l10n.jsx | 8 ++-- web/src/context/l10n.test.jsx | 50 ++++++++++------------ web/src/utils.js | 34 ++++++++++++++- 5 files changed, 66 insertions(+), 43 deletions(-) diff --git a/web/src/components/core/DBusError.jsx b/web/src/components/core/DBusError.jsx index bb4af21273..d2a325562e 100644 --- a/web/src/components/core/DBusError.jsx +++ b/web/src/components/core/DBusError.jsx @@ -21,6 +21,7 @@ import React from "react"; import { Button, EmptyState, EmptyStateIcon, EmptyStateBody, EmptyStateHeader } from "@patternfly/react-core"; +import { locationReload } from "~/utils"; import { _ } from "~/i18n"; import { @@ -35,7 +36,7 @@ const ErrorIcon = () => ; // TODO: an example const ReloadAction = () => ( - diff --git a/web/src/components/core/DBusError.test.jsx b/web/src/components/core/DBusError.test.jsx index 45cf56c271..ff901d9e19 100644 --- a/web/src/components/core/DBusError.test.jsx +++ b/web/src/components/core/DBusError.test.jsx @@ -23,6 +23,7 @@ import React from "react"; import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; +import * as utils from "~/utils"; import { DBusError } from "~/components/core"; @@ -35,20 +36,13 @@ describe("DBusError", () => { }); it("calls location.reload when user clicks on 'Reload'", async () => { + jest.spyOn(utils, "locationReload").mockImplementation(utils.noop); + const { user } = plainRender(, { layout: true }); const reloadButton = await screen.findByRole("button", { name: /Reload/i }); - // Mock location.reload - // https://remarkablemark.org/blog/2021/04/14/jest-mock-window-location-href - const { location } = window; - delete window.location; - window.location = { reload: jest.fn() }; - await user.click(reloadButton); - expect(window.location.reload).toHaveBeenCalled(); - - // restore windows.location - window.location = location; + expect(utils.locationReload).toHaveBeenCalled(); }); }); diff --git a/web/src/context/l10n.jsx b/web/src/context/l10n.jsx index 436c04cd77..d3a725b2fc 100644 --- a/web/src/context/l10n.jsx +++ b/web/src/context/l10n.jsx @@ -22,7 +22,7 @@ // @ts-check import React, { useCallback, useEffect, useState } from "react"; -import { useCancellablePromise } from "~/utils"; +import { useCancellablePromise, locationReload, setLocationSearch } from "~/utils"; import cockpit from "../lib/cockpit"; import { useInstallerClient } from "./installer"; @@ -174,11 +174,11 @@ function reload(newLanguage) { const query = new URLSearchParams(window.location.search); if (query.has("lang") && query.get("lang") !== newLanguage) { query.set("lang", newLanguage); - // Calling search() with a different value makes the browser to navigate + // Setting location search with a different value makes the browser to navigate // to the new URL. - window.location.search = query.toString(); + setLocationSearch(query.toString()); } else { - window.location.reload(); + locationReload(); } } diff --git a/web/src/context/l10n.test.jsx b/web/src/context/l10n.test.jsx index 47e975fbbe..dd932b6102 100644 --- a/web/src/context/l10n.test.jsx +++ b/web/src/context/l10n.test.jsx @@ -27,6 +27,7 @@ import { render, waitFor, screen } from "@testing-library/react"; import { L10nProvider } from "~/context/l10n"; import { InstallerClientProvider } from "./installer"; +import * as utils from "~/utils"; const getUILanguageFn = jest.fn().mockResolvedValue(); const setUILanguageFn = jest.fn().mockResolvedValue(); @@ -72,24 +73,14 @@ const TranslatedContent = () => { }; describe("L10nProvider", () => { - // remember the original object, we need to temporarily replace it with a mock - const origLocation = window.location; - const origNavigator = window.navigator; - - // mock window.location.reload and search beforeAll(() => { - delete window.location; - window.location = { reload: jest.fn(), search: "" }; + jest.spyOn(utils, "locationReload").mockImplementation(utils.noop); + jest.spyOn(utils, "setLocationSearch"); delete window.navigator; window.navigator = { languages: ["es-es", "cs-cz"] }; }); - afterAll(() => { - window.location = origLocation; - window.navigator = origNavigator; - }); - // remove the Cockpit language cookie after each test afterEach(() => { // setting a cookie with already expired date removes it @@ -117,8 +108,7 @@ describe("L10nProvider", () => { // children are displayed await screen.findByText("hello"); - expect(window.location.search).toEqual(""); - expect(window.location.reload).not.toHaveBeenCalled(); + expect(utils.locationReload).not.toHaveBeenCalled(); }); }); @@ -136,16 +126,16 @@ describe("L10nProvider", () => { ); - await waitFor(() => expect(window.location.reload).toHaveBeenCalled()); + await waitFor(() => expect(utils.locationReload).toHaveBeenCalled()); - // reload the component + // renders again after reloading render( ); - await waitFor(() => screen.getByText("hola")); + await waitFor(() => screen.getByText("hola")); expect(setUILanguageFn).toHaveBeenCalledWith("es_ES"); }); }); @@ -164,8 +154,10 @@ describe("L10nProvider", () => { ); - await waitFor(() => expect(window.location.reload).toHaveBeenCalled()); + await waitFor(() => expect(utils.locationReload).toHaveBeenCalled()); + + // renders again after reloading render( @@ -185,8 +177,10 @@ describe("L10nProvider", () => { ); - await waitFor(() => expect(window.location.reload).toHaveBeenCalled()); + await waitFor(() => expect(utils.locationReload).toHaveBeenCalled()); + + // renders again after reloading render( @@ -200,7 +194,7 @@ describe("L10nProvider", () => { describe("when the URL query parameter is set to '?lang=cs-CZ'", () => { beforeEach(() => { - window.location.search = "?lang=cs-CZ"; + history.replaceState(history.state, null, `http://localhost/?lang=cs-CZ`); }); describe("when the Cockpit language is already set to 'cs-cz'", () => { @@ -221,8 +215,8 @@ describe("L10nProvider", () => { expect(setUILanguageFn).not.toHaveBeenCalled(); expect(document.cookie).toEqual("CockpitLang=cs-cz"); - expect(window.location.reload).not.toHaveBeenCalled(); - expect(window.location.search).toEqual("?lang=cs-CZ"); + expect(utils.locationReload).not.toHaveBeenCalled(); + expect(utils.setLocationSearch).not.toHaveBeenCalled(); }); }); @@ -240,16 +234,17 @@ describe("L10nProvider", () => { ); - await waitFor(() => expect(window.location.search).toEqual("lang=cs-cz")); - // reload the component + await waitFor(() => expect(utils.setLocationSearch).toHaveBeenCalledWith("lang=cs-cz")); + + // renders again after reloading render( ); - await waitFor(() => screen.getByText("ahoj")); + await waitFor(() => screen.getByText("ahoj")); expect(setUILanguageFn).toHaveBeenCalledWith("cs_CZ"); }); }); @@ -267,7 +262,8 @@ describe("L10nProvider", () => { ); - await waitFor(() => expect(window.location.search).toEqual("lang=cs-cz")); + + await waitFor(() => expect(utils.setLocationSearch).toHaveBeenCalledWith("lang=cs-cz")); // reload the component render( @@ -275,8 +271,8 @@ describe("L10nProvider", () => { ); - await waitFor(() => screen.getByText("ahoj")); + await waitFor(() => screen.getByText("ahoj")); expect(setUILanguageFn).toHaveBeenCalledWith("cs_CZ"); }); }); diff --git a/web/src/utils.js b/web/src/utils.js index c313b6ae19..3cd85fb228 100644 --- a/web/src/utils.js +++ b/web/src/utils.js @@ -178,6 +178,36 @@ const hex = (value) => { */ const toValidationError = (issue) => ({ message: issue.description }); +/** + * Wrapper around window.location.reload + * @function + * + * It's needed mainly to ease testing because we can't override window in jest with jsdom anymore + * + * See below links + * - https://github.com/jsdom/jsdom/blob/master/Changelog.md#2100 + * - https://github.com/jsdom/jsdom/issues/3492 + */ +const locationReload = () => { + window.location.reload(); +}; + +/** + * Wrapper around window.location.search setter + * @function + * + * It's needed mainly to ease testing as we can't override window in jest with jsdom anymore + * + * See below links + * - https://github.com/jsdom/jsdom/blob/master/Changelog.md#2100 + * - https://github.com/jsdom/jsdom/issues/3492 + * + * @param {string} query + */ +const setLocationSearch = (query) => { + window.location.search = query; +}; + export { noop, partition, @@ -185,5 +215,7 @@ export { useCancellablePromise, useLocalStorage, hex, - toValidationError + toValidationError, + locationReload, + setLocationSearch };