-
Notifications
You must be signed in to change notification settings - Fork 1.8k
chore(NODE-3715): add code coverage generation to Evergreen tasks #3107
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
Conversation
.evergreen/run-tests.sh
Outdated
| UNIFIED=${UNIFIED:-} | ||
| MONGODB_URI=${MONGODB_URI:-} | ||
| TEST_NPM_SCRIPT=${TEST_NPM_SCRIPT:-check:test} | ||
| TEST_NPM_SCRIPT=${TEST_NPM_SCRIPT:-check: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.
This corresponds to nyc npm run test:all so the unit tests will be repeated on every variant. Maybe we want a new script for this to just run the int tests with 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.
Yeah, good catch! Would we want a separate command to run our unit tests with coverage in CI as well? (I'm assuming we run the unit tests in CI at some already...?)
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.
Yes we run them 🙂 they're part of the lint task, so if you go to that script you make it run nyc in front of the unit run there, and it would also get the upload after task just fine I think?
dariakp
left a comment
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.
LGTM :)
Description
This PR adds code coverage generation for each build in Evergreen.
What is changing?
This PR does the following
Is there new documentation needed for these changes?
Nope
Double check the following
npm run check:lintscript<type>(NODE-xxxx)<!>: <description>