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: fix test counting #47699

Closed
wants to merge 1 commit into from
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
2 changes: 1 addition & 1 deletion lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function setup(root) {
cancelled: 0,
skipped: 0,
todo: 0,
planned: 0,
topLevel: 0,
suites: 0,
},
};
Expand Down
21 changes: 6 additions & 15 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,11 @@ const {
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
ArrayPrototypeIndexOf,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSort,
ArrayPrototypeSplice,
Number,
ObjectAssign,
PromisePrototypeThen,
SafePromiseAll,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/test-runner/output/output_cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
11 changes: 8 additions & 3 deletions test/fixtures/test-runner/output/output_cli.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

on main, this diff would have been:

diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot
index 1d2ea1db9f..a4d54b29cd 100644
--- a/test/fixtures/test-runner/output/output_cli.snapshot
+++ b/test/fixtures/test-runner/output/output_cli.snapshot
@@ -675,7 +675,7 @@ not ok 66 - invalid subtest fail
 # Subtest: last test
-ok 2 - last test
+ok 67 - last test
   ---
   duration_ms: *
   ..

ok 67 - last test
---
duration_ms: *
...
1..67
# tests 81
# suites 0
# pass 37
# pass 38
# fail 25
# cancelled 3
# skipped 10
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/output/single.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Flags: --no-warnings
'use strict';
const test = require('node:test');
test('last test', () => {});
9 changes: 5 additions & 4 deletions test/parallel/test-runner-watch-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down