Tests: Setup Jest as an alternative test runner #1382

gziolo · 2017-06-22T17:36:38Z

This is a proof of concept for Jest integration with Gutenberg.

Wondering why Jest? See: https://gziolo.pl/2017/06/17/picking-jest-over-mocha/.

Sidenotes: It doesn't need Webpack processing. It still uses Chai and Sinon to make it possible to run with both Mocha and Jest and perform comparisons on one branch. I had to disable one test suite, because I didn't find a quick way to make it pass. It can be done later if we ever decide we replace Mocha with Jest.

Regular test run

npm run test-unit

With slow tests:

RUN_SLOW_TESTS=1 npm run test-unit

Only one test file (element/test/index.js):

npm run test-unit element/test/index.js

Code coverage

npm run test-unit:coverage

Watch mode

npm run test-unit:watch

By default it executes test for modified files only. You can change it using UI for subsequent runs.

IDE integration

Performance

Jest is configured to omit Webpack processing step. It makes it faster even when executed with all caches disabled. Jest shines on subsequent runs when it's able to take advantage of its caching mechanism.

Mocha

Before: test npm run test-unit

334 passing (295ms)
96 pending

real 0m16.125s
user 0m16.797s
sys 0m0.895s

Jest without cache

test npm run test-unit -- --no-cache

Test Suites: 2 skipped, 28 passed, 28 of 30 total
Tests: 96 skipped, 334 passed, 430 total
Snapshots: 0 total
Time: 9.363s
Ran all test suites.

real 0m13.121s
user 1m11.006s
sys 0m3.656s

Jest

test npm run test-unit

Test Suites: 2 skipped, 28 passed, 28 of 30 total
Tests: 96 skipped, 334 passed, 430 total
Snapshots: 0 total
Time: 3.484s
Ran all test suites.

real 0m5.961s
user 0m30.956s
sys 0m2.355s

TODO

Fix build/test/full-content.js test.
Replace Chai and Sinon with Jest matchers and spies.
Enable Eslint rule jest/valid-expect.
Remove Mocha?
Update testing docs

gziolo · 2017-06-23T07:57:05Z

blocks/test/full-content.js

@@ -86,7 +86,7 @@ function normalizeParsedBlocks( blocks ) {
 	} );
 }

