test_runner: added coverage threshold support for tests #51182

pulkit-30 · 2023-12-16T14:54:13Z

Added support for coverage threshold comparison.

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

example:

code:

import assert from "node:assert";
import test from "node:test";

test("run test", () => {
    assert.strictEqual(1,1)
})

output:

cmd: ./node --experimental-test-coverage=110 index.mjs

✔ run test (4.236583ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 32.217125
ℹ start of coverage report
ℹ ----------------------------------------------------------
ℹ file      | line % | branch % | funcs % | uncovered lines
ℹ ----------------------------------------------------------
ℹ index.mjs | 100.00 |   100.00 |  100.00 | 
ℹ ----------------------------------------------------------
ℹ all files | 100.00 |   100.00 |  100.00 |
ℹ ----------------------------------------------------------
ℹ coverage threshold for lines not met
ℹ end of coverage report

code:

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

run({ files: ['./index.test.mjs'], coverage: {
    lines: 100,
    branches: 100,
    functions: 100,
} })
  .compose(spec)
  .pipe(process.stdout);

output:

✔ hello test (10.640833ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 306.317042
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ index.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ ---------------------------------------------------------------
ℹ all files      | 100.00 |   100.00 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ end of coverage report

nodejs-github-bot · 2023-12-16T14:54:18Z

Review requested:

@nodejs/test_runner

meyfa · 2023-12-17T09:51:31Z

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.

lib/internal/test_runner/harness.js

pulkit-30 · 2023-12-18T08:05:05Z

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 👍🏼

pulkit-30 · 2023-12-19T13:10:12Z

lib/internal/test_runner/test.js

+        const failedCoverage = ArrayPrototypeFilter(
+          ObjectKeys(coverage.threshold), (key) => !coverage.threshold[key]);
+        if (failedCoverage.length > 0) {
+          console.error(`test coverage failed for ${ArrayPrototypeJoin(failedCoverage, ', ')}`);
+          process.exit(1);


here I added the logic to exit the test when coverage threshold doesn't match.

running test will be exited with status code 1.

output:

✔ hello test (0.64525ms) ✔ sub test (0.078625ms) ℹ tests 2 ℹ suites 0 ℹ pass 2 ℹ fail 0 ℹ cancelled 0 ℹ skipped 0 ℹ todo 0 ℹ duration_ms 6.242459 ℹ start of coverage report ℹ --------------------------------------------------------------- ℹ file | line % | branch % | funcs % | uncovered lines ℹ --------------------------------------------------------------- ℹ index.test.mjs | 100.00 | 100.00 | 100.00 | ℹ sub/index.mjs | 100.00 | 100.00 | 100.00 | ℹ sum/index.mjs | 100.00 | 100.00 | 100.00 | ℹ --------------------------------------------------------------- ℹ all files | 100.00 | 100.00 | 100.00 | ℹ --------------------------------------------------------------- ℹ minimum coverage failed for line. expected: 101 | actual: 100 ℹ minimum coverage failed for function. expected: 101 | actual: 100 ℹ minimum coverage failed for branch. expected: 101 | actual: 100 ℹ end of coverage report

cc @ljharb @meyfa

ljharb · 2023-12-19T15:38:54Z

It’s still missing “statements”.

pulkit-30 · 2023-12-19T17:42:56Z

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.

rluvaton · 2023-12-20T13:18:39Z

test/parallel/test-runner-run.mjs

@@ -500,3 +500,42 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
    for await (const _ of stream);
  });
 });
+
+describe('coverage report for test stream', () => {


You have 3 tests here with the same name, please rename

my bad 😞.
fixed now 🚀

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner labels Dec 16, 2023

pulkit-30 force-pushed the fix-48739 branch from 7aeb2bd to 17d653c Compare December 17, 2023 07:34

pulkit-30 marked this pull request as ready for review December 17, 2023 07:34

ljharb suggested changes Dec 17, 2023

View reviewed changes

lib/internal/test_runner/harness.js Outdated Show resolved Hide resolved

pulkit-30 force-pushed the fix-48739 branch 2 times, most recently from bfcfe72 to 52100c7 Compare December 19, 2023 13:07

pulkit-30 commented Dec 19, 2023

View reviewed changes

test_runner: added coverage threshold support for tests

bb211c1

pulkit-30 force-pushed the fix-48739 branch from 52100c7 to bb211c1 Compare December 20, 2023 10:41

fix tests

7e4adca

rluvaton reviewed Dec 20, 2023

View reviewed changes

fix test name

b2e1fe4

Nov	DEC	Jan
	24
2022	2023	2024

test_runner: added coverage threshold support for tests #51182

test_runner: added coverage threshold support for tests #51182

pulkit-30 commented Dec 16, 2023 •

edited

nodejs-github-bot commented Dec 16, 2023

meyfa commented Dec 17, 2023

ljharb left a comment

pulkit-30 commented Dec 18, 2023

pulkit-30 Dec 19, 2023 •

edited

ljharb commented Dec 19, 2023

pulkit-30 commented Dec 19, 2023 •

edited

rluvaton Dec 20, 2023

pulkit-30 Dec 20, 2023

test_runner: added coverage threshold support for tests #51182

Are you sure you want to change the base?

test_runner: added coverage threshold support for tests #51182

Conversation

pulkit-30 commented Dec 16, 2023 • edited

nodejs-github-bot commented Dec 16, 2023

meyfa commented Dec 17, 2023

ljharb left a comment

Choose a reason for hiding this comment

pulkit-30 commented Dec 18, 2023

pulkit-30 Dec 19, 2023 • edited

Choose a reason for hiding this comment

ljharb commented Dec 19, 2023

pulkit-30 commented Dec 19, 2023 • edited

rluvaton Dec 20, 2023

Choose a reason for hiding this comment

pulkit-30 Dec 20, 2023

Choose a reason for hiding this comment

pulkit-30 commented Dec 16, 2023 •

edited

pulkit-30 Dec 19, 2023 •

edited

pulkit-30 commented Dec 19, 2023 •

edited