chore(test runner): minor improvements (#11067)

- Types for fixture options and more.
- Refined type for deadline runner.

packages/playwright-core/src/client/browserType.ts

@@ -183,7 +183,7 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
       });
 
       const result = await raceAgainstDeadline(createBrowserPromise, deadline);
-      if (result.result) {
+      if (!result.timedOut) {
         return result.result;
       } else {
         closePipe();

packages/playwright-core/src/utils/async.ts

@@ -18,18 +18,18 @@ import { monotonicTime } from './utils';
 
 export class DeadlineRunner<T> {
   private _timer: NodeJS.Timer | undefined;
-  readonly result = new ManualPromise<{ result?: T, timedOut?: boolean }>();
+  readonly result = new ManualPromise<{ timedOut: true } | { result: T, timedOut: false }>();
 
   constructor(promise: Promise<T>, deadline: number) {
     promise.then(result => {
-      this._finish({ result });
+      this._finish({ result, timedOut: false });
     }).catch(e => {
       this._finish(undefined, e);
     });
     this.updateDeadline(deadline);
   }
 
-  private _finish(success?: { result?: T, timedOut?: boolean }, error?: any) {
+  private _finish(success?: { timedOut: true } | { result: T, timedOut: false }, error?: any) {
     if (this.result.isDone())
       return;
     this.updateDeadline(0);
@@ -58,7 +58,7 @@ export class DeadlineRunner<T> {
   }
 }
 
-export async function raceAgainstDeadline<T>(promise: Promise<T>, deadline: number): Promise<{ result?: T, timedOut?: boolean }> {
+export async function raceAgainstDeadline<T>(promise: Promise<T>, deadline: number) {
   return (new DeadlineRunner(promise, deadline)).result;
 }
 

packages/playwright-test/src/fixtures.ts

@@ -20,15 +20,18 @@ import { FixturesWithLocation, Location, WorkerInfo, TestInfo, TestStepInternal
 import { ManualPromise } from 'playwright-core/lib/utils/async';
 
 type FixtureScope = 'test' | 'worker';
+type FixtureOptions = { auto?: boolean, scope?: FixtureScope, option?: boolean };
+type FixtureTuple = [ value: any, options: FixtureOptions ];
 type FixtureRegistration = {
-  location: Location;
+  location: Location;  // Fixutre registration location.
   name: string;
   scope: FixtureScope;
   fn: Function | any;  // Either a fixture function, or a fixture value.
   auto: boolean;
-  deps: string[];
-  id: string;
-  super?: FixtureRegistration;
+  option: boolean;
+  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.
 };
 
 class Fixture {
@@ -105,11 +108,11 @@ class Fixture {
   }
 }
 
-function isFixtureTuple(value: any): value is [any, any] {
+function isFixtureTuple(value: any): value is FixtureTuple {
   return Array.isArray(value) && typeof value[1] === 'object' && ('scope' in value[1] || 'auto' in value[1] || 'option' in value[1]);
 }
 
-export function isFixtureOption(value: any): value is [any, any] {
+export function isFixtureOption(value: any): value is FixtureTuple {
   return isFixtureTuple(value) && !!value[1].option;
 }
 
@@ -124,11 +127,12 @@ export class FixturePool {
       for (const entry of Object.entries(fixtures)) {
         const name = entry[0];
         let value = entry[1];
-        let options: { auto: boolean, scope: FixtureScope } | undefined;
+        let options: Required<FixtureOptions> | undefined;
         if (isFixtureTuple(value)) {
           options = {
             auto: !!value[1].auto,
-            scope: value[1].scope || 'test'
+            scope: value[1].scope || 'test',
+            option: !!value[1].option,
           };
           value = value[0];
         }
@@ -141,9 +145,9 @@ export class FixturePool {
           if (previous.auto !== options.auto)
             throw errorWithLocations(`Fixture "${name}" has already been registered as a { auto: '${previous.scope}' } fixture.`, { location, name }, previous);
         } else if (previous) {
-          options = { auto: previous.auto, scope: previous.scope };
+          options = { auto: previous.auto, scope: previous.scope, option: previous.option };
         } else if (!options) {
-          options = { auto: false, scope: 'test' };
+          options = { auto: false, scope: 'test', option: false };
         }
 
         if (options.scope !== 'test' && options.scope !== 'worker')
@@ -152,7 +156,7 @@ export class FixturePool {
           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 });
 
         const deps = fixtureParameterNames(fn, location);
-        const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, deps, super: previous };
+        const registration: FixtureRegistration = { id: '', name, location, scope: options.scope, fn, auto: options.auto, option: options.option, deps, super: previous };
         registrationId(registration);
         this.registrations.set(name, registration);
       }

packages/playwright-test/src/runner.ts

@@ -100,8 +100,8 @@ export class Runner {
     try {
       const config = this._loader.fullConfig();
       const globalDeadline = config.globalTimeout ? config.globalTimeout + monotonicTime() : 0;
-      const { result, timedOut } = await raceAgainstDeadline(this._run(list, filePatternFilters, projectNames), globalDeadline);
-      if (timedOut) {
+      const result = await raceAgainstDeadline(this._run(list, filePatternFilters, projectNames), globalDeadline);
+      if (result.timedOut) {
         const actualResult: FullResult = { status: 'timedout' };
         if (this._didBegin)
           await this._reporter.onEnd?.(actualResult);
@@ -109,7 +109,7 @@ export class Runner {
           this._reporter.onError?.(createStacklessError(`Timed out waiting ${config.globalTimeout / 1000}s for the entire test run`));
         return actualResult;
       }
-      return result!;
+      return result.result;
     } catch (e) {
       const result: FullResult = { status: 'failed' };
       try {

packages/playwright-test/src/workerRunner.ts

@@ -194,9 +194,9 @@ export class WorkerRunner extends EventEmitter {
         if (!this._fatalError)
           this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running ${beforeAllModifier.type} modifier\n    at ${formatLocation(beforeAllModifier.location)}`));
         this.stop();
-      }
-      if (!!result.result)
+      } else if (!!result.result) {
         annotations.push({ type: beforeAllModifier.type, description: beforeAllModifier.description });
+      }
     }
 
     for (const hook of suite.hooks) {