Skip to content
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: check test coverage threshold #51370

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,94 @@ generated as part of the test runner output. If no tests are run, a coverage
report is not generated. See the documentation on
[collecting code coverage from tests][] for more details.

### `--check-coverage`

<!-- YAML
added:
- REPLACEME
-->

> Stability: 1 - Experimental

The `--check-coverage` CLI flag, used in conjunction with
the `--experimental-test-coverage` command, enforces that
the test coverage thresholds specified by check
flags are respected:

* [`--lines=threshold`][]: Sets the threshold for line coverage.
* [`--branches=threshold`][]: Sets the threshold for branch coverage.
* [`--functions=threshold`][]: Sets the threshold for function coverage.

To enforce global 100% coverage set the value of the each flag to 100:

```bash
node --test --experimental-test-coverage --check-coverage --lines=100 --branches=100 --functions=100
```
Comment on lines +913 to +917
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume some projects actually want this for one reason or another, but is it something we want to promote?

Copy link
Member Author

@marco-ippolito marco-ippolito Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume some projects actually want this for one reason or another, but is it something we want to promote?

I think that not having it means people using an external library for code coverage instead of the native one, rather than something built on top


If one of the specified coverage check falls below the threshold,
the test will exit with non zero code.

### `--lines=threshold`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that, if folks keep adding more and more flags for the test runner, they should at least be scoped, i.e., the name should be somewhat specific (e.g., --test-coverage-line-threshold) or the option should only be parsed in the context of the test runner (similar to python's -m, for example). Otherwise, we are just bloating the CLI with ambiguously named options.

Copy link
Member Author

