diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts index 11faab138a..125f9fb799 100644 --- a/packages/playwright-test/src/index.ts +++ b/packages/playwright-test/src/index.ts @@ -439,10 +439,9 @@ function formatStackFrame(frame: StackFrame) { } function hookType(testInfo: TestInfoImpl): 'beforeAll' | 'afterAll' | undefined { - if (testInfo._timeoutManager.hasRunnableType('beforeAll')) - return 'beforeAll'; - if (testInfo._timeoutManager.hasRunnableType('afterAll')) - return 'afterAll'; + const type = testInfo._timeoutManager.currentRunnableType(); + if (type === 'beforeAll' || type === 'afterAll') + return type; } type StackFrame = { diff --git a/packages/playwright-test/src/worker/fixtureRunner.ts b/packages/playwright-test/src/worker/fixtureRunner.ts index 9193b21062..46456588cb 100644 --- a/packages/playwright-test/src/worker/fixtureRunner.ts +++ b/packages/playwright-test/src/worker/fixtureRunner.ts @@ -17,7 +17,7 @@ import { formatLocation, debugTest, filterStackFile, serializeError } from '../util'; import { ManualPromise, zones } from 'playwright-core/lib/utils'; import type { TestInfoImpl, TestStepInternal } from './testInfo'; -import type { RunnableDescription, TimeoutManager } from './timeoutManager'; +import type { FixtureDescription, TimeoutManager } from './timeoutManager'; import { fixtureParameterNames, type FixturePool, type FixtureRegistration, type FixtureScope } from '../common/fixtures'; import type { WorkerInfo } from '../../types/test'; import type { Location } from '../../types/testReporter'; @@ -31,7 +31,7 @@ class Fixture { _useFuncFinished: ManualPromise | undefined; _selfTeardownComplete: Promise | undefined; _teardownWithDepsComplete: Promise | undefined; - _runnableDescription: RunnableDescription; + _runnableDescription: FixtureDescription; _deps = new Set(); _usages = new Set(); @@ -42,7 +42,6 @@ class Fixture { const title = this.registration.customTitle || this.registration.name; this._runnableDescription = { title, - type: 'fixture', phase: 'setup', location: registration.location, slot: this.registration.timeout === undefined ? undefined : { @@ -107,6 +106,7 @@ class Fixture { const workerInfo: WorkerInfo = { config: testInfo.config, parallelIndex: testInfo.parallelIndex, workerIndex: testInfo.workerIndex, project: testInfo.project }; const info = this.registration.scope === 'worker' ? workerInfo : testInfo; + testInfo._timeoutManager.setCurrentFixture(this._runnableDescription); const handleError = (e: any) => { this.failed = true; @@ -115,40 +115,38 @@ class Fixture { else throw e; }; + try { + const result = zones.preserve(async () => { + if (!shouldGenerateStep) + return await this.registration.fn(params, useFunc, info); - await testInfo._timeoutManager.runRunnable(this._runnableDescription, async () => { - try { - const result = zones.preserve(async () => { - if (!shouldGenerateStep) - return await this.registration.fn(params, useFunc, info); - - await testInfo._runAsStep({ - title: `fixture: ${this.registration.name}`, - category: 'fixture', - location: isInternalFixture ? this.registration.location : undefined, - }, async step => { - mutableStepOnStack = step; - return await this.registration.fn(params, useFunc, info); - }); + await testInfo._runAsStep({ + title: `fixture: ${this.registration.name}`, + category: 'fixture', + location: isInternalFixture ? this.registration.location : undefined, + }, async step => { + mutableStepOnStack = step; + return await this.registration.fn(params, useFunc, info); }); + }); - if (result instanceof Promise) - this._selfTeardownComplete = result.catch(handleError); - else - this._selfTeardownComplete = Promise.resolve(); - } catch (e) { - handleError(e); - } - await useFuncStarted; - if (shouldGenerateStep) { - mutableStepOnStack?.complete({}); - this._selfTeardownComplete?.then(() => { - afterStep?.complete({}); - }).catch(e => { - afterStep?.complete({ error: serializeError(e) }); - }); - } - }); + if (result instanceof Promise) + this._selfTeardownComplete = result.catch(handleError); + else + this._selfTeardownComplete = Promise.resolve(); + } catch (e) { + handleError(e); + } + await useFuncStarted; + if (shouldGenerateStep) { + mutableStepOnStack?.complete({}); + this._selfTeardownComplete?.then(() => { + afterStep?.complete({}); + }).catch(e => { + afterStep?.complete({ error: serializeError(e) }); + }); + } + testInfo._timeoutManager.setCurrentFixture(undefined); } async teardown(timeoutManager: TimeoutManager) { @@ -156,16 +154,20 @@ class Fixture { // When we are waiting for the teardown for the second time, // most likely after the first time did timeout, annotate current fixture // for better error messages. - this._runnableDescription.phase = 'teardown'; - await timeoutManager.runRunnable(this._runnableDescription, async () => { - await this._teardownWithDepsComplete; - }); + this._setTeardownDescription(timeoutManager); + await this._teardownWithDepsComplete; + timeoutManager.setCurrentFixture(undefined); return; } this._teardownWithDepsComplete = this._teardownInternal(timeoutManager); await this._teardownWithDepsComplete; } + private _setTeardownDescription(timeoutManager: TimeoutManager) { + this._runnableDescription.phase = 'teardown'; + timeoutManager.setCurrentFixture(this._runnableDescription); + } + private async _teardownInternal(timeoutManager: TimeoutManager) { if (typeof this.registration.fn !== 'function') return; @@ -179,11 +181,10 @@ class Fixture { } if (this._useFuncFinished) { debugTest(`teardown ${this.registration.name}`); - this._runnableDescription.phase = 'teardown'; - await timeoutManager.runRunnable(this._runnableDescription, async () => { - this._useFuncFinished!.resolve(); - await this._selfTeardownComplete; - }); + this._setTeardownDescription(timeoutManager); + this._useFuncFinished.resolve(); + await this._selfTeardownComplete; + timeoutManager.setCurrentFixture(undefined); } } finally { for (const dep of this._deps) diff --git a/packages/playwright-test/src/worker/timeoutManager.ts b/packages/playwright-test/src/worker/timeoutManager.ts index 8c478c88c6..72f1384bbc 100644 --- a/packages/playwright-test/src/worker/timeoutManager.ts +++ b/packages/playwright-test/src/worker/timeoutManager.ts @@ -24,22 +24,28 @@ export type TimeSlot = { elapsed: number; }; -export type RunnableDescription = { - type: 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown' | 'fixture'; - phase?: 'setup' | 'teardown'; - title?: string; +type RunnableDescription = { + type: 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown'; location?: Location; slot?: TimeSlot; // Falls back to test slot. }; +export type FixtureDescription = { + title: string; + phase: 'setup' | 'teardown'; + location?: Location; + slot?: TimeSlot; // Falls back to current runnable slot. +}; + export class TimeoutManager { private _defaultSlot: TimeSlot; - private _runnables: RunnableDescription[] = []; + private _runnable: RunnableDescription; + private _fixture: FixtureDescription | undefined; private _timeoutRunner: TimeoutRunner; constructor(timeout: number) { this._defaultSlot = { timeout, elapsed: 0 }; - this._runnables = [{ type: 'test', slot: this._defaultSlot }]; + this._runnable = { type: 'test', slot: this._defaultSlot }; this._timeoutRunner = new TimeoutRunner(timeout); } @@ -47,22 +53,12 @@ export class TimeoutManager { this._timeoutRunner.interrupt(); } - async runRunnable(runnable: RunnableDescription, cb: () => Promise): Promise { - let slot = this._currentSlot(); - slot.elapsed = this._timeoutRunner.elapsed(); - this._runnables.unshift(runnable); - slot = this._currentSlot(); - this._timeoutRunner.updateTimeout(slot.timeout, slot.elapsed); + setCurrentRunnable(runnable: RunnableDescription) { + this._updateRunnables(runnable, undefined); + } - try { - return await cb(); - } finally { - let slot = this._currentSlot(); - slot.elapsed = this._timeoutRunner.elapsed(); - this._runnables.splice(this._runnables.indexOf(runnable), 1); - slot = this._currentSlot(); - this._timeoutRunner.updateTimeout(slot.timeout, slot.elapsed); - } + setCurrentFixture(fixture: FixtureDescription | undefined) { + this._updateRunnables(this._runnable, fixture); } defaultSlotTimings() { @@ -95,12 +91,8 @@ export class TimeoutManager { this._timeoutRunner.updateTimeout(timeout); } - hasRunnableType(type: RunnableDescription['type']) { - return this._runnables.some(r => r.type === type); - } - - private _runnable(): RunnableDescription { - return this._runnables[0]!; + currentRunnableType() { + return this._runnable.type; } currentSlotDeadline() { @@ -108,59 +100,66 @@ export class TimeoutManager { } private _currentSlot() { - for (const runnable of this._runnables) { - if (runnable.slot) - return runnable.slot; - } - return this._defaultSlot; + return this._fixture?.slot || this._runnable.slot || this._defaultSlot; + } + + private _updateRunnables(runnable: RunnableDescription, fixture: FixtureDescription | undefined) { + let slot = this._currentSlot(); + slot.elapsed = this._timeoutRunner.elapsed(); + + this._runnable = runnable; + this._fixture = fixture; + + slot = this._currentSlot(); + this._timeoutRunner.updateTimeout(slot.timeout, slot.elapsed); } private _createTimeoutError(): TestInfoError { let message = ''; const timeout = this._currentSlot().timeout; - const runnable = this._runnable(); - switch (runnable.type) { + switch (this._runnable.type) { case 'test': { - message = `Test timeout of ${timeout}ms exceeded.`; - break; - } - case 'fixture': { - if (this._runnables.some(r => r.type === 'teardown')) { - message = `Worker teardown timeout of ${timeout}ms exceeded while ${runnable.phase === 'setup' ? 'setting up' : 'tearing down'} "${runnable.title}".`; - } else if (runnable.phase === 'setup') { - message = `Test timeout of ${timeout}ms exceeded while setting up "${runnable.title}".`; + if (this._fixture) { + if (this._fixture.phase === 'setup') { + message = `Test timeout of ${timeout}ms exceeded while setting up "${this._fixture.title}".`; + } else { + message = [ + `Test finished within timeout of ${timeout}ms, but tearing down "${this._fixture.title}" ran out of time.`, + `Please allow more time for the test, since teardown is attributed towards the test timeout budget.`, + ].join('\n'); + } } else { - message = [ - `Test finished within timeout of ${timeout}ms, but tearing down "${runnable.title}" ran out of time.`, - `Please allow more time for the test, since teardown is attributed towards the test timeout budget.`, - ].join('\n'); + message = `Test timeout of ${timeout}ms exceeded.`; } break; } case 'afterEach': case 'beforeEach': - message = `Test timeout of ${timeout}ms exceeded while running "${runnable.type}" hook.`; + message = `Test timeout of ${timeout}ms exceeded while running "${this._runnable.type}" hook.`; break; case 'beforeAll': case 'afterAll': - message = `"${runnable.type}" hook timeout of ${timeout}ms exceeded.`; + message = `"${this._runnable.type}" hook timeout of ${timeout}ms exceeded.`; break; case 'teardown': { - message = `Worker teardown timeout of ${timeout}ms exceeded.`; + if (this._fixture) + message = `Worker teardown timeout of ${timeout}ms exceeded while ${this._fixture.phase === 'setup' ? 'setting up' : 'tearing down'} "${this._fixture.title}".`; + else + message = `Worker teardown timeout of ${timeout}ms exceeded.`; break; } case 'skip': case 'slow': case 'fixme': case 'fail': - message = `"${runnable.type}" modifier timeout of ${timeout}ms exceeded.`; + message = `"${this._runnable.type}" modifier timeout of ${timeout}ms exceeded.`; break; } - const fixtureWithSlot = runnable.type === 'fixture' && runnable.slot ? runnable : undefined; + const fixtureWithSlot = this._fixture?.slot ? this._fixture : undefined; if (fixtureWithSlot) message = `Fixture "${fixtureWithSlot.title}" timeout of ${timeout}ms exceeded during ${fixtureWithSlot.phase}.`; message = colors.red(message); - const location = (fixtureWithSlot || runnable).location; + const location = (fixtureWithSlot || this._runnable).location; return { message, // Include location for hooks, modifiers and fixtures to distinguish between them. diff --git a/packages/playwright-test/src/worker/workerMain.ts b/packages/playwright-test/src/worker/workerMain.ts index d44a2ae12e..b6b520ea7c 100644 --- a/packages/playwright-test/src/worker/workerMain.ts +++ b/packages/playwright-test/src/worker/workerMain.ts @@ -153,11 +153,10 @@ 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 timeoutManager.runRunnable({ type: 'teardown' }, async () => { - await this._fixtureRunner.teardownScope('test', timeoutManager); - await this._fixtureRunner.teardownScope('worker', timeoutManager); - }); + await this._fixtureRunner.teardownScope('test', timeoutManager); + await this._fixtureRunner.teardownScope('worker', timeoutManager); }); if (timeoutError) this._fatalErrors.push(timeoutError); @@ -363,6 +362,7 @@ export class WorkerMain extends ProcessRunner { await this._runEachHooksForSuites(suites, 'beforeEach', testInfo, undefined); // Setup fixtures required by the test. + testInfo._timeoutManager.setCurrentRunnable({ type: 'test' }); testFunctionParams = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test'); }, 'allowSkips'); if (beforeHooksError) @@ -406,84 +406,82 @@ export class WorkerMain extends ProcessRunner { 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._timeoutManager.runRunnable({ type: 'afterEach', slot: afterHooksSlot }, async () => { - await testInfo._runAsStep({ category: 'hook', title: 'After Hooks' }, async step => { - testInfo._afterHooksStep = step; - let firstAfterHooksError: TestInfoError | undefined; - await testInfo._runWithTimeout(async () => { - // Note: do not wrap all teardown steps together, because failure in any of them - // does not prevent further teardown steps from running. + await testInfo._runAsStep({ category: 'hook', title: 'After Hooks' }, async step => { + testInfo._afterHooksStep = step; + let firstAfterHooksError: TestInfoError | undefined; + await testInfo._runWithTimeout(async () => { + // Note: do not wrap all teardown steps together, because failure in any of them + // does not prevent further teardown steps from running. - // Run "immediately upon test function finish" callback. - debugTest(`on-test-function-finish callback started`); - const didFinishTestFunctionError = await testInfo._runAndFailOnError(async () => testInfo._onDidFinishTestFunction?.()); - firstAfterHooksError = firstAfterHooksError || didFinishTestFunctionError; - debugTest(`on-test-function-finish callback finished`); + // Run "immediately upon test function finish" callback. + debugTest(`on-test-function-finish callback started`); + const didFinishTestFunctionError = await testInfo._runAndFailOnError(async () => testInfo._onDidFinishTestFunction?.()); + firstAfterHooksError = firstAfterHooksError || didFinishTestFunctionError; + debugTest(`on-test-function-finish callback finished`); - // Run "afterEach" hooks, unless we failed at beforeAll stage. - if (shouldRunAfterEachHooks) { - const afterEachError = await testInfo._runAndFailOnError(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo, afterHooksSlot)); - 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. - await testInfo._timeoutManager.runRunnable({ type: 'test', slot: afterHooksSlot }, async () => { - 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; - - // Run "afterAll" hooks for suites that are not shared with the next test. - // In case of failure the worker will be stopped and we have to make sure that afterAll - // hooks run before worker fixtures teardown. - for (const suite of reversedSuites) { - if (!nextSuites.has(suite) || testInfo._isFailure()) { - const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); - firstAfterHooksError = firstAfterHooksError || afterAllError; - } - } - }); - - }); - - if (testInfo._isFailure()) - this._isStopped = true; - - if (this._isStopped) { - // Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down. - // Mark as "cleaned up" early to avoid running cleanup twice. - this._didRunFullCleanup = true; - - // Give it more time for the full cleanup. - await testInfo._runWithTimeout(async () => { - 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 increase the test timeout to fix this issue. - await testInfo._timeoutManager.runRunnable({ type: 'test', slot: teardownSlot }, async () => { - 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; - - for (const suite of reversedSuites) { - const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); - firstAfterHooksError = firstAfterHooksError || afterAllError; - } - - 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; - }); - }); + // Run "afterEach" hooks, unless we failed at beforeAll stage. + if (shouldRunAfterEachHooks) { + const afterEachError = await testInfo._runAndFailOnError(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo, afterHooksSlot)); + firstAfterHooksError = firstAfterHooksError || afterEachError; } - if (firstAfterHooksError) - step.complete({ error: firstAfterHooksError }); + // 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)); + debugTest(`tearing down test scope finished`); + firstAfterHooksError = firstAfterHooksError || testScopeError; + + // Run "afterAll" hooks for suites that are not shared with the next test. + // In case of failure the worker will be stopped and we have to make sure that afterAll + // hooks run before worker fixtures teardown. + for (const suite of reversedSuites) { + if (!nextSuites.has(suite) || testInfo._isFailure()) { + const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo); + firstAfterHooksError = firstAfterHooksError || afterAllError; + } + } }); + + if (testInfo._isFailure()) + this._isStopped = true; + + if (this._isStopped) { + // Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down. + // Mark as "cleaned up" early to avoid running cleanup twice. + this._didRunFullCleanup = true; + + // Give it more time for the full cleanup. + await testInfo._runWithTimeout(async () => { + 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; + + 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; + }); + } + + if (firstAfterHooksError) + step.complete({ error: firstAfterHooksError }); }); this._currentTest = null; @@ -501,18 +499,17 @@ export class WorkerMain extends ProcessRunner { const actualScope = this._fixtureRunner.dependsOnWorkerFixturesOnly(modifier.fn, modifier.location) ? 'worker' : 'test'; if (actualScope !== scope) continue; - await testInfo._timeoutManager.runRunnable({ type: modifier.type, location: modifier.location, slot: timeSlot }, async () => { - debugTest(`modifier at "${formatLocation(modifier.location)}" started`); - const result = await testInfo._runAsStep({ - category: 'hook', - title: `${modifier.type} modifier`, - location: modifier.location, - }, () => this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, testInfo, scope)); - debugTest(`modifier at "${formatLocation(modifier.location)}" finished`); - if (result && extraAnnotations) - extraAnnotations.push({ type: modifier.type, description: modifier.description }); - testInfo[modifier.type](!!result, modifier.description); - }); + debugTest(`modifier at "${formatLocation(modifier.location)}" started`); + testInfo._timeoutManager.setCurrentRunnable({ type: modifier.type, location: modifier.location, slot: timeSlot }); + const result = await testInfo._runAsStep({ + category: 'hook', + title: `${modifier.type} modifier`, + location: modifier.location, + }, () => this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, testInfo, scope)); + debugTest(`modifier at "${formatLocation(modifier.location)}" finished`); + if (result && extraAnnotations) + extraAnnotations.push({ type: modifier.type, description: modifier.description }); + testInfo[modifier.type](!!result, modifier.description); } } @@ -528,20 +525,19 @@ export class WorkerMain extends ProcessRunner { try { // Separate time slot for each "beforeAll" hook. const timeSlot = { timeout: this._project.project.timeout, elapsed: 0 }; - await testInfo._timeoutManager.runRunnable({ type: 'beforeAll', location: hook.location, slot: timeSlot }, async () => { - await testInfo._runAsStep({ - category: 'hook', - title: `${hook.type} hook`, - location: hook.location, - }, async () => { - try { - await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); - } finally { - // Each beforeAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. - // Note: we must teardown even after beforeAll fails, because we'll run more beforeAlls. - await this._fixtureRunner.teardownScope('test', testInfo._timeoutManager); - } - }); + testInfo._timeoutManager.setCurrentRunnable({ type: 'beforeAll', location: hook.location, slot: timeSlot }); + await testInfo._runAsStep({ + category: 'hook', + title: `${hook.type} hook`, + location: hook.location, + }, async () => { + try { + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); + } finally { + // Each beforeAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. + // Note: we must teardown even after beforeAll fails, because we'll run more beforeAlls. + await this._fixtureRunner.teardownScope('test', testInfo._timeoutManager); + } }); } catch (e) { // Always run all the hooks, and capture the first error. @@ -565,20 +561,19 @@ 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 }; - await testInfo._timeoutManager.runRunnable({ type: 'afterAll', location: hook.location, slot: timeSlot }, async () => { - await testInfo._runAsStep({ - category: 'hook', - title: `${hook.type} hook`, - location: hook.location, - }, async () => { - try { - await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); - } finally { - // Each afterAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. - // Note: we must teardown even after afterAll fails, because we'll run more afterAlls. - await this._fixtureRunner.teardownScope('test', testInfo._timeoutManager); - } - }); + testInfo._timeoutManager.setCurrentRunnable({ type: 'afterAll', location: hook.location, slot: timeSlot }); + await testInfo._runAsStep({ + category: 'hook', + title: `${hook.type} hook`, + location: hook.location, + }, async () => { + try { + await this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'); + } finally { + // Each afterAll hook has its own scope for test fixtures. Attribute to the same runnable and timeSlot. + // Note: we must teardown even after afterAll fails, because we'll run more afterAlls. + await this._fixtureRunner.teardownScope('test', testInfo._timeoutManager); + } }); }); firstError = firstError || afterAllError; @@ -592,13 +587,12 @@ export class WorkerMain extends ProcessRunner { let error: Error | undefined; for (const hook of hooks) { try { - await testInfo._timeoutManager.runRunnable({ type, location: hook.location, slot: timeSlot }, async () => { - await testInfo._runAsStep({ - category: 'hook', - title: `${hook.type} hook`, - location: hook.location, - }, () => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test')); - }); + testInfo._timeoutManager.setCurrentRunnable({ type, location: hook.location, slot: timeSlot }); + await testInfo._runAsStep({ + category: 'hook', + title: `${hook.type} hook`, + location: hook.location, + }, () => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test')); } catch (e) { // Always run all the hooks, and capture the first error. error = error || e; diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index de6667eec5..f776ae48c8 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -477,7 +477,7 @@ test('should not report fixture teardown error twice', async ({ runInlineTest }) expect(countTimes(result.output, 'Oh my error')).toBe(2); }); -test('should report fixture teardown extend', async ({ runInlineTest }) => { +test('should not report fixture teardown timeout twice', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.spec.ts': ` import { test as base, expect } from '@playwright/test'; @@ -494,7 +494,8 @@ test('should report fixture teardown extend', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); expect(result.output).toContain('Test finished within timeout of 1000ms, but tearing down "fixture" ran out of time.'); - expect(result.output).toContain('base.extend'); + expect(result.output).not.toContain('base.extend'); // Should not point to the location. + // TODO: this should be "not.toContain" actually. expect(result.output).toContain('Worker teardown timeout of 1000ms exceeded while tearing down "fixture".'); });