diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 32d00d2751..b27ca4e633 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -100,16 +100,7 @@ export class FullConfigInternal { (this.config as any)[configInternalSymbol] = this; const workers = takeFirst(configCLIOverrides.workers, userConfig.workers, '50%'); - if (typeof workers === 'string') { - if (workers.endsWith('%')) { - const cpus = os.cpus().length; - this.config.workers = Math.max(1, Math.floor(cpus * (parseInt(workers, 10) / 100))); - } else { - this.config.workers = parseWorkers(workers); - } - } else { - this.config.workers = workers; - } + this.config.workers = typeof workers === 'string' ? parseWorkers(workers) : workers; const webServers = takeFirst(userConfig.webServer, null); if (Array.isArray(webServers)) { // multiple web server mode @@ -222,10 +213,16 @@ function resolveReporters(reporters: Config['reporter'], rootDir: string): Repor } function parseWorkers(workers: string) { + if (workers[workers.length - 1] === '%') { + if (/[^0-9]/.test(workers.slice(0, -1))) + throw new Error(`Workers ${workers} must be a number or percentage.`); + const percentage = parseInt(workers.slice(0, -1), 10); + const cpus = os.cpus().length; + return Math.max(1, Math.floor(cpus * (percentage / 100))); + } const parsedWorkers = parseInt(workers, 10); if (isNaN(parsedWorkers)) throw new Error(`Workers ${workers} must be a number or percentage.`); - return parsedWorkers; } diff --git a/packages/playwright/src/program.ts b/packages/playwright/src/program.ts index 79865f6a84..ad85dd04db 100644 --- a/packages/playwright/src/program.ts +++ b/packages/playwright/src/program.ts @@ -333,25 +333,30 @@ function resolveReporter(id: string) { } class OptionValidators { - static validateWorkers(value: string): string { - if (value.endsWith('%')) - return value; - if (isNaN(parseInt(value, 10))) + static validateNumber(value: string, options: { min?: number, max?: number }): string { + if (/[^0-9]/.test(value)) throw new InvalidArgumentError('Not a number.'); + const parsed = parseInt(value, 10); + if (isNaN(parsed)) + throw new InvalidArgumentError('Not a number.'); + if (options.min !== undefined && parsed < options.min) + throw new InvalidArgumentError(`Expected a number greater than ${options.min}.`); + if (options.max !== undefined && parsed > options.max) + throw new InvalidArgumentError(`Expected a number less than ${options.max}.`); return value; } - static validateNumber(value: string): string { - if (isNaN(parseInt(value, 10))) - throw new InvalidArgumentError('Not a number.'); + static validateWorkers(value: string): string { + if (value[value.length - 1] === '%') { + OptionValidators.validateNumber(value, { min: 1, max: 100 }); + return value; + } + OptionValidators.validateNumber(value, { min: 1 }); return value; } static validateShard(value: string): string { - if (!value.includes('/')) - throw new InvalidArgumentError('Shard should be in the form "current/all"'); - const [current, all] = value.split('/').map(v => parseInt(v, 10)); - if (isNaN(current) || isNaN(all)) + if (!/^\d+\/\d+$/.test(value)) throw new InvalidArgumentError('Shard should be in the form "current/all"'); return value; } @@ -373,21 +378,21 @@ const testOptions: [string, string, ((value: string) => string | number)?][] = [ ['--ignore-snapshots', `Ignore screenshot and snapshot expectations`], ['--last-failed', `Only re-run the failures`], ['--list', `Collect all the tests and report them, but do not run`], - ['--max-failures ', `Stop after the first N failures`, OptionValidators.validateNumber], + ['--max-failures ', `Stop after the first N failures`, value => OptionValidators.validateNumber(value, { min: 0 })], ['--no-deps', 'Do not run project dependencies'], ['--output ', `Folder for output artifacts (default: "test-results")`], ['--pass-with-no-tests', `Makes test run succeed even if no tests were found`], ['--project ', `Only run tests from the specified list of projects, supports '*' wildcard (default: run all projects)`], ['--quiet', `Suppress stdio`], - ['--repeat-each ', `Run each test N times (default: 1)`, OptionValidators.validateNumber], + ['--repeat-each ', `Run each test N times (default: 1)`, value => OptionValidators.validateNumber(value, { min: 0 })], ['--reporter ', `Reporter to use, comma-separated, can be ${builtInReporters.map(name => `"${name}"`).join(', ')} (default: "${defaultReporter}")`], - ['--retries ', `Maximum retry count for flaky tests, zero for no retries (default: no retries)`, OptionValidators.validateNumber], + ['--retries ', `Maximum retry count for flaky tests, zero for no retries (default: no retries)`, value => OptionValidators.validateNumber(value, { min: 0 })], ['--shard ', `Shard tests and execute only the selected shard, specify in the form "current/all", 1-based, for example "3/5"`, OptionValidators.validateShard], - ['--timeout ', `Specify test timeout threshold in milliseconds, zero for unlimited (default: ${defaultTimeout})`, OptionValidators.validateNumber], + ['--timeout ', `Specify test timeout threshold in milliseconds, zero for unlimited (default: ${defaultTimeout})`, value => OptionValidators.validateNumber(value, { min: 0 })], ['--trace ', `Force tracing mode, can be ${kTraceModes.map(mode => `"${mode}"`).join(', ')}`], ['--ui', `Run tests in interactive UI mode`], ['--ui-host ', 'Host to serve UI on; specifying this option opens UI in a browser tab'], - ['--ui-port ', 'Port to serve UI on, 0 for any free port; specifying this option opens UI in a browser tab', OptionValidators.validateNumber], + ['--ui-port ', 'Port to serve UI on, 0 for any free port; specifying this option opens UI in a browser tab', value => OptionValidators.validateNumber(value, { min: 0, max: 65_535 })], ['-u, --update-snapshots', `Update snapshots with actual results (default: only create missing snapshots)`], ['-j, --workers ', `Number of concurrent workers or percentage of logical CPU cores, use 1 to run in a single worker (default: 50%)`, OptionValidators.validateWorkers], ['-x', `Stop after the first failure`],