From eaaef29dbd42887ed49ad6255a9ed8fa53599202 Mon Sep 17 00:00:00 2001
From: ReaZzy <57872416+ReaZzy@users.noreply.github.com>
Date: Mon, 27 Jan 2025 23:31:14 +0100
Subject: [PATCH] fix: add validations to --shard cli parameter (#34463)
 (#34479)

---
 packages/playwright/src/program.ts  | 32 ++++++++++++++++++++++++++---
 tests/playwright-test/shard.spec.ts |  6 ++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/packages/playwright/src/program.ts b/packages/playwright/src/program.ts
index ebedca536b..511cca6d83 100644
--- a/packages/playwright/src/program.ts
+++ b/packages/playwright/src/program.ts
@@ -280,8 +280,6 @@ async function mergeReports(reportDir: string | undefined, opts: { [key: string]
 }
 
 function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrides {
-  const shardPair = options.shard ? options.shard.split('/').map((t: string) => parseInt(t, 10)) : undefined;
-
   let updateSnapshots: 'all' | 'changed' | 'missing' | 'none' | undefined;
   if (['all', 'changed', 'missing', 'none'].includes(options.updateSnapshots))
     updateSnapshots = options.updateSnapshots;
@@ -298,7 +296,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
     repeatEach: options.repeatEach ? parseInt(options.repeatEach, 10) : undefined,
     retries: options.retries ? parseInt(options.retries, 10) : undefined,
     reporter: resolveReporterOption(options.reporter),
-    shard: shardPair ? { current: shardPair[0], total: shardPair[1] } : undefined,
+    shard: resolveShardOption(options.shard),
     timeout: options.timeout ? parseInt(options.timeout, 10) : undefined,
     tsconfig: options.tsconfig ? path.resolve(process.cwd(), options.tsconfig) : undefined,
     ignoreSnapshots: options.ignoreSnapshots ? !!options.ignoreSnapshots : undefined,
@@ -341,6 +339,34 @@ function resolveReporterOption(reporter?: string): ReporterDescription[] | undef
   return reporter.split(',').map((r: string) => [resolveReporter(r)]);
 }
 
+function resolveShardOption(shard?: string): ConfigCLIOverrides['shard'] {
+  if (!shard)
+    return undefined;
+
+  const shardPair = shard.split('/');
+
+  if (shardPair.length !== 2) {
+    throw new Error(
+        `--shard "${shard}", expected format is "current/all", 1-based, for example "3/5".`,
+    );
+  }
+
+  const current = parseInt(shardPair[0], 10);
+  const total = parseInt(shardPair[1], 10);
+
+  if (isNaN(total) || total < 1)
+    throw new Error(`--shard "${shard}" total must be a positive number`);
+
+
+  if (isNaN(current) || current < 1 || current > total) {
+    throw new Error(
+        `--shard "${shard}" current must be a positive number, not greater than shard total`,
+    );
+  }
+
+  return { current, total };
+}
+
 function resolveReporter(id: string) {
   if (builtInReporters.includes(id as any))
     return id;
diff --git a/tests/playwright-test/shard.spec.ts b/tests/playwright-test/shard.spec.ts
index f73462c1f4..dcd75a9d23 100644
--- a/tests/playwright-test/shard.spec.ts
+++ b/tests/playwright-test/shard.spec.ts
@@ -139,6 +139,12 @@ test('should respect shard=3/4', async ({ runInlineTest }) => {
   ]);
 });
 
+test('should exit with shard=/3', async ({ runInlineTest }) => {
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/34463' });
+  const result = await runInlineTest(tests, { shard: '/3' });
+  expect(result.exitCode).toBe(1);
+});
+
 test('should not produce skipped tests for zero-sized shards', async ({ runInlineTest }) => {
   const result = await runInlineTest(tests, { shard: '10/10', workers: 1 });
   expect(result.exitCode).toBe(0);