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: wait for stream finish when --test-force-exit #54953

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
10 changes: 8 additions & 2 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const {
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypePushApply,
ArrayPrototypeReduce,
Expand All @@ -20,6 +21,7 @@ const {
RegExpPrototypeExec,
SafeMap,
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromisePrototypeFinally,
SafePromiseRace,
SafeSet,
Expand All @@ -41,7 +43,7 @@ const {
},
} = require('internal/errors');
const { MockTracker } = require('internal/test_runner/mock/mock');
const { TestsStream } = require('internal/test_runner/tests_stream');
const { TestsStream, kReporterStreams } = require('internal/test_runner/tests_stream');
const {
createDeferredCallback,
countCompletedTest,
Expand All @@ -65,6 +67,7 @@ const { TIMEOUT_MAX } = require('internal/timers');
const { fileURLToPath } = require('internal/url');
const { availableParallelism } = require('os');
const { innerOk } = require('internal/assert/utils');
const { finished } = require('stream/promises');
const { bigint: hrtime } = process.hrtime;
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
const kCancelledByParent = 'cancelledByParent';
Expand Down Expand Up @@ -973,7 +976,10 @@ class Test extends AsyncResource {
// any remaining ref'ed handles, then do that now. It is theoretically
// possible that a ref'ed handle could asynchronously create more tests,
// but the user opted into this behavior.
this.reporter.once('close', () => {
this.reporter.once('close', async () => {
await SafePromiseAllReturnVoid(
ArrayPrototypeMap(this.reporter[kReporterStreams], (stream) => finished(stream)),
);
process.exit();
});
this.harness.teardown();
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/test_runner/tests_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ const {
ArrayPrototypePush,
ArrayPrototypeShift,
NumberMAX_SAFE_INTEGER,
SafeWeakSet,
Symbol,
} = primordials;
const Readable = require('internal/streams/readable');

const kEmitMessage = Symbol('kEmitMessage');
const kReporterStreams = Symbol('kReporterStreams');

class TestsStream extends Readable {
#buffer;
#canPush;
Expand All @@ -18,6 +21,7 @@ class TestsStream extends Readable {
objectMode: true,
highWaterMark: NumberMAX_SAFE_INTEGER,
});
this[kReporterStreams] = new SafeWeakSet();
this.#buffer = [];
this.#canPush = true;
}
Expand Down Expand Up @@ -154,4 +158,4 @@ class TestsStream extends Readable {
}
}

module.exports = { TestsStream, kEmitMessage };
module.exports = { TestsStream, kEmitMessage, kReporterStreams };
5 changes: 4 additions & 1 deletion lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const {
} = require('internal/errors');
const { compose } = require('stream');
const { validateInteger } = require('internal/validators');
const { kReporterStreams } = require('internal/test_runner/tests_stream');

const coverageColors = {
__proto__: null,
Expand Down Expand Up @@ -297,7 +298,9 @@ function parseCommandLine() {

for (let i = 0; i < reportersMap.length; i++) {
const { reporter, destination } = reportersMap[i];
compose(rootReporter, reporter).pipe(destination);
const stream = compose(rootReporter, reporter);
stream.pipe(destination);
ArrayPrototypePush(rootReporter[kReporterStreams], destination);
Copy link
Member Author

@avivkeller avivkeller Sep 17, 2024

Choose a reason for hiding this comment

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

@cjihrig the issue was that I was originally appending stream, not destination.

}
});

Expand Down
4 changes: 2 additions & 2 deletions test/common/assertSnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ async function assertSnapshot(actual, filename = process.argv[1]) {
* @param {boolean} [options.tty] - whether to spawn the process in a pseudo-tty
* @returns {Promise<void>}
*/
async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...options } = {}) {
async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...options } = {}, snapshot = filename) {
if (tty && common.isWindows) {
test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
return;
Expand All @@ -88,7 +88,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
[path.join(__dirname, '../..', 'tools/pseudo-tty.py'), process.execPath, ...flags, filename] :
[...flags, filename];
const { stdout, stderr } = await common.spawnPromisified(executable, args, options);
await assertSnapshot(transform(`${stdout}${stderr}`), filename);
await assertSnapshot(transform(`${stdout}${stderr}`), snapshot);
}

module.exports = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
X.X

Failed tests:

✖ Failing test (*ms)
AssertionError [ERR_ASSERTION]: Failed
*
*
*
*
*
*
at new Promise (<anonymous>)
*
*
at Array.map (<anonymous>) {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: undefined,
expected: undefined,
operator: 'fail'
}
✖ Suite (*ms)
'1 subtest failed'
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="utf-8"?>
<testsuites>
<testsuite name="Suite" time="*" disabled="0" errors="0" tests="2" failures="1" skipped="0" hostname="HOSTNAME">
<testcase name="Failing test" time="*" classname="test" failure="Failed">
<failure type="testCodeFailure" message="Failed">
Error [ERR_TEST_FAILURE]: Failed
at new Promise (&lt;anonymous>)
at Array.map (&lt;anonymous>) {
code: 'ERR_TEST_FAILURE',
failureType: 'testCodeFailure',
cause: AssertionError [ERR_ASSERTION]: Failed
*
*
*
*
*
*
at new Promise (&lt;anonymous>)
*
*
at Array.map (&lt;anonymous>) {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: undefined,
expected: undefined,
operator: 'fail'
}
}
</failure>
</testcase>
<testcase name="Passing test" time="*" classname="test"/>
</testsuite>
<!-- tests 2 -->
<!-- suites 1 -->
<!-- pass 1 -->
<!-- fail 1 -->
<!-- cancelled 0 -->
<!-- skipped 0 -->
<!-- todo 0 -->
<!-- duration_ms * -->
</testsuites>
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
▶ Suite
✖ Failing test (*ms)
AssertionError [ERR_ASSERTION]: Failed
*
*
*
*
*
*
at new Promise (<anonymous>)
*
*
at Array.map (<anonymous>) {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: undefined,
expected: undefined,
operator: 'fail'
}

✔ Passing test (*ms)
▶ Suite (*ms)
ℹ tests 2
ℹ suites 1
ℹ pass 1
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms *

✖ failing tests:

*
✖ Failing test (*ms)
AssertionError [ERR_ASSERTION]: Failed
*
*
*
*
*
*
at new Promise (<anonymous>)
*
*
at Array.map (<anonymous>) {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: undefined,
expected: undefined,
operator: 'fail'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
TAP version 13
# Subtest: Suite
# Subtest: Failing test
not ok 1 - Failing test
---
duration_ms: *
location: '/test/fixtures/test-runner/output/test-runner-force-exit.js:(LINE):5'
failureType: 'testCodeFailure'
error: 'Failed'
code: 'ERR_ASSERTION'
name: 'AssertionError'
operator: 'fail'
stack: |-
*
*
*
*
*
*
new Promise (<anonymous>)
*
*
Array.map (<anonymous>)
...
# Subtest: Passing test
ok 2 - Passing test
---
duration_ms: *
...
1..2
not ok 1 - Suite
---
duration_ms: *
type: 'suite'
location: '/test/fixtures/test-runner/output/test-runner-force-exit.js:(LINE):1'
failureType: 'subtestsFailed'
error: '1 subtest failed'
code: 'ERR_TEST_FAILURE'
...
1..1
# tests 2
# suites 1
# pass 1
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
12 changes: 12 additions & 0 deletions test/fixtures/test-runner/output/test-runner-force-exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const { describe, test } = require('node:test');
const assert = require('node:assert');

describe('Suite', () => {
test('Failing test', () => {
assert.fail()
})

test('Passing test', () => {
assert.ok(true)
})
});
97 changes: 97 additions & 0 deletions test/parallel/test-runner-force-exit.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import * as common from '../common/index.mjs';
import * as tmpdir from '../common/tmpdir.js';
import * as fixtures from '../common/fixtures.mjs';
import * as snapshot from '../common/assertSnapshot.js';
import { describe, it } from 'node:test';
import { hostname } from 'node:os';
import { fileURLToPath } from 'node:url';
import { readFile } from 'node:fs/promises';

function replaceTestDuration(str) {
return str
.replaceAll(/duration_ms: [0-9.]+/g, 'duration_ms: *')
.replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *');
}

const root = fileURLToPath(new URL('../..', import.meta.url)).slice(0, -1);

const color = '(\\[\\d+m)';
const stackTraceBasePath = new RegExp(`${color}\\(${root.replaceAll(/[\\^$*+?.()|[\]{}]/g, '\\$&')}/?${color}(.*)${color}\\)`, 'g');

function replaceSpecDuration(str) {
return str
.replaceAll(/[0-9.]+ms/g, '*ms')
.replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *')
.replace(stackTraceBasePath, '$3');
}

function replaceJunitDuration(str) {
return str
.replaceAll(/time="[0-9.]+"/g, 'time="*"')
.replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *')
.replaceAll(hostname(), 'HOSTNAME')
.replace(stackTraceBasePath, '$3');
}

function removeWindowsPathEscaping(str) {
return common.isWindows ? str.replaceAll(/\\\\/g, '\\') : str;
}

function replaceTestLocationLine(str) {
return str.replaceAll(/(js:)(\d+)(:\d+)/g, '$1(LINE)$3');
}

const defaultTransform = snapshot.transform(
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
removeWindowsPathEscaping,
snapshot.replaceFullPaths,
snapshot.replaceWindowsPaths,
replaceTestDuration,
replaceTestLocationLine,
);

const transformers = {
junit: snapshot.transform(
replaceJunitDuration,
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
snapshot.replaceWindowsPaths,
),
spec: snapshot.transform(
replaceSpecDuration,
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
snapshot.replaceWindowsPaths,
),
dot: snapshot.transform(
replaceSpecDuration,
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
snapshot.replaceWindowsPaths,
),
};

describe('test runner output', { concurrency: true }, async () => {
tmpdir.refresh();
for (const reporter of ['dot', 'junit', 'spec', 'tap']) {
await it(reporter, async (t) => {
const output = tmpdir.resolve(`${reporter}.out`);
const spawned = await common.spawnPromisified(
process.execPath,
[
'--test',
'--test-force-exit',
`--test-reporter=${reporter}`,
`--test-reporter-destination=${output}`,
fixtures.path('test-runner/output/test-runner-force-exit.js'),
],
);
t.assert.deepStrictEqual(spawned, { code: 1, signal: null, stderr: '', stdout: '' });
const transformer = transformers[reporter] || defaultTransform;
const outputData = await readFile(output, 'utf-8');
await snapshot.assertSnapshot(transformer(outputData), fixtures.path(`test-runner/output/test-runner-force-exit-${reporter}.output`));
});
}
tmpdir.refresh();
});
Loading