test_runner: add support for coverage thresholds #54429

RedYetiDev · 2024-08-17T22:18:01Z

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:

Branch coverage: Use --test-coverage-branches=<threshold>
Function coverage: Use --test-coverage-functions=<threshold>
Line coverage: Use --test-coverage-lines=<threshold>

<threshold> should be an integer between 0 and 100. If an invalid value is provided, a TypeError 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:

$ node --experimental-test-coverage --test-coverage-lines=80 --test-coverage-branches=60 example.js

# start of coverage report
# ...
# ...
# ...
# end of coverage report
# Error: 53.45% line coverage does not meet the threshold of 80%.
# Error: 40.30% branch coverage does not meet the threshold of 60%.

nodejs-github-bot · 2024-08-17T22:18:06Z

Review requested:

@nodejs/test_runner

RedYetiDev · 2024-08-17T22:22:34Z

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

ljharb · 2024-08-17T22:28:00Z

there's always 4 thresholds: branch, function, line, and statements. can statements be added also?

RedYetiDev · 2024-08-17T22:49:57Z

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

RedYetiDev · 2024-08-18T00:44:28Z

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.

github-actions · 2024-08-18T00:45:38Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @RedYetiDev.

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.

marco-ippolito · 2024-08-18T09:02:17Z

I gave it a try in the past #51370 and it was blocked

RedYetiDev · 2024-08-18T11:37:07Z

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.

mcollina

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.

RedYetiDev · 2024-08-18T13:25:40Z

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.

marco-ippolito · 2024-08-18T16:58:10Z

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

ShogunPanda

LGTM!

tniessen · 2024-08-19T07:44:46Z

FWIW, there were two previous attempts at this: #51182 and #51370. I am not sure folks are convinced that we want to support this kind of arbitrary threshold.

mcollina

statements should be added too

RedYetiDev · 2024-08-19T13:16:53Z

statements should be added too

IIRC the coverage reporter doesn't support statements (but maybe it should in the future 🤔)

MoLow · 2024-08-20T04:54:19Z

there's always 4 thresholds: branch, function, line, and statements. can statements be added also?

@ljharb @mcollina statements coverage are only available when using instrumentation coverage from tools such as isnatbul.
V8 coverage dosent report that, you can see that c8 for instance doesn't have that either

MoLow

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)

test/parallel/test-runner-coverage.js

RedYetiDev · 2024-08-22T22:22:04Z

The one failed CI should've skipped the test, but I forgot to include the line common.skipIfInspectorDisabled();. It is resolved now.

ok 2438 parallel/test-runner-coverage # skip inspector disabled
  ---
  duration_ms: 339.45600
  ...
ok 2439 parallel/test-runner-cli-concurrency
  ---
  duration_ms: 1622.75700
  ...
ok 2440 parallel/test-runner-exit-code
  ---
  duration_ms: 1009.28400
  ...
not ok 2441 parallel/test-runner-coverage-thresholds
  ---
  duration_ms: 1316.08900
  severity: fail
  exitcode: 1
  stack: |-
    TAP version 13
    # Subtest: test passing --test-coverage-lines
    not ok 1 - test passing --test-coverage-lines
      ---
      duration_ms: 145.812425
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:58:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:66:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.start (node:internal/test_runner/test:786:17)
        startSubtestAfterBootstrap (node:internal/test_runner/harness:280:17)
      ...
    # Subtest: test failing --test-coverage-lines
    not ok 2 - test failing --test-coverage-lines
      ---
      duration_ms: 80.535613
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:72:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:80:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.processPendingSubtests (node:internal/test_runner/test:584:18)
        Test.postRun (node:internal/test_runner/test:983:19)
        Test.run (node:internal/test_runner/test:926:12)
        async startSubtestAfterBootstrap (node:internal/test_runner/harness:280:3)
      ...
    # Subtest: test out-of-range --test-coverage-lines (too high)
    ok 3 - test out-of-range --test-coverage-lines (too high)
      ---
      duration_ms: 61.481747
      ...
    # Subtest: test out-of-range --test-coverage-lines (too low)
    ok 4 - test out-of-range --test-coverage-lines (too low)
      ---
      duration_ms: 105.771413
      ...
    # Subtest: test passing --test-coverage-functions
    not ok 5 - test passing --test-coverage-functions
      ---
      duration_ms: 105.407257
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:58:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:66:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.processPendingSubtests (node:internal/test_runner/test:584:18)
        Test.postRun (node:internal/test_runner/test:983:19)
        Test.run (node:internal/test_runner/test:926:12)
        async Test.processPendingSubtests (node:internal/test_runner/test:584:7)
      ...
    # Subtest: test failing --test-coverage-functions
    not ok 6 - test failing --test-coverage-functions
      ---
      duration_ms: 106.136271
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:72:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:80:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.processPendingSubtests (node:internal/test_runner/test:584:18)
        Test.postRun (node:internal/test_runner/test:983:19)
        Test.run (node:internal/test_runner/test:926:12)
        async Test.processPendingSubtests (node:internal/test_runner/test:584:7)
      ...
    # Subtest: test out-of-range --test-coverage-functions (too high)
    ok 7 - test out-of-range --test-coverage-functions (too high)
      ---
      duration_ms: 57.876439
      ...
    # Subtest: test out-of-range --test-coverage-functions (too low)
    ok 8 - test out-of-range --test-coverage-functions (too low)
      ---
      duration_ms: 55.784104
      ...
    # Subtest: test passing --test-coverage-branches
    not ok 9 - test passing --test-coverage-branches
      ---
      duration_ms: 72.243671
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:58:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:66:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.processPendingSubtests (node:internal/test_runner/test:584:18)
        Test.postRun (node:internal/test_runner/test:983:19)
        Test.run (node:internal/test_runner/test:926:12)
        async Test.processPendingSubtests (node:internal/test_runner/test:584:7)
      ...
    # Subtest: test failing --test-coverage-branches
    not ok 10 - test failing --test-coverage-branches
      ---
      duration_ms: 98.27272
      location: '/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:72:3'
      failureType: 'testCodeFailure'
      error: |-
        The expression evaluated to a falsy value:
        
          assert(stdout.includes(getTapCoverageFixtureReport()))
        
      code: 'ERR_ASSERTION'
      name: 'AssertionError'
      expected: true
      actual: false
      operator: '=='
      stack: |-
        TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-coverage-thresholds.js:80:5)
        Test.runInAsyncScope (node:async_hooks:211:14)
        Test.run (node:internal/test_runner/test:887:25)
        Test.processPendingSubtests (node:internal/test_runner/test:584:18)
        Test.postRun (node:internal/test_runner/test:983:19)
        Test.run (node:internal/test_runner/test:926:12)
        async Test.processPendingSubtests (node:internal/test_runner/test:584:7)
      ...
    # Subtest: test out-of-range --test-coverage-branches (too high)
    ok 11 - test out-of-range --test-coverage-branches (too high)
      ---
      duration_ms: 57.432103
      ...
    # Subtest: test out-of-range --test-coverage-branches (too low)
    ok 12 - test out-of-range --test-coverage-branches (too low)
      ---
      duration_ms: 65.894466
      ...
    1..12
    # tests 12
    # suites 0
    # pass 6
    # fail 6
    # cancelled 0
    # skipped 0
    # todo 0
    # duration_ms 1038.128642
  ...
ok 2442 parallel/test-runner-filter-warning
  ---
  duration_ms: 115.44500
  ...
ok 2443 parallel/test-runner-filetest-location
  ---
  duration_ms: 333.65200
  ...
ok 2444 parallel/test-runner-force-exit-failure
  ---
  duration_ms: 859.59200
  ...
ok 2445 parallel/test-runner-extraneous-async-activity
  ---
  duration_ms: 1446.50900
  ...
ok 2446 parallel/test-runner-inspect # skip V8 inspector is disabled
  ---
  duration_ms: 272.56800
  ...

RedYetiDev · 2024-08-22T23:03:02Z

🎉 CI passed!

nodejs-github-bot · 2024-08-23T04:39:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/61367/

nodejs-github-bot · 2024-08-23T13:36:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/61378/

nodejs-github-bot · 2024-08-23T16:53:41Z

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/.ncu

https://github.com/nodejs/node/actions/runs/10529382777

nodejs-github-bot · 2024-08-23T17:02:12Z

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

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 17, 2024

RedYetiDev force-pushed the coverage-threshold branch from b978883 to 71a0be1 Compare August 17, 2024 22:51

RedYetiDev marked this pull request as ready for review August 18, 2024 00:43

RedYetiDev added notable-change PRs with changes that should be highlighted in changelogs. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 18, 2024

mcollina approved these changes Aug 18, 2024

View reviewed changes

ShogunPanda approved these changes Aug 19, 2024

View reviewed changes

mcollina requested changes Aug 19, 2024

View reviewed changes

MoLow approved these changes Aug 20, 2024

View reviewed changes

marco-ippolito reviewed Aug 20, 2024

View reviewed changes

test/parallel/test-runner-coverage.js Outdated Show resolved Hide resolved

marco-ippolito reviewed Aug 20, 2024

View reviewed changes

test/parallel/test-runner-coverage.js Outdated Show resolved Hide resolved

cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Aug 23, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 23, 2024

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 23, 2024

cjihrig approved these changes Aug 23, 2024

View reviewed changes

cjihrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 23, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 23, 2024

nodejs-github-bot merged commit 9edf4a0 into nodejs:main Aug 23, 2024
51 checks passed

RedYetiDev mentioned this pull request Aug 24, 2024

feat(learn): Collecting code coverage nodejs/nodejs.org#7006

Open

4 tasks

RafaelGSS mentioned this pull request Aug 25, 2024

v22.8.0 proposal #54560

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test_runner: add support for coverage thresholds #54429

test_runner: add support for coverage thresholds #54429

RedYetiDev commented Aug 17, 2024 •

edited

Loading

nodejs-github-bot commented Aug 17, 2024

RedYetiDev commented Aug 17, 2024

ljharb commented Aug 17, 2024

RedYetiDev commented Aug 17, 2024

RedYetiDev commented Aug 18, 2024

github-actions bot commented Aug 18, 2024

marco-ippolito commented Aug 18, 2024

RedYetiDev commented Aug 18, 2024

mcollina left a comment

RedYetiDev commented Aug 18, 2024

marco-ippolito commented Aug 18, 2024

ShogunPanda left a comment

tniessen commented Aug 19, 2024

mcollina left a comment

RedYetiDev commented Aug 19, 2024 •

edited

Loading

MoLow commented Aug 20, 2024

MoLow left a comment

RedYetiDev commented Aug 22, 2024

RedYetiDev commented Aug 22, 2024 •

edited

Loading

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

test_runner: add support for coverage thresholds #54429

test_runner: add support for coverage thresholds #54429

Conversation

RedYetiDev commented Aug 17, 2024 • edited Loading

Notable Change

nodejs-github-bot commented Aug 17, 2024

RedYetiDev commented Aug 17, 2024

ljharb commented Aug 17, 2024

RedYetiDev commented Aug 17, 2024

RedYetiDev commented Aug 18, 2024

github-actions bot commented Aug 18, 2024

marco-ippolito commented Aug 18, 2024

RedYetiDev commented Aug 18, 2024

mcollina left a comment

Choose a reason for hiding this comment

RedYetiDev commented Aug 18, 2024

marco-ippolito commented Aug 18, 2024

ShogunPanda left a comment

Choose a reason for hiding this comment

tniessen commented Aug 19, 2024

mcollina left a comment

Choose a reason for hiding this comment

RedYetiDev commented Aug 19, 2024 • edited Loading

MoLow commented Aug 20, 2024

MoLow left a comment

Choose a reason for hiding this comment

RedYetiDev commented Aug 22, 2024

RedYetiDev commented Aug 22, 2024 • edited Loading

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

nodejs-github-bot commented Aug 23, 2024

RedYetiDev commented Aug 17, 2024 •

edited

Loading

RedYetiDev commented Aug 19, 2024 •

edited

Loading

RedYetiDev commented Aug 22, 2024 •

edited

Loading