Skip to content

Don't build when executing test:unit or test:integration#153

Merged
usergenic merged 4 commits intomasterfrom
test-unit-all-the-things
Apr 17, 2018
Merged

Don't build when executing test:unit or test:integration#153
usergenic merged 4 commits intomasterfrom
test-unit-all-the-things

Conversation

@usergenic
Copy link
Contributor

Install steps already run build tasks and all package 'test' scripts also run build tasks, so travis is spending a lot of time unnecessarily running build tasks. Attempt to solve this by adding test:unit npm scripts to all packages and explicitly run those scripts instead.

Also removed the build gulp task dependencies on test:unit or test:integration where found.

@usergenic usergenic requested review from aomarks and rictic April 17, 2018 05:34
"test:watch": "tsc-then -- mocha",
"test": "npm run clean && npm run build && npm run test:unit && npm run lint",
"test:unit": "mocha",
"test:watch": "tsc-then -- npm run test:unit",
Copy link
Contributor

Choose a reason for hiding this comment

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

running mocha directly is generally better in test:watch, because if there's a failure when doing npm run foo then npm outputs like 20-30 lines about how the failure isn't npm's fault

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok.

"test:watch": "tsc-then -- mocha",
"test": "npm run build && npm run test:unit && npm run lint",
"test:unit": "mocha",
"test:watch": "tsc-then -- npm run test:unit",
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer just tsc-then -- mocha

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've changed all npm run test:unit inside of npm scripts to mocha where appropriate.

gulp.task('test', ['build'], function() {
gulp.task('test', ['build', 'test:unit']);

gulp.task('test:unit', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like build is missing a corresponding change to its package.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Added

"test:integration": "lerna run test:integration --stream",
"test:integration:windows": "lerna run test:integration --stream",
"test:unit": "lerna run test:unit --stream",
"test:unit:windows": "lerna run test:unit --stream --ignore polyserve"
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to --stream, nice catch

@usergenic
Copy link
Contributor Author

Note: I put the --stream flag on the lerna commands primarily because of integration tests. When you run any task with lerna that exceeds 10 minutes, that means no output is generated for those 10 minutes and travis just gives up. Using --stream ensures output is generated in realtime instead of after the entire task is completed, which makes travis happier. Its also just nice to get feedback that sh*t is happening.

…utput, and add test:unit script for build.
@usergenic usergenic requested a review from rictic April 17, 2018 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants