From a0d47a5a6ed6dd56ee2339f747c77dd8df722ff1 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 24 Sep 2026 11:25:51 -0700 Subject: [PATCH] fix(test runner): do not count statically skipped tests when sharding Fixes: https://github.com/microsoft/playwright/issues/42875 --- docs/src/test-sharding-js.md | 1 + packages/playwright/src/runner/testGroups.ts | 22 ++++---- tests/playwright-test/shard.spec.ts | 56 ++++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/docs/src/test-sharding-js.md b/docs/src/test-sharding-js.md index b1c0292c8a44a..67b8ef623af58 100644 --- a/docs/src/test-sharding-js.md +++ b/docs/src/test-sharding-js.md @@ -44,6 +44,7 @@ Without the fullyParallel setting, Playwright Test defaults to file-level granul - **With** `fullyParallel: true`: Tests are split at the individual test level, leading to more balanced shard execution. - **Without** `fullyParallel`: Tests are split at the file level, so to balance the shards, it's important to keep your test files small and evenly sized. - To ensure the most effective use of sharding, especially in CI environments, it is recommended to use `fullyParallel: true` when aiming for balanced distribution across shards. Otherwise, you may need to manually organize your test files to avoid imbalances. +- Tests that are statically skipped, for example with [`method: Test.skip`] or [`method: Test.fixme`], are not counted when balancing shards, because they do not run. ## Merging reports from multiple shards diff --git a/packages/playwright/src/runner/testGroups.ts b/packages/playwright/src/runner/testGroups.ts index 937ad8ac562d5..e7db48224be82 100644 --- a/packages/playwright/src/runner/testGroups.ts +++ b/packages/playwright/src/runner/testGroups.ts @@ -160,10 +160,12 @@ export function filterForShard(shard: { total: number, current: number }, weight // // Shards are still balanced by the number of tests, not files, // even in the case of non-paralleled files. - - let shardableTotal = 0; - for (const group of testGroups) - shardableTotal += group.tests.length; + // + // Statically skipped tests take no time, so they are not counted. + const activeSizes = testGroups.map(group => group.tests.filter(test => test.expectedStatus !== 'skipped').length); + // If all tests are skipped, balance them as usual. + const groupSizes = activeSizes.some(Boolean) ? activeSizes : testGroups.map(group => group.tests.length); + const shardableTotal = groupSizes.reduce((a, b) => a + b, 0); // Each shard gets some tests. const shardSizes = weights.map(w => Math.floor(w * shardableTotal / totalWeight)); @@ -180,12 +182,14 @@ export function filterForShard(shard: { total: number, current: number }, weight let current = 0; const result = new Set(); - for (const group of testGroups) { + testGroups.forEach((group, index) => { // Any test group goes to the shard that contains the first test of this group. - // So, this shard gets any group that starts at [from; to) - if (current >= from && current < to) + // So, this shard gets any group that starts at [from; to). + // Fully skipped groups go along with the preceding group. + const position = groupSizes[index] ? current : Math.max(current - 1, 0); + if (position >= from && position < to) result.add(group); - current += group.tests.length; - } + current += groupSizes[index]; + }); return result; } diff --git a/tests/playwright-test/shard.spec.ts b/tests/playwright-test/shard.spec.ts index 16ef26c2513a2..15774e8e373c5 100644 --- a/tests/playwright-test/shard.spec.ts +++ b/tests/playwright-test/shard.spec.ts @@ -357,3 +357,59 @@ test('should respect custom shard weights', async ({ runInlineTest }) => { ]); }); }); + +test('should not count statically skipped tests when sharding', async ({ runInlineTest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42875' }); + const tests = { + 'a.spec.ts': ` + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + test.skip('skip1', async () => {}); + test.fixme('skip2', async () => {}); + test.describe.skip('suite', () => { + test('skip3', async () => {}); + test('skip4', async () => {}); + }); + `, + 'b.spec.ts': ` + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + for (let i = 1; i <= 4; i++) { + test('test' + i, async () => { + console.log('\\n%%b-test' + i + '-done'); + }); + } + `, + }; + + await test.step('shard 1', async () => { + const result = await runInlineTest(tests, { shard: '1/2', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + expect(result.skipped).toBe(4); + expect(result.outputLines).toEqual(['b-test1-done', 'b-test2-done']); + }); + await test.step('shard 2', async () => { + const result = await runInlineTest(tests, { shard: '2/2', workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + expect(result.skipped).toBe(0); + expect(result.outputLines).toEqual(['b-test3-done', 'b-test4-done']); + }); +}); + +test('should shard when all tests are skipped', async ({ runInlineTest }) => { + const tests = { + 'a.spec.ts': ` + import { test } from '@playwright/test'; + test.describe.configure({ mode: 'parallel' }); + test.skip('skip1', async () => {}); + test.skip('skip2', async () => {}); + `, + }; + for (const shard of ['1/2', '2/2']) { + const result = await runInlineTest(tests, { shard, workers: 1 }); + expect(result.exitCode).toBe(0); + expect(result.skipped).toBe(1); + } +});