[web] Add function for mocking components#392
Conversation
web/src/App.test.jsx
Outdated
| jest.mock("@components/layout/DBusError", () => () => "D-BusError Mock"); | ||
| jest.mock("@components/layout/LoadingEnvironment", () => () => "LoadingEnvironment Mock"); |
There was a problem hiding this comment.
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 runTypeError: 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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.mockcalls and put/move them ant the very top- Component
DBusErroris now() => mockComponent("DBusError Mock")
- Component
-
A
test-utilsimport 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
DBusErrormock. -
But
mockComponenthas 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
Because in some cases the import order matters. To know more, read #392 (comment)
This reverts commit 5ef9c47.
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.
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 testends successfully.Notes
We cannot use the function directly in the
jest.mockcall because its second argument must be an inline function.A better name for the function is, as always, more than welcome. But it must be start with
mock.