diff --git a/lib/internal/test_runner/reporter/junit.js b/lib/internal/test_runner/reporter/junit.js index e5a3fa961eac..40dad3fa5fb8 100644 --- a/lib/internal/test_runner/reporter/junit.js +++ b/lib/internal/test_runner/reporter/junit.js @@ -16,21 +16,33 @@ const { } = primordials; const { inspectWithNoCustomRetry } = require('internal/errors'); +const { stripVTControlCharacters } = require('internal/util/inspect'); const { hostname } = require('os'); const inspectOptions = { __proto__: null, colors: false, breakLength: Infinity }; const HOSTNAME = hostname(); +// Characters that are not allowed anywhere in an XML 1.0 document, not even +// as character references: C0 controls other than tab, LF and CR, lone +// surrogates, and U+FFFE/U+FFFF. +const kInvalidXMLChars = /[^\t\n\r\x20-\uD7FF\uE000-\uFFFD\u{10000}-\u{10FFFF}]/gu; + +function sanitize(s = '') { + // Drop ANSI escape sequences entirely (e.g. colored assertion diffs), then + // replace any remaining illegal characters with U+FFFD. + return RegExpPrototypeSymbolReplace(kInvalidXMLChars, stripVTControlCharacters(s), '\uFFFD'); +} + function escapeAttribute(s = '') { return RegExpPrototypeSymbolReplace(/"/g, RegExpPrototypeSymbolReplace(/\n/g, escapeContent(s), ' '), '"'); } function escapeContent(s = '') { - return RegExpPrototypeSymbolReplace(/ c.tag === 'skipped')) || node?.attrs?.skipped; } +function setSuiteAttrs(node, tests) { + node.tag = 'testsuite'; + node.attrs.disabled = 0; + node.attrs.errors = 0; + node.attrs.tests = tests; + node.attrs.failures = ArrayPrototypeFilter(node.children, isFailure).length; + node.attrs.skipped = ArrayPrototypeFilter(node.children, isSkipped).length; +} + +// Give a test that started but never reported a result (e.g. because a test +// child process exited before all of its output was read) a valid element. +// A leaf is reported as a failure so that lost results don't look green. +function closeIncomplete(node) { + const tests = ArrayPrototypeFilter(node.children, (c) => c.comment == null).length; + if (tests > 0) { + setSuiteAttrs(node, tests); + node.attrs.hostname = HOSTNAME; + return; + } + const message = 'test did not report a result'; + node.tag = 'testcase'; + node.attrs.classname = 'test'; + node.attrs.failure = message; + ArrayPrototypePush(node.children, { + __proto__: null, nesting: node.nesting + 1, tag: 'failure', + attrs: { __proto__: null, type: 'incomplete', message }, + }); +} + module.exports = async function* junitReporter(source) { yield '\n'; yield '\n'; let currentSuite = null; const roots = []; + // Close any unfinished tests nested at `nesting` or deeper, so that later + // results are not attached to a test whose own result never arrived. + function closeUnfinished(nesting) { + while (currentSuite != null && currentSuite.nesting >= nesting) { + closeIncomplete(currentSuite); + currentSuite = currentSuite.parent; + } + } + function startTest(event) { + closeUnfinished(event.data.nesting); const originalSuite = currentSuite; currentSuite = { __proto__: null, @@ -95,11 +146,11 @@ module.exports = async function* junitReporter(source) { } case 'test:pass': case 'test:fail': { - if (!currentSuite) { - startTest({ __proto__: null, data: { __proto__: null, name: 'root', nesting: 0 } }); - } - if (currentSuite.attrs.name !== event.data.name || + closeUnfinished(event.data.nesting + 1); + if (currentSuite == null || + currentSuite.attrs.name !== event.data.name || currentSuite.nesting !== event.data.nesting) { + // No matching test:start was seen; start the test now. startTest(event); } const currentTest = currentSuite; @@ -109,12 +160,7 @@ module.exports = async function* junitReporter(source) { currentTest.attrs.time = NumberPrototypeToFixed(event.data.details.duration_ms / 1000, 6); const nonCommentChildren = ArrayPrototypeFilter(currentTest.children, (c) => c.comment == null); if (nonCommentChildren.length > 0) { - currentTest.tag = 'testsuite'; - currentTest.attrs.disabled = 0; - currentTest.attrs.errors = 0; - currentTest.attrs.tests = nonCommentChildren.length; - currentTest.attrs.failures = ArrayPrototypeFilter(currentTest.children, isFailure).length; - currentTest.attrs.skipped = ArrayPrototypeFilter(currentTest.children, isSkipped).length; + setSuiteAttrs(currentTest, nonCommentChildren.length); // A suite's `test:start` is emitted lazily (when its first subtest // reports), so derive the start time from the end minus the measured // duration rather than stamping the (late) test:start moment. @@ -165,6 +211,7 @@ module.exports = async function* junitReporter(source) { break; } } + closeUnfinished(0); for (const suite of roots) { yield treeToXML(suite); } diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 1b0956e4253c..b28bbd631fb1 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1525,6 +1525,8 @@ class Test extends AsyncResource { destination.on('unpipe', () => { if (!destination.closed && typeof destination.close === 'function') { destination.close(resolve); + } else if (destination.writableLength > 0) { + destination.write('', resolve); } else { resolve(); } diff --git a/test/fixtures/test-runner/junit-colored-diff.js b/test/fixtures/test-runner/junit-colored-diff.js new file mode 100644 index 000000000000..6119f6f41cdd --- /dev/null +++ b/test/fixtures/test-runner/junit-colored-diff.js @@ -0,0 +1,5 @@ +'use strict'; +const assert = require('node:assert'); +const { test } = require('node:test'); + +test('diff', () => assert.deepStrictEqual({ a: 1 }, { a: 2 })); diff --git a/test/parallel/test-runner-force-exit-stdout-drain.js b/test/parallel/test-runner-force-exit-stdout-drain.js new file mode 100644 index 000000000000..4fe012321099 --- /dev/null +++ b/test/parallel/test-runner-force-exit-stdout-drain.js @@ -0,0 +1,51 @@ +'use strict'; +// With --test-force-exit, buffered reporter output on stdout (both in test +// child processes and in the main runner) must be flushed before exiting. +// Refs: https://github.com/nodejs/node/issues/67149 +require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('node:assert'); +const { spawn } = require('node:child_process'); +const { writeFileSync } = require('node:fs'); +const { once } = require('node:events'); +const { setTimeout } = require('node:timers/promises'); +const { test } = require('node:test'); + +const kFiles = 8; +const kTestsPerFile = 300; + +tmpdir.refresh(); +const files = []; +for (let i = 0; i < kFiles; i++) { + const file = tmpdir.resolve(`t${i}.test.js`); + writeFileSync(file, `'use strict'; +const { test } = require('node:test'); +for (let i = 0; i < ${kTestsPerFile}; i++) { + test('test ' + i + ' ' + 'x'.repeat(1000), () => {}); +} +`); + files.push(file); +} + +test('no results are lost when stdout is read slowly', async () => { + const child = spawn(process.execPath, [ + '--test', + '--test-force-exit', + `--test-concurrency=${kFiles}`, + '--test-reporter=junit', + ...files, + ], { cwd: tmpdir.path }); + + // Let the pipes back up before reading anything. + child.stdout.pause(); + await setTimeout(1000); + let stdout = ''; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { stdout += chunk; }); + child.stdout.resume(); + + const [code] = await once(child, 'close'); + assert.strictEqual(code, 0); + assert.strictEqual(stdout.match(/\n$/); +}); diff --git a/test/parallel/test-runner-junit-invalid-xml.js b/test/parallel/test-runner-junit-invalid-xml.js new file mode 100644 index 000000000000..83f7d80f8734 --- /dev/null +++ b/test/parallel/test-runner-junit-invalid-xml.js @@ -0,0 +1,96 @@ +'use strict'; +// Regression tests for the junit reporter emitting malformed XML. +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const { test } = require('node:test'); +const { junit } = require('node:test/reporters'); + +// Characters that XML 1.0 forbids anywhere, even as character references. +const kInvalidXMLChars = /[^\t\n\r\x20-퟿-�\u{10000}-\u{10FFFF}]/u; + +function assertWellFormedChars(xml) { + assert.doesNotMatch(xml, kInvalidXMLChars); + for (const { 1: dec, 2: hex } of xml.matchAll(/&#(?:(\d+)|x([0-9a-fA-F]+));/g)) { + const code = dec !== undefined ? Number(dec) : Number.parseInt(hex, 16); + assert.doesNotMatch(String.fromCodePoint(code), kInvalidXMLChars, + `illegal character reference &#${dec ?? `x${hex}`};`); + } +} + +async function render(events) { + let out = ''; + for await (const chunk of junit((async function*() { yield* events; })())) { + out += chunk; + } + return out; +} + +const details = { duration_ms: 1 }; + +test('control characters are not written to the report', async () => { + const name = 'name\x00with\x1b[31mcontrols\x1b[39m\x07￾\uD800'; + const error = new Error('\x1b[32mexpected\x1b[39m\x0b\x0c\x00'); + const xml = await render([ + { type: 'test:start', data: { name, nesting: 0 } }, + { type: 'test:fail', data: { name, nesting: 0, details: { ...details, error } } }, + { type: 'test:diagnostic', data: { nesting: 0, message: 'diag\x00\x1b[1m' } }, + ]); + assertWellFormedChars(xml); + // ANSI escape sequences are stripped, other illegal characters are replaced. + assert.match(xml, /name="name�withcontrols���&#27;"/); + assert.match(xml, /message="expected���"/); + assert.match(xml, //); +}); + +test('colored assertion diffs produce a valid report', () => { + const child = spawnSync(process.execPath, [ + '--test-reporter=junit', + fixtures.path('test-runner', 'junit-colored-diff.js'), + ], { env: { ...process.env, FORCE_COLOR: '1' } }); + const xml = child.stdout.toString(); + assert.strictEqual(child.status, 1, child.stderr.toString()); + assert.match(xml, / { + const xml = await render([ + { type: 'test:start', data: { name: 'suite', nesting: 0 } }, + { type: 'test:start', data: { name: 'first', nesting: 1 } }, + { type: 'test:pass', data: { name: 'first', nesting: 1, details } }, + { type: 'test:start', data: { name: 'next file', nesting: 0 } }, + { type: 'test:pass', data: { name: 'next file', nesting: 0, details } }, + ]); + assert.doesNotMatch(xml, /undefined/); + assert.strictEqual(xml, [ + '', + '', + `\t`, + '\t\t', + '\t', + '\t', + '', + '', + ].join('\n')); +}); + +test('an unfinished leaf test is reported as a failure', async () => { + const xml = await render([ + { type: 'test:start', data: { name: 'lost', nesting: 0 } }, + { type: 'test:start', data: { name: 'ok', nesting: 0 } }, + { type: 'test:pass', data: { name: 'ok', nesting: 0, details } }, + ]); + assert.match(xml, /\n\t\t\n\t<\/testcase>/); + assert.match(xml, //); +}); + +test('results without a test:start are not wrapped in an undefined element', async () => { + const xml = await render([ + { type: 'test:pass', data: { name: 'a', nesting: 0, details } }, + { type: 'test:pass', data: { name: 'b', nesting: 0, details } }, + ]); + assert.doesNotMatch(xml, /undefined|name="root"/); + assert.match(xml, /^\t\n\t$/m); +});