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 <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
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‎

Lines changed: 3 additions & 1 deletion
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
returnglobalRoot;
211213
}

‎lib/internal/test_runner/runner.js‎

Lines changed: 4 additions & 1 deletion
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
constroot=createTestTree({__proto__: null, concurrency, timeout, signal });
491+
root.harness.shouldColorizeTestFiles||=shouldColorizeTestFiles(root);
492+
490493
if(process.env.NODE_TEST_CONTEXT!==undefined){
491494
returnroot.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
returnroot.reporter;
518521
}

‎lib/internal/test_runner/utils.js‎

Lines changed: 15 additions & 5 deletions
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
returnrequire(builtinPath);
129130
}
130131

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

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

168177
constreporterScope=newAsyncResource('TestReporterScope');
169-
constsetupTestReporters=reporterScope.bind(async(rootTest)=>{
178+
constsetupTestReporters=reporterScope.bind(async(rootReporter)=>{
170179
const{ reporters, destinations }=parseCommandLine();
171-
constreportersMap=awaitgetReportersMap(reporters,destinations,rootTest);
180+
constreportersMap=awaitgetReportersMap(reporters,destinations);
172181
for(leti=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‎

Lines changed: 19 additions & 0 deletions
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'eventonDuplexinstance/);
157157
});
158+
159+
it('should support stdout as a destination with spec reporter',async()=>{
160+
process.env.FORCE_COLOR='1';
161+
constfile=tmpdir.resolve(`${tmpFiles++}.txt`);
162+
constchild=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+
constfileConent=fs.readFileSync(file,'utf8');
167+
assert.match(fileConent,/nested/);
168+
assert.match(fileConent,/ok/);
169+
assert.match(fileConent,/failing/);
170+
assert.match(fileConent,/tests4/);
171+
assert.match(fileConent,/pass2/);
172+
assert.match(fileConent,/fail2/);
173+
assert.match(fileConent,/cancelled0/);
174+
assert.match(fileConent,/skipped0/);
175+
assert.match(fileConent,/todo0/);
176+
});
158177
});

‎test/parallel/test-runner-run.mjs‎

Lines changed: 13 additions & 0 deletions
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+
conststream=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+
forawait(const_ofstream);
480+
});
468481
});
469482

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

0 commit comments

Comments
(0)