-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
test_runner: add support for coverage thresholds #54429
Conversation
Review requested:
|
To reviewers, please send as any changes as you'd like. This is still a draft, so I'd love lots and lots of feedback |
there's always 4 thresholds: branch, function, line, and statements. can statements be added also? |
I don't believe the Node.js coverage reporter handlers statements |
b978883
to
71a0be1
Compare
I realize that this is passing local tests, and is not 'incomplete' per say, so it doesn't hurt to keep it in the review state. |
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
I gave it a try in the past #51370 and it was blocked |
Looking at yours, it looks like a much smarter way to do this than I did. If that didn't land, I doubt this will. |
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.
I was ok then and I'm ok now, but yeah, this should probably not land unless someone wants to move it through a @nodejs/tsc vote.
+1 with a TSC vote. If the consensus is to ship this feature, I think @marco-ippolito's PR is a better layout of this. |
Im happy if you want to takeover |
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!
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.
statements should be added too
IIRC the coverage reporter doesn't support statements (but maybe it should in the future 🤔) |
@ljharb @mcollina statements coverage are only available when using instrumentation coverage from tools such as isnatbul. |
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.
My main concern with previous attempts was lack of source map support. Now that we have that I thinks this is ok.
Also see my previous comment regarding lack of statements coverage (we might want to document that limitation, but it is a limitation that exists also in the coverage report/event, regardless of thresholds)
Commit Queue failed- Loading data for nodejs/node/pull/54429 ✔ Done loading data for nodejs/node/pull/54429 ----------------------------------- PR info ------------------------------------ Title test_runner: add support for coverage thresholds (#54429) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch RedYetiDev:coverage-threshold -> nodejs:main Labels semver-minor, notable-change, cli, author ready, coverage, commit-queue-squash, test_runner Commits 2 - test_runner: add support for coverage thresholds - (skip if inspector disabled) Committers 1 - RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> PR-URL: https://github.com/nodejs/node/pull/54429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/54429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - (skip if inspector disabled) ℹ This PR was created on Sat, 17 Aug 2024 22:18:01 GMT ✔ Approvals: 5 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/54429#pullrequestreview-2247072628 ✔ - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/54429#pullrequestreview-2244734218 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/54429#pullrequestreview-2246968179 ✔ - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/54429#pullrequestreview-2255436655 ✔ - Marco Ippolito (@marco-ippolito) (TSC): https://github.com/nodejs/node/pull/54429#pullrequestreview-2255452813 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-08-23T13:36:18Z: https://ci.nodejs.org/job/node-test-pull-request/61378/ - Querying data for job/node-test-pull-request/61378/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/10529382777 |
Landed in 9edf4a0 |
Co-Authored-By: Marco Ippolito <marcoippolito54@gmail.com> PR-URL: #54429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 PR-URL: TODO
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 PR-URL: #54560
Co-Authored-By: Marco Ippolito <marcoippolito54@gmail.com> PR-URL: #54429 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) #54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) #54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) #54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) #54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) #53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) #53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) #54394 PR-URL: #54560
Notable changes: net: * (SEMVER-MINOR) exclude ipv6 loopback addresses from server.listen (Giovanni Bucci) nodejs#54264 src: * (SEMVER-MINOR) add JS APIs for compile cache and NODE_DISABLE_COMPILE_CACHE (Joyee Cheung) nodejs#54501 src,lib: * (SEMVER-MINOR) add performance.uvMetricsInfo (Rafael Gonzaga) nodejs#54413 test_runner: * (SEMVER-MINOR) add support for coverage thresholds (Aviv Keller) nodejs#54429 * (SEMVER-MINOR) support running tests in process (Colin Ihrig) nodejs#53927 * (SEMVER-MINOR) defer inheriting hooks until run() (Colin Ihrig) nodejs#53927 vm: * (SEMVER-MINOR) introduce vanilla contexts via vm.constants.DONT_CONTEXTIFY (Joyee Cheung) nodejs#54394 PR-URL: nodejs#54560
Fixes #48739
This PR adds support for coverage thresholds.
The following new CLI flags are added:
--test-coverage-branches
--test-coverage-functions
--test-coverage-lines
Notable Change
Node.js now supports requiring code coverage to meet a specific threshold before the process exits successfully. To use this feature, you need to enable the
--experimental-test-coverage
flag.You can set thresholds for the following types of coverage:
--test-coverage-branches=<threshold>
--test-coverage-functions=<threshold>
--test-coverage-lines=<threshold>
<threshold>
should be an integer between 0 and 100. If an invalid value is provided, aTypeError
will be thrown.If the code coverage fails to meet the specified thresholds for any category, the process will exit with code
1
.For instance, to enforce a minimum of 80% line coverage and 60% branch coverage, you can run: