chore(test runner): prepare to per-fixture timeout (#11605)

This reworks DeadlineRunner to use exception to signal timeout. This way,
we'll be able to run fixtures against a shared deadline vs their own
deadline and still get an easy control-flow timeout handling.

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

@@ -26,7 +26,7 @@ import { envObjectToArray } from './clientHelper';
 import { assert, headersObjectToArray, monotonicTime } from '../utils/utils';
 import * as api from '../../types/types';
 import { kBrowserClosedError } from '../utils/errors';
-import { raceAgainstDeadline } from '../utils/async';
+import { raceAgainstTimeout } from '../utils/async';
 import type { Playwright } from './playwright';
 
 export interface BrowserServerLauncher {
@@ -157,32 +157,25 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
         }
       });
 
-      const createBrowserPromise = new Promise<Browser>(async (fulfill, reject) => {
-        try {
-          // For tests.
-          if ((params as any).__testHookBeforeCreateBrowser)
-            await (params as any).__testHookBeforeCreateBrowser();
+      const result = await raceAgainstTimeout(async () => {
+        // For tests.
+        if ((params as any).__testHookBeforeCreateBrowser)
+          await (params as any).__testHookBeforeCreateBrowser();
 
-          const playwright = await connection!.initializePlaywright();
-          if (!playwright._initializer.preLaunchedBrowser) {
-            reject(new Error('Malformed endpoint. Did you use launchServer method?'));
-            closePipe();
-            return;
-          }
-          playwright._setSelectors(this._playwright.selectors);
-          browser = Browser.from(playwright._initializer.preLaunchedBrowser!);
-          browser._logger = logger;
-          browser._shouldCloseConnectionOnClose = true;
-          browser._setBrowserType((playwright as any)[browser._name]);
-          browser._localUtils = this._playwright._utils;
-          browser.on(Events.Browser.Disconnected, closePipe);
-          fulfill(browser);
-        } catch (e) {
-          reject(e);
+        const playwright = await connection!.initializePlaywright();
+        if (!playwright._initializer.preLaunchedBrowser) {
+          closePipe();
+          throw new Error('Malformed endpoint. Did you use launchServer method?');
         }
-      });
-
-      const result = await raceAgainstDeadline(createBrowserPromise, deadline);
+        playwright._setSelectors(this._playwright.selectors);
+        browser = Browser.from(playwright._initializer.preLaunchedBrowser!);
+        browser._logger = logger;
+        browser._shouldCloseConnectionOnClose = true;
+        browser._setBrowserType((playwright as any)[browser._name]);
+        browser._localUtils = this._playwright._utils;
+        browser.on(Events.Browser.Disconnected, closePipe);
+        return browser;
+      }, deadline ? deadline - monotonicTime() : 0);
       if (!result.timedOut) {
         return result.result;
       } else {

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

@@ -16,50 +16,85 @@
 
 import { monotonicTime } from './utils';
 
-export class DeadlineRunner<T> {
-  private _timer: NodeJS.Timer | undefined;
-  readonly result = new ManualPromise<{ timedOut: true } | { result: T, timedOut: false }>();
+export class TimeoutRunnerError extends Error {}
 
-  constructor(promise: Promise<T>, deadline: number) {
-    promise.then(result => {
-      this._finish({ result, timedOut: false });
-    }).catch(e => {
-      this._finish(undefined, e);
-    });
-    this.updateDeadline(deadline);
+type TimeoutRunnerData = {
+  start: number,
+  timer: NodeJS.Timer | undefined,
+  timeoutPromise: ManualPromise<any>,
+};
+export class TimeoutRunner {
+  private _running: TimeoutRunnerData | undefined;
+  private _timeout: number;
+  private _elapsed: number;
+
+  constructor(timeout: number) {
+    this._timeout = timeout;
+    this._elapsed = 0;
   }
 
-  private _finish(success?: { timedOut: true } | { result: T, timedOut: false }, error?: any) {
-    if (this.result.isDone())
-      return;
-    this.updateDeadline(0);
-    if (success)
-      this.result.resolve(success);
-    else
-      this.result.reject(error);
+  async run<T>(cb: () => Promise<T>): Promise<T> {
+    const running = this._running = {
+      start: monotonicTime(),
+      timer: undefined,
+      timeoutPromise: new ManualPromise(),
+    };
+    try {
+      const resultPromise = Promise.race([
+        cb(),
+        running.timeoutPromise
+      ]);
+      this._updateTimeout(running, this._timeout);
+      return await resultPromise;
+    } finally {
+      this._elapsed += monotonicTime() - running.start;
+      this._updateTimeout(running, 0);
+      if (this._running === running)
+        this._running = undefined;
+    }
   }
 
   interrupt() {
-    this.updateDeadline(-1);
+    if (this._running)
+      this._updateTimeout(this._running, -1);
   }
 
-  updateDeadline(deadline: number) {
-    if (this._timer) {
-      clearTimeout(this._timer);
-      this._timer = undefined;
+  updateTimeout(timeout: number) {
+    this._timeout = timeout;
+    if (this._running)
+      this._updateTimeout(this._running, timeout);
+  }
+
+  resetTimeout(timeout: number) {
+    this._elapsed = 0;
+    this.updateTimeout(timeout);
+  }
+
+  private _updateTimeout(running: TimeoutRunnerData, timeout: number) {
+    if (running.timer) {
+      clearTimeout(running.timer);
+      running.timer = undefined;
     }
-    if (deadline === 0)
+    if (timeout === 0)
       return;
-    const timeout = deadline - monotonicTime();
+    const elapsed = (monotonicTime() - running.start) + this._elapsed;
+    timeout = timeout - elapsed;
     if (timeout <= 0)
-      this._finish({ timedOut: true });
+      running.timeoutPromise.reject(new TimeoutRunnerError());
     else
-      this._timer = setTimeout(() => this._finish({ timedOut: true }), timeout);
+      running.timer = setTimeout(() => running.timeoutPromise.reject(new TimeoutRunnerError()), timeout);
   }
 }
 
-export async function raceAgainstDeadline<T>(promise: Promise<T>, deadline: number) {
-  return (new DeadlineRunner(promise, deadline)).result;
+export async function raceAgainstTimeout<T>(cb: () => Promise<T>, timeout: number): Promise<{ result: T, timedOut: false } | { timedOut: true }> {
+  const runner = new TimeoutRunner(timeout);
+  try {
+    return { result: await runner.run(cb), timedOut: false };
+  } catch (e) {
+    if (e instanceof TimeoutRunnerError)
+      return { timedOut: true };
+    throw e;
+  }
 }
 
 export class ManualPromise<T> extends Promise<T> {

packages/playwright-test/src/runner.ts

@@ -20,7 +20,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import { promisify } from 'util';
 import { Dispatcher, TestGroup } from './dispatcher';
-import { createFileMatcher, createTitleMatcher, FilePatternFilter, monotonicTime, serializeError } from './util';
+import { createFileMatcher, createTitleMatcher, FilePatternFilter, serializeError } from './util';
 import { TestCase, Suite } from './test';
 import { Loader } from './loader';
 import { FullResult, Reporter, TestError } from '../types/testReporter';
@@ -37,7 +37,7 @@ import { ProjectImpl } from './project';
 import { Minimatch } from 'minimatch';
 import { Config, FullConfig } from './types';
 import { WebServer } from './webServer';
-import { raceAgainstDeadline } from 'playwright-core/lib/utils/async';
+import { raceAgainstTimeout } from 'playwright-core/lib/utils/async';
 
 const removeFolderAsync = promisify(rimraf);
 const readDirAsync = promisify(fs.readdir);
@@ -136,8 +136,7 @@ export class Runner {
     this._reporter = await this._createReporter(!!options.listOnly);
     try {
       const config = this._loader.fullConfig();
-      const globalDeadline = config.globalTimeout ? config.globalTimeout + monotonicTime() : 0;
-      const result = await raceAgainstDeadline(this._run(!!options.listOnly, options.filePatternFilter || [], options.projectFilter), globalDeadline);
+      const result = await raceAgainstTimeout(() => this._run(!!options.listOnly, options.filePatternFilter || [], options.projectFilter), config.globalTimeout);
       if (result.timedOut) {
         const actualResult: FullResult = { status: 'timedout' };
         if (this._didBegin)

packages/playwright-test/src/webServer.ts

@@ -18,8 +18,7 @@ import net from 'net';
 import os from 'os';
 import stream from 'stream';
 import debug from 'debug';
-import { monotonicTime } from './util';
-import { raceAgainstDeadline } from 'playwright-core/lib/utils/async';
+import { raceAgainstTimeout } from 'playwright-core/lib/utils/async';
 import { WebServerConfig } from './types';
 import { launchProcess } from 'playwright-core/lib/utils/processLauncher';
 
@@ -95,7 +94,7 @@ export class WebServer {
     const launchTimeout = this.config.timeout || 60 * 1000;
     const cancellationToken = { canceled: false };
     const { timedOut } = (await Promise.race([
-      raceAgainstDeadline(waitForSocket(this.config.port, 100, cancellationToken), launchTimeout + monotonicTime()),
+      raceAgainstTimeout(() => waitForSocket(this.config.port, 100, cancellationToken), launchTimeout),
       this._processExitedPromise,
     ]));
     cancellationToken.canceled = true;

packages/playwright-test/src/workerRunner.ts

@@ -29,7 +29,7 @@ import { Modifier, Suite, TestCase } from './test';
 import { Annotations, TestCaseType, TestError, TestInfo, TestInfoImpl, TestStepInternal, WorkerInfo } from './types';
 import { ProjectImpl } from './project';
 import { FixtureRunner } from './fixtures';
-import { DeadlineRunner, raceAgainstDeadline } from 'playwright-core/lib/utils/async';
+import { TimeoutRunner, raceAgainstTimeout, TimeoutRunnerError } from 'playwright-core/lib/utils/async';
 import { calculateSha1 } from 'playwright-core/lib/utils/utils';
 
 const removeFolderAsync = util.promisify(rimraf);
@@ -50,7 +50,7 @@ export class WorkerRunner extends EventEmitter {
   private _entries = new Map<string, TestEntry>();
   private _isStopped = false;
   private _runFinished = Promise.resolve();
-  private _currentDeadlineRunner: DeadlineRunner<any> | undefined;
+  private _currentTimeoutRunner: TimeoutRunner | undefined;
   _currentTest: TestData | null = null;
 
   constructor(params: WorkerInitParams) {
@@ -64,7 +64,7 @@ export class WorkerRunner extends EventEmitter {
       this._isStopped = true;
 
       // Interrupt current action.
-      this._currentDeadlineRunner?.interrupt();
+      this._currentTimeoutRunner?.interrupt();
 
       // TODO: mark test as 'interrupted' instead.
       if (this._currentTest && this._currentTest.testInfo.status === 'passed')
@@ -83,10 +83,10 @@ export class WorkerRunner extends EventEmitter {
 
   private async _teardownScopes() {
     // TODO: separate timeout for teardown?
-    const result = await raceAgainstDeadline((async () => {
+    const result = await raceAgainstTimeout(async () => {
       await this._fixtureRunner.teardownScope('test');
       await this._fixtureRunner.teardownScope('worker');
-    })(), this._deadline());
+    }, this._project.config.timeout);
     if (result.timedOut && !this._fatalError)
       this._fatalError = { message: colors.red(`Timeout of ${this._project.config.timeout}ms exceeded while shutting down environment`) };
   }
@@ -115,10 +115,6 @@ export class WorkerRunner extends EventEmitter {
     this.stop();
   }
 
-  private _deadline() {
-    return this._project.config.timeout ? monotonicTime() + this._project.config.timeout : 0;
-  }
-
   private async _loadIfNeeded() {
     if (this._loader)
       return;
@@ -189,7 +185,7 @@ export class WorkerRunner extends EventEmitter {
       if (!this._fixtureRunner.dependsOnWorkerFixturesOnly(beforeAllModifier.fn, beforeAllModifier.location))
         continue;
       // TODO: separate timeout for beforeAll modifiers?
-      const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunFunction(beforeAllModifier.fn, this._workerInfo, undefined), this._deadline());
+      const result = await raceAgainstTimeout(() => this._fixtureRunner.resolveParametersAndRunFunction(beforeAllModifier.fn, this._workerInfo, undefined), this._project.config.timeout);
       if (result.timedOut) {
         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)}`));
@@ -224,7 +220,7 @@ export class WorkerRunner extends EventEmitter {
   private async _runTestOrAllHook(test: TestCase, annotations: Annotations, retry: number) {
     const startTime = monotonicTime();
     const startWallTime = Date.now();
-    let deadlineRunner: DeadlineRunner<any> | undefined;
+    let timeoutRunner: TimeoutRunner;
     const testId = test._id;
 
     const baseOutputDir = (() => {
@@ -313,8 +309,8 @@ export class WorkerRunner extends EventEmitter {
         if (!testInfo.timeout)
           return; // Zero timeout means some debug mode - do not set a timeout.
         testInfo.timeout = timeout;
-        if (deadlineRunner)
-          deadlineRunner.updateDeadline(deadline());
+        if (timeoutRunner)
+          timeoutRunner.updateTimeout(timeout);
       },
       _addStep: data => {
         const stepId = `${data.category}@${data.title}@${++lastStepId}`;
@@ -381,10 +377,6 @@ export class WorkerRunner extends EventEmitter {
     this._currentTest = testData;
     setCurrentTestInfo(testInfo);
 
-    const deadline = () => {
-      return testInfo.timeout ? startTime + testInfo.timeout : 0;
-    };
-
     this.emit('testBegin', buildTestBeginPayload(testData, startWallTime));
 
     if (testInfo.expectedStatus === 'skipped') {
@@ -396,29 +388,18 @@ export class WorkerRunner extends EventEmitter {
     // Update the fixture pool - it may differ between tests, but only in test-scoped fixtures.
     this._fixtureRunner.setPool(test._pool!);
 
-    this._currentDeadlineRunner = deadlineRunner = new DeadlineRunner(this._runTestWithBeforeHooks(test, testInfo), deadline());
-    const result = await deadlineRunner.result;
-    // Do not overwrite test failure upon hook timeout.
-    if (result.timedOut && testInfo.status === 'passed')
-      testInfo.status = 'timedOut';
+    this._currentTimeoutRunner = timeoutRunner = new TimeoutRunner(testInfo.timeout);
+    await this._runWithTimeout(timeoutRunner, () => this._runTestWithBeforeHooks(test, testInfo), testInfo);
     testInfo.duration = monotonicTime() - startTime;
 
-    if (!result.timedOut) {
-      this._currentDeadlineRunner = deadlineRunner = new DeadlineRunner(this._runAfterHooks(test, testInfo), deadline());
-      deadlineRunner.updateDeadline(deadline());
-      const hooksResult = await deadlineRunner.result;
-      // Do not overwrite test failure upon hook timeout.
-      if (hooksResult.timedOut && testInfo.status === 'passed')
-        testInfo.status = 'timedOut';
-    } else {
+    if (testInfo.status === 'timedOut') {
       // A timed-out test gets a full additional timeout to run after hooks.
-      const newDeadline = this._deadline();
-      this._currentDeadlineRunner = deadlineRunner = new DeadlineRunner(this._runAfterHooks(test, testInfo), newDeadline);
-      await deadlineRunner.result;
+      timeoutRunner.resetTimeout(testInfo.timeout);
     }
+    await this._runWithTimeout(timeoutRunner, () => this._runAfterHooks(test, testInfo), testInfo);
 
     testInfo.duration = monotonicTime() - startTime;
-    this._currentDeadlineRunner = undefined;
+    this._currentTimeoutRunner = undefined;
     this._currentTest = null;
     setCurrentTestInfo(null);
 
@@ -496,10 +477,10 @@ export class WorkerRunner extends EventEmitter {
 
     let teardownError1: TestError | undefined;
     if (test._type === 'test')
-      teardownError1 = await this._runFn(() => this._runHooks(test.parent!, 'afterEach', testInfo), testInfo, 'disallowSkips');
+      teardownError1 = await this._runFn(() => this._runHooks(test.parent!, 'afterEach', testInfo), testInfo);
     // Continue teardown even after the failure.
 
-    const teardownError2 = await this._runFn(() => this._fixtureRunner.teardownScope('test'), testInfo, 'disallowSkips');
+    const teardownError2 = await this._runFn(() => this._fixtureRunner.teardownScope('test'), testInfo);
     step.complete(teardownError1 || teardownError2);
   }
 
@@ -524,7 +505,19 @@ export class WorkerRunner extends EventEmitter {
       throw error;
   }
 
-  private async _runFn(fn: Function, testInfo: TestInfoImpl, skips: 'allowSkips' | 'disallowSkips'): Promise<TestError | undefined> {
+  private async _runWithTimeout(timeoutRunner: TimeoutRunner, cb: () => Promise<any>, testInfo: TestInfoImpl): Promise<void> {
+    try {
+      await timeoutRunner.run(cb);
+    } catch (error) {
+      if (!(error instanceof TimeoutRunnerError))
+        throw error;
+      // Do not overwrite existing failure upon hook/teardown timeout.
+      if (testInfo.status === 'passed')
+        testInfo.status = 'timedOut';
+    }
+  }
+
+  private async _runFn(fn: Function, testInfo: TestInfoImpl, skips?: 'allowSkips'): Promise<TestError | undefined> {
     try {
       await fn();
     } catch (error) {