Skip to content

Commit

Permalink
test_runner: add 'test:summary' event
Browse files Browse the repository at this point in the history
This commit adds a new 'test:summary' event to the test runner's
reporting interface. This new event serves two purposes:

- The test runner internals no longer change the process exit
  code. This may be important to run() users.
- The reporting interface now has a single event that can identify
  passing or failing test runs.

Refs: nodejs#53867
Refs: nodejs#54812
  • Loading branch information
cjihrig committed Sep 8, 2024
1 parent 9404d3a commit b5254f9
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 13 deletions.
4 changes: 2 additions & 2 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ if (isUsingInspector() && options.isolation === 'process') {
options.globPatterns = ArrayPrototypeSlice(process.argv, 1);

debug('test runner configuration:', options);
run(options).on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
run(options).on('test:summary', (data) => {
if (!data.success) {
process.exitCode = kGenericUserError;
}
});
15 changes: 9 additions & 6 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function createTestTree(rootTestOptions, globalOptions) {
suites: 0,
};
},
success: true,
counters: null,
shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
teardown: null,
Expand Down Expand Up @@ -124,7 +125,7 @@ function createProcessEventHandler(eventName, rootTest) {
}

rootTest.diagnostic(msg);
process.exitCode = kGenericUserError;
rootTest.harness.success = false;
return;
}

Expand All @@ -146,7 +147,7 @@ function configureCoverage(rootTest, globalOptions) {
const msg = `Warning: Code coverage could not be enabled. ${err}`;

rootTest.diagnostic(msg);
process.exitCode = kGenericUserError;
rootTest.harness.success = false;
}
}

Expand All @@ -165,7 +166,7 @@ function collectCoverage(rootTest, coverage) {
const msg = `Warning: Could not ${op} code coverage. ${err}`;

rootTest.diagnostic(msg);
process.exitCode = kGenericUserError;
rootTest.harness.success = false;
}

return summary;
Expand Down Expand Up @@ -239,14 +240,16 @@ function lazyBootstrapRoot() {
if (!globalRoot) {
// This is where the test runner is bootstrapped when node:test is used
// without the --test flag or the run() API.
const entryFile = process.argv?.[1];
const rootTestOptions = {
__proto__: null,
entryFile: process.argv?.[1],
entryFile,
loc: entryFile ? [1, 1, entryFile] : undefined,
};
const globalOptions = parseCommandLine();
createTestTree(rootTestOptions, globalOptions);
globalRoot.reporter.on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
globalRoot.reporter.on('test:summary', (data) => {
if (!data.success) {
process.exitCode = kGenericUserError;
}
});
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const {
SymbolDispose,
} = primordials;
const { getCallerLocation } = internalBinding('util');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
const { addAbortListener } = require('internal/events/abort_listener');
const { queueMicrotask } = require('internal/process/task_queues');
const { AsyncResource } = require('async_hooks');
Expand Down Expand Up @@ -1026,12 +1025,14 @@ class Test extends AsyncResource {
for (let i = 0; i < coverages.length; i++) {
const { threshold, actual, name } = coverages[i];
if (actual < threshold) {
process.exitCode = kGenericUserError;
harness.success = false;
reporter.diagnostic(nesting, loc, `Error: ${NumberPrototypeToFixed(actual, 2)}% ${name} coverage does not meet threshold of ${threshold}%.`);
}
}
}

reporter.summary(nesting, loc, harness.success, harness.counters);

if (harness.watching) {
this.reported = false;
harness.resetCounters();
Expand Down
9 changes: 9 additions & 0 deletions lib/internal/test_runner/tests_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,15 @@ class TestsStream extends Readable {
});
}

summary(nesting, loc, success, counts) {
this[kEmitMessage]('test:summary', {
__proto__: null,
success,
counts,
...loc,
});
}

end() {
this.#tryPush(null);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,10 @@ function countCompletedTest(test, harness = test.root.harness) {
harness.counters.todo++;
} else if (test.cancelled) {
harness.counters.cancelled++;
harness.success = false;
} else if (!test.passed) {
harness.counters.failed++;
harness.success = false;
} else {
harness.counters.passed++;
}
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-runner-reporters.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('node:test reporters', { concurrency: true }, () => {
testFile]);
assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/);
assert.match(stdout, /{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:summary":2,"test:diagnostic":\d+}$/);
assert.strictEqual(stdout.slice(0, filename.length + 2), `${filename} {`);
});
});
Expand All @@ -125,7 +125,7 @@ describe('node:test reporters', { concurrency: true }, () => {
assert.strictEqual(child.stderr.toString(), '');
assert.match(
child.stdout.toString(),
/^package: reporter-cjs{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/,
/^package: reporter-cjs{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:summary":2,"test:diagnostic":\d+}$/,
);
});

Expand All @@ -136,7 +136,7 @@ describe('node:test reporters', { concurrency: true }, () => {
assert.strictEqual(child.stderr.toString(), '');
assert.match(
child.stdout.toString(),
/^package: reporter-esm{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/,
/^package: reporter-esm{"test:enqueue":5,"test:dequeue":5,"test:complete":5,"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:summary":2,"test:diagnostic":\d+}$/,
);
});

Expand Down

0 comments on commit b5254f9

Please sign in to comment.