Skip to content

Commit

Permalink
[refactor] Migrate from Mocha+Chai to Jest (#6079)
Browse files Browse the repository at this point in the history
* [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.
  • Loading branch information
xtinec authored and williaster committed Oct 15, 2018
1 parent 46c8667 commit 9029701
Show file tree
Hide file tree
Showing 177 changed files with 2,549 additions and 2,176 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ commands are invoked.

### JavaScript testing

We use [Mocha](https://mochajs.org/), [Chai](http://chaijs.com/) and [Enzyme](http://airbnb.io/enzyme/) to test Javascript. Tests can be run with:
We use [Jest](https://jestjs.io/) and [Enzyme](http://airbnb.io/enzyme/) to test Javascript. Tests can be run with:

```bash
cd superset/assets/spec
Expand Down
9 changes: 8 additions & 1 deletion superset/assets/.babelrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
{
"presets" : ["airbnb", "react", "env"],
"plugins": ["lodash", "syntax-dynamic-import", "react-hot-loader/babel"]
"plugins": ["lodash", "syntax-dynamic-import", "react-hot-loader/babel"],
"env": {
"test": {
"plugins": [
"babel-plugin-dynamic-import-node"
]
}
}
}
8 changes: 2 additions & 6 deletions superset/assets/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@
"react/no-string-refs": 0,
"indent": 0,
"no-multi-spaces": 0,
"padded-blocks": 0,
"no-only-tests/no-only-tests": 2
},
"plugins": [
"no-only-tests"
]
"padded-blocks": 0
}
}
11 changes: 11 additions & 0 deletions superset/assets/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module.exports = {
testRegex: '\\/spec\\/.*_spec\\.jsx?$',
moduleNameMapper: {
'\\.(css|less)$': '<rootDir>/spec/__mocks__/styleMock.js',
'\\.(gif|ttf|eot|svg)$': '<rootDir>/spec/__mocks__/fileMock.js',
},
setupTestFrameworkScriptFile: '<rootDir>/spec/helpers/shim.js',
testURL: 'http://localhost',
collectCoverageFrom: ['src/**/*.{js,jsx}'],
coverageDirectory: '<rootDir>/coverage/',
};
14 changes: 6 additions & 8 deletions superset/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
"test": "spec"
},
"scripts": {
"tdd": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*' --watch --recursive",
"test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js 'spec/**/*_spec.*'",
"test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/shim.js",
"cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/shim.js --require ignore-styles 'spec/**/*_spec.*'",
"tdd": "jest --watch",
"test": "jest",
"cover": "jest --maxWorkers=8 --coverage",
"dev": "webpack --mode=development --colors --progress --debug --watch",
"dev-server": "webpack-dev-server --mode=development --progress",
"prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress",
Expand Down Expand Up @@ -134,7 +133,7 @@
"babel-cli": "^6.14.0",
"babel-core": "^6.10.4",
"babel-eslint": "^8.2.2",
"babel-istanbul": "^0.12.2",
"babel-jest": "^23.6.0",
"babel-loader": "^7.1.4",
"babel-plugin-css-modules-transform": "^1.1.0",
"babel-plugin-dynamic-import-node": "^1.2.0",
Expand All @@ -143,7 +142,6 @@
"babel-polyfill": "^6.23.0",
"babel-preset-airbnb": "^2.1.1",
"babel-preset-env": "^1.7.0",
"chai": "^4.0.2",
"clean-webpack-plugin": "^0.1.19",
"css-loader": "^1.0.0",
"cypress": "^3.0.3",
Expand All @@ -154,6 +152,7 @@
"eslint-config-prettier": "^2.9.0",
"eslint-plugin-cypress": "^2.0.1",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-jest": "^21.24.1",
"eslint-plugin-jsx-a11y": "^5.1.1",
"eslint-plugin-no-only-tests": "^2.0.1",
"eslint-plugin-prettier": "^2.6.0",
Expand All @@ -163,13 +162,12 @@
"gl": "^4.0.4",
"ignore-styles": "^5.0.1",
"imports-loader": "^0.7.1",
"istanbul": "^1.0.0-alpha",
"jest": "^23.6.0",
"jsdom": "9.12.0",
"less": "^2.6.1",
"less-loader": "^4.1.0",
"mini-css-extract-plugin": "^0.4.0",
"minimist": "^1.2.0",
"mocha": "^3.5.3",
"npm-check-updates": "^2.14.2",
"optimize-css-assets-webpack-plugin": "^5.0.1",
"po2json": "^0.4.5",
Expand Down
11 changes: 9 additions & 2 deletions superset/assets/spec/.eslintrc
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
{
"plugins": [
"jest",
"no-only-tests"
],
"env": {
"mocha": true
"jest/globals": true
},
"extends": ["plugin:jest/recommended"],
"rules": {
"import/no-extraneous-dependencies": ["error", {"devDependencies": true}]
"import/no-extraneous-dependencies": ["error", {"devDependencies": true}],
"jest/consistent-test-it": "error",
"no-only-tests/no-only-tests": "error"
}
}
1 change: 1 addition & 0 deletions superset/assets/spec/__mocks__/fileMock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'test-file-stub';
1 change: 1 addition & 0 deletions superset/assets/spec/__mocks__/styleMock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};
11 changes: 0 additions & 11 deletions superset/assets/spec/helpers/shim.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
/* eslint no-native-reassign: 0 */
import 'babel-polyfill';
import chai from 'chai';
import jsdom from 'jsdom';
import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';

configure({ adapter: new Adapter() });

require('babel-register')({
// NOTE: If `dynamic-import-node` is in .babelrc alongside
// `syntax-dynamic-import` it breaks webpack's bundle splitting capability.
// So only load during runtime on the node-side (in tests)
plugins: ['dynamic-import-node'],
});

const exposedProperties = ['window', 'navigator', 'document'];

global.jsdom = jsdom.jsdom;
Expand Down Expand Up @@ -46,9 +38,6 @@ global.XMLHttpRequest = global.window.XMLHttpRequest;

global.sinon = require('sinon');

global.expect = chai.expect;
global.assert = chai.assert;

global.sinon.useFakeXMLHttpRequest();

global.window.XMLHttpRequest = global.XMLHttpRequest;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import { expect } from 'chai';
import { shallow } from 'enzyme';

import CollectionTable from '../../../src/CRUD/CollectionTable';
Expand All @@ -21,13 +20,13 @@ describe('CollectionTable', () => {
});

it('is valid', () => {
expect(React.isValidElement(el)).to.equal(true);
expect(React.isValidElement(el)).toBe(true);
});

it('renders a table', () => {
const length = mockDatasource['7__table'].columns.length;
expect(wrapper.find('table')).to.have.lengthOf(1);
expect(wrapper.find('tbody tr.row')).to.have.lengthOf(length);
expect(wrapper.find('table')).toHaveLength(1);
expect(wrapper.find('tbody tr.row')).toHaveLength(length);
});

});
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from 'react';
import { expect } from 'chai';
import { shallow } from 'enzyme';
import { Button } from 'react-bootstrap';
import Select from 'react-virtualized-select';
Expand All @@ -20,19 +19,19 @@ describe('AddSliceContainer', () => {
});

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

it('renders 2 selects', () => {
expect(wrapper.find(Select)).to.have.lengthOf(2);
expect(wrapper.find(Select)).toHaveLength(2);
});

it('renders a button', () => {
expect(wrapper.find(Button)).to.have.lengthOf(1);
expect(wrapper.find(Button)).toHaveLength(1);
});

it('renders a disabled button if no datasource is selected', () => {
expect(wrapper.find(Button).dive().find('.btn[disabled=true]')).to.have.length(1);
expect(wrapper.find(Button).dive().find('.btn[disabled=true]')).toHaveLength(1);
});

it('renders an enabled button if datasource is selected', () => {
Expand All @@ -42,7 +41,7 @@ describe('AddSliceContainer', () => {
datasourceId: datasourceValue.split('__')[0],
datasourceType: datasourceValue.split('__')[1],
});
expect(wrapper.find(Button).dive().find('.btn[disabled=false]')).to.have.length(1);
expect(wrapper.find(Button).dive().find('.btn[disabled=false]')).toHaveLength(1);
});

it('formats explore url', () => {
Expand All @@ -53,6 +52,6 @@ describe('AddSliceContainer', () => {
datasourceType: datasourceValue.split('__')[1],
});
const formattedUrl = '/superset/explore/?form_data=%7B%22viz_type%22%3A%22table%22%2C%22datasource%22%3A%221__table%22%7D';
expect(wrapper.instance().exploreUrl()).to.equal(formattedUrl);
expect(wrapper.instance().exploreUrl()).toBe(formattedUrl);
});
});
11 changes: 5 additions & 6 deletions superset/assets/spec/javascripts/chart/Chart_spec.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import { shallow } from 'enzyme';
import { expect } from 'chai';
import sinon from 'sinon';

import { chart as initChart } from '../../../src/chart/chartReducer';
Expand Down Expand Up @@ -52,7 +51,7 @@ describe('Chart', () => {
height: 100,
});
wrapper.instance().componentDidUpdate(prevProp);
expect(stub.callCount).to.equals(0);
expect(stub.callCount).toBe(0);
});

it('should call after chart stop loading', () => {
Expand All @@ -61,22 +60,22 @@ describe('Chart', () => {
chartStatus: 'success',
});
wrapper.instance().componentDidUpdate(prevProp);
expect(stub.callCount).to.equals(1);
expect(stub.callCount).toBe(1);
});

it('should call after resize', () => {
wrapper.setProps({
chartStatus: 'rendered',
height: 100,
});
expect(stub.callCount).to.equals(1);
expect(stub.callCount).toBe(1);
});
});

describe('render', () => {
it('should render ChartBody after loading is completed', () => {
expect(wrapper.find(Loading)).to.have.length(1);
expect(wrapper.find(ChartBody)).to.have.length(0);
expect(wrapper.find(Loading)).toHaveLength(1);
expect(wrapper.find(ChartBody)).toHaveLength(0);
});
});
});
Loading

0 comments on commit 9029701

Please sign in to comment.