From 98fa308683f22171fe27ba7c3235aedc069451de Mon Sep 17 00:00:00 2001 From: Kaho Date: Sat, 18 Mar 2023 15:10:01 +0800 Subject: [PATCH] test_runner: report failing tests after summary 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: https://github.com/nodejs/node/issues/47110 --- lib/internal/test_runner/reporter/spec.js | 18 +++++- lib/internal/test_runner/test.js | 2 + lib/internal/test_runner/tests_stream.js | 4 ++ .../test-runner/custom_reporters/custom.js | 2 +- .../test_runner_output_spec_reporter.out | 60 +++++++++++++++++++ test/parallel/test-runner-reporters.js | 6 +- .../test_runner_default_reporter.out | 2 + 7 files changed, 88 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index 3637c74111c4b7..703cae338b38ac 100644 --- a/lib/internal/test_runner/reporter/spec.js +++ b/lib/internal/test_runner/reporter/spec.js @@ -3,6 +3,7 @@ const { ArrayPrototypeJoin, ArrayPrototypePop, + ArrayPrototypePush, ArrayPrototypeShift, ArrayPrototypeUnshift, hardenRegExp, @@ -36,6 +37,7 @@ class SpecReporter extends Transform { #stack = []; #reported = []; #indentMemo = new SafeMap(); + #failedTests = []; constructor() { super({ writableObjectMode: true }); @@ -57,8 +59,8 @@ 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; @@ -66,6 +68,7 @@ class SpecReporter extends Transform { 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) { @@ -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) { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index ffbf2d257aed62..ac94ca9b38c859 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -655,6 +655,8 @@ class Test extends AsyncResource { this.reporter.coverage(this.nesting, kFilename, this.harness.coverage); } + this.reporter.endOfTests(); + this.reporter.push(null); } } diff --git a/lib/internal/test_runner/tests_stream.js b/lib/internal/test_runner/tests_stream.js index c32d57d3b4cf71..fe81606aba86a8 100644 --- a/lib/internal/test_runner/tests_stream.js +++ b/lib/internal/test_runner/tests_stream.js @@ -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 }); } diff --git a/test/fixtures/test-runner/custom_reporters/custom.js b/test/fixtures/test-runner/custom_reporters/custom.js index aa85eab14acff4..0f7e748d2225ee 100644 --- a/test/fixtures/test-runner/custom_reporters/custom.js +++ b/test/fixtures/test-runner/custom_reporters/custom.js @@ -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; diff --git a/test/message/test_runner_output_spec_reporter.out b/test/message/test_runner_output_spec_reporter.out index 3ff9aefe7a42ce..fe3e2fce8a76af 100644 --- a/test/message/test_runner_output_spec_reporter.out +++ b/test/message/test_runner_output_spec_reporter.out @@ -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' diff --git a/test/parallel/test-runner-reporters.js b/test/parallel/test-runner-reporters.js index 81074abc9fc838..840a116c59d9ac 100644 --- a/test/parallel/test-runner-reporters.js +++ b/test/parallel/test-runner-reporters.js @@ -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} {`); }); }); @@ -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}$/, ); }); @@ -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}$/, ); }); diff --git a/test/pseudo-tty/test_runner_default_reporter.out b/test/pseudo-tty/test_runner_default_reporter.out index 795b7e556d13d8..ec7fa16c742c39 100644 --- a/test/pseudo-tty/test_runner_default_reporter.out +++ b/test/pseudo-tty/test_runner_default_reporter.out @@ -17,3 +17,5 @@ [34m* skipped 1[39m [34m* todo 0[39m [34m* duration_ms *[39m +[31m* should fail [90m(*ms)[39m +*