chore: add floating promise warning to hooks (#34861)

packages/playwright/src/worker/floatingPromiseScope.ts

@@ -43,6 +43,10 @@ export class FloatingPromiseScope {
     return promiseProxy;
   }
 
+  clear() {
+    this._floatingCalls.clear();
+  }
+
   hasFloatingPromises(): boolean {
     return this._floatingCalls.size > 0;
   }

packages/playwright/src/worker/workerMain.ts

@@ -321,6 +321,15 @@ export class WorkerMain extends ProcessRunner {
     let shouldRunAfterEachHooks = false;
 
     testInfo._allowSkips = true;
+
+    // Create warning if any of the async calls were not awaited in various stages.
+    const checkForFloatingPromises = (functionDescription: string) => {
+      if (!testInfo._floatingPromiseScope.hasFloatingPromises())
+        return;
+      testInfo.annotations.push({ type: 'warning', description: `Some async calls were not awaited by the end of ${functionDescription}. This can cause flakiness.` });
+      testInfo._floatingPromiseScope.clear();
+    };
+
     await testInfo._runAsStage({ title: 'setup and test' }, async () => {
       await testInfo._runAsStage({ title: 'start tracing', runnable: { type: 'test' } }, async () => {
         // Ideally, "trace" would be an config-level option belonging to the
@@ -360,6 +369,8 @@ export class WorkerMain extends ProcessRunner {
         testFunctionParams = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test', { type: 'test' });
       });
 
+      checkForFloatingPromises('beforeAll/beforeEach hooks');
+
       if (testFunctionParams === null) {
         // Fixture setup failed or was skipped, we should not run the test now.
         return;
@@ -369,9 +380,7 @@ export class WorkerMain extends ProcessRunner {
         // Now run the test itself.
         const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]").
         await fn(testFunctionParams, testInfo);
-        // Create warning if any of the async calls were not awaited.
+        checkForFloatingPromises('the test');
-        if (testInfo._floatingPromiseScope.hasFloatingPromises())
-          testInfo.annotations.push({ type: 'warning', description: 'Some async calls were not awaited by the end of the test. This can cause flakiness.' });
       });
     }).catch(() => {});  // Ignore the top-level error, it is already inside TestInfo.errors.
 
@@ -429,6 +438,8 @@ export class WorkerMain extends ProcessRunner {
         throw firstAfterHooksError;
     }).catch(() => {});  // Ignore the top-level error, it is already inside TestInfo.errors.
 
+    checkForFloatingPromises('afterAll/afterEach hooks');
+
     if (testInfo._isFailure())
       this._isStopped = true;
 

tests/playwright-test/warnings.spec.ts

@@ -45,6 +45,7 @@ test.describe('await', () => {
     });
     expect(exitCode).toBe(1);
     expect(stdout).toContain(warningSnippet);
+    expect(stdout).toContain('the test');
     expect(stdout).toContain('custom test name');
   });
 
@@ -53,7 +54,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           expect(page.locator('div')).toHaveText('A');
         });
       `
@@ -80,7 +81,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           await expect(page.locator('div')).toHaveText('A');
         });
       `
@@ -89,6 +90,20 @@ test.describe('await', () => {
     expect(stdout).not.toContain(warningSnippet);
   });
 
+  test('should not warn when using then on expects when passing', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent('<div>A</div>');
+          expect(page.locator('div')).toHaveText('A').then(() => {});
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).not.toContain(warningSnippet);
+  });
+
   test('should warn about missing await on reject', async ({ runInlineTest }) => {
     const { exitCode, stdout } = await runInlineTest({
       'a.test.ts': `
@@ -120,7 +135,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           test.step('step', () => {});
           await expect(page.locator('div')).toHaveText('A');
         });
@@ -135,7 +150,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           await test.step('step', () => {});
           await expect(page.locator('div')).toHaveText('A');
         });
@@ -150,7 +165,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           test.step.skip('step', () => {});
           await expect(page.locator('div')).toHaveText('A');
         });
@@ -165,7 +180,7 @@ test.describe('await', () => {
       'a.test.ts': `
         import { test, expect } from '@playwright/test';
         test('test', async ({ page }) => {
-          await page.setContent('data:text/html,<div>A</div>');
+          await page.setContent('<div>A</div>');
           const expectPromise = expect(page.locator('div')).toHaveText('A');
           expect(expectPromise instanceof Promise).toBeTruthy();
         });
@@ -173,4 +188,75 @@ test.describe('await', () => {
     });
     expect(exitCode).toBe(0);
   });
+
+  test('should warn about missing await in before hooks', async ({ runInlineTest }) => {
+    const group = ['beforeAll', 'beforeEach'];
+    for (const hook of group) {
+      await test.step(hook, async () => {
+        const { exitCode, stdout } = await runInlineTest({
+          'a.test.ts': `
+            import { test, expect } from '@playwright/test';
+            let page;
+            test.${hook}(async ({ browser }) => {
+              page = await browser.newPage();
+              await page.setContent('<div>A</div>');
+              expect(page.locator('div')).toHaveText('A');
+            });
+            test('test ${hook}', async () => {
+              await expect(page.locator('div')).toBeVisible();
+            });
+          `
+        });
+
+        expect(exitCode).toBe(0);
+        expect(stdout).toContain(warningSnippet);
+        expect(stdout).toContain(`${group[0]}/${group[1]} hooks`);
+      });
+    }
+  });
+
+  test.describe('should warn about missing await in after hooks', () => {
+    const group = ['afterAll', 'afterEach'];
+    for (const hook of group) {
+      test(hook, async ({ runInlineTest }) => {
+        const { exitCode, stdout } = await runInlineTest({
+          'a.test.ts': `
+            import { test, expect } from '@playwright/test';
+            let page;
+            test('test ${hook}', async ({ browser }) => {
+              await expect(Promise.resolve()).resolves.toBe(undefined);
+            });
+            test.${hook}(async () => {
+              expect(Promise.resolve()).resolves.toBe(undefined);
+            });
+          `
+        });
+
+        expect(exitCode).toBe(0);
+        expect(stdout).toContain(warningSnippet);
+        expect(stdout).toContain(`${group[0]}/${group[1]} hooks`);
+      });
+    }
+  });
+
+  test('should warn about missing await across hooks and test', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test.beforeAll(async () => {
+          expect(Promise.resolve()).resolves.toBe(undefined);
+        });
+        test('test', async () => {
+          expect(Promise.resolve()).resolves.toBe(undefined);
+        });
+        test.afterEach(async () => {
+          expect(Promise.resolve()).resolves.toBe(undefined);
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain(`${warningSnippet} by the end of beforeAll/beforeEach hooks.`);
+    expect(stdout).toContain(`${warningSnippet} by the end of the test.`);
+    expect(stdout).toContain(`${warningSnippet} by the end of afterAll/afterEach hooks.`);
+  });
 });