[refactor] Migrate from Mocha+Chai to Jest #6079

xtinec · 2018-10-11T23:55:24Z

This change migrates all the existing unit tests

to Jest's global expect and matchers from chai's imported expect, asserts and matchers.
to Jest's describe/test from mocha's describe/it

The majority of the mechanical changes to tests are achieved through running jest-codemods. The only two note-worthy manual tweaks:

Setting a testURL of http://localhost in jest config and adjusting a few tests to leverage this value instead of relying on about:blank.
Re-enabling ExploreChartPanel_spec which was previously commented out as we cannot have empty tests with nothing in them with Jest. :)

This change also removes dependencies to Mocha and Chai.

@williaster @kristw @mistercrunch @betodealmeida

williaster · 2018-10-12T00:03:23Z

superset/assets/package.json

-    "test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js",
+    "tdd": "jest --watch",
+    "test": "jest",
+    "test:one": "jest",


should we remove this one since it's the same as test? I added test:one for mocha just a couple months ago.

Yep, we can.

williaster · 2018-10-12T17:24:28Z

superset/assets/spec/javascripts/addSlice/AddSliceContainer_spec.jsx

@@ -19,40 +18,40 @@ describe('AddSliceContainer', () => {
    wrapper = shallow(<AddSliceContainer {...defaultProps} />);
  });

