[web] Add function for mocking components #392

dgdavid · 2023-01-11T21:25:45Z

Problem

After cutting down wrappers for laying out the UI in #391, some unit tests using mocked components started to complain about Unable to find an element with the text: ....

This happens because we're using getByText Testing Libarry's query with the exact text we're looking for. The text is still there, but it is no longer "alone". I.e., it's not explicitly wrapped by a layout node and thus, the query throws an error.

An example can be found at this moment after rebasing #381 on top of master for solving some conflicts.

FAIL src/components/overview/Overview.test.jsx
  ● renders the Overview and the Install button

    Unable to find an element with the text: Users Configuration. [...]

    ...

              <div
                class="stack content"
              >
                Network Configuration
              </div>
            </section>
            <div>
              Storage Section
            </div>
            Software Section
            ,
            Users Configuration
          </main>

^{See https://github.com/yast/d-installer/actions/runs/3893089545/jobs/6645272239#step:7:312 (as long as the link remains available)}

Solution

The simplest and direct solution is to manually fix failed tests by wrapping mocked content in a div, span, or whatever other HTML element of our taste. At the end, it's a mock. But it's a suboptimal fix.

Instead, the solution proposed here is to use a util function for mocking components. This give us a unified way of producing mocked components and can ensure that their content is somehow isolated from other nodes, making getByText queries happy again.

Testing

npm run test ends successfully.

Notes

We cannot use the function directly in the jest.mock call because its second argument must be an inline function.

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

TypeError: src/App.test.jsx: The second argument of jest.mockmust be an inline function.
A better name for the function is, as always, more than welcome. But it must be start with mock.

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

    ReferenceError: src/App.test.jsx: The module factory of jest.mock() is not allowed to reference any out-of-scope variables.
    Invalid variable access: fakeComponent
    Allowed objects: [...]
    Note: This is a precaution to guard against uninitialized mock variables. If it is ensured that the mock is required lazily, variable names prefixed with mock (case insensitive) are permitted.

coveralls · 2023-01-11T21:28:32Z

Coverage: 76.344% (+0.02%) from 76.325% when pulling b00320a on add-mocking-function into f975706 on master.

dgdavid · 2023-01-11T21:43:18Z

web/src/App.test.jsx

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


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?

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?

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.

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).

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

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.

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.

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.

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.

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

To avoid having the failure > TypeError: Cannot read properties of undefined (reading 'mockComponent') when using the recently introduced mockComonent function for components living in the layout namespace. See #392 (comment)

joseivanlopez

LGTM

Because in some cases the import order matters. To know more, read #392 (comment)

This reverts commit 5ef9c47.

dgdavid added 2 commits January 11, 2023 13:37

[web] Add an util function for mocking components

d26f476

[web] Use mockComponent function

9b8e62c

[web] Update changelog

e222ec3

dgdavid commented Jan 11, 2023

View reviewed changes

dgdavid added 3 commits January 12, 2023 13:14

[web] Fix wrong reference in the changelog

f2af721

[web] Use the default Layout import in test-utils.js

1a42fc6

To avoid having the failure > TypeError: Cannot read properties of undefined (reading 'mockComponent') when using the recently introduced mockComonent function for components living in the layout namespace. See #392 (comment)

[web] Use mockComponent for layout components too

08f711f

joseivanlopez approved these changes Jan 12, 2023

View reviewed changes

dgdavid added 2 commits January 12, 2023 16:22

[web] Split test-utils in two files

5ef9c47

Because in some cases the import order matters. To know more, read #392 (comment)

Revert "[web] Split test-utils in two files"

b00320a

This reverts commit 5ef9c47.

imobachgs approved these changes Jan 12, 2023

View reviewed changes

dgdavid merged commit 1cad583 into master Jan 12, 2023

dgdavid deleted the add-mocking-function branch January 12, 2023 17:21

dgdavid mentioned this pull request Jan 17, 2023

[service][web] Allow downloading logs from web UI #379

Merged

imobachgs mentioned this pull request Feb 13, 2023

Release 0.7 #423

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Add function for mocking components #392

[web] Add function for mocking components #392

dgdavid commented Jan 11, 2023 •

edited

Loading

coveralls commented Jan 11, 2023 •

edited

Loading

dgdavid Jan 11, 2023 •

edited

Loading

dgdavid Jan 11, 2023

imobachgs Jan 12, 2023 •

edited

Loading

imobachgs Jan 12, 2023

dgdavid Jan 12, 2023

imobachgs Jan 12, 2023

dgdavid Jan 12, 2023

dgdavid Jan 12, 2023

dgdavid Jan 12, 2023

dgdavid Jan 12, 2023

joseivanlopez left a comment

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

[web] Add function for mocking components #392

[web] Add function for mocking components #392

Conversation

dgdavid commented Jan 11, 2023 • edited Loading

Problem

Solution

Testing

Notes

coveralls commented Jan 11, 2023 • edited Loading

dgdavid Jan 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imobachgs Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseivanlopez left a comment

Choose a reason for hiding this comment

dgdavid commented Jan 11, 2023 •

edited

Loading

coveralls commented Jan 11, 2023 •

edited

Loading

dgdavid Jan 11, 2023 •

edited

Loading

imobachgs Jan 12, 2023 •

edited

Loading