PR feedback

Author: octogonz

libraries/rush-lib/src/cli/parsing/ParseParallelism.ts

@@ -5,39 +5,67 @@ import * as os from 'node:os';
 
 import { IS_WINDOWS } from '../../utilities/executionUtilities';
 
+export function getNumberOfCores(): number {
+  return os.availableParallelism?.() ?? os.cpus().length;
+}
+
+/**
+ * Since the JSON value is a string, it must be a percentage like "50%",
+ * which we convert to a number based on the available parallelism.
+ * For example, if the available parallelism (not the -p flag) is 8 and the weight is "50%",
+ * then the resulting weight will be 4.
+ *
+ * @param weight
+ * @returns
+ */
+export function parseParallelismPercent(weight: string, numberOfCores: number = getNumberOfCores()): number {
+  const percentageRegExp: RegExp = /^\d+(\.\d+)?%$/;
+
+  if (!percentageRegExp.test(weight)) {
+    throw new Error(`Expecting a percentage string like "12%" or "34.56%".`);
+  }
+
+  const percentValue: number = parseFloat(weight.slice(0, -1));
+
+  if (percentValue <= 0) {
+    throw new Error(`Invalid percentage value of "${percentValue}": value must be greater than zero`);
+  }
+
+  if (percentValue > 100) {
+    throw new Error(`Invalid percentage value of "${percentValue}": value must not exceed 100%`);
+  }
+
+  // Use as much CPU as possible, so we round down the weight here
+  return Math.max(1, Math.floor((percentValue / 100) * numberOfCores));
+}
+
 /**
  * Parses a command line specification for desired parallelism.
  * Factored out to enable unit tests
  */
 export function parseParallelism(
   rawParallelism: string | undefined,
-  numberOfCores: number = os.availableParallelism?.() ?? os.cpus().length
+  numberOfCores: number = getNumberOfCores()
 ): number {
   if (rawParallelism) {
+    rawParallelism = rawParallelism.trim();
+
     if (rawParallelism === 'max') {
       return numberOfCores;
-    } else {
-      const parallelismAsNumber: number = Number(rawParallelism);
-
-      if (typeof rawParallelism === 'string' && rawParallelism.trim().endsWith('%')) {
-        const parsedPercentage: number = Number(rawParallelism.trim().replace(/\%$/, ''));
-
-        if (parsedPercentage <= 0 || parsedPercentage > 100) {
-          throw new Error(
-            `Invalid percentage value of '${rawParallelism}', value cannot be less than '0%' or more than '100%'`
-          );
-        }
-
-        const workers: number = Math.floor((parsedPercentage / 100) * numberOfCores);
-        return Math.max(workers, 1);
-      } else if (!isNaN(parallelismAsNumber)) {
-        return Math.max(parallelismAsNumber, 1);
-      } else {
-        throw new Error(
-          `Invalid parallelism value of '${rawParallelism}', expected a number, a percentage, or 'max'`
-        );
-      }
     }
+
+    if (rawParallelism.endsWith('%')) {
+      return parseParallelismPercent(rawParallelism, numberOfCores);
+    }
+
+    const parallelismAsNumber: number = Number(rawParallelism);
+    if (!isNaN(parallelismAsNumber)) {
+      return Math.max(parallelismAsNumber, 1);
+    }
+
+    throw new Error(
+      `Invalid parallelism value of "${rawParallelism}": expected a number, a percentage string, or "max"`
+    );
   } else {
     // If an explicit parallelism number wasn't provided, then choose a sensible
     // default.

libraries/rush-lib/src/cli/parsing/test/__snapshots__/ParseParallelism.test.ts.snap

@@ -1,5 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`parseParallelism throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of 'tequila', expected a number, a percentage, or 'max'"`;
+exports[`parseParallelism throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of \\"tequila\\": expected a number, a percentage string, or \\"max\\""`;
 
-exports[`parseParallelism throwsErrorOnInvalidParallelismPercentage 1`] = `"Invalid percentage value of '200%', value cannot be less than '0%' or more than '100%'"`;
+exports[`parseParallelism throwsErrorOnInvalidParallelismPercentage 1`] = `"Invalid percentage value of \\"200\\": value must not exceed 100%"`;

libraries/rush-lib/src/logic/operations/WeightedOperationPlugin.ts

@@ -1,8 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import os from 'node:os';
-
 import { Async } from '@rushstack/node-core-library';
 
 import type { Operation } from './Operation';
@@ -14,6 +12,7 @@ import type {
 import type { IOperationSettings, RushProjectConfiguration } from '../../api/RushProjectConfiguration';
 import type { IOperationExecutionResult } from './IOperationExecutionResult';
 import type { OperationExecutionRecord } from './OperationExecutionRecord';
+import { parseParallelismPercent } from '../../cli/parsing/ParseParallelism';
 
 const PLUGIN_NAME: 'WeightedOperationPlugin' = 'WeightedOperationPlugin';
 
@@ -33,29 +32,6 @@ function weightOperations(
   context: ICreateOperationsContext
 ): Map<Operation, IOperationExecutionResult> {
   const { projectConfigurations } = context;
-  const availableParallelism: number = os.availableParallelism();
-
-  const percentageRegExp: RegExp = /^[1-9][0-9]*(\.\d+)?%$/;
-
-  /**
-   * Since the JSON value is a string, it must be a percentage like "50%",
-   * which we convert to a number based on the available parallelism.
-   * For example, if the available parallelism (not the -p flag) is 8 and the weight is "50%",
-   * then the resulting weight will be 4.
-   *
-   * @param weight
-   * @returns
-   */
-  function _tryConvertPercentWeight(weight: `${number}%`): number {
-    if (!percentageRegExp.test(weight)) {
-      throw new Error(`Because the JSON value is a string, expecting a percentage like "23.4%".`);
-    }
-
-    const percentValue: number = parseFloat(weight.slice(0, -1));
-
-    // Use as much CPU as possible, so we round down the weight here
-    return Math.max(1, Math.floor((percentValue / 100) * availableParallelism));
-  }
 
   for (const [operation, record] of operations) {
     const { runner } = record as OperationExecutionRecord;
@@ -71,7 +47,7 @@ function weightOperations(
           operation.weight = operationSettings.weight;
         } else if (typeof operationSettings.weight === 'string') {
           try {
-            operation.weight = _tryConvertPercentWeight(operationSettings.weight);
+            operation.weight = parseParallelismPercent(operationSettings.weight);
           } catch (error) {
             throw new Error(
               `${operation.name} (invalid weight: ${JSON.stringify(operationSettings.weight)}) ${(error as Error).message}`