chore: small test runner changes in preparation of global fixtures (#13899)

The main change is splitting up options from the config from other
fixtures to ensure unique location for them.

packages/playwright-test/src/fixtures.ts

@@ -22,20 +22,31 @@ import type { TestInfoImpl } from './testInfo';
 import type { FixtureDescription, TimeoutManager } from './timeoutManager';
 
 type FixtureScope = 'test' | 'worker';
+const kScopeOrder: FixtureScope[] = ['test', 'worker'];
 type FixtureOptions = { auto?: boolean, scope?: FixtureScope, option?: boolean, timeout?: number | undefined };
 type FixtureTuple = [ value: any, options: FixtureOptions ];
 type FixtureRegistration = {
-  location: Location;  // Fixutre registration location.
+  // Fixture registration location.
+  location: Location;
+  // Fixture name comes from test.extend() call.
   name: string;
   scope: FixtureScope;
-  fn: Function | any;  // Either a fixture function, or a fixture value.
+  // Either a fixture function, or a fixture value.
+  fn: Function | any;
+  // Auto fixtures always run without user explicitly mentioning them.
   auto: boolean;
+  // An "option" fixture can have a value set in the config.
   option: boolean;
+  // Custom title to be used instead of the name, internal-only.
   customTitle?: string;
+  // Fixture with a separate timeout does not count towards the test time.
   timeout?: number;
-  deps: string[];  // Names of the dependencies, ({ foo, bar }) => {...}
-  id: string;  // Unique id, to differentiate between fixtures with the same name.
-  super?: FixtureRegistration;  // A fixture override can use the previous version of the fixture.
+  // Names of the dependencies, comes from the declaration "({ foo, bar }) => {...}"
+  deps: string[];
+  // Unique id, to differentiate between fixtures with the same name.
+  id: string;
+  // A fixture override can use the previous version of the fixture.
+  super?: FixtureRegistration;
 };
 
 class Fixture {
@@ -174,7 +185,7 @@ export class FixturePool {
           options = { auto: false, scope: 'test', option: false, timeout: undefined, customTitle: undefined };
         }
 
-        if (options.scope !== 'test' && options.scope !== 'worker')
+        if (!kScopeOrder.includes(options.scope))
           throw errorWithLocations(`Fixture "${name}" has unknown { scope: '${options.scope}' }.`, { location, name });
         if (options.scope === 'worker' && disallowWorkerFixtures)
           throw errorWithLocations(`Cannot use({ ${name} }) in a describe group, because it forces a new worker.\nMake it top-level in the test file or put in the configuration file.`, { location, name });
@@ -203,8 +214,8 @@ export class FixturePool {
           else
             throw errorWithLocations(`Fixture "${registration.name}" has unknown parameter "${name}".`, registration);
         }
-        if (registration.scope === 'worker' && dep.scope === 'test')
-          throw errorWithLocations(`Worker fixture "${registration.name}" cannot depend on a test fixture "${name}".`, registration, dep);
+        if (kScopeOrder.indexOf(registration.scope) > kScopeOrder.indexOf(dep.scope))
+          throw errorWithLocations(`${registration.scope} fixture "${registration.name}" cannot depend on a ${dep.scope} fixture "${name}".`, registration, dep);
         if (!markers.has(dep)) {
           visit(dep);
         } else if (markers.get(dep) === 'visiting') {

packages/playwright-test/src/loader.ts

@@ -283,7 +283,7 @@ export class Loader {
     const screenshotsDir = takeFirst((projectConfig as any).screenshotsDir, (config as any).screenshotsDir, path.join(testDir, '__screenshots__', process.platform, name));
     return {
       _fullyParallel: takeFirst(projectConfig.fullyParallel, config.fullyParallel, undefined),
-      _expect: takeFirst(projectConfig.expect, config.expect, undefined),
+      _expect: takeFirst(projectConfig.expect, config.expect, {}),
       grep: takeFirst(projectConfig.grep, config.grep, baseFullConfig.grep),
       grepInvert: takeFirst(projectConfig.grepInvert, config.grepInvert, baseFullConfig.grepInvert),
       outputDir,
@@ -346,7 +346,7 @@ class ProjectSuiteBuilder {
 
   private _buildTestTypePool(testType: TestTypeImpl): FixturePool {
     if (!this._testTypePools.has(testType)) {
-      const fixtures = this._applyConfigUseOptions(testType, this._config.use);
+      const fixtures = this._applyConfigUseOptions(testType, this._config.use || {});
       const pool = new FixturePool(fixtures);
       this._testTypePools.set(testType, pool);
     }
@@ -417,18 +417,25 @@ class ProjectSuiteBuilder {
   }
 
   private _applyConfigUseOptions(testType: TestTypeImpl, configUse: Fixtures): FixturesWithLocation[] {
-    return testType.fixtures.map(f => {
-      const configKeys = new Set(Object.keys(configUse || {}));
-      const resolved = { ...f.fixtures };
-      for (const [key, value] of Object.entries(resolved)) {
-        if (!isFixtureOption(value) || !configKeys.has(key))
-          continue;
-        // Apply override from config file.
-        const override = (configUse as any)[key];
-        (resolved as any)[key] = [override, value[1]];
+    const configKeys = new Set(Object.keys(configUse));
+    if (!configKeys.size)
+      return testType.fixtures;
+    const result: FixturesWithLocation[] = [];
+    for (const f of testType.fixtures) {
+      const optionsFromConfig: Fixtures = {};
+      const originalFixtures: 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]];
+        else
+          (originalFixtures as any)[key] = value;
       }
-      return { fixtures: resolved, location: f.location };
-    });
+      if (Object.entries(optionsFromConfig).length)
+        result.push({ fixtures: optionsFromConfig, location: { file: `project#${this._index}`, line: 1, column: 1 } });
+      if (Object.entries(originalFixtures).length)
+        result.push({ fixtures: originalFixtures, location: f.location });
+    }
+    return result;
   }
 }
 

tests/playwright-test/fixture-errors.spec.ts

@@ -194,7 +194,7 @@ test('should throw when worker fixture depends on a test fixture', async ({ runI
       test('works', async ({bar}) => {});
     `,
   });
-  expect(result.output).toContain('Worker fixture "bar" cannot depend on a test fixture "foo".');
+  expect(result.output).toContain('worker fixture "bar" cannot depend on a test fixture "foo".');
   expect(result.output).toContain(`f.spec.ts:5`);
   expect(result.exitCode).toBe(1);
 });
@@ -302,7 +302,7 @@ test('should throw when overridden worker fixture depends on a test fixture', as
       test2('works', async ({bar}) => {});
     `,
   });
-  expect(result.output).toContain('Worker fixture "bar" cannot depend on a test fixture "foo".');
+  expect(result.output).toContain('worker fixture "bar" cannot depend on a test fixture "foo".');
   expect(result.exitCode).toBe(1);
 });