@marco-ippolito marco-ippolito Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that, if folks keep adding more and more flags for the test runner, they should at least be scoped, i.e., the name should be somewhat specific (e.g., --test-coverage-line-threshold) or the option should only be parsed in the context of the test runner (similar to python's -m, for example). Otherwise, we are just bloating the CLI with ambiguously named options.

I do agree that flags need to be scoped but also not too long as they become hard to read when lined. Can you elaborate on how the option should be parsed in test runner context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One idea may be to let --check-coverage take a string argument with the additional options, for example:

--check-coverage=lines=80,branches=100

# or
--check-coverage=lines:80,branches:100

# or support passing multiple times, one aspect each
--check-coverage lines=80 --check-coverage branches=100

I know this increases parsing complexity and may not be aligned with other CLI flags in Node.js (though I haven't checked them all). There is, however, precedence in other CLIs such as the Docker CLI:

$ docker run --env VAR1=value1 --env VAR2=value2 ubuntu


<!-- YAML
added:
- REPLACEME
-->

> Stability: 1 - Experimental

The `--lines` CLI flag, used in conjunction with the `--check-coverage` flag,
enforces a coverage threshold check for lines of code covered by the test.
It is expressed as a numerical value between `0` and `100`,
representing the percentage (e.g., 80 for 80% coverage).
If the coverage falls below the threshold,
the test will exit with non zero code.

```bash
node --test --experimental-test-coverage --check-coverage --lines=80
```

### `--branches=threshold`

<!-- YAML
added:
- REPLACEME
-->

> Stability: 1 - Experimental

The `--branches` CLI flag, used in conjunction with the `--check-coverage` flag,
enforces a coverage threshold check for branches of code covered by the test.
It is expressed as a numerical value between `0` and `100`,
representing the percentage (e.g., 80 for 80% coverage).
If the coverage falls below the threshold,
the test will exit with non zero code.

```bash
node --test --experimental-test-coverage --check-coverage --branches=80
```

### `--functions=threshold`

<!-- YAML
added:
- REPLACEME
-->

> Stability: 1 - Experimental

The `--functions` CLI flag, used in conjunction with
the `--check-coverage`flag, enforces a coverage threshold check
for functions covered by the test.
It is expressed as a numerical value between `0` and `100`,
representing the percentage (e.g., 80 for 80% coverage).
If the coverage falls below the threshold,
the test will exit with non zero code.

```bash
node --test --experimental-test-coverage --check-coverage --functions=80
```

### `--experimental-vm-modules`

<!-- YAML
Expand Down Expand Up @@ -2945,14 +3033,17 @@ done
[`--allow-fs-read`]: #--allow-fs-read
[`--allow-fs-write`]: #--allow-fs-write
[`--allow-worker`]: #--allow-worker
[`--branches=threshold`]: #--branchesthreshold
[`--build-snapshot`]: #--build-snapshot
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--experimental-default-type=module`]: #--experimental-default-typetype
[`--experimental-sea-config`]: single-executable-applications.md#generating-single-executable-preparation-blobs
[`--experimental-wasm-modules`]: #--experimental-wasm-modules
[`--functions=threshold`]: #--functionsthreshold
[`--heap-prof-dir`]: #--heap-prof-dir
[`--import`]: #--importmodule
[`--lines=threshold`]: #--linesthreshold
[`--openssl-config`]: #--openssl-configfile
[`--preserve-symlinks`]: #--preserve-symlinks
[`--print`]: #-p---print-script
Expand Down
15 changes: 15 additions & 0 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,20 @@ if (anAlwaysFalseCondition) {
}
```

By using the CLI flag [`--check-coverage`][]
in conjunction with the `--experimental-test-coverage` flag,
it is possible to enforce specific test coverage threshold checks
(`--lines=threshold`, `--branches=threshold`, `--functions=threshold`).
When enabled, it evaluates the test coverage achieved during
the execution of tests and determines whether it meets
the specified coverage thresholds.
If the coverage check falls below the specified threshold,
the program will exit with a non zero code.

```bash
node --test --experimental-test-coverage --check-coverage --lines=100 --branches=100 --functions=100
```

### Coverage reporters

The tap and spec reporters will print a summary of the coverage statistics.
Expand Down Expand Up @@ -2966,6 +2980,7 @@ added:

[TAP]: https://testanything.org/
[TTY]: tty.md
[`--check-coverage`]: cli.md#--check-coverage
[`--experimental-test-coverage`]: cli.md#--experimental-test-coverage
[`--import`]: cli.md#--importmodule
[`--test-concurrency`]: cli.md#--test-concurrency
Expand Down
18 changes: 18 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,24 @@ Allow spawning process when using the permission model.
.It Fl -allow-worker
Allow creating worker threads when using the permission model.
.
.It Fl -check-coverage
Enforce test coverage threshold checks
when used with the
.Fl -experimental-test-coverage
flag.
.
.It Fl -lines
Enforce a minimum threshold of
lines of code covered by test coverage (0 - 100).
.
.It Fl -branches
Enforce a minimum threshold of
branches covered by test coverage (0 - 100).
.
.It Fl -functions
Enforce a minimum threshold of
functions covered by test coverage (0 - 100).
.
.It Fl -completion-bash
Print source-able bash completion script for Node.js.
.
Expand Down
24 changes: 22 additions & 2 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
FunctionPrototype,
MathMax,
Number,
NumberPrototypeToFixed,
ObjectSeal,
PromisePrototypeThen,
PromiseResolve,
Expand Down Expand Up @@ -58,6 +59,7 @@ const {
const { setTimeout } = require('timers');
const { TIMEOUT_MAX } = require('internal/timers');
const { availableParallelism } = require('os');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
const { bigint: hrtime } = process.hrtime;
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
const kCancelledByParent = 'cancelledByParent';
Expand All @@ -75,7 +77,7 @@ const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
.add('uncaughtException').add('unhandledRejection');
const { testNamePatterns, testOnlyFlag } = parseCommandLine();
const { testNamePatterns, testOnlyFlag, coverageThreshold } = parseCommandLine();
let kResistStopPropagation;

function stopTest(timeout, signal) {
Expand Down Expand Up @@ -753,7 +755,25 @@ class Test extends AsyncResource {
reporter.diagnostic(nesting, loc, `duration_ms ${this.duration()}`);

if (coverage) {
reporter.coverage(nesting, loc, coverage);
if (coverageThreshold) {
const { lines, branches, functions } = coverageThreshold;
const { coveredLinePercent, coveredBranchPercent, coveredFunctionPercent } = coverage.totals;

const checks = [
{ __proto__: null, expected: lines, actual: coveredLinePercent, type: 'Lines' },
{ __proto__: null, expected: branches, actual: coveredBranchPercent, type: 'Branches' },
{ __proto__: null, expected: functions, actual: coveredFunctionPercent, type: 'Functions' },
];

for (const { actual, expected, type } of checks) {
if (actual < expected) {
const msg = `ERROR: ${type} coverage (${NumberPrototypeToFixed(actual, 2)}%) does not meet expected threshold (${expected}%)\n`;
process.exitCode = kGenericUserError;
reporter.stderr(loc, msg);
}
}
}
reporter.coverage(nesting, loc, coverage, coverageThreshold);
}

reporter.end();
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/test_runner/tests_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,12 @@ class TestsStream extends Readable {
this[kEmitMessage]('test:stdout', { __proto__: null, message, ...loc });
}

coverage(nesting, loc, summary) {
coverage(nesting, loc, summary, coverageThreshold) {
this[kEmitMessage]('test:coverage', {
__proto__: null,
nesting,
summary,
coverageThreshold,
...loc,
});
}
Expand Down
41 changes: 41 additions & 0 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,46 @@ function parseCommandLine() {

const isTestRunner = getOptionValue('--test');
const coverage = getOptionValue('--experimental-test-coverage');
const checkCoverage = getOptionValue('--check-coverage');
let coverageThreshold;

if (checkCoverage) {
const lines = getOptionValue('--lines');

if (lines < 0 || lines > 100) {
throw new ERR_INVALID_ARG_VALUE(
'--lines',
lines,
'must be a value between 0 and 100',
);
}

const branches = getOptionValue('--branches');
if (branches < 0 || branches > 100) {
throw new ERR_INVALID_ARG_VALUE(
'--branches',
branches,
'must be a value between 0 and 100',
);
}

const functions = getOptionValue('--functions');
if (functions < 0 || functions > 100) {
throw new ERR_INVALID_ARG_VALUE(
'--functions',
functions,
'must be a value between 0 and 100',
);
}

coverageThreshold = {
__proto__: null,
lines,
branches,
functions,
};
}

const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8';
let destinations;
Expand Down Expand Up @@ -244,6 +284,7 @@ function parseCommandLine() {
__proto__: null,
isTestRunner,
coverage,
coverageThreshold,
testOnlyFlag,
testNamePatterns,
reporters,
Expand Down
12 changes: 12 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,18 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
AddOption("--experimental-test-coverage",
"enable code coverage in the test runner",
&EnvironmentOptions::test_runner_coverage);
AddOption("--check-coverage",
"check that coverage falls within the thresholds provided",
&EnvironmentOptions::test_runner_check_coverage);
AddOption("--lines",
"coverage threshold for lines of code",
&EnvironmentOptions::test_runner_check_coverage_lines);
AddOption("--branches",
"coverage threshold for code branches",
&EnvironmentOptions::test_runner_check_coverage_branches);
AddOption("--functions",
"coverage threshold for functions",
&EnvironmentOptions::test_runner_check_coverage_functions);
AddOption("--test-name-pattern",
"run tests whose name matches this regular expression",
&EnvironmentOptions::test_name_pattern);
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ class EnvironmentOptions : public Options {
uint64_t test_runner_concurrency = 0;
uint64_t test_runner_timeout = 0;
bool test_runner_coverage = false;
bool test_runner_check_coverage = false;
uint64_t test_runner_check_coverage_lines = 0;
uint64_t test_runner_check_coverage_branches = 0;
uint64_t test_runner_check_coverage_functions = 0;
std::vector<std::string> test_name_pattern;
std::vector<std::string> test_reporter;
std::vector<std::string> test_reporter_destination;
Expand Down
25 changes: 25 additions & 0 deletions test/fixtures/test-runner/output/coverage_check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Flags: --expose-internals --experimental-test-coverage --check-coverage --lines=100 --branches=100 --functions=100

'use strict';
require('../../../common');
const { TestCoverage } = require('internal/test_runner/coverage');
const { test, mock } = require('node:test');

mock.method(TestCoverage.prototype, 'summary', () => {
return {
files: [],
totals: {
totalLineCount: 100,
totalBranchCount: 100,
totalFunctionCount: 100,
coveredLineCount: 100,
coveredBranchCount: 100,
coveredFunctionCount: 100,
coveredLinePercent: 100,
coveredBranchPercent: 100,
coveredFunctionPercent: 100
}
}
});

test('ok');
23 changes: 23 additions & 0 deletions test/fixtures/test-runner/output/coverage_check.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
TAP version 13
# Subtest: ok
ok 1 - ok
---
duration_ms: *
...
1..1
# tests 1
# suites 0
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
# start of coverage report
# -----------------------------------------------------
# file | line % | branch % | funcs % | uncovered lines
# -----------------------------------------------------
# -----------------------------------------------------
# all… | 100.00 | 100.00 | 100.00 |
# -----------------------------------------------------
# end of coverage report
25 changes: 25 additions & 0 deletions test/fixtures/test-runner/output/coverage_insufficient.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Flags: --expose-internals --experimental-test-coverage --check-coverage --lines=100 --branches=100 --functions=100

'use strict';
require('../../../common');
const { TestCoverage } = require('internal/test_runner/coverage');
const { test, mock } = require('node:test');

mock.method(TestCoverage.prototype, 'summary', () => {
return {
files: [],
totals: {
totalLineCount: 0,
totalBranchCount: 0,
totalFunctionCount: 0,
coveredLineCount: 0,
coveredBranchCount: 0,
coveredFunctionCount: 0,
coveredLinePercent: 0,
coveredBranchPercent: 0,
coveredFunctionPercent: 0
}
}
});

test('ok');
Loading