Skip to content

Improve handling of transpiled packages in unit tests #14432

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.

Sign up for GitHub

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

Merged
gziolo merged 1 commit into from
Mar 14, 2019
Merged

Improve handling of transpiled packages in unit tests #14432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
"test-e2e": "wp-scripts test-e2e --config packages/e2e-tests/jest.config.js",
"test-e2e:watch": "npm run test-e2e -- --watch",
"test-php": "npm run lint-php && npm run test-unit-php",
"test-unit": "wp-scripts test-unit-js --config test/unit/jest.config.json",
"test-unit": "wp-scripts test-unit-js --config test/unit/jest.config.js",
"test-unit:update": "npm run test-unit -- --updateSnapshot",
"test-unit:watch": "npm run test-unit -- --watch",
"test-unit-php": "docker-compose run --rm wordpress_phpunit phpunit",
Expand Down
2 changes: 1 addition & 1 deletion packages/block-serialization-default-parser/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { jsTester, phpTester } from '@wordpress/block-serialization-spec-parser/
/**
* Internal dependencies
*/
import { parse } from '../';
import { parse } from '../src';

describe( 'block-serialization-default-parser-js', jsTester( parse ) );

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/disabled/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Component } from '@wordpress/element';
import Disabled from '../';

jest.mock( '@wordpress/dom', () => {
const focus = require.requireActual( '@wordpress/dom' ).focus;
const focus = require.requireActual( '../../../../dom/src' ).focus;
Copy link
Member

@oandregal oandregal Mar 14, 2019

Choose a reason for hiding this comment

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

Can we add @worpress/dom back to the jest config?

Copy link
Member Author

@gziolo gziolo Mar 14, 2019

Choose a reason for hiding this comment

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

require.requireActual is problematic. If you use as @wordpress/dom, it will load it from node_modules and try to use main from packages.json - it skips module name mapper (edited). The mocking logic has its own life 🙂


return {
focus: {
Expand Down
30 changes: 30 additions & 0 deletions test/unit/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* External dependencies
*/
const glob = require( 'glob' ).sync;

// Finds all packages which are transpiled with Babel to force Jest to use their source code.
const transpiledPackageNames = glob( 'packages/*/src/index.js' )
Copy link
Member

@oandregal oandregal Mar 14, 2019

Choose a reason for hiding this comment

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

This will also pick docgen (which shouldn't be transpiled but at the moment it is).

Copy link
Member Author

@gziolo gziolo Mar 14, 2019

Choose a reason for hiding this comment

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

Yes, this is the same story as with transpliling it :) We should address it separately.

Copy link
Member

@aduth aduth Mar 18, 2019

Choose a reason for hiding this comment

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

I'm wondering if we ought to start codifying / documenting where we have these conventions, i.e. "sources should have a src/index.js file".

Copy link
Member Author

@gziolo gziolo Mar 19, 2019

Choose a reason for hiding this comment

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

I would not only document them but maybe try to go further and centralize the source of those lists. I will open a follow-up issue.

Copy link
Member Author

@gziolo gziolo Mar 19, 2019

Choose a reason for hiding this comment

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

#14508 is up for grabs :)

.map( ( fileName ) => fileName.split( '/' )[ 1 ] );

module.exports = {
rootDir: '../../',
moduleNameMapper: {
[ `@wordpress\\/(${ transpiledPackageNames.join( '|' ) })$` ]: 'packages/$1/src',
},
preset: '@wordpress/jest-preset-default',
setupFiles: [
'core-js/fn/symbol/async-iterator',
'<rootDir>/test/unit/config/gutenberg-phase.js',
],
testURL: 'http://localhost',
testPathIgnorePatterns: [
'/node_modules/',
'/packages/e2e-tests',
'<rootDir>/.*/build/',
'<rootDir>/.*/build-module/',
],
transformIgnorePatterns: [
'node_modules/(?!(simple-html-tokenizer)/)',
],
};
22 changes: 0 additions & 22 deletions test/unit/jest.config.json
View file
Open in desktop

This file was deleted.