Migrate tests to Jest #5359

montogeek · 2017-02-22T14:29:23Z

Q	A
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Deprecations?	No
Spec Compliancy?	Yes
Tests Added/Pass?	Yes
Fixed Tickets	Fixes #5197
License	MIT
Doc PR	No
Dependency Changes	Yes

Migrated tests infrastructure from Mocha to Jest 19.
All tests pass locally:

Test Suites: 86 passed, 86 total
Tests:       7 skipped, 1720 passed, 1727 total
Snapshots:   0 total
Time:        92.977s
Ran all test suites.

Previously a timeout of 10 seconds was set globally for all tests, I moved it locally to just babel-cli tests: https://github.com/babel/babel/compare/7.0...montogeek:feat/migrate-test-to-jest?expand=1#diff-ab3a365851f93959d9be46529b9b1500R147

Migrated babel-helper-transform-fixture-test-runner to use Jest as well since it is used as a test utility.

lerna version was reverted to 2.0.0-beta.23 to make relative requires work, not sure what it is necessary to make it work with latest beta release.

There is more work to do, delete all Mocha related code, update Makefile, etc. I wanted to submit it to see I am doing things right

mention-bot · 2017-02-22T14:29:27Z

@montogeek, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ChauTNguyen, @motiz88 and @hzoo to be potential reviewers.

oleksandr-kuzmenko · 2017-02-22T14:45:21Z

I think if we want to add yarn.lock we all should to start using yarn. This is a good idea, as for me. But then we have to edit Makefile and CONTRIBUTING.md. I think this is better done through a separate PR. I could do this.

montogeek · 2017-02-22T14:53:14Z

@Alxpy In 7.0 branch, Yarn is used:
https://github.com/babel/babel/blob/7.0/CONTRIBUTING.md, search for Yarn
https://github.com/babel/babel/blob/7.0/yarn.lock
https://github.com/babel/babel/blob/7.0/Makefile#L71

oleksandr-kuzmenko · 2017-02-22T14:56:23Z

@montogeek I have not seen that this is PR in 7.0 =)
maybe it's worth to rename PR? [7.0] Migrate tests to Jest

babel-bot · 2017-02-22T16:47:02Z

Hey @montogeek! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

Jessidhia · 2017-02-23T05:30:28Z

package.json

    "gulp-util": "^3.0.7",
    "gulp-watch": "^4.3.5",
-    "lerna": "2.0.0-beta.37",
+    "lerna": "2.0.0-beta.23",


Why the rollback?

From the PR description:

lerna version was reverted to 2.0.0-beta.23 to make relative requires work, not sure what it is necessary to make it work with latest beta release.

Does this mean that it started failing at beta.24?

@hzoo Didn't try every beta, will try later what version make it fails

@hzoo It definitely breaks with v2.0.0-beta.28, in v2.0.0-beta.27 works

Jessidhia · 2017-02-23T05:35:32Z

packages/babel-types/test/converters.js

        t.toStatement(node);
-      });
-      assert.strictEqual(t.toStatement(node, /* ignore = */ true), false);
+      }).toThrow();


Would be nice to specify what should be thrown (jest can take in a constructor (for instanceof check), string or regexp), but that definitely should be done in a later PR.

Jessidhia · 2017-02-23T05:37:54Z

packages/babel-helper-transform-fixture-test-runner/src/index.js

 testContext.global = testContext;

+const jest = {};
+jest.expect = global.expect;


Not adding expect to the testContext?

It would also be really nice to get describe/it working inside the exec fixtures, but that would need a significant rework of how they are written.

Is this assignment necessary here? Why wouldn't you just use expect instead of jest.expect?
Also you can remove chai from imports.

@thymikee Because expect is used in the code: https://github.com/babel/babel/pull/5359/files/e6ed1494d94cae8094b890ebe93cbe46f5ec4718#diff-37b0c62f94b613936e9195f14a3320aeR123

I can't remove chai because is still used, but maybe we can't remove the vm stuff, I'm investigating it

aaronang · 2017-02-23T06:57:40Z

.eslintrc

    "node": true,
-    "mocha": true
+    "jest": true,
+    "jasmine": true


Why do we need Jasmine? Isn't this: https://facebook.github.io/jest/docs/setup-teardown.html#scoping the reason why you would actually want Jasmine?