-describe( 'full post content fixture', () => {
+describe.skip( 'full post content fixture', () => {


I still need to find a way to make this test suite pass.

@gziolo This suite can probably be rewritten to use Jest snapshots. What is the current issue with it?

All of them fail, but I didn't even try to fix them. I just wanted to validate if Gutenberg can work with Jest without any changes in test files.

I started my parental leave this week so I might not have time to debug it in the upcoming days or weeks, but I'm happy to help with reviews using my mobile :)

gziolo · 2017-06-23T07:57:53Z

components/form-token-field/test/index.js

-			// We need a high timeout here to accomodate Travis CI
-			this.timeout( 10000 );
+			// We need a high timeout for Mocha here to accomodate Travis CI
+			if ( this.timeout ) {


Jest doesn't provide such method.

gziolo · 2017-06-23T07:58:26Z

components/form-token-field/test/index.js

-	if ( ! process.env.RUN_SLOW_TESTS ) {
-		return;
-	}
+const maybeDescribe = process.env.RUN_SLOW_TESTS ? describe : describe.skip;


You can't have test suite without any tests so it complains without skipping.

gziolo · 2017-06-23T07:59:07Z

package.json

+      "**/utils/**/*.js"
+    ],
+    "coveragePathIgnorePatterns": [
+      "<rootDir>/components/clipboard-button/index.js",


Coverage errors on those 3 files. I didn't try to understand why.

It fails because nyc or istanbul doesn't like param destructuring in function definition that uses a default value ... Example:

function Notice( { status, content, onRemove = noop } ) { ... }

I think it needs to be fixed somewhere else. I can investigate it further later.

gziolo · 2017-06-23T08:00:00Z

test/pgejs-transform.js

+
+module.exports = {
+	process( src ) {
+		// Description of PEG.js options: https://github.com/pegjs/pegjs#javascript-api


I followed pegjs-loader from Webpack and it looks like it works properly :)

gziolo · 2017-06-23T08:01:06Z

test/setup-test-framework.js

@@ -0,0 +1,21 @@
+require( 'core-js/modules/es7.object.values' );


This is tricky part. I had to add it explicitly here. babel-jest should do it instead, but apparently there is something happening that breaks it.

This should probably be a code comment.

Yes, will add in another PR.

gziolo · 2017-06-23T08:01:35Z

test/setup-test-framework.js

+global.before = global.beforeAll;
+global.context = global.describe;
+
+global.wp = {


We need it to support wp.element.* syntax. I should probably add comment in code, too :)

gziolo · 2017-06-23T08:26:37Z

components/form-token-field/test/index.js

@@ -438,12 +438,6 @@ describe( 'FormTokenField', function() {
 			expect( textInputNode.prop( 'value' ) ).to.equal( ' quux' );
 		} );

-		it( 'should skip empty tokens at the beginning of a paste', function() {


It is copy and paste from the test above.

youknowriad · 2017-06-23T11:32:06Z

Copying my comments for our slack discussion here:

Before introducing a new testing framework to Core, we need to consider the different options and Jest is a great test runner (maybe the best right now) 🙂

One of the issues we currently have with our mocha setup is that we build the tests using webpack (pegs-loader, sass-loader, wp global variables are all handled by webpack) before running them, this has some costs. We can’t run a single test without building the entire suite which is not very performant.

This PR solves this issue 🎉 but the solution has some downsides: we need to “recreate” (or find a hack) the webpack loaders one by one. Also, this makes our test code and built code slightly different.

I personally think the niceties that come with Jest surpass the downsides: Performance, coverage, snapshots.

It would be great to have others' opinion.

ntwb · 2017-07-02T03:59:22Z

Found this an interesting read, results were not what I expected:

https://medium.com/dailyjs/javascript-test-runners-benchmark-3a78d4117b4

p.s. I do like watch, cache, snapshots features of Jest FWIW

gziolo · 2017-07-02T06:55:12Z

Found this an interesting read, results were not what I expected

I saw that the other day, too. This benchmark is very interesting but I noticed that Jest is executed with default configuration, which means it needs to setup a fresh jsdom instance for every test suite. You can pick node test env instead which should drastically speed up things and will align with what Mocha and Jasmine offer out of the box. Jest's default setup is optimized for code that is run in the browser.

I asked post's author to add Jest setup that uses node as test env. I'm looking forward to see how it would impact results.

gziolo · 2017-07-03T14:22:50Z

@ntwb I got confirmation form benchmark's author that Jest is 30% faster with test env set to node. See https://medium.com/@vitaliypotapov/thanks-for-suggestion-jest-is-really-30-faster-with-env-node-option-the-chart-4309ba8aba81. It still slower in this particular case though.

gziolo · 2017-07-06T06:07:25Z

Fixed build/test/full-content.js test which was the only blocker. It turned out I missed to include blocks import in the setup phase.

If we are fine with using Jest I'm more than happy to remove Mocha and update all references in documentation.

ntwb · 2017-07-06T06:35:07Z

.gitignore

 gutenberg.pot
+.idea


IDE specific entries in .gitignore should not be in the repo, users should manage these locally in ~/.gitignore_global

See also https://core.trac.wordpress.org/ticket/34345

Thanks for tip, this was bothering me since forever :)

ntwb · 2017-07-06T06:38:10Z

package.json

@@ -59,12 +59,14 @@
    "enzyme": "^2.8.2",
    "eslint": "^3.17.1",
    "eslint-config-wordpress": "^1.1.0",
+    "eslint-plugin-jest": "20.0.3",


This should include a ~ for updating via minor semantic version releases.

ntwb · 2017-07-06T06:38:15Z

package.json

    "eslint-plugin-jsx-a11y": "^4.0.0",
    "eslint-plugin-react": "^6.10.3",
    "expose-loader": "^0.7.3",
    "extract-text-webpack-plugin": "^2.1.0",
    "gettext-parser": "^1.2.2",
    "glob": "^7.1.1",
+    "jest": "20.0.4",


This should include a ~ for updating via minor semantic version releases.

I see ^ is used in other places, shouldn't it be used here, too?

I think we should use ^ to include all non-blocking updates, I guess it's also the default when using npm install --save

It should be ~, linters and for the most part rules will inherit the same semantic behaviour from the linter, in this case ESLint and by proxy it's shared plugins and shared configs:

Via ESLint Semantic Version Policy:
"According to our policy, any minor update may report more errors than the previous release (ex: from a bug fix). As such, we recommend using the tilde (~) in package.json e.g. "eslint": "~3.1.0" to guarantee the results of your builds."

I don't follow:

How this relates to Jest specifically

Why this means we can't use ^

I think this is best left for a separate PR.

youknowriad · 2017-07-06T07:29:34Z

Let's remove mocha in this PR, we don't want to maintain two runners IMO.

…ases

gziolo · 2017-07-06T15:31:34Z

I removed Mocha and updated most of its references.

There are two cleanup tasks left which can be done in the follow up PR if you agree:

Replace Chai and Sinon with Jest matchers and spies.
Enable Eslint rule jest/valid-expect and remove mocha Eslint env.

nylen · 2017-07-06T22:22:36Z

.eslintrc.json

-		"mocha": true
-	},
+		"mocha": true,
+		"jest/globals": true


This doesn't include before and after, which means we still need mocha: true? I'm a bit surprised by that.

I know. I will fix it with jest-codemods soon.

Done with #1788.

nylen · 2017-07-06T22:27:30Z

Not having to wait for webpack builds to run the tests is a huge improvement. Let's try it.

gziolo · 2017-07-07T17:13:56Z

Full API migration from Mocha to Jest is ready to review #1788.

Work around for link selection.

gziolo self-assigned this Jun 23, 2017

gziolo commented Jun 23, 2017

View reviewed changes

gziolo force-pushed the try/jest branch from d9e3cd5 to cb91afa Compare June 23, 2017 08:31

gziolo requested review from nylen, youknowriad, aduth, mtias, jasmussen and ellatrix June 23, 2017 11:02

gziolo mentioned this pull request Jul 3, 2017

Test: Setup unit testing using Jest WordPress/packages#9

Merged

gziolo force-pushed the try/jest branch 2 times, most recently from 0c3fde7 to e118d4d Compare July 6, 2017 06:00

gziolo force-pushed the try/jest branch from e118d4d to df73d90 Compare July 6, 2017 06:20

ntwb requested changes Jul 6, 2017

View reviewed changes

gziolo force-pushed the try/jest branch from ab411ff to 89ed92b Compare July 6, 2017 11:38

gziolo added 6 commits July 6, 2017 13:42

Tests: Setup Jest as an alternative test runner

b5db30f

Tests: Temporarily skip failing full content block test

5a3cbdd

Tests: Add coverega setup and watch mode for Jest runner

32941f0

Tests: Refactor Jest configuration

08a0a2e

Tests: Fix Eslint errors

67bf781

Tests: Update newly added tests to pass with Jest

e3e7e54

gziolo force-pushed the try/jest branch from 89ed92b to 46478d0 Compare July 6, 2017 14:59

gziolo added 3 commits July 6, 2017 17:28

Tests: Improve performance by lazy loading content of entry point ali…

0250c89

…ases

Tests: Ignore notice component in code coverage

7fc46cd

Tests: Remove Mocha test runner

7375464

gziolo force-pushed the try/jest branch from 46478d0 to 7375464 Compare July 6, 2017 15:29

nylen mentioned this pull request Jul 6, 2017

Fix warnings during tests and prevent them from reoccurring #1769

Merged

nylen reviewed Jul 6, 2017

View reviewed changes

nylen merged commit 1406fe8 into master Jul 6, 2017

nylen deleted the try/jest branch July 6, 2017 22:27

This was referenced Jul 6, 2017

Fix pgejs typo in tests #1771

Merged

Track and report test coverage #1783

Merged

gziolo mentioned this pull request Jul 7, 2017

Tests: Refactor tests to use only Jest API #1788

Merged

nylen mentioned this pull request Jul 8, 2017

Only run 2 tasks at once during Travis JS build #1808

Merged

aduth mentioned this pull request Sep 20, 2017

Testing: Evaluate Jest alternatives #2757

Closed

youknowriad pushed a commit that referenced this pull request Jan 17, 2020

Merge pull request #1382 from wordpress-mobile/hotfix/1.11.1

72cbc1e

Work around for link selection.

		@@ -0,0 +1,21 @@
		require( 'core-js/modules/es7.object.values' );

		gutenberg.pot
		.idea

Tests: Setup Jest as an alternative test runner #1382

Tests: Setup Jest as an alternative test runner #1382

Conversation

gziolo commented Jun 22, 2017 • edited Loading

Regular test run

Code coverage

Watch mode

IDE integration

Performance

Mocha

Jest without cache

Jest

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo Jul 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Jun 23, 2017

ntwb commented Jul 2, 2017

gziolo commented Jul 2, 2017 • edited Loading

gziolo commented Jul 3, 2017

gziolo commented Jul 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Jul 6, 2017

gziolo commented Jul 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nylen commented Jul 6, 2017

gziolo commented Jul 7, 2017

gziolo commented Jun 22, 2017 •

edited

Loading

gziolo Jul 6, 2017 •

edited

Loading

gziolo Jun 23, 2017 •

edited

Loading

gziolo commented Jul 2, 2017 •

edited

Loading