Tests: Improve configuration and mocking strategy#8188
Conversation
|
Kind of unrelated, but can I ask why we need to mock the "Button" component, it's not really clear to me? |
tofumatt
left a comment
There was a problem hiding this comment.
So, I dig it and this is nicer, but those imports are quite something.
I filed #8190 to deal with them on a broader scale, but I'd really like it if the imports in here were absolute. Right now it's really tough to figure out where those files actually are located.
Code looks good though, just minor things to fix and then merge!
| */ | ||
| import MoreMenu from '../index'; | ||
|
|
||
| jest.mock( '../../../../../packages/components/src/button' ); |
There was a problem hiding this comment.
That is a nigh-unusable import path? I know there's discussion around just using absolute paths everywhere (I brought it up in a chat and should really file an issue... so: #8190).
But this should be absolute if possible. It's really unusable to try to figure out what this is importing.
There was a problem hiding this comment.
Agreed, it took me some time to build them properly 😅
In the first place, I want to get rid of those mocks completely with #7005 which will sort of resolve this issue :D
| @@ -0,0 +1,17 @@ | |||
| // [TEMPORARY]: Button uses React.forwardRef, added in react@16.3.0 but not yet | |||
| const apiFetch = jest.fn( () => { | ||
| return apiFetch.mockReturnValue; | ||
| } ); | ||
| apiFetch.mockReturnValue = 'mock this value by overriding apiFetch.mockReturnValue'; |
There was a problem hiding this comment.
Maybe make this a full sentence? 'Mock this value by overriding apiFetch.mockReturnValue.'
There was a problem hiding this comment.
I copied it over - can tweak 👍
| "clean:packages": "rimraf ./packages/*/build ./packages/*/build-module ./packages/*/build-style", | ||
| "prebuild:packages": "npm run clean:packages && lerna run build && cross-env INCLUDE_PACKAGES=babel-plugin-import-jsx-pragma,postcss-themes SKIP_JSX_PRAGMA_TRANSFORM=1 node ./bin/packages/build.js", | ||
| "build:packages": "cross-env EXCLUDE_PACKAGES=babel-plugin-import-jsx-pragma,postcss-themes node ./bin/packages/build.js", | ||
| "prebuild:packages": "npm run clean:packages && lerna run build && cross-env INCLUDE_PACKAGES=babel-plugin-import-jsx-pragma,postcss-themes,jest-console SKIP_JSX_PRAGMA_TRANSFORM=1 node ./bin/packages/build.js", |
There was a problem hiding this comment.
Why are we including these other packages in the prebuild?
There was a problem hiding this comment.
postcss-themesis necessary to build CSS styles for packages, so we need its build version upfrontjest-consoledoesn't strictly need to be there; however, I put it there to be able to runnpm run prebuild:package && npm testand verify that packages can be tested without transpiled code
Description
I encountered some issues with our current test setup when trying to debug failing tests in #8189.
This PR tries to improve our test configuration by:
jest.mockcalls.moduleNameMapperto always use source code rather than build files.wpglobal aliasing in the test setup.__mocks__folder from the build output for all packages.We should tackle #7005 next. It should be easier to identify tests which need to be updated since we can easily identify tests that depend on
Buttonby searching forjest.mockcall.How has this been tested?
npm run prebuild:packages- to ensure there are no build files created for production code.npm test- to make sure all tests pass.Checklist: