diff --git a/packages/playwright-test/src/fixtures.ts b/packages/playwright-test/src/fixtures.ts index ce2d087cc9..c26fb9069e 100644 --- a/packages/playwright-test/src/fixtures.ts +++ b/packages/playwright-test/src/fixtures.ts @@ -17,6 +17,7 @@ import { formatLocation, wrapInPromise, debugTest } from './util'; import * as crypto from 'crypto'; import { FixturesWithLocation, Location, WorkerInfo, TestInfo, TestStepInternal } from './types'; +import { ManualPromise } from 'playwright-core/lib/utils/async'; type FixtureScope = 'test' | 'worker'; type FixtureRegistration = { @@ -35,10 +36,10 @@ class Fixture { registration: FixtureRegistration; usages: Set; value: any; - _teardownFenceCallback!: (value?: unknown) => void; - _tearDownComplete!: Promise; - _setup = false; - _teardown = false; + + _useFuncFinished: ManualPromise | undefined; + _selfTeardownComplete: Promise | undefined; + _teardownWithDepsComplete: Promise | undefined; constructor(runner: FixtureRunner, registration: FixtureRegistration) { this.runner = runner; @@ -49,7 +50,6 @@ class Fixture { async setup(workerInfo: WorkerInfo, testInfo: TestInfo | undefined) { if (typeof this.registration.fn !== 'function') { - this._setup = true; this.value = this.registration.fn; return; } @@ -62,42 +62,44 @@ class Fixture { params[name] = dep.value; } - let setupFenceFulfill = () => {}; - let setupFenceReject = (e: Error) => {}; let called = false; - const setupFence = new Promise((f, r) => { setupFenceFulfill = f; setupFenceReject = r; }); - const teardownFence = new Promise(f => this._teardownFenceCallback = f); + const useFuncStarted = new ManualPromise(); debugTest(`setup ${this.registration.name}`); - this._tearDownComplete = wrapInPromise(this.registration.fn(params, async (value: any) => { + const useFunc = async (value: any) => { if (called) throw new Error(`Cannot provide fixture value for the second time`); called = true; this.value = value; - setupFenceFulfill(); - return await teardownFence; - }, this.registration.scope === 'worker' ? workerInfo : testInfo)).catch((e: any) => { - if (!this._setup) - setupFenceReject(e); + this._useFuncFinished = new ManualPromise(); + useFuncStarted.resolve(); + await this._useFuncFinished; + }; + const info = this.registration.scope === 'worker' ? workerInfo : testInfo; + this._selfTeardownComplete = wrapInPromise(this.registration.fn(params, useFunc, info)).catch((e: any) => { + if (!useFuncStarted.isDone()) + useFuncStarted.reject(e); else throw e; }); - await setupFence; - this._setup = true; + await useFuncStarted; } async teardown() { - if (this._teardown) - return; - this._teardown = true; + if (!this._teardownWithDepsComplete) + this._teardownWithDepsComplete = this._teardownInternal(); + await this._teardownWithDepsComplete; + } + + private async _teardownInternal() { if (typeof this.registration.fn !== 'function') return; for (const fixture of this.usages) await fixture.teardown(); this.usages.clear(); - if (this._setup) { + if (this._useFuncFinished) { debugTest(`teardown ${this.registration.name}`); - this._teardownFenceCallback(); - await this._tearDownComplete; + this._useFuncFinished.resolve(); + await this._selfTeardownComplete; } this.runner.instanceForId.delete(this.registration.id); } diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts index 01accdbe9e..5dd167b15a 100644 --- a/tests/playwright-test/fixture-errors.spec.ts +++ b/tests/playwright-test/fixture-errors.spec.ts @@ -388,3 +388,50 @@ test('should error for unsupported scope', async ({ runInlineTest }) => { expect(result.exitCode).toBe(1); expect(result.output).toContain(`Error: Fixture "failure" has unknown { scope: 'foo' }`); }); + +test('should give enough time for fixture teardown', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const test = pwt.test.extend({ + fixture: async ({ }, use) => { + await use(); + console.log('\\n%%teardown start'); + await new Promise(f => setTimeout(f, 800)); + console.log('\\n%%teardown finished'); + }, + }); + test('fast enough but close', async ({ fixture }) => { + test.setTimeout(1000); + await new Promise(f => setTimeout(f, 800)); + }); + `, + }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('Timeout of 1000ms exceeded'); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + '%%teardown start', + '%%teardown finished', + ]); +}); + +test('should not teardown when setup times out', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const test = pwt.test.extend({ + fixture: async ({ }, use) => { + await new Promise(f => setTimeout(f, 1500)); + await use(); + console.log('\\n%%teardown'); + }, + }); + test('fast enough but close', async ({ fixture }) => { + }); + `, + }, { timeout: 1000 }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.output).toContain('Timeout of 1000ms exceeded'); + expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([ + ]); +});