Skip to content

Upgrade to babel@7 #1043

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

Merged
merged 6 commits into from
Oct 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Run tests for all versions of React in a single jest process
  • Loading branch information
Andarist committed Oct 15, 2018
commit 25679b66eee80b0920d3eeb5ca71b233476ca2d8
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
],
"scripts": {
"build:commonjs": "cross-env BABEL_ENV=commonjs babel src --out-dir lib",
"build:es": "cross-env babel src --out-dir es",
"build:es": "babel src --out-dir es",
"build:umd": "cross-env NODE_ENV=development rollup -c -o dist/react-redux.js",
"build:umd:min": "cross-env NODE_ENV=production rollup -c -o dist/react-redux.min.js",
"build": "npm run build:commonjs && npm run build:es && npm run build:umd && npm run build:umd:min",
Expand Down
8 changes: 0 additions & 8 deletions test/jest.config.js

This file was deleted.

43 changes: 30 additions & 13 deletions test/run-tests.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,34 @@
const { readdirSync } = require('fs')
const { join } = require('path')
const npmRun = require('npm-run')
const version = process.env.REACT || '16.4'
const LATEST_VERSION = '16.4'
const version = process.env.REACT || LATEST_VERSION

try {
require('./install-test-deps.js')
if (version.toLowerCase() === 'all') {
readdirSync(join(__dirname, 'react')).forEach(version => {
npmRun.execSync(`cd test/react/${version} && npm test -- -c ../../jest.config.js`, { stdio: 'inherit' })
})
} else {
npmRun.execSync(`cd test/react/${version} && npm test -- -c ../../jest.config.js`, { stdio: 'inherit' })
const jestConfig = {
testURL: 'http://localhost',
collectCoverage: true,
coverageDirectory: `${__dirname}/coverage`,
transform: {
'.js$': `${__dirname}/babel-transformer.jest.js`,
},
}

require('./install-test-deps.js')

if (version.toLowerCase() === 'all') {
const allVersionsConfig = {
...jestConfig,
rootDir: __dirname,
// every directory has the same coverage, so we collect it only from one
collectCoverageFrom: [`react/${LATEST_VERSION}/src/**.js`]
}
npmRun.execSync(`node ./node_modules/.bin/jest -c '${JSON.stringify(allVersionsConfig)}'`, { stdio: 'inherit' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems to me that node ./node_modules/.bin/jest can be used when testing a specific version too, combining this with rootDir option would allow us to remove usage of npm-run here, removing cd dance & removing 'artificial' test scripts from nested package.jsons

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

First, if we're using npm-run, then this should just be jest -c .... That module automatically adds the .bin dir to the PATH.

We could actually make use of jest --projects in this case. That might be easier, as each version of React would be testable in isolation and without as much tooling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, if we're using npm-run, then this should just be jest -c .... That module automatically adds the .bin dir to the PATH.

Cool, didnt know that - gonna chang it in a minute.

We could actually make use of jest --projects in this case. That might be easier, as each version of React would be testable in isolation and without as much tooling.

I thought about it, but I think that would require keeping a copy of the jest config per each react version and is IMHO annoying in a long run (when it needs updating).

Also I wasnt sure how to collect coverage in --projects case as we wont to collect it only from a single project.

Copy link
Member

Choose a reason for hiding this comment

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

Damn, it doesn't do per-project coverage? That seems like an oversight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it might have, we just have exactly the same coverage here per project as noticed by @cellog. I guess overcollecting the coverage doesnt do us harm, but im not sure how CI takes the coverage for its report. Is the path configurable? Gonna look into that later as im on the mobile right now. If its configurable then i guess we might just point to the latest react directory and be done with it (leting jest collect in each directory).

There is still an issue of having to duplicate jest config all over the place though (if u want to use the projects feature)

} else {
try {
const specificVersionConfig = {
...jestConfig,
rootDir: `${__dirname}/react/${version}`,
}
npmRun.execSync(`cd test/react/${version} && npm test -- -c '${JSON.stringify(specificVersionConfig)}'`, { stdio: 'inherit' })
} finally {
npmRun.execSync('cd ../../..')
}
} finally {
npmRun.execSync('cd ../../..')
}