-  it('uses table as default visType', () => {
-    expect(wrapper.state().visType).to.equal('table');
+  test('uses table as default visType', () => {


it might be useful to also add the eslint-plugin-jest while adding jest for consistent styles from the start. for example within describe blocks it recommends using it over test. (that one is auto-fixable so shouldn't be much work 😄 )

I noticed a mix in the file changes here.

Took a quick look at eslint-plugin-jest. I like the rules it advocates. Good suggestion!

Let me add it to this PR so we don't change it to test and the back to it. 😛

williaster · 2018-10-12T17:32:48Z

superset/assets/spec/helpers/shim.js

@@ -46,9 +45,6 @@ global.XMLHttpRequest = global.window.XMLHttpRequest;

 global.sinon = require('sinon');


In a future PR I think we can actually also remove sinon because jest has support for spies, stubs, etc. I doubt that's code-mod-able, tho.

Totally agree. I didn't do it in the same PR to limit the scope. :) Yeah, for sinon, we need to modify things manually.

williaster

LGTM! thanks again for doing this, it's amazing 🙌

sorry the cypress build is broken, I introduced a flaky test, will fix now.

xtinec · 2018-10-12T17:52:30Z

Looks like I have some linting errors to fix as well. :) On it.

kristw · 2018-10-12T18:42:00Z

Ah. It seems like you have to deal with Istanbul as well.
npm run cover fails. That command is still calling mocha under the hood.
Perhaps time to get rid of babel-istanbul (deprecated) and substitute with babel-plugin-istanbul
https://github.com/istanbuljs/babel-plugin-istanbul

Rooting for you. This PR is gold 🥇

williaster · 2018-10-12T21:21:04Z

@kristw I agree we should remove babel-istanbul but do we need babel-plugin-istanbul?

jest computes coverage on its own, so we could change the coverage script to simply be
"cover": "jest --coverage"

note this is compatible with our current setup for coveralls, I mirrored this in superset-ui which uses jest

This change migrates all the existing unit tests - to Jest's global expect and matchers from chai's imported expect, asserts and matchers. - to Jest's describe/test from mocha's describe/it The majority of the mechanical changes to tests are achieved through running jest-codemods. The only two note-worthy manual tweaks: 1. Setting a testURL of http://localhost in jest config and adjusting a few tests to leverage this value instead of relying on about:blank. 2. Re-enabling ExploreChartPanel_spec which was previously commented out as we cannot have empty tests with nothing in it with Jest. :) This change also removes dependencies to Mocha and Chai.

…arn run lint --fix` The only noteworthy change is the one in eslintrc for tests. The env has been updated from mocha to jest.

- One small fix in sqllab's Timer Spec for a test that is not using the spy it created for testing. - Deletion of a duplicated test caught by eslint-plugin-jest.

- Remove dependency on stand-alone istanbul and babel-istanbul as they're built-into jest. Yes!

kristw · 2018-10-12T22:05:50Z

@williaster That is even better. No need for istanbul then.

xtinec · 2018-10-12T22:12:00Z

@kristw @williaster yep, that's exactly what I did as well. "cover": "jest --coverage"
I added a config to jest to only compute coverage for files under src.

codecov-io · 2018-10-13T01:40:47Z

Codecov Report

Merging #6079 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6079   +/-   ##
=======================================
  Coverage   77.56%   77.56%           
=======================================
  Files          47       47           
  Lines        9484     9484           
=======================================
  Hits         7356     7356           
  Misses       2128     2128

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1e8778...1dfe55f. Read the comment docs.

… codecov - remove dynamic import in shim.js now that it is set in babelrc for tests only.

williaster

🔥

* [refactor] Migrate from Mocha+Chai to Jest This change migrates all the existing unit tests - to Jest's global expect and matchers from chai's imported expect, asserts and matchers. - to Jest's describe/test from mocha's describe/it The majority of the mechanical changes to tests are achieved through running jest-codemods. The only two note-worthy manual tweaks: 1. Setting a testURL of http://localhost in jest config and adjusting a few tests to leverage this value instead of relying on about:blank. 2. Re-enabling ExploreChartPanel_spec which was previously commented out as we cannot have empty tests with nothing in it with Jest. :) This change also removes dependencies to Mocha and Chai. * Remove the test:one command as it now does the same thing as test. * Fixing lint errors. The diff looks large but is large done through `yarn run lint --fix` The only noteworthy change is the one in eslintrc for tests. The env has been updated from mocha to jest. * Adding eslint-plugin-jest and further modify tests. - One small fix in sqllab's Timer Spec for a test that is not using the spy it created for testing. - Deletion of a duplicated test caught by eslint-plugin-jest. * - Make istanbul coverage work with Jest. - Remove dependency on stand-alone istanbul and babel-istanbul as they're built-into jest. Yes! * Attempt to fix dynamic imports in tests. * run sequentially and log heap usage * - tweaking maxworkers for travis and specifying coverageDirectory for codecov - remove dynamic import in shim.js now that it is set in babelrc for tests only.

xtinec force-pushed the migrate-from-mocha-to-jest branch from 5792b4f to 9a62140 Compare October 11, 2018 23:56

williaster reviewed Oct 12, 2018

View reviewed changes

williaster approved these changes Oct 12, 2018

View reviewed changes

xtinec added 5 commits October 12, 2018 14:28

Remove the test:one command as it now does the same thing as test.

5fffcf3

Fixing lint errors. The diff looks large but is large done through `y…

632f92c

…arn run lint --fix` The only noteworthy change is the one in eslintrc for tests. The env has been updated from mocha to jest.

Adding eslint-plugin-jest and further modify tests.

4606334

- One small fix in sqllab's Timer Spec for a test that is not using the spy it created for testing. - Deletion of a duplicated test caught by eslint-plugin-jest.

- Make istanbul coverage work with Jest.

3437946

- Remove dependency on stand-alone istanbul and babel-istanbul as they're built-into jest. Yes!

xtinec force-pushed the migrate-from-mocha-to-jest branch from 88bceb1 to 3437946 Compare October 12, 2018 21:32

kristw approved these changes Oct 12, 2018

View reviewed changes

Attempt to fix dynamic imports in tests.

b768f36

xtinec force-pushed the migrate-from-mocha-to-jest branch from ef155eb to f32b583 Compare October 12, 2018 23:19

run sequentially and log heap usage

c4fc33c

xtinec force-pushed the migrate-from-mocha-to-jest branch 2 times, most recently from be6e39b to 1df4883 Compare October 13, 2018 01:40

xtinec force-pushed the migrate-from-mocha-to-jest branch from 1df4883 to fd7bbac Compare October 13, 2018 01:52

- tweaking maxworkers for travis and specifying coverageDirectory for…

1dfe55f

… codecov - remove dynamic import in shim.js now that it is set in babelrc for tests only.

xtinec force-pushed the migrate-from-mocha-to-jest branch from 298cc23 to 1dfe55f Compare October 13, 2018 02:25

williaster approved these changes Oct 15, 2018

View reviewed changes

williaster merged commit 9029701 into apache:master Oct 15, 2018

matrix-compute mentioned this pull request Dec 27, 2023

[Snyk] Security upgrade axios from 1.6.0 to 1.6.3 matrix-compute/superset#29

Open

laxmanraparthi mentioned this pull request Dec 27, 2023

[Snyk] Security upgrade axios from 0.25.0 to 1.6.3 ShareChat/superset#102

Open

laxmanraparthi mentioned this pull request Jan 5, 2024

[Snyk] Security upgrade axios from 0.25.0 to 1.6.4 ShareChat/superset#111

Open

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[refactor] Migrate from Mocha+Chai to Jest #6079

[refactor] Migrate from Mocha+Chai to Jest #6079

xtinec commented Oct 11, 2018 •

edited

Loading

williaster Oct 12, 2018

xtinec Oct 12, 2018

williaster Oct 12, 2018 •

edited

Loading

xtinec Oct 12, 2018

williaster Oct 12, 2018 •

edited

Loading

xtinec Oct 12, 2018

williaster left a comment

xtinec commented Oct 12, 2018

kristw commented Oct 12, 2018 •

edited

Loading

williaster commented Oct 12, 2018 •

edited

Loading

kristw commented Oct 12, 2018

xtinec commented Oct 12, 2018

codecov-io commented Oct 13, 2018 •

edited

Loading

williaster left a comment

		@@ -46,9 +45,6 @@ global.XMLHttpRequest = global.window.XMLHttpRequest;

		global.sinon = require('sinon');

[refactor] Migrate from Mocha+Chai to Jest #6079

[refactor] Migrate from Mocha+Chai to Jest #6079

Conversation

xtinec commented Oct 11, 2018 • edited Loading

williaster Oct 12, 2018

Choose a reason for hiding this comment

xtinec Oct 12, 2018

Choose a reason for hiding this comment

williaster Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

xtinec Oct 12, 2018

Choose a reason for hiding this comment

williaster Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

xtinec Oct 12, 2018

Choose a reason for hiding this comment

williaster left a comment

Choose a reason for hiding this comment

xtinec commented Oct 12, 2018

kristw commented Oct 12, 2018 • edited Loading

williaster commented Oct 12, 2018 • edited Loading

kristw commented Oct 12, 2018

xtinec commented Oct 12, 2018

codecov-io commented Oct 13, 2018 • edited Loading

Codecov Report

williaster left a comment

Choose a reason for hiding this comment

xtinec commented Oct 11, 2018 •

edited

Loading

williaster Oct 12, 2018 •

edited

Loading

williaster Oct 12, 2018 •

edited

Loading

kristw commented Oct 12, 2018 •

edited

Loading

williaster commented Oct 12, 2018 •

edited

Loading

codecov-io commented Oct 13, 2018 •

edited

Loading