From 2d7cac0c5b42b601426e0aa6265e8b0b3738d256 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 24 Apr 2023 15:04:59 +0300 Subject: [PATCH] test_runner: fix test counting PR-URL: https://github.com/nodejs/node/pull/47675 Fixes: https://github.com/nodejs/node/issues/47365 Fixes: https://github.com/nodejs/node/issues/47696 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- lib/internal/test_runner/harness.js | 2 +- lib/internal/test_runner/runner.js | 21 ++++++------------- lib/internal/test_runner/test.js | 2 +- lib/internal/test_runner/utils.js | 2 +- .../fixtures/test-runner/output/output_cli.js | 2 +- .../test-runner/output/output_cli.snapshot | 11 +++++++--- test/fixtures/test-runner/output/single.js | 4 ++++ test/parallel/test-runner-watch-mode.mjs | 9 ++++---- 8 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 test/fixtures/test-runner/output/single.js diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 7457ca62ef52be..a501efacdfa92d 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -176,7 +176,7 @@ function setup(root) { cancelled: 0, skipped: 0, todo: 0, - planned: 0, + topLevel: 0, suites: 0, }, }; diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2e9395ea61c12c..0c31e75f70c455 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -5,14 +5,11 @@ const { ArrayPrototypeFilter, ArrayPrototypeForEach, ArrayPrototypeIncludes, - ArrayPrototypeIndexOf, ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSlice, ArrayPrototypeSome, ArrayPrototypeSort, - ArrayPrototypeSplice, - Number, ObjectAssign, PromisePrototypeThen, SafePromiseAll, @@ -207,7 +204,7 @@ class FileTest extends Test { const diagnostics = YAMLToJs(node.diagnostics); const cancelled = kCanceledTests.has(diagnostics.error?.failureType); - const testNumber = nesting === 0 ? (Number(node.id) + this.testNumber - 1) : node.id; + const testNumber = nesting === 0 ? (this.root.harness.counters.topLevel + 1) : node.id; const method = pass ? 'ok' : 'fail'; this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive); if (nesting === 0) { @@ -335,17 +332,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { throw err; } }); - const promise = subtest.start(); - if (filesWatcher) { - return PromisePrototypeThen(promise, () => { - const index = ArrayPrototypeIndexOf(root.subtests, subtest); - if (index !== -1) { - ArrayPrototypeSplice(root.subtests, index, 1); - root.waitingOn--; - } - }); - } - return promise; + return subtest.start(); } function watchFiles(testFiles, root, inspectPort, testNamePatterns) { @@ -361,6 +348,10 @@ function watchFiles(testFiles, root, inspectPort, testNamePatterns) { runningProcess.kill(); await once(runningProcess, 'exit'); } + if (!runningSubtests.size) { + // Reset the topLevel counter + root.harness.counters.topLevel = 0; + } await runningSubtests.get(file); runningSubtests.set(file, runTestFile(file, root, inspectPort, filesWatcher, testNamePatterns)); }, undefined, (error) => { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 45b3c39e013a0d..8388a1797f1808 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -637,7 +637,7 @@ class Test extends AsyncResource { this.parent.processPendingSubtests(); } else if (!this.reported) { this.reported = true; - this.reporter.plan(this.nesting, kFilename, this.root.harness.counters.planned); + this.reporter.plan(this.nesting, kFilename, this.root.harness.counters.topLevel); for (let i = 0; i < this.diagnostics.length; i++) { this.reporter.diagnostic(this.nesting, kFilename, this.diagnostics[i]); diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index e97f92484b8b8d..0b7cbc1de73cab 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -227,7 +227,7 @@ function parseCommandLine() { function countCompletedTest(test, harness = test.root.harness) { if (test.nesting === 0) { - harness.counters.planned++; + harness.counters.topLevel++; } if (test.reportedType === 'suite') { harness.counters.suites++; diff --git a/test/fixtures/test-runner/output/output_cli.js b/test/fixtures/test-runner/output/output_cli.js index 3a90d7c1991709..50ef07233314b1 100644 --- a/test/fixtures/test-runner/output/output_cli.js +++ b/test/fixtures/test-runner/output/output_cli.js @@ -5,5 +5,5 @@ const fixtures = require('../../../common/fixtures'); const spawn = require('node:child_process').spawn; spawn(process.execPath, - ['--no-warnings', '--test', '--test-reporter', 'tap', fixtures.path('test-runner/output/output.js')], + ['--no-warnings', '--test', '--test-reporter', 'tap', fixtures.path('test-runner/output/output.js'), fixtures.path('test-runner/output/single.js')], { stdio: 'inherit' }); diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot index 5aba59661451a9..a4d54b29cd7778 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -672,10 +672,15 @@ not ok 66 - invalid subtest fail # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -1..66 -# tests 80 +# Subtest: last test +ok 67 - last test + --- + duration_ms: * + ... +1..67 +# tests 81 # suites 0 -# pass 37 +# pass 38 # fail 25 # cancelled 3 # skipped 10 diff --git a/test/fixtures/test-runner/output/single.js b/test/fixtures/test-runner/output/single.js new file mode 100644 index 00000000000000..e099ec3c375bb7 --- /dev/null +++ b/test/fixtures/test-runner/output/single.js @@ -0,0 +1,4 @@ +// Flags: --no-warnings +'use strict'; +const test = require('node:test'); +test('last test', () => {}); diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index 1fbc7a7c584e92..e7f17da8e5e142 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -14,13 +14,14 @@ async function testWatch({ files, fileToUpdate }) { child.stdout.on('data', (data) => { stdout += data.toString(); - const matches = stdout.match(/test has ran/g); - if (matches?.length >= 1) ran1.resolve(); - if (matches?.length >= 2) ran2.resolve(); + const testRuns = stdout.match(/ - test has ran/g); + if (testRuns?.length >= 1) ran1.resolve(); + if (testRuns?.length >= 2) ran2.resolve(); }); await ran1.promise; - const interval = setInterval(() => writeFileSync(fileToUpdate, readFileSync(fileToUpdate, 'utf8')), 50); + const content = readFileSync(fileToUpdate, 'utf8'); + const interval = setInterval(() => writeFileSync(fileToUpdate, content), 10); await ran2.promise; clearInterval(interval); child.kill();