Skip to content

[web] Add function for mocking components #392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dgdavid merged 8 commits into from
Jan 12, 2023
Merged

[web] Add function for mocking components #392

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d26f476
[web] Add an util function for mocking components
dgdavid Jan 11, 2023
9b8e62c
[web] Use mockComponent function
dgdavid Jan 11, 2023
e222ec3
[web] Update changelog
dgdavid Jan 11, 2023
f2af721
[web] Fix wrong reference in the changelog
dgdavid Jan 12, 2023
1a42fc6
[web] Use the default Layout import in test-utils.js
dgdavid Jan 12, 2023
08f711f
[web] Use mockComponent for layout components too
dgdavid Jan 12, 2023
5ef9c47
[web] Split test-utils in two files
dgdavid Jan 12, 2023
b00320a
Revert "[web] Split test-utils in two files"
dgdavid Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions web/package/cockpit-d-installer.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Wed Jan 11 20:37:26 UTC 2023 - David Diaz <dgonzalez@suse.com>

- Testing: use a mocking function to make mocked components
consistent across the test suite (gh#yast/d-installer#392).

-------------------------------------------------------------------
Wed Jan 11 11:54:29 UTC 2023 - David Diaz <dgonzalez@suse.com>

Expand Down
22 changes: 11 additions & 11 deletions web/src/App.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,26 @@

import React from "react";
import { act, screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { createClient } from "@client";
import { installerRender, mockComponent } from "@/test-utils";
import App from "./App";
import { createClient } from "@client";
import { STARTUP, CONFIG, INSTALL } from "@client/phase";
import { IDLE, BUSY } from "@client/status";

jest.mock("@client");

jest.mock('react-router-dom', () => ({
Outlet: mockComponent("Content"),
}));

// Mock some components,
// See https://www.chakshunyu.com/blog/how-to-mock-a-react-component-in-jest/#default-export

jest.mock("@components/questions/Questions", () => () => <div>Questions Mock</div>);
jest.mock("@components/layout/DBusError", () => () => <div>D-BusError Mock</div>);
jest.mock("@components/layout/DBusError", () => () => "D-BusError Mock");
jest.mock("@components/layout/LoadingEnvironment", () => () => "LoadingEnvironment Mock");

@dgdavid dgdavid Jan 11, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to understand why I couldn't use the mockComponent function for components living in the @components/layout scope. When try to do so, Jest complains with

FAIL src/App.test.jsx
  ● Test suite failed to run

   TypeError: Cannot read properties of undefined (reading 'mockComponent')

     > 38 | jest.mock("@components/layout/DBusError", () => mockComponent("D-BusError Mock"));

If those components are moved to, let's say, @components/core and imported and mocked from there, it works.

If, instead, we keep them in @components/layout BUT we remove the import { Layout } from "@components/layout"; from src/test-utils.js and stop using it there, it works too.

Obviously, we cannot stop wrapping content into <Layout> for all tests just because of this. Just trying to find the explanation for that complaint.

@imobachgs, Do you have any hint about what is going on here?

@dgdavid dgdavid Jan 11, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I found a solution. However, I still not understanding why the error described above ¯\_(ツ)_/¯

diff --git a/web/src/test-utils.js b/web/src/test-utils.js
index 6d8ce2d85..3fc8579c5 100644
--- a/web/src/test-utils.js
+++ b/web/src/test-utils.js
@@ -25,7 +25,7 @@ import { render } from "@testing-library/react";
 
 import { createClient } from "@client/index";
 import { InstallerClientProvider } from "@context/installer";
-import { Layout } from "@components/layout";
+import Layout from "@components/layout/Layout";

What do you think?

@imobachgs imobachgs Jan 12, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is related to the fact that Jest executes the test-utils.js first, so you are accidentally already loading the DBusError (and any other layout-related component). I do not know how Jest mocking works internally, but the proposed patch looks good.

@imobachgs imobachgs Jan 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that, as a rule of thumb, we should avoid loading any application component from the test-utils.js (we can make an exception just with the layout).

@dgdavid dgdavid Jan 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you are accidentally already loading the DBusError

Which makes no sense to me. It's importing only (apparently) the named Layout export... maybe something is happening when re-exporting components from layout/index.js... Don't know

but the propose patch looks good.

Ok, I'll send it

@imobachgs imobachgs Jan 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the root of the issue is that you are importing something that it is already mocked, and that mock calls to a function which is not defined yet.

@dgdavid dgdavid Jan 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right! In my understanding, all imports in src/components/layout/index.js get processed when importing { Layout } from "@components/layout" (??). However, due to the Jest hoisting, the DBusError component at this point is already mocked and tries to execute the jest.mock inline function. Unfortunately, the mockComponent function is not available: it has not been imported yet.

So, something like...

  • $ npm run tests src/App.test.jsx

    • Jest found these jest.mock calls and put/move them ant the very top

      • Component DBusError is now () => mockComponent("DBusError Mock")

    • A test-utils import is found and processed

    • The import { Layout } from "@components/layout"; is found and processed

    • The src/components/layout/index.js contains an implicit import to DBusError

    • Jest executes the DBusError mock.

    • But mockComponent has not been defined nor imported yet.

      TypeError: Cannot read properties of undefined (reading 'mockComponent')

🤯 🤯 🤯

Another solution proposed by @joseivanlopez is to move the mockComponent function to another file and import the function BEFORE importing the current test-utils. Fun fact: I made that test yesterday, but forgot about the order and imported it AFTER. 🤦‍♂️

It worked as expected. I'm not sure when to do it: as part of this PR (i.e., create a test-util directory and split current utils into at least two files) or later when we have more utils. In any case, I think we should stick to the default Layout import.

@dgdavid dgdavid Jan 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI,

Searching more information about the topic, I have found that interesting article https://www.coolcomputerclub.com/posts/jest-hoist-await/ It's almost two years old. Maybe mentioned top-levels awaits are nearest to be a reality in the Jest world nowadays.

@dgdavid dgdavid Jan 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another solution proposed by @joseivanlopez is to move the mockComponent function to another file and import the function BEFORE importing the current test-utils

Done at 5ef9c47. We can revert the commit if it looks too much.

@dgdavid dgdavid Jan 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally, after discussing it online we agreed on reverting 5ef9c47.

17:05:03 <imobach> jilopez, dgdavid: basically, as a user, I find rather tricky that you have to import mocks/ and renderers/ in that specific order

17:06:21 <imobach> jilopez, dgdavid: I consider not loading stuff that we do not need in the test-utils.js as a better alternative

jest.mock("@components/core/InstallationProgress", () => () => "InstallationProgress Mock");
jest.mock("@components/core/InstallationFinished", () => () => "InstallationFinished Mock");
jest.mock("@components/network/TargetIpsPopup", () => () => "Target IPs Mock");
jest.mock('react-router-dom', () => ({
Outlet: () => <div>Content</div>,
}));
jest.mock("@components/questions/Questions", () => mockComponent("Questions Mock"));
jest.mock("@components/core/InstallationProgress", () => mockComponent("InstallationProgress Mock"));
jest.mock("@components/core/InstallationFinished", () => mockComponent("InstallationFinished Mock"));
jest.mock("@components/network/TargetIpsPopup", () => mockComponent("Target IPs Mock"));

const callbacks = {};
const getStatusFn = jest.fn();
Expand Down
6 changes: 3 additions & 3 deletions web/src/Main.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@

import React from "react";
import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";

import Main from "@/Main";

jest.mock("@components/questions/Questions", () => () => <div>Questions Mock</div>);
jest.mock("@components/questions/Questions", () => mockComponent("Questions Mock"));
jest.mock('react-router-dom', () => ({
Outlet: () => <div>Content</div>,
Outlet: mockComponent("Content"),
}));

it("renders the Questions component and the content", async () => {
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/core/InstallationProgress.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import React from "react";

import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";

import InstallationProgress from "./InstallationProgress";

jest.mock("@components/core/ProgressReport", () => () => "ProgressReport Mock");
jest.mock("@components/core/ProgressReport", () => mockComponent("ProgressReport Mock"));

describe("InstallationProgress", () => {
it("uses 'Installing' as title", async () => {
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/layout/DBusError.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import React from "react";

import { screen } from "@testing-library/react";
import { plainRender } from "@/test-utils";
import { plainRender, mockComponent } from "@/test-utils";

import { DBusError } from "@components/layout";

jest.mock("@components/network/TargetIpsPopup", () => () => "IP Mock");
jest.mock("@components/network/TargetIpsPopup", () => mockComponent("IP Mock"));

describe("DBusError", () => {
it("includes a generic D-Bus connection problem message", () => {
Expand Down
6 changes: 3 additions & 3 deletions web/src/components/network/Network.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@

import React from "react";
import { screen, within, waitFor } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import Network from "@components/network/Network";
import { ConnectionTypes } from "@client/network";
import { createClient } from "@client";

jest.mock("@client");
jest.mock("@components/network/NetworkWiredStatus", () => () => <div>Wired Connections</div>);
jest.mock("@components/network/NetworkWifiStatus", () => () => <div>WiFi Connections</div>);
jest.mock("@components/network/NetworkWiredStatus", () => mockComponent("Wired Connections"));
jest.mock("@components/network/NetworkWifiStatus", () => mockComponent("WiFi Connections"));

const networkSettings = { wifiScanSupported: false, hostname: "test" };
let settingsFn = jest.fn().mockReturnValue(networkSettings);
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/network/WifiHiddenNetworkForm.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import React from "react";

import { screen } from "@testing-library/react";
import { plainRender } from "@/test-utils";
import { plainRender, mockComponent } from "@/test-utils";

import { WifiHiddenNetworkForm } from "@components/network";

jest.mock("@components/network/WifiConnectionForm", () => () => "WifiConnectionForm mock");
jest.mock("@components/network/WifiConnectionForm", () => mockComponent("WifiConnectionForm mock"));

describe("WifiHiddenNetworkForm", () => {
describe("when it is visible", () => {
Expand Down
6 changes: 3 additions & 3 deletions web/src/components/network/WifiNetworkListItem.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
import React from "react";

import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";

import { WifiNetworkListItem } from "@components/network";

jest.mock("@components/network/WifiConnectionForm", () => () => "WifiConnectionForm mock");
jest.mock("@components/network/WifiNetworkMenu", () => () => "WifiNetworkMenu mock");
jest.mock("@components/network/WifiConnectionForm", () => mockComponent("WifiConnectionForm mock"));
jest.mock("@components/network/WifiNetworkMenu", () => mockComponent("WifiNetworkMenu mock"));

const onSelectCallback = jest.fn();
const fakeNetwork = {
Expand Down
12 changes: 6 additions & 6 deletions web/src/components/overview/Overview.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import React from "react";
import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import Overview from "./Overview";
import { createClient } from "@client";

Expand Down Expand Up @@ -50,11 +50,11 @@ jest.mock('react-router-dom', () => ({
useNavigate: () => jest.fn()
}));

jest.mock("@components/language/LanguageSelector", () => () => "Language Selector");
jest.mock("@components/overview/StorageSection", () => () => <div>Storage Section</div>);
jest.mock("@components/network/Network", () => () => "Network Configuration");
jest.mock("@components/users/Users", () => () => "Users Configuration");
jest.mock("@components/core/InstallButton", () => () => "Install Button");
jest.mock("@components/language/LanguageSelector", () => mockComponent("Language Selector"));
jest.mock("@components/overview/StorageSection", () => mockComponent("Storage Section"));
jest.mock("@components/network/Network", () => mockComponent("Network Configuration"));
jest.mock("@components/users/Users", () => mockComponent("Users Configuration"));
jest.mock("@components/core/InstallButton", () => mockComponent("Install Button"));

beforeEach(() => {
mockProduct = { id: "openSUSE", name: "openSUSE Tumbleweed" };
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/overview/StorageSection.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@

import React from "react";
import { act, screen, waitFor } from "@testing-library/react";
import { installerRender, createCallbackMock } from "@/test-utils";
import { installerRender, createCallbackMock, mockComponent } from "@/test-utils";
import { createClient } from "@client";
import { BUSY, IDLE } from "@client/status";
import { StorageSection } from "@components/overview";

const mockUseNavigate = jest.fn();
jest.mock("@client");
jest.mock("@components/core/InstallerSkeleton", () => () => "Loading storage");
jest.mock("@components/core/InstallerSkeleton", () => mockComponent("Loading storage"));
jest.mock("react-router-dom", () => ({
...jest.requireActual("react-router-dom"),
useNavigate: () => mockUseNavigate
Expand Down
6 changes: 3 additions & 3 deletions web/src/components/questions/Questions.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
import React from "react";

import { act, screen, waitFor } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import { createClient } from "@client";

import { Questions } from "@components/questions";

jest.mock("@client");
jest.mock("@components/questions/GenericQuestion", () => () => "A Generic question mock");
jest.mock("@components/questions/LuksActivationQuestion", () => () => "A LUKS activation question mock");
jest.mock("@components/questions/GenericQuestion", () => mockComponent("A Generic question mock"));
jest.mock("@components/questions/LuksActivationQuestion", () => mockComponent("A LUKS activation question mock"));

const handlers = {};
const genericQuestion = { id: 1, type: 'generic' };
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/software/ProductSelectionPage.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import React from "react";
import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import { ProductSelectionPage } from "@components/software";
import { createClient } from "@client";

Expand All @@ -38,7 +38,7 @@ const products = [
}
];
jest.mock("@client");
jest.mock("@components/network/TargetIpsPopup", () => () => "Target IPs Mock");
jest.mock("@components/network/TargetIpsPopup", () => mockComponent("Target IPs Mock"));

const mockUseNavigate = jest.fn();
jest.mock("react-router-dom", () => ({
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/storage/ProposalActionsSection.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

import React from "react";
import { screen } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import { ProposalActionsSection } from "@components/storage";

jest.mock("@components/storage/ProposalActions", () => () => "ProposalActions content");
jest.mock("@components/storage/ProposalActions", () => mockComponent("ProposalActions content"));

const proposal = {};

Expand Down
8 changes: 4 additions & 4 deletions web/src/components/storage/ProposalPage.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import React from "react";
import { screen, waitForElementToBeRemoved } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import { createClient } from "@client";
import { ProposalPage } from "@components/storage";

Expand All @@ -40,11 +40,11 @@ jest.mock("react-router-dom", () => ({
useNavigate: () => jest.fn()
}));

jest.mock("@components/core/InstallerSkeleton", () => () => "Loading proposal");
jest.mock("@components/core/InstallerSkeleton", () => mockComponent("Loading proposal"));
jest.mock("@components/storage/ProposalTargetSection", () => FakeProposalTargetSection);

jest.mock("@components/storage/ProposalSettingsSection", () => () => <div>Settings section</div>);
jest.mock("@components/storage/ProposalActionsSection", () => () => <div>Actions section</div>);
jest.mock("@components/storage/ProposalSettingsSection", () => mockComponent("Settings section"));
jest.mock("@components/storage/ProposalActionsSection", () => mockComponent("Actions section"));

let proposal;

Expand Down
4 changes: 2 additions & 2 deletions web/src/components/storage/ProposalTargetSection.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import React from "react";
import { screen, waitFor, within } from "@testing-library/react";
import { installerRender } from "@/test-utils";
import { installerRender, mockComponent } from "@/test-utils";
import { ProposalTargetSection } from "@components/storage";

const FakeProposalTargetForm = ({ id, onSubmit }) => {
Expand All @@ -33,7 +33,7 @@ const FakeProposalTargetForm = ({ id, onSubmit }) => {
return <form id={id} onSubmit={accept} aria-label="Target form" />;
};

jest.mock("@components/storage/ProposalSummary", () => () => "Proposal summary");
jest.mock("@components/storage/ProposalSummary", () => mockComponent("Proposal summary"));
jest.mock("@components/storage/ProposalTargetForm", () => FakeProposalTargetForm);

const proposal = {
Expand Down
16 changes: 15 additions & 1 deletion web/src/test-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,18 @@ const createCallbackMock = () => {
return [on, callbacks];
};

export { installerRender, plainRender, createCallbackMock };
/**
* Returns fake component with given content
*
* @param {React.ReactNode} content - content for the fake component
* @param {object} [options] - Options for building the fake component
* @param {string} [options.wrapper="div"] - the HTML element to be used for wrapping given content
*
* @return a function component
*/
const mockComponent = (content, { wrapper } = { wrapper: "div" }) => {
const Wrapper = wrapper;
return () => <Wrapper>{content}</Wrapper>;
};

export { installerRender, plainRender, createCallbackMock, mockComponent };