diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 5a35b96cda..8d1d2c548a 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -653,10 +653,16 @@ function createTestGroups(rootSuite: Suite, workers: number): TestGroup[] { const groups = new Map, parallelWithHooks: TestGroup, }>>(); @@ -681,29 +687,34 @@ function createTestGroups(rootSuite: Suite, workers: number): TestGroup[] { if (!withRequireFile) { withRequireFile = { general: createGroup(test), - parallel: [], + parallel: new Map(), parallelWithHooks: createGroup(test), }; withWorkerHash.set(test._requireFile, withRequireFile); } + // Note that a parallel suite cannot be inside a serial suite. This is enforced in TestType. let insideParallel = false; - let insideSerial = false; + let outerMostSerialSuite: Suite | undefined; let hasAllHooks = false; for (let parent: Suite | undefined = test.parent; parent; parent = parent.parent) { - insideSerial = insideSerial || parent._parallelMode === 'serial'; - // Serial cancels out any enclosing parallel. - insideParallel = insideParallel || (!insideSerial && parent._parallelMode === 'parallel'); + if (parent._parallelMode === 'serial') + outerMostSerialSuite = parent; + insideParallel = insideParallel || parent._parallelMode === 'parallel'; hasAllHooks = hasAllHooks || parent._hooks.some(hook => hook.type === 'beforeAll' || hook.type === 'afterAll'); } if (insideParallel) { - if (hasAllHooks) { + if (hasAllHooks && !outerMostSerialSuite) { withRequireFile.parallelWithHooks.tests.push(test); } else { - const group = createGroup(test); + const key = outerMostSerialSuite || test; + let group = withRequireFile.parallel.get(key); + if (!group) { + group = createGroup(test); + withRequireFile.parallel.set(key, group); + } group.tests.push(test); - withRequireFile.parallel.push(group); } } else { withRequireFile.general.tests.push(test); @@ -714,10 +725,14 @@ function createTestGroups(rootSuite: Suite, workers: number): TestGroup[] { const result: TestGroup[] = []; for (const withWorkerHash of groups.values()) { for (const withRequireFile of withWorkerHash.values()) { + // Tests without parallel mode should run serially as a single group. if (withRequireFile.general.tests.length) result.push(withRequireFile.general); - result.push(...withRequireFile.parallel); + // Parallel test groups without beforeAll/afterAll can be run independently. + 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); let lastGroup: TestGroup | undefined; for (const test of withRequireFile.parallelWithHooks.tests) { diff --git a/tests/playwright-test/test-serial.spec.ts b/tests/playwright-test/test-serial.spec.ts index 758ff3b2d3..2ca9a31e54 100644 --- a/tests/playwright-test/test-serial.spec.ts +++ b/tests/playwright-test/test-serial.spec.ts @@ -330,22 +330,41 @@ test('test.describe.serial should work inside test.describe.parallel', async ({ test.describe.parallel('parallel suite', () => { test.describe.serial('serial suite', () => { test('one', async ({}) => { - await new Promise(f => setTimeout(f, 1000)); - console.log('\\n%%one'); + await new Promise(f => setTimeout(f, 2000)); + console.log('\\n%%1-one'); }); test('two', async ({}) => { - await new Promise(f => setTimeout(f, 500)); - console.log('\\n%%two'); + await new Promise(f => setTimeout(f, 1000)); + console.log('\\n%%1-two'); + }); + }); + + test.describe.serial('serial suite 2', () => { + test('one', async ({}) => { + await new Promise(f => setTimeout(f, 2000)); + console.log('\\n%%2-one'); + }); + test('two', async ({}) => { + await new Promise(f => setTimeout(f, 1000)); + console.log('\\n%%2-two'); }); }); }); `, }, { workers: 2 }); expect(result.exitCode).toBe(0); - expect(result.passed).toBe(2); - expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ - '%%one', - '%%two', + expect(result.passed).toBe(4); + expect(result.output).toContain('Running 4 tests using 2 workers'); + const lines = result.output.split('\n').filter(line => line.startsWith('%%')); + // First test in each worker started before the second test in the other. + // This means they were actually running in parallel. + expect(lines.indexOf('%%1-one')).toBeLessThan(lines.indexOf('%%2-two')); + expect(lines.indexOf('%%2-one')).toBeLessThan(lines.indexOf('%%1-two')); + expect(lines.sort()).toEqual([ + '%%1-one', + '%%1-two', + '%%2-one', + '%%2-two', ]); });