From 86bbf6a4381700d02a9c4f4b7cf5a2c41e1f6094 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Tue, 16 Jul 2024 17:05:46 -0400 Subject: [PATCH] test_runner: consolidate option parsing This commit consolidates all option parsing for the test runner in the parseCommandLine() internal helper function. The exception is a couple of temporary flags used for feature gating which will eventually become no-ops. This consolidation is prep work for supporting running test files in the test runner process. PR-URL: https://github.com/nodejs/node/pull/53849 Reviewed-By: James M Snell Reviewed-By: Moshe Atlow Reviewed-By: Chemi Atlow --- lib/internal/main/test_runner.js | 58 +++++++--------------------- lib/internal/test_runner/coverage.js | 24 ++++++------ lib/internal/test_runner/utils.js | 41 ++++++++++++++++++++ 3 files changed, 68 insertions(+), 55 deletions(-) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 5d4140bef94a62..3f86ddb84bb64e 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -1,26 +1,16 @@ 'use strict'; -const { - NumberParseInt, - RegExpPrototypeExec, - StringPrototypeSplit, -} = primordials; - const { prepareMainThreadExecution, markBootstrapComplete, } = require('internal/process/pre_execution'); -const { getOptionValue } = require('internal/options'); const { isUsingInspector } = require('internal/util/inspector'); const { run } = require('internal/test_runner/runner'); -const { setupTestReporters } = require('internal/test_runner/utils'); -const { exitCodes: { kGenericUserError } } = internalBinding('errors'); const { - codes: { - ERR_INVALID_ARG_VALUE, - }, -} = require('internal/errors'); - + parseCommandLine, + setupTestReporters, +} = require('internal/test_runner/utils'); +const { exitCodes: { kGenericUserError } } = internalBinding('errors'); let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => { debug = fn; }); @@ -28,7 +18,14 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => { prepareMainThreadExecution(false); markBootstrapComplete(); -let concurrency = getOptionValue('--test-concurrency') || true; +const { + perFileTimeout, + runnerConcurrency, + shard, + watchMode, +} = parseCommandLine(); + +let concurrency = runnerConcurrency; let inspectPort; if (isUsingInspector()) { @@ -38,39 +35,12 @@ if (isUsingInspector()) { inspectPort = process.debugPort; } -let shard; -const shardOption = getOptionValue('--test-shard'); -if (shardOption) { - if (!RegExpPrototypeExec(/^\d+\/\d+$/, shardOption)) { - process.exitCode = kGenericUserError; - - throw new ERR_INVALID_ARG_VALUE( - '--test-shard', - shardOption, - 'must be in the form of /', - ); - } - - const { 0: indexStr, 1: totalStr } = StringPrototypeSplit(shardOption, '/'); - - const index = NumberParseInt(indexStr, 10); - const total = NumberParseInt(totalStr, 10); - - shard = { - __proto__: null, - index, - total, - }; -} - -const timeout = getOptionValue('--test-timeout') || Infinity; - const options = { concurrency, inspectPort, - watch: getOptionValue('--watch'), + watch: watchMode, setup: setupTestReporters, - timeout, + timeout: perFileTimeout, shard, }; debug('test runner configuration:', options); diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index 1ef13d89285cee..23f479260292e7 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -25,18 +25,20 @@ const { readFileSync, } = require('fs'); const { setupCoverageHooks } = require('internal/util'); -const { getOptionValue } = require('internal/options'); const { tmpdir } = require('os'); const { join, resolve, relative, matchesGlob } = require('path'); const { fileURLToPath } = require('internal/url'); const { kMappings, SourceMap } = require('internal/source_map/source_map'); +const { parseCommandLine } = require('internal/test_runner/utils'); const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/; const kIgnoreRegex = /\/\* node:coverage ignore next (?\d+ )?\*\//; const kLineEndingRegex = /\r?\n$/u; const kLineSplitRegex = /(?<=\r?\n)/u; const kStatusRegex = /\/\* node:coverage (?enable|disable) \*\//; -const excludeFileGlobs = getOptionValue('--test-coverage-exclude'); -const includeFileGlobs = getOptionValue('--test-coverage-include'); +const { + coverageExcludeGlobs, + coverageIncludeGlobs, +} = parseCommandLine(); class CoverageLine { constructor(line, startOffset, src, length = src?.length) { @@ -498,18 +500,18 @@ function shouldSkipFileCoverage(url, workingDirectory) { const relativePath = relative(workingDirectory, absolutePath); // This check filters out files that match the exclude globs. - if (excludeFileGlobs?.length > 0) { - for (let i = 0; i < excludeFileGlobs.length; ++i) { - if (matchesGlob(relativePath, excludeFileGlobs[i]) || - matchesGlob(absolutePath, excludeFileGlobs[i])) return true; + if (coverageExcludeGlobs?.length > 0) { + for (let i = 0; i < coverageExcludeGlobs.length; ++i) { + if (matchesGlob(relativePath, coverageExcludeGlobs[i]) || + matchesGlob(absolutePath, coverageExcludeGlobs[i])) return true; } } // This check filters out files that do not match the include globs. - if (includeFileGlobs?.length > 0) { - for (let i = 0; i < includeFileGlobs.length; ++i) { - if (matchesGlob(relativePath, includeFileGlobs[i]) || - matchesGlob(absolutePath, includeFileGlobs[i])) return false; + if (coverageIncludeGlobs?.length > 0) { + for (let i = 0; i < coverageIncludeGlobs.length; ++i) { + if (matchesGlob(relativePath, coverageIncludeGlobs[i]) || + matchesGlob(absolutePath, coverageIncludeGlobs[i])) return false; } return true; } diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index b8576dbe6673b5..82af14c38e8baa 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -9,6 +9,7 @@ const { MathFloor, MathMax, MathMin, + NumberParseInt, NumberPrototypeToFixed, ObjectGetOwnPropertyDescriptor, RegExp, @@ -19,6 +20,7 @@ const { StringPrototypePadStart, StringPrototypeRepeat, StringPrototypeSlice, + StringPrototypeSplit, } = primordials; const { AsyncResource } = require('async_hooks'); @@ -196,13 +198,19 @@ function parseCommandLine() { const forceExit = getOptionValue('--test-force-exit'); const sourceMaps = getOptionValue('--enable-source-maps'); const updateSnapshots = getOptionValue('--test-update-snapshots'); + const watchMode = getOptionValue('--watch'); const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child'; const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8'; + let coverageExcludeGlobs; + let coverageIncludeGlobs; let destinations; + let perFileTimeout; let reporters; + let runnerConcurrency; let testNamePatterns; let testSkipPatterns; let testOnlyFlag; + let shard; if (isChildProcessV8) { kBuiltinReporters.set('v8-serializer', 'internal/test_runner/reporter/v8-serializer'); @@ -232,9 +240,31 @@ function parseCommandLine() { } if (isTestRunner) { + perFileTimeout = getOptionValue('--test-timeout') || Infinity; + runnerConcurrency = getOptionValue('--test-concurrency') || true; testOnlyFlag = false; testNamePatterns = null; + + const shardOption = getOptionValue('--test-shard'); + if (shardOption) { + if (!RegExpPrototypeExec(/^\d+\/\d+$/, shardOption)) { + throw new ERR_INVALID_ARG_VALUE( + '--test-shard', + shardOption, + 'must be in the form of /', + ); + } + + const indexAndTotal = StringPrototypeSplit(shardOption, '/'); + shard = { + __proto__: null, + index: NumberParseInt(indexAndTotal[0], 10), + total: NumberParseInt(indexAndTotal[1], 10), + }; + } } else { + perFileTimeout = Infinity; + runnerConcurrency = 1; const testNamePatternFlag = getOptionValue('--test-name-pattern'); testOnlyFlag = getOptionValue('--test-only'); testNamePatterns = testNamePatternFlag?.length > 0 ? @@ -247,11 +277,21 @@ function parseCommandLine() { ArrayPrototypeMap(testSkipPatternFlag, (re) => convertStringToRegExp(re, '--test-skip-pattern')) : null; } + if (coverage) { + coverageExcludeGlobs = getOptionValue('--test-coverage-exclude'); + coverageIncludeGlobs = getOptionValue('--test-coverage-include'); + } + globalTestOptions = { __proto__: null, isTestRunner, coverage, + coverageExcludeGlobs, + coverageIncludeGlobs, forceExit, + perFileTimeout, + runnerConcurrency, + shard, sourceMaps, testOnlyFlag, testNamePatterns, @@ -259,6 +299,7 @@ function parseCommandLine() { updateSnapshots, reporters, destinations, + watchMode, }; return globalTestOptions;