I think we can remove this, I added is because this reason:

Previously a timeout of 10 seconds was set globally for all tests, I moved it locally to just babel-cli tests: https://github.com/babel/babel/compare/7.0...montogeek:feat/migrate-test-to-jest?expand=1#diff-ab3a365851f93959d9be46529b9b1500R147

But I made that config global and forgot to change it, thanks for pointing it out!

babel-bot · 2017-02-23T13:22:34Z

Hey @montogeek! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

aaronang · 2017-02-23T17:16:19Z

test/testSetupFile.js

@@ -0,0 +1 @@
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 70000;


Is there a reason why this is set to 70000? If I understand it correctly, previously in Mocha it was set to 10000.

To try to make run tests on CI

On my Mac, tests run within 10000

montogeek · 2017-02-23T18:23:23Z

I do not have a lot of experience with Bash, but if somebody does and have a better way to handle this: e989503

Please do

aaronang

By any chance, do you maybe an idea why the tests have to be run sequentially instead of parallel on Travis?

aaronang · 2017-02-23T19:25:18Z

.travis.yml

 script:
-  - 'if [ "$JOB" = "test" ]; then make test-ci; fi'
-  - 'if [ "$JOB" = "test-coverage" ]; then make test-ci-coverage; fi'
+  - 'if [ "$JOB" = "test" ]; then CI=true make test-ci; fi'


You don't need to pass CI as a variable because it is set by Travis by default: https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables.

Oh! Thanks, they have to run sequentially because Travis machine resources are limited, by default Jest spawn child processes that consume more resources

@aaronang

By any chance, do you maybe an idea why the tests have to be run sequentially instead of parallel on Travis?

It's because most CI services don't handle parallel test runs well. You can read more about this in Jest's doc on Troubleshooting CI

thymikee · 2017-02-23T21:44:33Z

packages/babel-traverse/test/scope.js

    });

