fix(fixtures): await fixture teardown when shutting down the worker (#11033)

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<Fixture>;
   value: any;
-  _teardownFenceCallback!: (value?: unknown) => void;
+
-  _tearDownComplete!: Promise<void>;
+  _useFuncFinished: ManualPromise<void> | undefined;
-  _setup = false;
+  _selfTeardownComplete: Promise<void> | undefined;
-  _teardown = false;
+  _teardownWithDepsComplete: Promise<void> | 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<void>((f, r) => { setupFenceFulfill = f; setupFenceReject = r; });
+    const useFuncStarted = new ManualPromise<void>();
-    const teardownFence = new Promise(f => this._teardownFenceCallback = f);
     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();
+      this._useFuncFinished = new ManualPromise<void>();
-      return await teardownFence;
+      useFuncStarted.resolve();
-    }, this.registration.scope === 'worker' ? workerInfo : testInfo)).catch((e: any) => {
+      await this._useFuncFinished;
-      if (!this._setup)
+    };
-        setupFenceReject(e);
+    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;
+    await useFuncStarted;
-    this._setup = true;
   }
 
   async teardown() {
-    if (this._teardown)
+    if (!this._teardownWithDepsComplete)
-      return;
+      this._teardownWithDepsComplete = this._teardownInternal();
-    this._teardown = true;
+    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();
+      this._useFuncFinished.resolve();
-      await this._tearDownComplete;
+      await this._selfTeardownComplete;
     }
     this.runner.instanceForId.delete(this.registration.id);
   }

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([
+  ]);
+});