fix(test runner): when sharding with beforeAll, use shards total inst…

…ead of workers (#33083)

Otherwise, we might split the `beforeAll`-grouped test group into
`workers` parts instead of `shard.total` parts as the user would expect.

Fixes #33077.

packages/playwright/src/runner/loadUtils.ts

@@ -176,8 +176,11 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
   if (config.config.shard) {
     // Create test groups for top-level projects.
     const testGroups: TestGroup[] = [];
-    for (const projectSuite of rootSuite.suites)
-      testGroups.push(...createTestGroups(projectSuite, config.config.workers));
+    for (const projectSuite of rootSuite.suites) {
+      // Split beforeAll-grouped tests into "config.shard.total" groups when needed.
+      // Later on, we'll re-split them between workers by using "config.workers" instead.
+      testGroups.push(...createTestGroups(projectSuite, config.config.shard.total));
+    }
 
     // Shard test groups.
     const testGroupsInThisShard = filterForShard(config.config.shard, testGroups);

packages/playwright/src/runner/testGroups.ts

@@ -24,7 +24,7 @@ export type TestGroup = {
   tests: TestCase[];
 };
 
-export function createTestGroups(projectSuite: Suite, workers: number): TestGroup[] {
+export function createTestGroups(projectSuite: Suite, expectedParallelism: number): TestGroup[] {
   // This function groups tests that can be run together.
   // Tests cannot be run together when:
   // - They belong to different projects - requires different workers.
@@ -116,7 +116,7 @@ export function createTestGroups(projectSuite: Suite, workers: number): TestGrou
       result.push(...withRequireFile.parallel.values());
 
       // Tests with beforeAll/afterAll should try to share workers as much as possible.
-      const parallelWithHooksGroupSize = Math.ceil(withRequireFile.parallelWithHooks.tests.length / workers);
+      const parallelWithHooksGroupSize = Math.ceil(withRequireFile.parallelWithHooks.tests.length / expectedParallelism);
       let lastGroup: TestGroup | undefined;
       for (const test of withRequireFile.parallelWithHooks.tests) {
         if (!lastGroup || lastGroup.tests.length >= parallelWithHooksGroupSize) {

tests/playwright-test/shard.spec.ts

@@ -284,3 +284,43 @@ test('should not shard mode:default suites', async ({ runInlineTest }) => {
     expect(result.outputLines).toEqual(['beforeAll2', 'test4', 'test5']);
   }
 });
+
+test('should shard tests with beforeAll based on shards total instead of workers', {
+  annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/33077' },
+}, async ({ runInlineTest }) => {
+  const tests = {
+    'a.spec.ts': `
+      import { test } from '@playwright/test';
+
+      test.describe.configure({ mode: 'parallel' });
+      test.beforeAll(() => {
+        console.log('\\n%%beforeAll');
+      });
+
+      for (let i = 1; i <= 8; i++) {
+        test('test ' + i, async ({ }) => {
+          console.log('\\n%%test' + i);
+        });
+      }
+    `,
+  };
+
+  {
+    const result = await runInlineTest(tests, { shard: '1/4', workers: 1 });
+    expect(result.exitCode).toBe(0);
+    expect(result.passed).toBe(2);
+    expect(result.outputLines).toEqual(['beforeAll', 'test1', 'test2']);
+  }
+  {
+    const result = await runInlineTest(tests, { shard: '2/4', workers: 1 });
+    expect(result.exitCode).toBe(0);
+    expect(result.passed).toBe(2);
+    expect(result.outputLines).toEqual(['beforeAll', 'test3', 'test4']);
+  }
+  {
+    const result = await runInlineTest(tests, { shard: '7/8', workers: 6 });
+    expect(result.exitCode).toBe(0);
+    expect(result.passed).toBe(1);
+    expect(result.outputLines).toEqual(['beforeAll', 'test7']);
+  }
+});