chore: use closures to set current runnable (#27293)

packages/playwright/src/worker/testInfo.ts

@@ -21,6 +21,7 @@ import type { TestInfoError, TestInfo, TestStatus, FullProject, FullConfig } fro
 import type { AttachmentPayload, StepBeginPayload, StepEndPayload, WorkerInitParams } from '../common/ipc';
 import type { TestCase } from '../common/test';
 import { TimeoutManager } from './timeoutManager';
+import type { RunnableType, TimeSlot, RunnableDescription } from './timeoutManager';
 import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
 import type { Location } from '../../types/testReporter';
 import { getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString } from '../util';
@@ -227,6 +228,12 @@ export class TestInfoImpl implements TestInfo {
     this.duration = this._timeoutManager.defaultSlotTimings().elapsed | 0;
   }
 
+  async _runWithRunnableAndFailOnError(runnable: RunnableDescription, cb: () => Promise<void>): Promise<TestInfoError | undefined> {
+    return await this._timeoutManager.withRunnable(runnable, async () => {
+      return await this._runAndFailOnError(cb);
+    });
+  }
+
   async _runAndFailOnError(fn: () => Promise<void>, skips?: 'allowSkips'): Promise<TestInfoError | undefined> {
     try {
       await fn();
@@ -348,6 +355,21 @@ export class TestInfoImpl implements TestInfo {
     this.errors.push(error);
   }
 
+  async _runAsStepWithRunnable<T>(
+    stepInfo: Omit<TestStepInternal, 'complete' | 'wallTime' | 'parentStepId' | 'stepId' | 'steps'> & {
+      wallTime?: number,
+      runnableType: RunnableType;
+      runnableSlot?: TimeSlot;
+    }, cb: (step: TestStepInternal) => Promise<T>): Promise<T> {
+    return await this._timeoutManager.withRunnable({
+      type: stepInfo.runnableType,
+      slot: stepInfo.runnableSlot,
+      location: stepInfo.location,
+    }, async () => {
+      return await this._runAsStep(stepInfo, cb);
+    });
+  }
+
   async _runAsStep<T>(stepInfo: Omit<TestStepInternal, 'complete' | 'wallTime' | 'parentStepId' | 'stepId' | 'steps'> & { wallTime?: number }, cb: (step: TestStepInternal) => Promise<T>): Promise<T> {
     const step = this._addStep({ wallTime: Date.now(), ...stepInfo });
     return await zones.run('stepZone', step, async () => {

packages/playwright/src/worker/timeoutManager.ts

@@ -24,8 +24,10 @@ export type TimeSlot = {
   elapsed: number;
 };
 
-type RunnableDescription = {
+export type RunnableType = 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown';
-  type: 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown';
+
+export type RunnableDescription = {
+  type: RunnableType;
   location?: Location;
   slot?: TimeSlot;  // Falls back to test slot.
 };
@@ -39,13 +41,15 @@ export type FixtureDescription = {
 
 export class TimeoutManager {
   private _defaultSlot: TimeSlot;
+  private _defaultRunnable: RunnableDescription;
   private _runnable: RunnableDescription;
   private _fixture: FixtureDescription | undefined;
   private _timeoutRunner: TimeoutRunner;
 
   constructor(timeout: number) {
     this._defaultSlot = { timeout, elapsed: 0 };
-    this._runnable = { type: 'test', slot: this._defaultSlot };
+    this._defaultRunnable = { type: 'test', slot: this._defaultSlot };
+    this._runnable = this._defaultRunnable;
     this._timeoutRunner = new TimeoutRunner(timeout);
   }
 
@@ -53,8 +57,15 @@ export class TimeoutManager {
     this._timeoutRunner.interrupt();
   }
 
-  setCurrentRunnable(runnable: RunnableDescription) {
+  async withRunnable<R>(runnable: RunnableDescription, cb: () => Promise<R>): Promise<R> {
-    this._updateRunnables(runnable, undefined);
+    const existingRunnable = this._runnable;
+    const effectiveRunnable = { ...this._runnable, ...runnable };
+    this._updateRunnables(effectiveRunnable, undefined);
+    try {
+      return await cb();
+    } finally {
+      this._updateRunnables(existingRunnable, undefined);
+    }
   }
 
   setCurrentFixture(fixture: FixtureDescription | undefined) {

packages/playwright/src/worker/workerMain.ts

@@ -147,13 +147,14 @@ export class WorkerMain extends ProcessRunner {
   private async _teardownScopes() {
     // TODO: separate timeout for teardown?
     const timeoutManager = new TimeoutManager(this._project.project.timeout);
-    timeoutManager.setCurrentRunnable({ type: 'teardown' });
+    await timeoutManager.withRunnable({ type: 'teardown' }, async () => {
-    const timeoutError = await timeoutManager.runWithTimeout(async () => {
+      const timeoutError = await timeoutManager.runWithTimeout(async () => {
-      await this._fixtureRunner.teardownScope('test', timeoutManager);
+        await this._fixtureRunner.teardownScope('test', timeoutManager);
-      await this._fixtureRunner.teardownScope('worker', timeoutManager);
+        await this._fixtureRunner.teardownScope('worker', timeoutManager);
+      });
+      if (timeoutError)
+        this._fatalErrors.push(timeoutError);
     });
-    if (timeoutError)
-      this._fatalErrors.push(timeoutError);
   }
 
   unhandledError(error: Error | any) {
@@ -366,10 +367,9 @@ export class WorkerMain extends ProcessRunner {
 
           // Run "beforeEach" hooks. Once started with "beforeEach", we must run all "afterEach" hooks as well.
           shouldRunAfterEachHooks = true;
-          await this._runEachHooksForSuites(suites, 'beforeEach', testInfo, undefined);
+          await this._runEachHooksForSuites(suites, 'beforeEach', testInfo);
 
           // Setup fixtures required by the test.
-          testInfo._timeoutManager.setCurrentRunnable({ type: 'test' });
           testFunctionParams = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test');
         }, 'allowSkips');
         if (beforeHooksError)
@@ -409,13 +409,9 @@ export class WorkerMain extends ProcessRunner {
       this._skipRemainingTestsInSuite = didFailBeforeAllForSuite;
     }
 
-    let afterHooksSlot: TimeSlot | undefined;
+    // A timed-out test gets a full additional timeout to run after hooks.
-    if (testInfo._didTimeout) {
+    const afterHooksSlot = testInfo._didTimeout ? { timeout: this._project.project.timeout, elapsed: 0 } : undefined;
-      // A timed-out test gets a full additional timeout to run after hooks.
+    await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterEach', runnableSlot: afterHooksSlot }, async step => {
-      afterHooksSlot = { timeout: this._project.project.timeout, elapsed: 0 };
-      testInfo._timeoutManager.setCurrentRunnable({ type: 'afterEach', slot: afterHooksSlot });
-    }
-    await testInfo._runAsStep({ category: 'hook', title: 'After Hooks' }, async step => {
       testInfo._afterHooksStep = step;
       let firstAfterHooksError: TestInfoError | undefined;
       await testInfo._runWithTimeout(async () => {
@@ -430,15 +426,16 @@ export class WorkerMain extends ProcessRunner {
 
         // Run "afterEach" hooks, unless we failed at beforeAll stage.
         if (shouldRunAfterEachHooks) {
-          const afterEachError = await testInfo._runAndFailOnError(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo, afterHooksSlot));
+          const afterEachError = await testInfo._runAndFailOnError(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo));
           firstAfterHooksError = firstAfterHooksError || afterEachError;
         }
 
         // Teardown test-scoped fixtures. Attribute to 'test' so that users understand
         // they should probably increase the test timeout to fix this issue.
-        testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: afterHooksSlot });
         debugTest(`tearing down test scope started`);
-        const testScopeError = await testInfo._runAndFailOnError(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager));
+        const testScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'test' }, () => {
+          return this._fixtureRunner.teardownScope('test', testInfo._timeoutManager);
+        });
         debugTest(`tearing down test scope finished`);
         firstAfterHooksError = firstAfterHooksError || testScopeError;
 
@@ -466,24 +463,28 @@ export class WorkerMain extends ProcessRunner {
           debugTest(`running full cleanup after the failure`);
 
           const teardownSlot = { timeout: this._project.project.timeout, elapsed: 0 };
-          // Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue.
+          await testInfo._timeoutManager.withRunnable({ type: 'test', slot: teardownSlot }, async () => {
-          testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: teardownSlot });
+            // Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue.
-          debugTest(`tearing down test scope started`);
+            debugTest(`tearing down test scope started`);
-          const testScopeError = await testInfo._runAndFailOnError(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager));
+            const testScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'test' }, () => {
-          debugTest(`tearing down test scope finished`);
+              return this._fixtureRunner.teardownScope('test', testInfo._timeoutManager);
-          firstAfterHooksError = firstAfterHooksError || testScopeError;
+            });
+            debugTest(`tearing down test scope finished`);
+            firstAfterHooksError = firstAfterHooksError || testScopeError;
 
-          for (const suite of reversedSuites) {
+            for (const suite of reversedSuites) {
-            const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
+              const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
-            firstAfterHooksError = firstAfterHooksError || afterAllError;
+              firstAfterHooksError = firstAfterHooksError || afterAllError;
-          }
+            }
 
-          // Attribute to 'teardown' because worker fixtures are not perceived as a part of a test.
+            // Attribute to 'teardown' because worker fixtures are not perceived as a part of a test.
-          testInfo._timeoutManager.setCurrentRunnable({ type: 'teardown', slot: teardownSlot });
+            debugTest(`tearing down worker scope started`);
-          debugTest(`tearing down worker scope started`);
+            const workerScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'teardown' }, () => {
-          const workerScopeError = await testInfo._runAndFailOnError(() => this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager));
+              return this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager);
-          debugTest(`tearing down worker scope finished`);
+            });
-          firstAfterHooksError = firstAfterHooksError || workerScopeError;
+            debugTest(`tearing down worker scope finished`);
+            firstAfterHooksError = firstAfterHooksError || workerScopeError;
+          });
         });
       }
 
@@ -507,11 +508,12 @@ export class WorkerMain extends ProcessRunner {
       if (actualScope !== scope)
         continue;
       debugTest(`modifier at "${formatLocation(modifier.location)}" started`);
-      testInfo._timeoutManager.setCurrentRunnable({ type: modifier.type, location: modifier.location, slot: timeSlot });
+      const result = await testInfo._runAsStepWithRunnable({
-      const result = await testInfo._runAsStep({
         category: 'hook',
         title: `${modifier.type} modifier`,
         location: modifier.location,
+        runnableType: modifier.type,
+        runnableSlot: timeSlot,
       }, () => this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, testInfo, scope));
       debugTest(`modifier at "${formatLocation(modifier.location)}" finished`);
       if (result && extraAnnotations)
@@ -532,11 +534,12 @@ export class WorkerMain extends ProcessRunner {
       try {
         // Separate time slot for each "beforeAll" hook.
         const timeSlot = { timeout: this._project.project.timeout, elapsed: 0 };
-        testInfo._timeoutManager.setCurrentRunnable({ type: 'beforeAll', location: hook.location, slot: timeSlot });
+        await testInfo._runAsStepWithRunnable({
-        await testInfo._runAsStep({
           category: 'hook',
           title: `${hook.title}`,
           location: hook.location,
+          runnableType: 'beforeAll',
+          runnableSlot: timeSlot,
         }, async () => {
           try {
             await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only');
@@ -568,11 +571,12 @@ export class WorkerMain extends ProcessRunner {
       const afterAllError = await testInfo._runAndFailOnError(async () => {
         // Separate time slot for each "afterAll" hook.
         const timeSlot = { timeout: this._project.project.timeout, elapsed: 0 };
-        testInfo._timeoutManager.setCurrentRunnable({ type: 'afterAll', location: hook.location, slot: timeSlot });
+        await testInfo._runAsStepWithRunnable({
-        await testInfo._runAsStep({
           category: 'hook',
           title: `${hook.title}`,
           location: hook.location,
+          runnableType: 'afterAll',
+          runnableSlot: timeSlot,
         }, async () => {
           try {
             await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only');
@@ -589,16 +593,16 @@ export class WorkerMain extends ProcessRunner {
     return firstError;
   }
 
-  private async _runEachHooksForSuites(suites: Suite[], type: 'beforeEach' | 'afterEach', testInfo: TestInfoImpl, timeSlot: TimeSlot | undefined) {
+  private async _runEachHooksForSuites(suites: Suite[], type: 'beforeEach' | 'afterEach', testInfo: TestInfoImpl) {
     const hooks = suites.map(suite => suite._hooks.filter(hook => hook.type === type)).flat();
     let error: Error | undefined;
     for (const hook of hooks) {
       try {
-        testInfo._timeoutManager.setCurrentRunnable({ type, location: hook.location, slot: timeSlot });
+        await testInfo._runAsStepWithRunnable({
-        await testInfo._runAsStep({
           category: 'hook',
           title: `${hook.title}`,
           location: hook.location,
+          runnableType: type,
         }, () => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test'));
       } catch (e) {
         // Always run all the hooks, and capture the first error.