Skip to content

Commit 69c78ca

Browse files
cjihrigRafaelGSS
authored andcommitted
test_runner: return setup() from parseCommandLine()
Now that parseCommandLine() returns run() compatible arguments, it makes sense to return setupTestReporters() as the setup() argument to run(). This also removes another problematic use of parseCommandLine() in setupTestReporters(). PR-URL: #54353 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent ed1ede8 commit 69c78ca

File tree

4 files changed

+15
-18
lines changed

4 files changed

+15
-18
lines changed

lib/internal/main/test_runner.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ const {
1010
} = require('internal/process/pre_execution');
1111
const { isUsingInspector } = require('internal/util/inspector');
1212
const { run } = require('internal/test_runner/runner');
13-
const {
14-
parseCommandLine,
15-
setupTestReporters,
16-
} = require('internal/test_runner/utils');
13+
const { parseCommandLine } = require('internal/test_runner/utils');
1714
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
1815
let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
1916
debug = fn;
@@ -31,7 +28,6 @@ if (isUsingInspector()) {
3128
options.inspectPort = process.debugPort;
3229
}
3330

34-
options.setup = setupTestReporters;
3531
options.globPatterns = ArrayPrototypeSlice(process.argv, 1);
3632

3733
debug('test runner configuration:', options);

lib/internal/test_runner/harness.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test')
2121
const {
2222
parseCommandLine,
2323
reporterScope,
24-
setupTestReporters,
2524
shouldColorizeTestFiles,
2625
} = require('internal/test_runner/utils');
2726
const { queueMicrotask } = require('internal/process/task_queues');
@@ -231,13 +230,14 @@ function lazyBootstrapRoot() {
231230
__proto__: null,
232231
entryFile: process.argv?.[1],
233232
};
234-
createTestTree(rootTestOptions, parseCommandLine());
233+
const globalOptions = parseCommandLine();
234+
createTestTree(rootTestOptions, globalOptions);
235235
globalRoot.reporter.on('test:fail', (data) => {
236236
if (data.todo === undefined || data.todo === false) {
237237
process.exitCode = kGenericUserError;
238238
}
239239
});
240-
globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter);
240+
globalRoot.harness.bootstrapPromise = globalOptions.setup(globalRoot.reporter);
241241
}
242242
return globalRoot;
243243
}

lib/internal/test_runner/runner.js

+1
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ function run(options = kEmptyObject) {
573573
// parseCommandLine() should not be used here. However, The existing run()
574574
// behavior has relied on it, so removing it must be done in a semver major.
575575
...parseCommandLine(),
576+
setup, // This line can be removed when parseCommandLine() is removed here.
576577
};
577578
const root = createTestTree(rootTestOptions, globalOptions);
578579

lib/internal/test_runner/utils.js

+10-10
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,6 @@ async function getReportersMap(reporters, destinations) {
176176
}
177177

178178
const reporterScope = new AsyncResource('TestReporterScope');
179-
const setupTestReporters = reporterScope.bind(async (rootReporter) => {
180-
const { reporters, destinations } = parseCommandLine();
181-
const reportersMap = await getReportersMap(reporters, destinations);
182-
for (let i = 0; i < reportersMap.length; i++) {
183-
const { reporter, destination } = reportersMap[i];
184-
compose(rootReporter, reporter).pipe(destination);
185-
}
186-
});
187-
188179
let globalTestOptions;
189180

190181
function parseCommandLine() {
@@ -281,6 +272,15 @@ function parseCommandLine() {
281272
coverageIncludeGlobs = getOptionValue('--test-coverage-include');
282273
}
283274

275+
const setup = reporterScope.bind(async (rootReporter) => {
276+
const reportersMap = await getReportersMap(reporters, destinations);
277+
278+
for (let i = 0; i < reportersMap.length; i++) {
279+
const { reporter, destination } = reportersMap[i];
280+
compose(rootReporter, reporter).pipe(destination);
281+
}
282+
});
283+
284284
globalTestOptions = {
285285
__proto__: null,
286286
isTestRunner,
@@ -292,6 +292,7 @@ function parseCommandLine() {
292292
forceExit,
293293
only,
294294
reporters,
295+
setup,
295296
shard,
296297
sourceMaps,
297298
testNamePatterns,
@@ -480,7 +481,6 @@ module.exports = {
480481
kDefaultPattern,
481482
parseCommandLine,
482483
reporterScope,
483-
setupTestReporters,
484484
shouldColorizeTestFiles,
485485
getCoverageReport,
486486
};

0 commit comments

Comments
 (0)