Skip to content

Commit

Permalink
test_runner: report failing tests after summary
Browse files Browse the repository at this point in the history
Re-output failing tests after summary has been printed.
This behavior follows other popular test runners (e.g. jest, mocha, etc...).

Updates SpecReporter:
1. When there is a 'test:fail' event, the failed test will be stored.
2. Introduce a new 'test:eot' event in which all the failed tests will be dumped.

Fixes: #47110
  • Loading branch information
HinataKah0 committed Mar 19, 2023
1 parent 30d92e8 commit 98fa308
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 6 deletions.
18 changes: 16 additions & 2 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
ArrayPrototypeJoin,
ArrayPrototypePop,
ArrayPrototypePush,
ArrayPrototypeShift,
ArrayPrototypeUnshift,
hardenRegExp,
Expand Down Expand Up @@ -36,6 +37,7 @@ class SpecReporter extends Transform {
#stack = [];
#reported = [];
#indentMemo = new SafeMap();
#failedTests = [];

constructor() {
super({ writableObjectMode: true });
Expand All @@ -57,15 +59,16 @@ class SpecReporter extends Transform {
RegExpPrototypeSymbolSplit(
hardenRegExp(/\r?\n/),
inspectWithNoCustomRetry(err, inspectOptions),
), `\n${indent} `);
return `\n${indent} ${message}\n`;
), indent !== undefined ? `\n${indent} ` : '');
return `${indent !== undefined ? `\n${indent} ` : ''}${message}\n`;
}
#handleEvent({ type, data }) {
let color = colors[type] ?? white;
let symbol = symbols[type] ?? ' ';

switch (type) {
case 'test:fail':
ArrayPrototypePush(this.#failedTests, data);
case 'test:pass': {
const subtest = ArrayPrototypeShift(this.#stack); // This is the matching `test:start` event
if (subtest) {
Expand Down Expand Up @@ -103,6 +106,17 @@ class SpecReporter extends Transform {
break;
case 'test:diagnostic':
return `${color}${this.#indent(data.nesting)}${symbol}${data.message}${white}\n`;
case 'test:eot':
color = colors['test:fail'];
symbol = symbols['test:fail'];
let results = [];
for (let i = 0; i < this.#failedTests.length; i++) {
const failed = this.#failedTests[i];
const duration_ms = failed.details?.duration_ms ? ` ${gray}(${failed.details.duration_ms}ms)${white}` : '';
const result = `${color}${symbol}${failed.file} ${failed.name}${duration_ms}\n${this.#formatError(failed.details?.error)}`;
ArrayPrototypePush(results, result);
}
return ArrayPrototypeJoin(results, '');
}
}
_transform({ type, data }, encoding, callback) {
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,8 @@ class Test extends AsyncResource {
this.reporter.coverage(this.nesting, kFilename, this.harness.coverage);
}

this.reporter.endOfTests();

this.reporter.push(null);
}
}
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/test_runner/tests_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class TestsStream extends Readable {
this.#emit('test:start', { __proto__: null, nesting, file, name });
}

endOfTests() {
this.#emit('test:eot', null);
}

diagnostic(nesting, file, message) {
this.#emit('test:diagnostic', { __proto__: null, nesting, file, message });
}
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/test-runner/custom_reporters/custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path');
module.exports = async function * customReporter(source) {
const counters = {};
for await (const event of source) {
if (event.data.file) {
if (event.data?.file) {
assert.strictEqual(event.data.file, path.resolve(__dirname, '../reporters.js'));
}
counters[event.type] = (counters[event.type] ?? 0) + 1;
Expand Down
60 changes: 60 additions & 0 deletions test/message/test_runner_output_spec_reporter.out
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,63 @@
skipped 10
todo 5
duration_ms *
* sync fail todo (*ms)
*
* sync fail todo with message (*ms)
*
* sync throw fail (*ms)
*
* async throw fail (*ms)
*
* async skip fail (*ms)
*
* async assertion fail (*ms)
*
* reject fail (*ms)
*
* +sync throw fail (*ms)
*
* subtest sync throw fail (*ms)
'1 subtest failed'
* sync throw non-error fail (*ms)
Symbol(thrown symbol from sync throw non-error fail)
* +long running (*ms)
'test did not finish before its parent and was cancelled'
* top level (*ms)
'1 subtest failed'
* sync skip option is false fail (*ms)
*
* callback fail (*ms)
*
* callback also returns a Promise (*ms)
'passed a callback but also returned a Promise'
* callback throw (*ms)
*
* callback called twice (*ms)
'callback invoked multiple times'
* callback called twice in future tick (*ms)
*
* callback async throw (*ms)
*
* custom inspect symbol fail (*ms)
customized
* custom inspect symbol that throws fail (*ms)
{ foo: 1, [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]] }
* sync throw fails at first (*ms)
*
* sync throw fails at second (*ms)
*
* subtest sync throw fails (*ms)
'2 subtests failed'
* timed out async test (*ms)
'test timed out after 5ms'
* timed out callback test (*ms)
'test timed out after 5ms'
* rejected thenable (*ms)
'custom error'
* unfinished test with uncaughtException (*ms)
*
* unfinished test with unhandledRejection (*ms)
*
* invalid subtest fail (*ms)
'test could not be started because its parent finished'
6 changes: 3 additions & 3 deletions test/parallel/test-runner-reporters.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('node:test reporters', { concurrency: true }, () => {
testFile]);
assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /{"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/);
assert.match(stdout, /{"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+,"test:eot":1}$/);
assert.strictEqual(stdout.slice(0, filename.length + 2), `${filename} {`);
});
});
Expand All @@ -102,7 +102,7 @@ describe('node:test reporters', { concurrency: true }, () => {
assert.strictEqual(child.stderr.toString(), '');
assert.match(
child.stdout.toString(),
/^package: reporter-cjs{"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+}$/,
/^package: reporter-cjs{"test:start":4,"test:pass":2,"test:fail":2,"test:plan":2,"test:diagnostic":\d+,"test:eot":1}$/,
);
});

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

Expand Down
2 changes: 2 additions & 0 deletions test/pseudo-tty/test_runner_default_reporter.out
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
[34m* skipped 1[39m
[34m* todo 0[39m
[34m* duration_ms *[39m
[31m* should fail [90m(*ms)[39m
*

0 comments on commit 98fa308

Please sign in to comment.