From 638ebd6dd622a881749c344a93eaa17d5581e2cb Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Mon, 15 Nov 2021 13:17:26 -0800
Subject: [PATCH] fix(test runner): do not validate fixtures in tests/hooks
 that are never run (#10328)

---
 packages/playwright-test/src/project.ts      | 25 ++++++++++--------
 packages/playwright-test/src/workerRunner.ts |  9 +++----
 tests/playwright-test/fixtures.spec.ts       | 27 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/packages/playwright-test/src/project.ts b/packages/playwright-test/src/project.ts
index e94304f443..ce3e0de962 100644
--- a/packages/playwright-test/src/project.ts
+++ b/packages/playwright-test/src/project.ts
@@ -80,12 +80,6 @@ export class ProjectImpl {
   }
 
   private _cloneEntries(from: Suite, to: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean): boolean {
-    for (const hook of from._allHooks) {
-      const clone = hook._clone();
-      clone._pool = this.buildPool(hook);
-      clone._projectIndex = this.index;
-      to._addAllHook(clone);
-    }
     for (const entry of from._entries) {
       if (entry instanceof Suite) {
         const suite = entry._clone();
@@ -95,22 +89,31 @@ export class ProjectImpl {
           to.suites.pop();
         }
       } else {
-        const pool = this.buildPool(entry);
         const test = entry._clone();
         test.retries = this.config.retries;
-        test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`;
         test._id = `${entry._ordinalInFile}@${entry._requireFile}#run${this.index}-repeat${repeatEachIndex}`;
-        test._pool = pool;
         test._repeatEachIndex = repeatEachIndex;
         test._projectIndex = this.index;
         to._addTest(test);
         if (!filter(test)) {
           to._entries.pop();
-          to.suites.pop();
+          to.tests.pop();
+        } else {
+          const pool = this.buildPool(entry);
+          test._workerHash = `run${this.index}-${pool.digest}-repeat${repeatEachIndex}`;
+          test._pool = pool;
         }
       }
     }
-    return to._entries.length > 0;
+    if (!to._entries.length)
+      return false;
+    for (const hook of from._allHooks) {
+      const clone = hook._clone();
+      clone._pool = this.buildPool(hook);
+      clone._projectIndex = this.index;
+      to._addAllHook(clone);
+    }
+    return true;
   }
 
   cloneFileSuite(suite: Suite, repeatEachIndex: number, filter: (test: TestCase) => boolean): Suite | undefined {
diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts
index 5028e1f3df..61bd146971 100644
--- a/packages/playwright-test/src/workerRunner.ts
+++ b/packages/playwright-test/src/workerRunner.ts
@@ -27,7 +27,7 @@ import { Loader } from './loader';
 import { Modifier, Suite, TestCase } from './test';
 import { Annotations, TestError, TestInfo, TestInfoImpl, TestStepInternal, WorkerInfo } from './types';
 import { ProjectImpl } from './project';
-import { FixturePool, FixtureRunner } from './fixtures';
+import { FixtureRunner } from './fixtures';
 import { DeadlineRunner, raceAgainstDeadline } from 'playwright-core/lib/utils/async';
 
 const removeFolderAsync = util.promisify(rimraf);
@@ -148,15 +148,14 @@ export class WorkerRunner extends EventEmitter {
       this._entries = new Map(runPayload.entries.map(e => [ e.testId, e ]));
       await this._loadIfNeeded();
       const fileSuite = await this._loader.loadTestFile(runPayload.file);
-      let anyPool: FixturePool | undefined;
       const suite = this._project.cloneFileSuite(fileSuite, this._params.repeatEachIndex, test => {
         if (!this._entries.has(test._id))
           return false;
-        anyPool = test._pool;
         return true;
       });
-      if (suite && anyPool) {
-        this._fixtureRunner.setPool(anyPool);
+      if (suite) {
+        const firstPool = suite.allTests()[0]._pool!;
+        this._fixtureRunner.setPool(firstPool);
         await this._runSuite(suite, []);
       }
       if (this._failedTest)
diff --git a/tests/playwright-test/fixtures.spec.ts b/tests/playwright-test/fixtures.spec.ts
index bf8a409935..2728b325ac 100644
--- a/tests/playwright-test/fixtures.spec.ts
+++ b/tests/playwright-test/fixtures.spec.ts
@@ -699,3 +699,30 @@ test('worker teardown errors reflected in timed-out tests', async ({ runInlineTe
   expect(result.output).toContain('Timeout of 1000ms exceeded.');
   expect(result.output).toContain('Rejecting!');
 });
+
+test('should not complain about fixtures of non-focused tests', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const { test } = pwt;
+      test.only('works', () => {});
+      test('unknown fixture', ({ foo }) => {});
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+});
+
+test('should not complain about fixtures of unused hooks', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const { test } = pwt;
+      test.only('works', () => {});
+      test.describe('unused suite', () => {
+        test.beforeAll(({ foo }) => {});
+        test('unused test', () => {});
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+});