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: Migrate client side tests from Mocha to Jest #16119

Merged
merged 29 commits into from Sep 28, 2017

Conversation

@gziolo
Member

gziolo commented Jul 11, 2017

Similar to #14364 where we migrate server side tests to Jest. This one turned out to be huge :)

Test Suites: 928 passed, 928 total
Tests: 10 skipped, 8691 passed, 8701 total
Snapshots: 0 total
Time: 49.127s
Ran all test suites.

real 0m50.955s
user 5m16.126s
sys 0m20.892s

It takes 50-60 seconds on average to complete full tests run on MBP 2015. It is a bit slower on first run, because Jest bootstraps some nice caching mechanisms to speed up things on subsequent runs.

It is faster than with what we have with Mocha at the moment:

8692 passing (1m)
9 pending

...
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
...

real 1m54.982s
user 1m56.019s
sys 0m4.815s

I have some memory issues which Mocha on my machine, so this test result might be inaccurate. It still takes 10 seconds more to process all tests if we skip Mocha shutdown phase. I'm not even mentioning that Jest works properly all the time and doesn't error at all!

There is still one test that doesn't work with Jest, but we should rather remove it completely, because it's too complex.

Changes introduced

This PR enables to use Jest test runner in place of Mocha. To make transition smoother and this already huge PR a bit smaller we still keep Chai and Sinon as they were configured. In theory we can keep such setup forever. I don't see any reason why we wouldn't leave decision if developer wants to use Jest expectations and spies or code using Chai assertions and Sinon spies.

With this PR we are also removing no longer needed test helpers:

  • immutableChai - we no longer have tests that use this helper.
  • test/helpers/react/empty-component - we already have components/empty-component.
  • useFakeDom - we also use node test env as default. It is possible to set jsdom test env per test suite using special comment.
  • useFilesystemMocks - jest.mock handles it.

At the end of full migration we will be able to remove also:

  • nock-control - since we introduced integration tests, we should move all tests triggering network requests there.
  • use-mockery - it turned out that jest.mock + test isolation is much faster than mockery. We will also no longer need to put require statements inside before block once this PR lands.

In the long run we could get rid of all remaining helpers. This is going to be possible because Jest provides isolation between test files.

How to run tests

npm run test-client
npm run test-client:ci - version tailored for CircleCI
npm run test-client:watch - run with watch mode enabled, by default it executes tests only for modified files

Possible improvements

  • Add code coverage.
  • Use jest-codemods to replace Mocha specific methods with their Jest equivalents.
  • Use jest-codemods to replace Chai expectations with Jest equivalents.
@matticbot

This comment has been minimized.

@matticbot matticbot added the [Size] XL label Jul 11, 2017

@gziolo gziolo force-pushed the update/client-tests-jest branch from 7af5255 to a79b9e5 Jul 12, 2017

@gziolo gziolo changed the base branch from update/non-tests-subfolder to master Jul 12, 2017

@gziolo gziolo force-pushed the update/client-tests-jest branch 2 times, most recently from 5b1708b to 0d57f5c Jul 13, 2017

@gziolo

This comment has been minimized.

Member

gziolo commented Jul 13, 2017

@nb @ehg I'm quite close to have Jest running with Calypso client code:

Test Suites: 57 skipped, 856 passed, 856 of 913 total
Tests: 580 skipped, 8039 passed, 8619 total
Snapshots: 0 total
Time: 94.387s

I updated over 160 tests, skipped 57 tests to make full test run green. I think we can manually convert 99% of skipped tests and it seems like I can do it myself during my parental leave with the very limited time spent at the computer. I removed useFakeDom completely and it looks like that we need to replace mockery with Jest mocks and we should be done :)

One more note, it looks like execution time locally should be about the same to what we have at the moment once I introduce all performance optimizations I found online.

@gziolo gziolo requested review from nb and ehg Jul 13, 2017

@gziolo gziolo force-pushed the update/client-tests-jest branch 7 times, most recently from cc0a407 to 1fce24f Jul 13, 2017

@gziolo

This comment has been minimized.

Member

gziolo commented Jul 17, 2017

I was able to speed up execution of all tests from ~90 to ~50 seconds by using this code shared by @gdborton. It works like a charm for both sinon and enzyme. I also removed obsolete calls to the following helpers useSandbox and useFilesystemMocks.

Test Suites: 54 skipped, 859 passed, 859 of 913 total
Tests: 541 skipped, 8086 passed, 8627 total
Snapshots: 0 total
Time: 53.702s
Ran all test suites.

We have still 54 test suites to updated or remove completely :)

@gziolo gziolo force-pushed the update/client-tests-jest branch 2 times, most recently from e2e53dd to 71345d7 Jul 17, 2017

@gziolo

This comment has been minimized.

Member

gziolo commented Jul 19, 2017

@blowery it looks like jest.mock + test isolation is much faster than mockery. We will also no longer need to put require statements inside before block.

@gziolo gziolo requested a review from blowery Jul 19, 2017

@gziolo gziolo force-pushed the update/client-tests-jest branch 3 times, most recently from 2973dea to 9cc606e Jul 24, 2017

gziolo added some commits Jul 14, 2017

@gziolo gziolo force-pushed the update/client-tests-jest branch from 38110bc to 5c0ac36 Sep 28, 2017

@gziolo gziolo merged commit 6a32d36 into master Sep 28, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@gziolo gziolo deleted the update/client-tests-jest branch Sep 28, 2017

@Automattic Automattic deleted a comment from matticbot Sep 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment