test_runner: added coverage threshold support for tests

[pulkit-30]

fixes #48739

Added support for coverage threshold comparison.

1. pass coverage threshold value via cli.
2. if threshold didn;t match then log it.

example:

code:

import { test } from 'node:test';
import assert from 'node:assert';

test('running test', {}, () => {
    assert.strictEqual(1, 1);
});

output:

cmd: ./node --experimental-test-coverage --experimental-minimal-test-coverage 80 index.mjs

✔ running test (0.998708ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 8.136042
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ index.test.mjs |  77.78 |    66.67 |  100.00 | 8-9
ℹ ---------------------------------------------------------------
ℹ all files      |  77.78 |    66.67 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ minimum coverage failed for line. expected: 80% | actual: 77.78%
ℹ minimum coverage failed for branch. expected: 80% | actual: 66.67%
ℹ end of coverage report

code:

import { spec } from 'node:test/reporters';
import { run } from 'node:test';
import process from 'node:process';
import path from 'node:path';

run({
  files: [path.resolve('./index.test.mjs')],
  coverage: true,
  minimumCoverage: {
    branch: 80,
    function: 80,
    line: 80,
  },
})
  .compose(spec)
  .pipe(process.stdout);

output:

✔ running test (1.034708ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 90.49975
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ index.test.mjs |  77.78 |    66.67 |  100.00 | 8-9
ℹ ---------------------------------------------------------------
ℹ all files      |  77.78 |    66.67 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ minimum coverage failed for line. expected: 80% | actual: 77.78%
ℹ minimum coverage failed for branch. expected: 80% | actual: 66.67%
ℹ end of coverage report

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[meyfa]

I think this should also set the exit code to a nonzero value such that CI systems will mark the job as failed.

[ljharb]

I agree on the exit code; also, coverage always has 4 metrics, lines, functions, branches, and statements.

[pulkit-30]

I think this should also set the exit code to a nonzero value such that CI systems will mark the job as failed.
I agree on the exit code;

working on it 👍🏼

[ljharb]

It’s still missing “statements”.

[pulkit-30]

It’s still missing “statements”.

I think we don't collect statement coverage summary.
https://github.com/nodejs/node/blob/main/lib/internal/test_runner/coverage.js#L241-L263

@MoLow please confirm about statement summary here and how can we support that.

[pulkit-30]

PTAL @nodejs/test_runner

[atlowChemi]

Overall LGTM, should add documentation though

[pulkit-30]

Overall LGTM, should add documentation though

Thanks for review 🙌🏼 . will add documentation.

It’s still missing “statements”.

I think we don't collect statement coverage summary. https://github.com/nodejs/node/blob/main/lib/internal/test_runner/coverage.js#L241-L263

please confirm about statement summary here and how can we support that.

@atlowChemi Any idea about this?

[tniessen]

There's a reason we don't use such a threshold in this repository -- it tends to be a fragile indicator.

On a side note, the first commit message does not adhere to our requirements and must be amended.

[tniessen]

The documentation doesn't say anything about what coverage metric is meant here.

[ljharb]

Indeed; typically in code coverage there are 4 metrics (statements remains missing), and all 4 are independently able to be set to a threshold.

[benjamingr]

Really good work, I'm not convinced we need/want the feature but the code and everyting else looks solid <3

[pulkit-30]

Really good work, I'm not convinced we need/want the feature but the code and everyting else looks solid <3

Thanks @benjamingr,
I see this feature request. I don't want to force the inclusion of this feature, but I'm curious if there's an alternative method to ensure 100% code coverage.

Additionally, it might be beneficial to consider adding a coverage option within the test_runner/run() function. WDYT @nodejs/test_runner

[tniessen]

I'm curious if there's an alternative method to ensure 100% code coverage.

FWIW, Node.js could always emit coverage data in some standardized format that could then be consumed by a different tool (e.g., codecov or so).

Also, "100% code coverage" is a somewhat odd metric. It by no means guarantees the absence of bugs yet is almost impossible to achieve in many cases.

[MoLow]

FWIW, Node.js could always emit coverage data in some standardized format that could then be consumed by a different tool (e.g., codecov or so).

node already does that https://nodejs.org/api/test.html#event-testcoverage

[cjihrig]

This appears to have stalled out and now has conflicts. I'm going to close this for now.