-    test("label", function () {
+    it("label", function () {


Jest supports either test or it so this is unnecessary (unless you like it better 👍)

thymikee · 2017-02-23T21:48:19Z

scripts/test-cov.sh

+runSequentially=""
+
+if [ "$CI" ]; then
+  runSequentially="--i"


Not sure why this works, but technically it should be either --runInBand or -i.

You are right. If I may, in addition, I would suggest using --runInBand because it might be more self-explanatory than the alias -i.

thymikee · 2017-02-23T21:48:35Z

scripts/test.sh


-$node node_modules/mocha/bin/_mocha `scripts/_get-test-directories.sh` --opts test/mocha.opts --grep "$TEST_GREP"
+if [ "$CI" ]; then
+  runSequentially="--i"


skovhus · 2017-02-24T06:21:41Z

Great work.

Out of curiosity, did you use Jeat Codemods? https://github.com/skovhus/jest-codemods

The codemod should be able to handle most of this. And if not, then let me know. : )

loganfsmyth · 2017-03-15T01:07:13Z

Ouch, ran straight into jestjs/jest#1652 when trying to debug this :(

loganfsmyth · 2017-03-15T01:08:17Z

Followed immediately by node-inspector/node-inspector#905 :(

babel-bot · 2017-03-15T02:23:26Z

Hey @montogeek! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

babel-bot · 2017-03-15T04:00:49Z

Hey @loganfsmyth! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

loganfsmyth · 2017-03-15T04:04:15Z

I pushed a new commit to try something, but it looks like an unrelated issue caused some failures.

babel-bot · 2017-03-15T04:25:40Z

Hey @loganfsmyth! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

loganfsmyth · 2017-03-15T04:26:54Z

@montogeek I did my best, I have no idea why it's running out of memory now. It passes in parallel but I think running with -i it fails.

thymikee · 2017-03-15T08:07:45Z

@loganfsmyth can you try using Jest from master? We had an issue with babel-polyfill (which jest was auto requiring if found) causing memory leaks, and forgot to merge the fix into v19. It will be available in v19.1 which should happen this week

montogeek · 2017-03-15T14:17:33Z

@thymikee How can I install Jest from master? Since it is a Lerna repo, I can't just do yarn add -D https://github.com/facebook/jest#master

thymikee · 2017-03-15T14:55:23Z

Setting this directly in package.json doesn't work?

"jest": "facebook/jest"

montogeek · 2017-03-15T15:14:13Z

It does, I ran yarn on node_modules/jest, the postinstall hook runs without problem, then I ran ./node_modules/jest/jest but it throws an error saying that cannot find jest-util module

thymikee · 2017-03-15T15:33:27Z

Let's wait to 19.1 then

cpojer · 2017-03-17T00:57:16Z

jest@next was tagged.

loganfsmyth · 2017-03-17T05:58:40Z

I have to admit I'm very concerned with this PR because of the debugging issues in #5359 (comment). I use --inspect for all of my debugging these days, and falling back to node-inspector failed miserably for me.

cpojer · 2017-03-17T06:09:12Z

There is a third party module jest-environment-node-debug which should work with --inspect until node/v8 finally implement --inspect for contexts. Currently they are blocked on the deprecation of the old debugger :(

cpojer · 2017-05-11T11:50:18Z

It seems like everybody is on board with this change; Jest 20 just came out with a ton of new features. @montogeek, are you planning on bringing this over the finish line? :)

montogeek · 2017-05-11T12:29:41Z

@cpojer I do, will take a look today!

xtuc · 2017-09-11T07:32:41Z

Can we close this in favor of #6179? It includes @montogeek work.

Migrate tests to Jest

c6ae9cb

montogeek added 2 commits February 22, 2017 09:36

Update ESLint configuration

b2bb5cb

Update script/test.sh to Jest

69b3dca

montogeek added 3 commits February 22, 2017 09:59

Update scripts/test-cov.sh

cdbbc6a

Increase and make jasmine.DEFAULT_TIMEOUT_INTERVAL global

fb5626b

Update Jest textRegex option

e6ed149

existentialism added PR: Internal 🏠 A type of pull request used for our changelog categories WIP labels Feb 23, 2017

Jessidhia reviewed Feb 23, 2017

View reviewed changes

aaronang reviewed Feb 23, 2017

View reviewed changes

Remove ESLint jasmine env option

cb88551

aaronang reviewed Feb 23, 2017

View reviewed changes

montogeek added 3 commits February 23, 2017 12:28

Run tests serially

879e0d0

Specify Error should be thrown

5299c8e

Run test serially only in CI env

e989503

aaronang suggested changes Feb 23, 2017

View reviewed changes

montogeek added 2 commits February 23, 2017 15:19

Delete unnecessary var

9a7c9cb

Set jasmine.DEFAULT_TIMEOUT_INTERVAL to 10 seconds

6bc1b35

thymikee reviewed Feb 23, 2017

View reviewed changes

Update Jest option when test run on CI

ae26805

Add transformIgnorePatterns to Jest config

9fef81a

Ensure that the working directory is restored between tests.

a6ea0d4

Use jest@next and run tests in parallel in CI

e5ef23b

danez mentioned this pull request Aug 31, 2017

WIP: Migrate to jest #6179

Closed

babel-bot added the Has PR label Aug 31, 2017

danez closed this Aug 31, 2017

danez reopened this Aug 31, 2017

danez changed the base branch from 7.0 to master August 31, 2017 18:25

jridgewell closed this Sep 12, 2017

montogeek deleted the feat/migrate-test-to-jest branch March 22, 2018 13:23

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019

		@@ -0,0 +1 @@
		jasmine.DEFAULT_TIMEOUT_INTERVAL = 70000; No newline at end of file

Uh oh!

Migrate tests to Jest #5359

Migrate tests to Jest #5359

Uh oh!

Conversation

montogeek commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mention-bot commented Feb 22, 2017

Uh oh!

oleksandr-kuzmenko commented Feb 22, 2017

Uh oh!

montogeek commented Feb 22, 2017

Uh oh!

oleksandr-kuzmenko commented Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montogeek Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronang Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

babel-bot commented Feb 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montogeek Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montogeek commented Feb 23, 2017

Uh oh!

aaronang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thymikee Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronang Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montogeek commented Feb 22, 2017 •

edited

Loading

oleksandr-kuzmenko commented Feb 22, 2017 •

edited

Loading

montogeek Feb 23, 2017 •

edited

Loading

aaronang Feb 23, 2017 •

edited

Loading

montogeek Feb 23, 2017 •

edited

Loading

thymikee Feb 23, 2017 •

edited

Loading

aaronang Feb 23, 2017 •

edited

Loading

montogeek commented Mar 15, 2017 •

edited

Loading