diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index 40ce96c99a..d28e123958 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/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): Promise { + return await this._timeoutManager.withRunnable(runnable, async () => { + return await this._runAndFailOnError(cb); + }); + } + async _runAndFailOnError(fn: () => Promise, skips?: 'allowSkips'): Promise { try { await fn(); @@ -348,6 +355,21 @@ export class TestInfoImpl implements TestInfo { this.errors.push(error); } + async _runAsStepWithRunnable( + stepInfo: Omit & { + wallTime?: number, + runnableType: RunnableType; + runnableSlot?: TimeSlot; + }, cb: (step: TestStepInternal) => Promise): Promise { + return await this._timeoutManager.withRunnable({ + type: stepInfo.runnableType, + slot: stepInfo.runnableSlot, + location: stepInfo.location, + }, async () => { + return await this._runAsStep(stepInfo, cb); + }); + } + async _runAsStep(stepInfo: Omit & { wallTime?: number }, cb: (step: TestStepInternal) => Promise): Promise { const step = this._addStep({ wallTime: Date.now(), ...stepInfo }); return await zones.run('stepZone', step, async () => { diff --git a/packages/playwright/src/worker/timeoutManager.ts b/packages/playwright/src/worker/timeoutManager.ts index 72f1384bbc..56c73c8d2b 100644 --- a/packages/playwright/src/worker/timeoutManager.ts +++ b/packages/playwright/src/worker/timeoutManager.ts @@ -24,8 +24,10 @@ export type TimeSlot = { elapsed: number; }; -type RunnableDescription = { - type: 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown'; +export type RunnableType = '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) { - this._updateRunnables(runnable, undefined); + async withRunnable(runnable: RunnableDescription, cb: () => Promise): Promise { + 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) { diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index cb1c5e5225..b72b7e3450 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/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' }); - const timeoutError = await timeoutManager.runWithTimeout(async () => { - await this._fixtureRunner.teardownScope('test', timeoutManager); - await this._fixtureRunner.teardownScope('worker', timeoutManager); + await timeoutManager.withRunnable({ type: 'teardown' }, async () => { + const timeoutError = await timeoutManager.runWithTimeout(async () => { + await this._fixtureRunner.teardownScope('test', 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; - if (testInfo._didTimeout) { - // A timed-out test gets a full additional timeout to run after hooks. - 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 => { + // A timed-out test gets a full additional timeout to run after hooks. + const afterHooksSlot = testInfo._didTimeout ? { timeout: this._project.project.timeout, elapsed: 0 } : undefined; + await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterEach', runnableSlot: afterHooksSlot }, 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. - testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: teardownSlot }); - debugTest(`tearing down test scope started`); - const testScopeError = await testInfo._runAndFailOnError(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager)); - debugTest(`tearing down test scope finished`); - firstAfterHooksError = firstAfterHooksError || testScopeError; + await testInfo._timeoutManager.withRunnable({ type: 'test', slot: teardownSlot }, async () => { + // Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue. + debugTest(`tearing down test scope started`); + const testScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'test' }, () => { + return this._fixtureRunner.teardownScope('test', testInfo._timeoutManager); + }); + debugTest(`tearing down test scope finished`); + firstAfterHooksError = firstAfterHooksError || testScopeError; - for (const suite of reversedSuites) { - const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); - firstAfterHooksError = firstAfterHooksError || afterAllError; - } + for (const suite of reversedSuites) { + const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); + firstAfterHooksError = firstAfterHooksError || afterAllError; + } - // 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`); - const workerScopeError = await testInfo._runAndFailOnError(() => this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager)); - debugTest(`tearing down worker scope finished`); - firstAfterHooksError = firstAfterHooksError || workerScopeError; + // Attribute to 'teardown' because worker fixtures are not perceived as a part of a test. + debugTest(`tearing down worker scope started`); + const workerScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'teardown' }, () => { + return this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager); + }); + 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._runAsStep({ + const result = await testInfo._runAsStepWithRunnable({ 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._runAsStep({ + await testInfo._runAsStepWithRunnable({ 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._runAsStep({ + await testInfo._runAsStepWithRunnable({ 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._runAsStep({ + await testInfo._runAsStepWithRunnable({ 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.