-
Notifications
You must be signed in to change notification settings - Fork 67
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
MONGOSH-269 - Add nyc for test coverage #385
Conversation
@@ -17,8 +17,8 @@ | |||
"clean": "lerna clean -y && rm -Rf node_modules", | |||
"check": "lerna run check --since HEAD --exclude-dependents", | |||
"check-ci": "lerna run check", | |||
"test": "lerna run test", | |||
"test-ci": "lerna run test-ci", | |||
"test": "rimraf .nyc_output && lerna exec -- nyc --no-clean --cwd ../.. --reporter=none npm run test && npm run report-coverage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use some pretest
or magic package to allow &&
in windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&&
should work just fine in npm scripts in Windows ... there are some issues with coverage generation that only seem to affect Windows, though, I'm currently looking into that (turns out to be istanbuljs/nyc#1310)
@@ -17,8 +17,8 @@ | |||
"clean": "lerna clean -y && rm -Rf node_modules", | |||
"check": "lerna run check --since HEAD --exclude-dependents", | |||
"check-ci": "lerna run check", | |||
"test": "lerna run test", | |||
"test-ci": "lerna run test-ci", | |||
"test": "rimraf .nyc_output && lerna exec -- nyc --no-clean --cwd ../.. --reporter=none npm run test && npm run report-coverage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to have coverage always or as a separate job like test-cov
? won't it take more to run tests with coverage? or is intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll take a bit longer, yes... but npm test
for the full repo already takes at least 10 minutes, so it's not like it's something that one would run on a regular basis during development (I only do it before submitting a PR, and admittedly only sometimes ... CI is there to do that work for us)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh i see, the coverage runs only on the root package .. didn't notice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was referring to test
and test-ci
of the root package :)
No description provided.