From a7500c18d6880dee8c8f37c3267b06aa5112d957 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Tue, 31 May 2022 15:24:20 -0700
Subject: [PATCH] feat(test runner): allow serial suites inside parallel
 (#14530)

Also works for `fullyParallel` mode.
---
 packages/playwright-test/src/runner.ts    |  5 ++-
 packages/playwright-test/src/testType.ts  |  4 --
 tests/playwright-test/test-serial.spec.ts | 47 +++++++++++++++++++++--
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts
index e6740771ee..9c86d72abe 100644
--- a/packages/playwright-test/src/runner.ts
+++ b/packages/playwright-test/src/runner.ts
@@ -668,9 +668,12 @@ function createTestGroups(rootSuite: Suite, workers: number): TestGroup[] {
       }
 
       let insideParallel = false;
+      let insideSerial = false;
       let hasAllHooks = false;
       for (let parent: Suite | undefined = test.parent; parent; parent = parent.parent) {
-        insideParallel = insideParallel || parent._parallelMode === 'parallel';
+        insideSerial = insideSerial || parent._parallelMode === 'serial';
+        // Serial cancels out any enclosing parallel.
+        insideParallel = insideParallel || (!insideSerial && parent._parallelMode === 'parallel');
         hasAllHooks = hasAllHooks || parent._hooks.some(hook => hook.type === 'beforeAll' || hook.type === 'afterAll');
       }
 
diff --git a/packages/playwright-test/src/testType.ts b/packages/playwright-test/src/testType.ts
index f429ca5b87..0d8c918f99 100644
--- a/packages/playwright-test/src/testType.ts
+++ b/packages/playwright-test/src/testType.ts
@@ -126,8 +126,6 @@ export class TestTypeImpl {
     for (let parent: Suite | undefined = suite; parent; parent = parent.parent) {
       if (parent._parallelMode === 'serial' && child._parallelMode === 'parallel')
         throw errorWithLocation(location, 'describe.parallel cannot be nested inside describe.serial');
-      if (parent._parallelMode === 'parallel' && child._parallelMode === 'serial')
-        throw errorWithLocation(location, 'describe.serial cannot be nested inside describe.parallel');
     }
 
     setCurrentlyLoadingFileSuite(child);
@@ -152,8 +150,6 @@ export class TestTypeImpl {
     for (let parent: Suite | undefined = suite.parent; parent; parent = parent.parent) {
       if (parent._parallelMode === 'serial' && suite._parallelMode === 'parallel')
         throw errorWithLocation(location, 'describe.parallel cannot be nested inside describe.serial');
-      if (parent._parallelMode === 'parallel' && suite._parallelMode === 'serial')
-        throw errorWithLocation(location, 'describe.serial cannot be nested inside describe.parallel');
     }
   }
 
diff --git a/tests/playwright-test/test-serial.spec.ts b/tests/playwright-test/test-serial.spec.ts
index 2ab6402e7d..758ff3b2d3 100644
--- a/tests/playwright-test/test-serial.spec.ts
+++ b/tests/playwright-test/test-serial.spec.ts
@@ -323,16 +323,55 @@ test('test.describe.serial should work with test.fail and retries', async ({ run
   ]);
 });
 
-test('test.describe.serial should throw inside test.describe.parallel', async ({ runInlineTest }) => {
+test('test.describe.serial should work inside test.describe.parallel', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'a.test.ts': `
       const { test } = pwt;
       test.describe.parallel('parallel suite', () => {
         test.describe.serial('serial suite', () => {
+          test('one', async ({}) => {
+            await new Promise(f => setTimeout(f, 1000));
+            console.log('\\n%%one');
+          });
+          test('two', async ({}) => {
+            await new Promise(f => setTimeout(f, 500));
+            console.log('\\n%%two');
+          });
         });
       });
     `,
-  });
-  expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('a.test.ts:7:23: describe.serial cannot be nested inside describe.parallel');
+  }, { workers: 2 });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(2);
+  expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
+    '%%one',
+    '%%two',
+  ]);
+});
+
+test('test.describe.serial should work with fullyParallel', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { fullyParallel: true };
+    `,
+    'a.test.ts': `
+      const { test } = pwt;
+      test.describe.serial('serial suite', () => {
+        test('one', async ({}) => {
+          await new Promise(f => setTimeout(f, 1000));
+          console.log('\\n%%one');
+        });
+        test('two', async ({}) => {
+          await new Promise(f => setTimeout(f, 500));
+          console.log('\\n%%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',
+  ]);
 });