chore: use fake pool on the runner side (#20241)

packages/playwright-test/src/poolBuilder.ts

@@ -20,18 +20,30 @@ import type { TestTypeImpl } from './testType';
 import type { Fixtures, FixturesWithLocation, FullProjectInternal } from './types';
 
 export class PoolBuilder {
-  private _project: FullProjectInternal;
+  private _project: FullProjectInternal | undefined;
   private _testTypePools = new Map<TestTypeImpl, FixturePool>();
+  private _type: 'loader' | 'worker';
 
-  constructor(project: FullProjectInternal) {
+  static buildForLoader(suite: Suite) {
+    new PoolBuilder('loader').buildPools(suite);
+  }
+
+  static createForWorker(project: FullProjectInternal) {
+    return new PoolBuilder('worker', project);
+  }
+
+  private constructor(type: 'loader' | 'worker', project?: FullProjectInternal) {
+    this._type = type;
     this._project = project;
   }
 
-  buildPools(suite: Suite, repeatEachIndex: number) {
+  buildPools(suite: Suite) {
     suite.forEachTest(test => {
       const pool = this._buildPoolForTest(test);
-      test._workerHash = `run${this._project._id}-${pool.digest}-repeat${repeatEachIndex}`;
-      test._pool = pool;
+      if (this._type === 'loader')
+        test._poolDigest = pool.digest;
+      if (this._type === 'worker')
+        test._pool = pool;
     });
   }
 
@@ -58,15 +70,16 @@ export class PoolBuilder {
 
   private _buildTestTypePool(testType: TestTypeImpl): FixturePool {
     if (!this._testTypePools.has(testType)) {
-      const fixtures = this._applyConfigUseOptions(testType, this._project.use || {});
+      const fixtures = this._project ? this._applyConfigUseOptions(this._project, testType) : testType.fixtures;
       const pool = new FixturePool(fixtures);
       this._testTypePools.set(testType, pool);
     }
     return this._testTypePools.get(testType)!;
   }
 
-  private _applyConfigUseOptions(testType: TestTypeImpl, configUse: Fixtures): FixturesWithLocation[] {
-    const configKeys = new Set(Object.keys(configUse));
+  private _applyConfigUseOptions(project: FullProjectInternal, testType: TestTypeImpl): FixturesWithLocation[] {
+    const projectUse = project.use || {};
+    const configKeys = new Set(Object.keys(projectUse));
     if (!configKeys.size)
       return testType.fixtures;
     const result: FixturesWithLocation[] = [];
@@ -75,12 +88,12 @@ export class PoolBuilder {
       const optionsFromConfig: Fixtures = {};
       for (const [key, value] of Object.entries(f.fixtures)) {
         if (isFixtureOption(value) && configKeys.has(key))
-          (optionsFromConfig as any)[key] = [(configUse as any)[key], value[1]];
+          (optionsFromConfig as any)[key] = [(projectUse as any)[key], value[1]];
       }
       if (Object.entries(optionsFromConfig).length) {
         // Add config options immediately after original option definition,
         // so that any test.use() override it.
-        result.push({ fixtures: optionsFromConfig, location: { file: `project#${this._project._id}`, line: 1, column: 1 }, fromConfig: true });
+        result.push({ fixtures: optionsFromConfig, location: { file: `project#${project._id}`, line: 1, column: 1 }, fromConfig: true });
       }
     }
     return result;

packages/playwright-test/src/runner.ts

@@ -283,19 +283,12 @@ export class Runner {
     const config = this._configLoader.fullConfig();
     const fatalErrors: TestError[] = [];
     const allTestFiles = new Set<string>();
-    const testLoader = new TestLoader(config);
     for (const files of filesByProject.values())
       files.forEach(file => allTestFiles.add(file));
 
-    // Add all tests.
-    const preprocessRoot = new Suite('', 'root');
-    for (const file of allTestFiles) {
-      const fileSuite = await testLoader.loadTestFile(file, 'runner');
-      if (fileSuite._loadError)
-        fatalErrors.push(fileSuite._loadError);
-      // We have to clone only if there maybe subsequent calls of this method.
-      preprocessRoot._addSuite(fileSuite);
-    }
+    // Load all tests.
+    const { rootSuite: preprocessRoot, loadErrors } = await this._loadTests(allTestFiles);
+    fatalErrors.push(...loadErrors);
 
     // Complain about duplicate titles.
     fatalErrors.push(...createDuplicateTitlesErrors(config, preprocessRoot));
@@ -321,7 +314,6 @@ export class Runner {
 
     const rootSuite = new Suite('', 'root');
     for (const [project, files] of filesByProject) {
-      const poolBuilder = new PoolBuilder(project);
       const grepMatcher = createTitleMatcher(project.grep);
       const grepInvertMatcher = project.grepInvert ? createTitleMatcher(project.grepInvert) : null;
 
@@ -346,13 +338,29 @@ export class Runner {
           if (!filterTests(builtSuite, titleMatcher))
             continue;
           projectSuite._addSuite(builtSuite);
-          poolBuilder.buildPools(builtSuite, repeatEachIndex);
         }
       }
     }
     return { rootSuite, fatalErrors };
   }
 
+  private async _loadTests(testFiles: Set<string>): Promise<{ rootSuite: Suite, loadErrors: TestError[] }> {
+    const config = this._configLoader.fullConfig();
+    const testLoader = new TestLoader(config);
+    const loadErrors: TestError[] = [];
+    const rootSuite = new Suite('', 'root');
+    for (const file of testFiles) {
+      const fileSuite = await testLoader.loadTestFile(file, 'loader');
+      if (fileSuite._loadError)
+        loadErrors.push(fileSuite._loadError);
+      // We have to clone only if there maybe subsequent calls of this method.
+      rootSuite._addSuite(fileSuite);
+    }
+    // Generate hashes.
+    PoolBuilder.buildForLoader(rootSuite);
+    return { rootSuite, loadErrors };
+  }
+
   private _filterForCurrentShard(rootSuite: Suite, testGroups: TestGroup[]) {
     const shard = this._configLoader.fullConfig().shard;
     if (!shard)

packages/playwright-test/src/suiteUtils.ts

@@ -52,6 +52,9 @@ export function buildFileSuiteForProject(project: FullProjectInternal, suite: Su
         break;
       }
     }
+    // We only compute / set digest in the runner.
+    if (test._poolDigest)
+      test._workerHash = `${project._id}-${test._poolDigest}-${repeatEachIndex}`;
   });
 
   return result;

packages/playwright-test/src/test.ts

@@ -160,8 +160,9 @@ export class TestCase extends Base implements reporterTypes.TestCase {
 
   _testType: TestTypeImpl;
   id = '';
-  _workerHash = '';
   _pool: FixturePool | undefined;
+  _poolDigest = '';
+  _workerHash = '';
   _projectId = '';
   // Annotations that are not added from within a test (like fixme and skip), should not
   // be re-added each time we retry a test.
@@ -200,6 +201,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
     const test = new TestCase(this.title, this.fn, this._testType, this.location);
     test._only = this._only;
     test._requireFile = this._requireFile;
+    test._poolDigest = this._poolDigest;
     test.expectedStatus = this.expectedStatus;
     test.annotations = this.annotations.slice();
     test._annotateWithInheritence = this._annotateWithInheritence;

packages/playwright-test/src/testLoader.ts

@@ -34,7 +34,7 @@ export class TestLoader {
     this._fullConfig = fullConfig;
   }
 
-  async loadTestFile(file: string, environment: 'runner' | 'worker'): Promise<Suite> {
+  async loadTestFile(file: string, environment: 'loader' | 'worker'): Promise<Suite> {
     if (cachedFileSuites.has(file))
       return cachedFileSuites.get(file)!;
     const suite = new Suite(path.relative(this._fullConfig.rootDir, file) || path.basename(file), 'file');

packages/playwright-test/src/workerRunner.ts

@@ -175,7 +175,7 @@ export class WorkerRunner extends ProcessRunner {
     this._configLoader = await ConfigLoader.deserialize(this._params.config);
     this._testLoader = new TestLoader(this._configLoader.fullConfig());
     this._project = this._configLoader.fullConfig().projects.find(p => p._id === this._params.projectId)!;
-    this._poolBuilder = new PoolBuilder(this._project);
+    this._poolBuilder = PoolBuilder.createForWorker(this._project);
   }
 
   async runTestGroup(runPayload: RunPayload) {
@@ -188,7 +188,7 @@ export class WorkerRunner extends ProcessRunner {
       const suite = buildFileSuiteForProject(this._project, fileSuite, this._params.repeatEachIndex);
       const hasEntries = filterTests(suite, test => entries.has(test.id));
       if (hasEntries) {
-        this._poolBuilder.buildPools(suite, this._params.repeatEachIndex);
+        this._poolBuilder.buildPools(suite);
         this._extraSuiteAnnotations = new Map();
         this._activeSuites = new Set();
         this._didRunFullCleanup = false;

tests/playwright-test/fixtures.spec.ts

@@ -714,30 +714,3 @@ test('worker teardown errors reflected in timed-out tests', async ({ runInlineTe
   expect(result.output).toContain('Test 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);
-});

tests/playwright-test/playwright.config.ts

@@ -24,6 +24,7 @@ const outputDir = path.join(__dirname, '..', '..', 'test-results');
 const config: Config = {
   timeout: 30000,
   forbidOnly: !!process.env.CI,
+  fullyParallel: !process.env.CI,
   workers: process.env.CI ? 2 : undefined,
   preserveOutput: process.env.CI ? 'failures-only' : 'always',
   snapshotPathTemplate: '__screenshots__/{testFilePath}/{arg}{ext}',

tests/playwright-test/web-server.spec.ts

@@ -279,7 +279,7 @@ test('should be able to specify the baseURL without the server', async ({ runInl
 });
 
 test('should be able to specify a custom baseURL with the server', async ({ runInlineTest }, { workerIndex }) => {
-  const customWebServerPort = workerIndex + 10500;
+  const customWebServerPort = workerIndex * 2 + 10500;
   const webServerPort = customWebServerPort + 1;
   const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
     res.end('<html><body>hello</body></html>');
@@ -458,7 +458,7 @@ test('should send Accept header', async ({ runInlineTest, server }) => {
 });
 
 test('should create multiple servers', async ({ runInlineTest }, { workerIndex }) => {
-  const port = workerIndex + 10500;
+  const port = workerIndex * 2 + 10500;
   const result = await runInlineTest({
     'test.spec.ts': `
         const { test } = pwt;
@@ -533,7 +533,7 @@ test('should create multiple servers', async ({ runInlineTest }, { workerIndex }
 
 test.describe('baseURL with plugins', () => {
   test('plugins do not set it', async ({ runInlineTest }, { workerIndex }) => {
-    const port = workerIndex + 10500;
+    const port = workerIndex * 2 + 10500;
     const result = await runInlineTest({
       'test.spec.ts': `
           import { webServer } from '@playwright/test/lib/plugins';
@@ -553,7 +553,7 @@ test.describe('baseURL with plugins', () => {
   });
 
   test('legacy config sets it alongside plugin', async ({ runInlineTest }, { workerIndex }) => {
-    const port = workerIndex + 10500;
+    const port = workerIndex * 2 + 10500;
     const result = await runInlineTest({
       'test.spec.ts': `
           import { webServer } from '@playwright/test/lib/plugins';