test_runner: added coverage support with watch mode

[pulkit-30]

fix #51253

code:

with following test files

// test1.test.mjs
import { test } from 'node:test';

test('test1', () => {})

// test2.test.mjs
import { test } from 'node:test';

test('test2', () => {})

run node --test --experimental-test-coverage --watch

output:

✔ test1 (1.952625ms)
✔ test2 (2.08075ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 228.890917
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ test1.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ test2.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ ---------------------------------------------------------------
ℹ all files      | 100.00 |   100.00 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ end of coverage report

• when i update test2.test.mjs

✔ test1 (1.952625ms)
✔ test2 (2.08075ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 228.890917
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ test1.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ test2.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ ---------------------------------------------------------------
ℹ all files      | 100.00 |   100.00 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ end of coverage report
✔ test2 (3.161208ms)
ℹ start of coverage report
ℹ ---------------------------------------------------------------
ℹ file           | line % | branch % | funcs % | uncovered lines
ℹ ---------------------------------------------------------------
ℹ test2.test.mjs | 100.00 |   100.00 |  100.00 | 
ℹ ---------------------------------------------------------------
ℹ all files      | 100.00 |   100.00 |  100.00 |
ℹ ---------------------------------------------------------------
ℹ end of coverage report

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[MoLow]

I propose this implementation, which is a little simpler:

diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 2f18b0bcf0..29dc7ca495 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -177,6 +177,7 @@ function setup(root) {
   root.harness = {
     __proto__: null,
     bootstrapComplete: false,
+    watchMode: false,
     coverage: FunctionPrototypeBind(collectCoverage, null, root, coverage),
     counters: {
       __proto__: null,
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index c4194923cc..810a3ee13e 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -78,7 +78,7 @@ const {
   exitCodes: { kGenericUserError },
 } = internalBinding('errors');
 
-const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
+const kFilterArgs = new SafeSet(['--test', '--experimental-test-coverage', '--watch']);
 const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
 const kDiagnosticsFilterArgs = ['tests', 'suites', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
 
@@ -107,7 +107,7 @@ function createTestFileList() {
 }
 
 function filterExecArgv(arg, i, arr) {
-  return !ArrayPrototypeIncludes(kFilterArgs, arg) &&
+  return !kFilterArgs.has(arg) &&
   !ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
 }
 
@@ -425,11 +425,10 @@ function watchFiles(testFiles, opts) {
   });
   if (opts.signal) {
     kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
-    opts.signal.addEventListener(
-      'abort',
-      () => opts.root.postRun(),
-      { __proto__: null, once: true, [kResistStopPropagation]: true },
-    );
+    opts.signal.addEventListener('abort', () => {
+      opts.root.postRun();
+      opts.root.reporter.end();
+    }, { __proto__: null, once: true, [kResistStopPropagation]: true });
   }
 
   return filesWatcher;
@@ -497,8 +496,12 @@ function run(options = kEmptyObject) {
   let filesWatcher;
   const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only };
   if (watch) {
+    root.harness.watchMode = true;
     filesWatcher = watchFiles(testFiles, opts);
-    postRun = undefined;
+    postRun = () => {
+      kFilterArgs.delete('--experimental-test-coverage');
+      root.postRun();
+    };
   }
   const runFiles = () => {
     root.harness.bootstrapComplete = true;
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 4eb02c5d4e..b28d00dce2 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -756,7 +756,9 @@ class Test extends AsyncResource {
         reporter.coverage(nesting, loc, coverage);
       }
 
-      reporter.end();
+      if (harness.watchMode === false) {
+        reporter.end();
+      }
     }
   }

[pulkit-30]

yeah looks clean 👍🏼

[nodejs-github-bot]

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

[rluvaton]

Left some comments, great job

[nodejs-github-bot]

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

[atlowChemi]

@pulkit-30 the failure is related to the changes:

not ok 2474 parallel/test-runner-watch-coverage
  ---
  duration_ms: 2215.81000
  severity: fail
  exitcode: 1
  stack: |-
    TAP version 13
    # Subtest: test runner must report coverage report with watch mode
        # Subtest: should report coverage report with watch mode
        not ok 1 - should report coverage report with watch mode
          ---
          duration_ms: 1797.885443
          location: 'file:///home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-watch-coverage.mjs:80:3'
          failureType: 'testCodeFailure'
          error: |-
            The expression evaluated to a falsy value:
            
              assert(stdout.includes(getCoverageFixtureReport('tap')))
            
          code: 'ERR_ASSERTION'
          name: 'AssertionError'
          expected: true
          actual: false
          operator: '=='
          stack: |-
            TestContext.<anonymous> (file:///home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-watch-coverage.mjs:82:5)
            process.processTicksAndRejections (node:internal/process/task_queues:95:5)
            async Test.run (node:internal/test_runner/test:632:9)
            async Promise.all (index 0)
            async Suite.run (node:internal/test_runner/test:951:7)
            async startSubtest (node:internal/test_runner/harness:219:3)
          ...
        1..1
    not ok 1 - test runner must report coverage report with watch mode
      ---
      duration_ms: 1801.223591
      type: 'suite'
      location: 'file:///home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-runner-watch-coverage.mjs:79:1'
      failureType: 'subtestsFailed'
      error: '1 subtest failed'
      code: 'ERR_TEST_FAILURE'
      ...
    1..1
    # tests 1
    # suites 1
    # pass 0
    # fail 1
    # cancelled 0
    # skipped 0
    # todo 0
    # duration_ms 1826.192055
  ...

[nodejs-github-bot]

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

[MoLow]

there are still 2 related failures:
https://ci.nodejs.org/job/node-test-binary-windows-js-suites/25342/RUN_SUBSET=1,nodes=win2022-COMPILED_BY-vs2022/
https://ci.nodejs.org/job/node-test-commit-linux-containered/41219/nodes=ubuntu2204_sharedlibs_withoutssl_x64/

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[cjihrig]

The linked issue is now closed, so I'll go ahead and close this as well. Thanks for the PR though.