Skip to content
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.

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

Tests: Improve configuration and mocking strategy #8188

Merged
merged 4 commits into from
Jul 26, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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 __mocks__/@wordpress/api-fetch/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const apiFetch = jest.fn( () => {
return apiFetch.mockReturnValue;
} );
apiFetch.mockReturnValue = 'mock this value by overriding apiFetch.mockReturnValue';
Copy link
Member

Choose a reason for hiding this comment

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

Maybe make this a full sentence? 'Mock this value by overriding apiFetch.mockReturnValue.'

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied it over - can tweak 👍


export default apiFetch;
5 changes: 4 additions & 1 deletion bin/packages/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ function buildFileFor( file, silent, environment ) {
function buildPackage( packagePath ) {
const srcDir = path.resolve( packagePath, SRC_DIR );
const files = glob.sync( `${ srcDir }/**/*.js`, {
ignore: `${ srcDir }/**/test/**/*.js`,
ignore: [
`${ srcDir }/**/test/**/*.js`,
`${ srcDir }/**/__mocks__/**/*.js`,
],
nodir: true,
} );

Expand Down
2 changes: 2 additions & 0 deletions edit-post/components/header/more-menu/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { mount } from 'enzyme';
*/
import MoreMenu from '../index';

jest.mock( '../../../../../packages/components/src/button' );
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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


describe( 'MoreMenu', () => {
it( 'should match snapshot', () => {
const wrapper = mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { SlotFillProvider } from '@wordpress/components';
*/
import PluginPostPublishPanel from '../';

jest.mock( '../../../../../packages/components/src/button' );

describe( 'PluginPostPublishPanel', () => {
test( 'renders fill properly', () => {
const wrapper = mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { SlotFillProvider } from '@wordpress/components';
*/
import PluginPrePublishPanel from '../';

jest.mock( '../../../../../packages/components/src/button' );

describe( 'PluginPrePublishPanel', () => {
test( 'renders fill properly', () => {
const wrapper = mount(
Expand Down
2 changes: 2 additions & 0 deletions editor/components/post-preview-button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow } from 'enzyme';
*/
import { PostPreviewButton } from '../';

jest.mock( '../../../../packages/components/src/button' );

describe( 'PostPreviewButton', () => {
describe( 'setPreviewWindowLink()', () => {
it( 'should do nothing if there is no preview window', () => {
Expand Down
2 changes: 2 additions & 0 deletions editor/components/post-publish-button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow } from 'enzyme';
*/
import { PostPublishButton } from '../';

jest.mock( '../../../../packages/components/src/button' );

describe( 'PostPublishButton', () => {
describe( 'disabled', () => {
it( 'should be disabled if post is currently saving', () => {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@
"scripts": {
"prebuild": "npm run check-engines",
"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",
Copy link
Member

Choose a reason for hiding this comment

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

Why are we including these other packages in the prebuild?

Copy link
Member Author

@gziolo gziolo Aug 1, 2018

Choose a reason for hiding this comment

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

  • postcss-themes is necessary to build CSS styles for packages, so we need its build version upfront
  • jest-console doesn't strictly need to be there; however, I put it there to be able to run npm run prebuild:package && npm test and verify that packages can be tested without transpiled code

"build:packages": "cross-env EXCLUDE_PACKAGES=babel-plugin-import-jsx-pragma,jest-console,postcss-themes node ./bin/packages/build.js",
"build": "npm run build:packages && cross-env NODE_ENV=production webpack",
"check-engines": "check-node-version --package",
"ci": "concurrently \"npm run lint\" \"npm run test-unit:coverage-ci\"",
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/autocomplete/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { ENTER, ESCAPE, UP, DOWN, SPACE } from '@wordpress/keycodes';
*/
import EnhancedAutocomplete, { Autocomplete } from '../';

jest.useFakeTimers();
jest.mock( '../../button' );

class FakeEditor extends Component {
// we want to change the editor contents manually so don't let react update it
Expand Down
17 changes: 17 additions & 0 deletions packages/components/src/button/__mocks__/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// [TEMPORARY]: Button uses React.forwardRef, added in [email protected] but not yet
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the docs! 👍

// supported by Enzyme as of [email protected] . This mock unwraps
// the ref forwarding, so any tests relying on this behavior will fail.
//
// See: https://github.com/airbnb/enzyme/issues/1604
// See: https://github.com/airbnb/enzyme/pull/1592/files

const { Component } = require( 'react' );
const { Button: RawButton } = require.requireActual( '../index' );

class Button extends Component {
render() {
return RawButton( this.props );
}
}

export default Button;
3 changes: 0 additions & 3 deletions packages/components/src/button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ import { createRef } from '@wordpress/element';
*/
import ButtonWithForwardedRef, { Button } from '../';

// [TEMPORARY]: Only needed so long as Enzyme does not support React.forwardRef
jest.unmock( '../' );

describe( 'Button', () => {
describe( 'basic rendering', () => {
it( 'should render a button element with only one class', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/color-palette/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow } from 'enzyme';
*/
import ColorPalette from '../';

jest.mock( '../../button' );

describe( 'ColorPalette', () => {
const colors = [ { name: 'red', color: '#f00' }, { name: 'white', color: '#fff' }, { name: 'blue', color: '#00f' } ];
const currentColor = '#f00';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import request, {
isRequestMethod,
} from '../request';

jest.mock( '@wordpress/api-fetch' );

describe( 'request', () => {
const actualResponse = {
json: () => Promise.resolve( {} ),
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/icon-button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow } from 'enzyme';
*/
import IconButton from '../';

jest.mock( '../../button' );

describe( 'IconButton', () => {
describe( 'basic rendering', () => {
it( 'should render an top level element with only a class property', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/menu-item/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow } from 'enzyme';
*/
import MenuItem from '../';

jest.mock( '../../button' );

describe( 'MenuItem', () => {
test( 'should match snapshot when only label provided', () => {
const wrapper = shallow(
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/panel/test/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { shallow, mount } from 'enzyme';
*/
import PanelBody from '../body';

jest.mock( '../../button' );

describe( 'PanelBody', () => {
describe( 'basic rendering', () => {
it( 'should render an empty div with the matching className', () => {
Expand Down

This file was deleted.

2 changes: 2 additions & 0 deletions packages/core-data/src/test/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import apiFetch from '@wordpress/api-fetch';
import { getMethodName, defaultEntities, getKindEntities } from '../entities';
import { addEntities } from '../actions';

jest.mock( '@wordpress/api-fetch' );

describe( 'getMethodName', () => {
it( 'Should return the right method name for an entity with the root kind', () => {
const methodName = getMethodName( 'root', 'postType' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { basename } = require( 'path' );
* Internal dependencies
*/

const CustomTemplatedPathPlugin = require( '../../' );
const CustomTemplatedPathPlugin = require( '@wordpress/custom-templated-path-webpack-plugin' );

module.exports = {
mode: 'development',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
const LibraryExportDefaultPlugin = require( '../../' );
const LibraryExportDefaultPlugin = require( '@wordpress/library-export-default-webpack-plugin' );

module.exports = {
mode: 'development',
Expand Down
2 changes: 2 additions & 0 deletions packages/nux/src/components/dot-tip/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { noop } from 'lodash';
*/
import { DotTip } from '..';

jest.mock( '../../../../../components/src/button' );

describe( 'DotTip', () => {
it( 'should not render anything if invisible', () => {
const wrapper = shallow(
Expand Down
9 changes: 4 additions & 5 deletions test/unit/jest.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
"<rootDir>/packages/.*/benchmark/"
],
"moduleNameMapper": {
"@wordpress\\/(components|editor|utils|edit-post|core-blocks)$": "$1",
"@wordpress\\/(a-z0-9-)$": "packages/$1/src"
"@wordpress\\/(components|editor|edit-post|core-blocks)$": "$1",
"@wordpress\\/(block-serialization-spec-parser|is-shallow-equal)$": "packages/$1",
"@wordpress\\/([a-z0-9-]+)$": "packages/$1/src"
},
"preset": "@wordpress/jest-preset-default",
"setupFiles": [
"core-js/fn/symbol/async-iterator",
"<rootDir>/test/unit/setup-wp-aliases.js",
"<rootDir>/test/unit/setup-mocks.js"
"core-js/fn/symbol/async-iterator"
],
"testPathIgnorePatterns": [
"/node_modules/",
Expand Down
25 changes: 0 additions & 25 deletions test/unit/setup-mocks.js

This file was deleted.

14 changes: 0 additions & 14 deletions test/unit/setup-wp-aliases.js

This file was deleted.