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: nodejs#53849 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
ehsankhfr · Jul 18, 2024 · 86bbf6a · 86bbf6a
1 parent ed340d0
commit 86bbf6a
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 55 deletions.
diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
@@ -1,34 +1,31 @@
 '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;
 });
 
 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 <index>/<total>',
-    );
-  }
-
-  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
@@ -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 (?<count>\d+ )?\*\//;
 const kLineEndingRegex = /\r?\n$/u;
 const kLineSplitRegex = /(?<=\r?\n)/u;
 const kStatusRegex = /\/\* node:coverage (?<status>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
@@ -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 <index>/<total>',
+        );
+      }
+
+      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,18 +277,29 @@ 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,
     testSkipPatterns,
     updateSnapshots,
     reporters,
     destinations,
+    watchMode,
   };
 
   return globalTestOptions;