Skip to content

Commit 979cebc

Browse files
pulkit-30RafaelGSS
authored andcommitted
test_runner: fixed test object is incorrectly passed to setup()
PR-URL: #50982 Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 47548d9 commit 979cebc

File tree

5 files changed

+54
-7
lines changed

5 files changed

+54
-7
lines changed

lib/internal/test_runner/harness.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
parseCommandLine,
2424
reporterScope,
2525
setupTestReporters,
26+
shouldColorizeTestFiles,
2627
} = require('internal/test_runner/utils');
2728
const { bigint: hrtime } = process.hrtime;
2829

@@ -205,7 +206,8 @@ function getGlobalRoot() {
205206
process.exitCode = kGenericUserError;
206207
}
207208
});
208-
reportersSetup = setupTestReporters(globalRoot);
209+
reportersSetup = setupTestReporters(globalRoot.reporter);
210+
globalRoot.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(globalRoot);
209211
}
210212
return globalRoot;
211213
}

lib/internal/test_runner/runner.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ const {
7070
convertStringToRegExp,
7171
countCompletedTest,
7272
kDefaultPattern,
73+
shouldColorizeTestFiles,
7374
} = require('internal/test_runner/utils');
7475
const { Glob } = require('internal/fs/glob');
7576
const { once } = require('events');
@@ -487,6 +488,8 @@ function run(options = kEmptyObject) {
487488
}
488489

489490
const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
491+
root.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(root);
492+
490493
if (process.env.NODE_TEST_CONTEXT !== undefined) {
491494
return root.reporter;
492495
}
@@ -512,7 +515,7 @@ function run(options = kEmptyObject) {
512515
});
513516
};
514517

515-
PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles), postRun);
518+
PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root.reporter)), runFiles), postRun);
516519

517520
return root.reporter;
518521
}

lib/internal/test_runner/utils.js

+15-5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
ArrayPrototypeFlatMap,
66
ArrayPrototypePush,
77
ArrayPrototypeReduce,
8+
ArrayPrototypeSome,
89
ObjectGetOwnPropertyDescriptor,
910
MathFloor,
1011
MathMax,
@@ -128,10 +129,18 @@ function tryBuiltinReporter(name) {
128129
return require(builtinPath);
129130
}
130131

131-
async function getReportersMap(reporters, destinations, rootTest) {
132+
function shouldColorizeTestFiles(rootTest) {
133+
// This function assumes only built-in destinations (stdout/stderr) supports coloring
134+
const { reporters, destinations } = parseCommandLine();
135+
return ArrayPrototypeSome(reporters, (_, index) => {
136+
const destination = kBuiltinDestinations.get(destinations[index]);
137+
return destination && shouldColorize(destination);
138+
});
139+
}
140+
141+
async function getReportersMap(reporters, destinations) {
132142
return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
133143
const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i]);
134-
rootTest.harness.shouldColorizeTestFiles ||= shouldColorize(destination);
135144

136145
// Load the test reporter passed to --test-reporter
137146
let reporter = tryBuiltinReporter(name);
@@ -166,12 +175,12 @@ async function getReportersMap(reporters, destinations, rootTest) {
166175
}
167176

168177
const reporterScope = new AsyncResource('TestReporterScope');
169-
const setupTestReporters = reporterScope.bind(async (rootTest) => {
178+
const setupTestReporters = reporterScope.bind(async (rootReporter) => {
170179
const { reporters, destinations } = parseCommandLine();
171-
const reportersMap = await getReportersMap(reporters, destinations, rootTest);
180+
const reportersMap = await getReportersMap(reporters, destinations);
172181
for (let i = 0; i < reportersMap.length; i++) {
173182
const { reporter, destination } = reportersMap[i];
174-
compose(rootTest.reporter, reporter).pipe(destination);
183+
compose(rootReporter, reporter).pipe(destination);
175184
}
176185
});
177186

@@ -421,5 +430,6 @@ module.exports = {
421430
parseCommandLine,
422431
reporterScope,
423432
setupTestReporters,
433+
shouldColorizeTestFiles,
424434
getCoverageReport,
425435
};

test/parallel/test-runner-reporters.js

+19
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,23 @@ describe('node:test reporters', { concurrency: true }, () => {
155155
assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
156156
assert.match(child.stderr.toString(), /Emitted 'error' event on Duplex instance/);
157157
});
158+
159+
it('should support stdout as a destination with spec reporter', async () => {
160+
process.env.FORCE_COLOR = '1';
161+
const file = tmpdir.resolve(`${tmpFiles++}.txt`);
162+
const child = spawnSync(process.execPath,
163+
['--test', '--test-reporter', 'spec', '--test-reporter-destination', file, testFile]);
164+
assert.strictEqual(child.stderr.toString(), '');
165+
assert.strictEqual(child.stdout.toString(), '');
166+
const fileConent = fs.readFileSync(file, 'utf8');
167+
assert.match(fileConent, / nested/);
168+
assert.match(fileConent, / ok/);
169+
assert.match(fileConent, / failing/);
170+
assert.match(fileConent, / tests 4/);
171+
assert.match(fileConent, / pass 2/);
172+
assert.match(fileConent, / fail 2/);
173+
assert.match(fileConent, / cancelled 0/);
174+
assert.match(fileConent, / skipped 0/);
175+
assert.match(fileConent, / todo 0/);
176+
});
158177
});

test/parallel/test-runner-run.mjs

+13
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,19 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
465465
code: 'ERR_INVALID_ARG_TYPE'
466466
}));
467467
});
468+
469+
it('should pass instance of stream to setup', async () => {
470+
const stream = run({
471+
files: [join(testFixtures, 'default-behavior/test/random.cjs')],
472+
setup: common.mustCall((root) => {
473+
assert.strictEqual(root.constructor.name, 'TestsStream');
474+
}),
475+
});
476+
stream.on('test:fail', common.mustNotCall());
477+
stream.on('test:pass', common.mustCall());
478+
// eslint-disable-next-line no-unused-vars
479+
for await (const _ of stream);
480+
});
468481
});
469482

470483
it('should run with no files', async () => {

0 commit comments

Comments
 (0)