Skip to content

Commit

Permalink
Use build matrix to run tests in different envs
Browse files Browse the repository at this point in the history
  • Loading branch information
mjackson committed Oct 31, 2018
1 parent 90ecb35 commit 46a39c9
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 80 deletions.
8 changes: 6 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
sudo: false
language: node_js
node_js:
- "10"
cache:
directories:
- packages/react-router/node_modules
- packages/react-router-config/node_modules
- packages/react-router-dom/node_modules
- packages/react-router-native/node_modules
- website/node_modules
node_js:
- "10"
env:
- TEST_ENV=cjs
- TEST_ENV=umd
- TEST_ENV=source
install:
- npm ci
before_script:
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"build": "del cjs esm umd && rollup -c",
"prepublishOnly": "del cjs esm umd && rollup -c",
"lint": "eslint modules",
"test": "node ./scripts/test.js"
"test": "jest"
},
"peerDependencies": {
"react": ">=15",
Expand Down
25 changes: 0 additions & 25 deletions packages/react-router-config/scripts/test.js

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react-router-dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"build": "del cjs esm umd && rollup -c",
"prepublishOnly": "del cjs esm umd && rollup -c",
"lint": "eslint modules",
"test": "node ./scripts/test.js"
"test": "jest"
},
"peerDependencies": {
"react": ">=15"
Expand Down
25 changes: 0 additions & 25 deletions packages/react-router-dom/scripts/test.js

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react-router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"build": "del cjs esm umd && rollup -c",
"prepublishOnly": "del cjs esm umd && rollup -c",
"lint": "eslint modules",
"test": "node ./scripts/test.js"
"test": "jest"
},
"peerDependencies": {
"react": ">=15"
Expand Down
25 changes: 0 additions & 25 deletions packages/react-router/scripts/test.js

This file was deleted.

4 comments on commit 46a39c9

@pshrmn
Copy link
Contributor

@pshrmn pshrmn commented on 46a39c9 Nov 4, 2018

Choose a reason for hiding this comment

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

Does Travis have to build the packages for each environment in the matrix? Running Travis is taking ~3 as long now.

@mjackson
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, Travis runs a new VM for each build so it has to build everything from scratch. But ya, it's a bummer that it takes 3x as long to run the build.

The one thing I like about this approach is that it isolates test output from the different builds. So we can see at a glance if the UMD build is failing, for example. With the test script approach it's a bit harder to see because I have to scroll through the output.

The best approach would probably be to have 1 build but a more flexible test script that lets us isolate output from different builds more cleanly. So the workflow would be like:

  • Build everything
  • Run all source tests, for all packages
  • Run all CJS tests, for all packages
  • Run all UMD tests, for all packages

We could probably just add a flag to the test script, kinda like the --no-website flag we currently have in the build script. So, e.g. node scripts/test.js --module-format=umd, or something like that.

@timdorr
Copy link
Member

@timdorr timdorr commented on 46a39c9 Nov 5, 2018

Choose a reason for hiding this comment

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

Maybe we should wrap the rollup outputs array with some checks for TEST_ENV to only build the desired formats?

const config = []

if (process.env.TEST_ENV == 'cjs' || !process.env.TEST_ENV) {
  config.push({ // CJS config goes here... })
}

export default config

@mjackson
Copy link
Member Author

Choose a reason for hiding this comment

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

@timdorr Ya, that'd work great if we decide to keep things in separate processes. Right now I'm leaning toward just doing what they do in the React repo and providing a --type flag to allow people to build/test whichever things they want. By default we build/test everything.

Please sign in to comment.