WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Issues mocking @wordpress dependencies in unit tests when using wp-scripts #42442

Open chrisvanpatten opened 2 years ago

chrisvanpatten commented 2 years ago

Description

While using wp-scripts to run unit tests of a block, I've encountered an issue with mocking @wordpress/data dependencies. For instance I would expect the following to allow me to mock useSelect:

jest.mock("@wordpress/data", () => {
  const module = jest.requireActual("@wordpress/data");

  return {
    ...module,
    useSelect: jest.fn().mockReturnValue({}),
  };
});

However, running the test returns the following error:

 FAIL  src/index.test.js
  ● Test suite failed to run

    TypeError: Cannot read property 'AsyncModeProvider' of undefined

       6 |   const module = jest.requireActual("@wordpress/data");
       7 |
    >  8 |   return {
         |   ^
       9 |     ...module,
      10 |     useSelect: jest.fn().mockReturnValue({}),
      11 |   };

      at Object.get [as AsyncModeProvider] (node_modules/@wordpress/data/build/index.js:11:31)
      at src/index.test.js:8:3

The configuration in my test case is the standard Jest config from wp-scripts with the addition of @testing-library/react as the renderer, and @testing-library/jest-dom for some additional expectations. However, the issue can be reproduced even without any of the testing-library packages installed (and in fact can be reproduced without any tests in the suite, as the mocking happens outside of the tests).

It appears to be some kind of issue with the way the @wordpress/data package is built for distribution, but it's tough to say for sure.

Step-by-step reproduction instructions

I've created a limited test case here: https://github.com/chrisvanpatten/wp-scripts-unit-test-case

$ git clone git@github.com:chrisvanpatten/wp-scripts-unit-test-case.git
$ cd wp-scripts-unit-test-case
$ nvm use
$ npm install
$ npm run test

Screenshots, screen recording, code snippet

Screen Shot 2022-07-14 at 2 43 39 PM

Environment info

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

talldan commented 2 years ago

@chrisvanpatten I've seen this approach used in the Gutenberg codebase: https://github.com/WordPress/gutenberg/blob/0fa1141d41685835175aa5b8341b34378d83430e/packages/block-directory/src/components/downloadable-blocks-list/test/index.js#L17-L21

Does that help at all?

It may not if you're using the built code in your test 🤔

chrisvanpatten commented 2 years ago

@talldan Yeah unfortunately that only works when working with the source; there's a special module mapper in Gutenberg's own test config that maps the module path to the internal packages. When working with the built/distributed code from npm, it won't work. https://github.com/WordPress/gutenberg/blob/trunk/test/unit/jest.config.js#L13-L16

chrisvanpatten commented 2 years ago

(See also #14168)

chrisvanpatten commented 2 years ago

Help me @gziolo you're my only hope 😅

gziolo commented 2 years ago

The following works:

jest.mock("@wordpress/data", () => {
  return {
    useSelect: jest.fn().mockReturnValue({}),
  };
});

Out of curiosity, why do you need to expose all other APIs from the mock?

gziolo commented 2 years ago

The issue is that for some reasons the mock is evaluated twice when using jest.requireActual:

Screenshot 2022-07-20 at 13 41 20

When I test with:

jest.mock("@wordpress/data", () => {
  const api = jest.requireActual("@wordpress/data");

  console.log( 'mock' );

  return {
    useSelect: jest.fn().mockReturnValue({}),
  };
});

Some paths look surprising in the repository included to reproduce the issue (with all my changes removed):

Screenshot 2022-07-20 at 13 43 33

Maybe this workaround from Gutenberg would be useful for the Jest preset used in the project before Jest 28 is out:

https://github.com/WordPress/gutenberg/blob/5d2490845464e83cb535e43b7e39cb7320998ca0/test/unit/jest.config.js#L36-L38

https://github.com/WordPress/gutenberg/blob/5d2490845464e83cb535e43b7e39cb7320998ca0/test/unit/scripts/babel-transformer.js#L7-L8

chrisvanpatten commented 2 years ago

@gziolo Removing the requireActual worked perfectly! Applying the Babel transformer from the Gutenberg repo did not work — I was still getting the same error as before about "Cannot read property 'AsyncModeProvider' of undefined" but I'll re-review after Jest 28 is available and integrated into wp-scripts.

Thank you!

chrisvanpatten commented 2 years ago

@gziolo unfortunately I remembered why I wanted to use requireActual

I've updated the test suite to show an example of a component which imports from @wordpress/components. When you don't requireActual so the rest of the package is available, you start getting errors about other missed dependencies in the component tree, and then end up having to mock each and every one…

 FAIL  src/index.test.js
  ● Test suite failed to run

    TypeError: (0 , _data.combineReducers) is not a function

      1 | import { useSelect } from "@wordpress/data";
    > 2 | import { BaseControl } from "@wordpress/components";
        | ^
      3 |
      4 | function MyComponent(props) {
      5 |   const { prefix } = props;

      at Object.<anonymous> (node_modules/@wordpress/rich-text/build/store/@wordpress/rich-text/src/store/reducer.js:40:16)

The hope would be to avoid having to mock every nested dependency, although maybe there's a better way to handle this?

chrisvanpatten commented 2 years ago

This StackOverflow answer about using a Proxy seems relevant: https://stackoverflow.com/a/70174052

chrisvanpatten commented 2 years ago

Also pushed an example using a very simple example with Jest 28 and a custom config — fully removing wp-scripts — and the error persists, which I think really indicates it's an issue with how the packages are bundled. https://github.com/chrisvanpatten/wp-scripts-unit-test-case/pull/2

gziolo commented 2 years ago

My bet is that we are dealing with the challenges of Dual CommonJS/ES module packages:

When an application is using a package that provides both CommonJS and ES module sources, there is a risk of certain bugs if both versions of the package get loaded.

chrisvanpatten commented 2 years ago

@gziolo curious what you think of this solution, using an overloaded defineProperty to overwrite the getters/setters: https://github.com/chrisvanpatten/wp-scripts-unit-test-case/pull/3/files

gziolo commented 2 years ago

@chrisvanpatten, I hope that Jest will resolve it by adding full support for ESM 😃 I guess everything that works is good enough in the meantime.

chihsuan commented 1 day ago

👋 I found a workaround for the issue. I think the issue is that @wordpress/data has circular imports in its codebase, and somehow, Jest.requireActual doesn't work with it.

To fix it, we can import @wordpress/data in test setup file to load it first before we mock it so when Jest.requireActual is called, AsyncModeProvider is available. Just need to make sure the @wordpress/data used in the test is the same one loaded in test setup file. Hope this helps!