Skip to content
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

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

Merged
merged 8 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,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');
Copy link
Contributor

@williaster williaster Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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