feat: Warn on floating promises (#34845)

packages/playwright/src/common/testType.ts

@@ -25,6 +25,7 @@ import { wrapFunctionWithLocation } from '../transform/transform';
 import type { FixturesWithLocation } from './config';
 import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test';
 import type { Location } from '../../types/testReporter';
+import type { TestInfoImpl } from '../worker/testInfo';
 
 
 const testTypeSymbol = Symbol('testType');
@@ -261,10 +262,14 @@ export class TestTypeImpl {
     suite._use.push({ fixtures, location });
   }
 
-  async _step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
+  _step<T>(expectation: 'pass'|'skip', title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
     const testInfo = currentTestInfo();
     if (!testInfo)
       throw new Error(`test.step() can only be called from a test`);
+    return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, title, body, options));
+  }
+
+  private async _stepInternal<T>(expectation: 'pass'|'skip', testInfo: TestInfoImpl, title: string, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
     const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
     return await currentZone().with('stepZone', step).run(async () => {
       try {

packages/playwright/src/matchers/expect.ts

@@ -381,8 +381,10 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
         setMatcherCallContext({ expectInfo: this._info, testInfo, step: step.info });
         const callback = () => matcher.call(target, ...args);
         const result = currentZone().with('stepZone', step).run(callback);
-        if (result instanceof Promise)
-          return result.then(finalizer).catch(reportStepError);
+        if (result instanceof Promise) {
+          const promise = result.then(finalizer).catch(reportStepError);
+          return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise);
+        }
         finalizer();
         return result;
       } catch (e) {

packages/playwright/src/reporters/base.ts

@@ -270,6 +270,7 @@ export class TerminalReporter implements ReporterV2 {
     if (full && summary.failuresToPrint.length && !this._omitFailures)
       this._printFailures(summary.failuresToPrint);
     this._printSlowTests();
+    this._printWarnings();
     this._printSummary(summaryMessage);
   }
 
@@ -289,6 +290,28 @@ export class TerminalReporter implements ReporterV2 {
       console.log(this.screen.colors.yellow('  Consider running tests from slow files in parallel, see https://playwright.dev/docs/test-parallel.'));
   }
 
+  private _printWarnings() {
+    const warningTests = this.suite.allTests().filter(test => test.annotations.some(a => a.type === 'warning'));
+    const encounteredWarnings = new Map<string, Array<TestCase>>();
+    for (const test of warningTests) {
+      for (const annotation of test.annotations) {
+        if (annotation.type !== 'warning' || annotation.description === undefined)
+          continue;
+        let tests = encounteredWarnings.get(annotation.description);
+        if (!tests) {
+          tests = [];
+          encounteredWarnings.set(annotation.description, tests);
+        }
+        tests.push(test);
+      }
+    }
+    for (const [description, tests] of encounteredWarnings) {
+      console.log(this.screen.colors.yellow('  Warning: ') + description);
+      for (const test of tests)
+        console.log(this.formatTestHeader(test, { indent: '    ', mode: 'default' }));
+    }
+  }
+
   private _printSummary(summary: string) {
     if (summary.trim())
       console.log(summary);

packages/playwright/src/worker/floatingPromiseScope.ts

@@ -0,0 +1,49 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export class FloatingPromiseScope {
+  readonly _floatingCalls: Set<Promise<any>> = new Set();
+
+  /**
+   * Enables a promise API call to be tracked by the test, alerting if unawaited.
+   *
+   * **NOTE:** Returning from an async function wraps the result in a promise, regardless of whether the return value is a promise. This will automatically mark the promise as awaited. Avoid this.
+   */
+  wrapPromiseAPIResult<T>(promise: Promise<T>): Promise<T> {
+    const promiseProxy = new Proxy(promise, {
+      get: (target, prop, receiver) => {
+        if (prop === 'then') {
+          return (...args: any[]) => {
+            this._floatingCalls.delete(promise);
+
+            const originalThen = Reflect.get(target, prop, receiver) as Promise<T>['then'];
+            return originalThen.call(target, ...args);
+          };
+        } else {
+          return Reflect.get(target, prop, receiver);
+        }
+      }
+    });
+
+    this._floatingCalls.add(promise);
+
+    return promiseProxy;
+  }
+
+  hasFloatingPromises(): boolean {
+    return this._floatingCalls.size > 0;
+  }
+}

packages/playwright/src/worker/testInfo.ts

@@ -23,6 +23,7 @@ import { TimeoutManager, TimeoutManagerError, kMaxDeadline } from './timeoutMana
 import { debugTest, filteredStackTrace, formatLocation, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util';
 import { TestTracing } from './testTracing';
 import { testInfoError } from './util';
+import { FloatingPromiseScope } from './floatingPromiseScope';
 
 import type { RunnableDescription } from './timeoutManager';
 import type { FullProject, TestInfo, TestStatus, TestStepInfo } from '../../types/test';
@@ -67,6 +68,7 @@ export class TestInfoImpl implements TestInfo {
   readonly _startTime: number;
   readonly _startWallTime: number;
   readonly _tracing: TestTracing;
+  readonly _floatingPromiseScope: FloatingPromiseScope = new FloatingPromiseScope();
 
   _wasInterrupted = false;
   _lastStepId = 0;

packages/playwright/src/worker/workerMain.ts

@@ -369,6 +369,9 @@ 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.
+        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.
 

tests/playwright-test/warnings.spec.ts

@@ -0,0 +1,176 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test, expect } from './playwright-test-fixtures';
+
+const warningSnippet = 'Some async calls were not awaited';
+
+test.describe.configure({ mode: 'parallel' });
+
+test.describe('await', () => {
+  test('should not care about non-API promises', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test } from '@playwright/test';
+        test('test', () => {
+          new Promise(() => {});
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).not.toContain(warningSnippet);
+  });
+
+  test('should warn about missing await on expects when failing', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('custom test name', async ({ page }) => {
+          expect(page.locator('div')).toHaveText('A', { timeout: 100 });
+        });
+      `
+    });
+    expect(exitCode).toBe(1);
+    expect(stdout).toContain(warningSnippet);
+    expect(stdout).toContain('custom test name');
+  });
+
+  test('should warn about missing await 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('data:text/html,<div>A</div>');
+          expect(page.locator('div')).toHaveText('A');
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain(warningSnippet);
+  });
+
+  test('should not warn when not missing await on expects when failing', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await expect(page.locator('div')).toHaveText('A', { timeout: 100 });
+        });
+      `
+    });
+    expect(exitCode).toBe(1);
+    expect(stdout).not.toContain(warningSnippet);
+  });
+
+  test('should not warn when not missing await 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('data:text/html,<div>A</div>');
+          await expect(page.locator('div')).toHaveText('A');
+        });
+      `
+    });
+    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': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          expect(Promise.reject(new Error('foo'))).rejects.toThrow('foo');
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain(warningSnippet);
+  });
+
+  test('should warn about missing await on reject.not', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          expect(Promise.reject(new Error('foo'))).rejects.not.toThrow('foo');
+        });
+      `
+    });
+    expect(exitCode).toBe(1);
+    expect(stdout).toContain(warningSnippet);
+  });
+
+  test('should warn about missing await on test.step', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent('data:text/html,<div>A</div>');
+          test.step('step', () => {});
+          await expect(page.locator('div')).toHaveText('A');
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain(warningSnippet);
+  });
+
+  test('should not warn when not missing await on test.step', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent('data:text/html,<div>A</div>');
+          await test.step('step', () => {});
+          await expect(page.locator('div')).toHaveText('A');
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).not.toContain(warningSnippet);
+  });
+
+  test('should warn about missing await on test.step.skip', async ({ runInlineTest }) => {
+    const { exitCode, stdout } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent('data:text/html,<div>A</div>');
+          test.step.skip('step', () => {});
+          await expect(page.locator('div')).toHaveText('A');
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain(warningSnippet);
+  });
+
+  test('traced promise should be instanceof Promise', async ({ runInlineTest }) => {
+    const { exitCode } = await runInlineTest({
+      'a.test.ts': `
+        import { test, expect } from '@playwright/test';
+        test('test', async ({ page }) => {
+          await page.setContent('data:text/html,<div>A</div>');
+          const expectPromise = expect(page.locator('div')).toHaveText('A');
+          expect(expectPromise instanceof Promise).toBeTruthy();
+        });
+      `
+    });
+    expect(exitCode).toBe(0);
+  });
+});