8000 PR Feedback #3263 · psy-repos-typescript/rushstack@59ebb5f · GitHub
[go: up one dir, main page]
Skip to content

Commit 59ebb5f

Browse files
wberniclanton
wbern
authored and
iclanton
committed
PR Feedback microsoft#3263 
1 parent bd8915a commit 59ebb5f

File tree

5 files changed

+51
-35
lines changed

5 files changed

+51
-35
lines changed

‎apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
342342
environmentVariable: EnvironmentVariableNames.RUSH_PARALLELISM,
343343
description:
344344
'Specifies the maximum number of concurrent processes to launch during a build.' +
345-
' The COUNT should be a positive integer, a percentage value (eg. "50%") or the word "max"' +
345+
' The COUNT should be a positive integer, a percentage value (eg. "50%%") or the word "max"' +
346346
' to specify a count that is equal to the number of CPU cores. If this parameter is omitted,' +
347347
' then the default value depends on the operating system and number of CPU cores.'
348348
});

‎apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,13 @@ Optional arguments:
140140
-p COUNT, --parallelism COUNT
141141
Specifies the maximum number of concurrent processes
142142
to launch during a build. The COUNT should be a
143-
positive integer or else the word \\"max\\" to specify a
144-
count that is equal to the number of CPU cores. If
145-
this parameter is omitted, then the default value
146-
depends on the operating system and number of CPU
147-
cores. This parameter may alternatively be specified
148-
via the RUSH_PARALLELISM environment variable.
143+
positive integer, a percentage value (eg. \\"50%\\") or
144+
the word \\"max\\" to specify a count that is equal to
145+
the number of CPU cores. If this parameter is omitted,
146+
then the default value depends on the operating
147+
system and number of CPU cores. This parameter may
148+
alternatively be specified via the RUSH_PARALLELISM
149+
environment variable.
149150
--timeline After the build is complete, print additional
150151
statistics and CPU usage information, including an
151152
ASCII chart of the start and stop times for each
@@ -383,12 +384,13 @@ Optional arguments:
383384
-p COUNT, --parallelism COUNT
384385
Specifies the maximum number of concurrent processes
385386
to launch during a build. The COUNT should be a
386-
positive integer or else the word \\"max\\" to specify a
387-
count that is equal to the number of CPU cores. If
388-
this parameter is omitted, then the default value
389-
depends on the operating system and number of CPU
390-
cores. This parameter may alternatively be specified
391-
via the RUSH_PARALLELISM environment variable.
387+
positive integer, a percentage value (eg. \\"50%\\") or
388+
the word \\"max\\" to specify a count that is equal to
389+
the number of CPU cores. If this parameter is omitted,
390+
then the default value depends on the operating
391+
system and number of CPU cores. This parameter may
392+
alternatively be specified via the RUSH_PARALLELISM
393+
environment variable.
392394
--timeline After the build is complete, print additional
393395
statistics and CPU usage information, including an
394396
ASCII chart of the start and stop times for each
@@ -933,12 +935,13 @@ Optional arguments:
933935
-p COUNT, --parallelism COUNT
934936
Specifies the maximum number of concurrent processes
935937
to launch during a build. The COUNT should be a
936-
positive integer or else the word \\"max\\" to specify a
937-
count that is equal to the number of CPU cores. If
938-
this parameter is omitted, then the default value
939-
depends on the operating system and number of CPU
940-
cores. This parameter may alternatively be specified
941-
via the RUSH_PARALLELISM environment variable.
938+
positive integer, a percentage value (eg. \\"50%\\") or
939+
the word \\"max\\" to specify a count that is equal to
940+
the number of CPU cores. If this parameter is omitted,
941+
then the default value depends on the operating
942+
system and number of CPU cores. This parameter may
943+
alternatively be specified via the RUSH_PARALLELISM
944+
environment variable.
942945
--timeline After the build is complete, print additional
943946
statistics and CPU usage information, including an
944947
ASCII chart of the start and stop times for each

‎apps/rush-lib/src/logic/operations/OperationExecutionManager.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,21 @@ export class OperationExecutionManager {
159159
} else {
160160
const parsed: number = parseInt(parallelism, 10);
161161

162-
if (isNaN(parsed)) {
163-
throw new Error(
164-
`Invalid parallelism value of '${parallelism}', expected a number, a percentage, or 'max'`
165-
);
166-
}
167-
168162
let parallelismInt: number = parsed > 0 ? parsed : 1;
169163

170-
if (
171-
typeof parallelism === 'string' &&
172-
parallelism.trim().endsWith('%') &&
173-
parsed > 0 &&
174-
parsed <= 100
175-
) {
164+
if (typeof parallelism === 'string' && parallelism.trim().endsWith('%')) {
165+
if (parsed <= 0 || parsed > 100) {
166+
throw new Error(
167+
`Invalid percentage value of '${parallelism}', value cannot be less than '0%' or more than '100%'`
168+
);
169+
}
170+
176171
const workers: number = Math.floor((parsed / 100) * numberOfCores);
177172
parallelismInt = Math.max(workers, 1);
173+
} else if (isNaN(parsed)) {
174+
throw new Error(
175+
`Invalid parallelism value of '${parallelism}', expected a number, a percentage, or 'max'`
176+
);
178177
}
179178

180179
this._parallelism = parallelismInt;

‎apps/rush-lib/src/logic/operations/test/OperationExecutionManager.test.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,34 @@ describe(OperationExecutionManager.name, () => {
7676
})
7777
).toThrowErrorMatchingSnapshot();
7878
});
79-
});
8079

81-
describe('Constructor', () => {
8280
it('createsWithPercentageBasedParallelism', () => {
8381
expect(
8482
() =>
8583
new OperationExecutionManager(new Set(), {
8684
quietMode: false,
8785
debugMode: false,
8886
parallelism: '50%',
87+
showTimeline: false,
8988
changedProjectsOnly: false,
90-
destination: mockWritable,
91-
repoCommandLineConfiguration: undefined!
89+
destination: mockWritable
9290
})
9391
).toBeInstanceOf(Function);
9492
});
93+
94+
it('throwsErrorOnInvalidParallelismPercentage', () => {
95+
expect(
96+
() =>
97+
new OperationExecutionManager(new Set(), {
98+
quietMode: false,
99+
debugMode: false,
100+
parallelism: '200%',
101+
showTimeline: false,
102+
changedProjectsOnly: false,
103+
destination: mockWritable
104+
})
105+
).toThrowErrorMatchingSnapshot();
106+
});
95107
});
96108

97109
describe('Error logging', () => {

‎apps/rush-lib/src/logic/operations/test/__snapshots__/OperationExecutionManager.test.ts.snap

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
exports[`OperationExecutionManager Constructor throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of 'tequila', expected a number, a percentage, or 'max'"`;
44

5+
exports[`OperationExecutionManager Constructor throwsErrorOnInvalidParallelismPercentage 1`] = `"Invalid percentage value of '200%', value cannot be less than '0%' or more than '100%'"`;
6+
57
exports[`OperationExecutionManager Error logging printedStderrAfterError 1`] = `"An error occurred."`;
68

79
exports[`OperationExecutionManager Error logging printedStderrAfterError 2`] = `

0 commit comments

Comments
 (0)
0