feat: Display warnings only on test failure

packages/playwright/src/common/testType.ts

@@ -25,7 +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';
+import type { TestInfoImpl, TestStepInternal } from '../worker/testInfo';
 
 
 const testTypeSymbol = Symbol('testType');
@@ -266,11 +266,11 @@ export class TestTypeImpl {
     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));
+    const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
+    return testInfo._floatingPromiseScope.wrapPromiseAPIResult(this._stepInternal(expectation, testInfo, step, body, options), step.location);
   }
 
-  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 });
+  private async _stepInternal<T>(expectation: 'pass'|'skip', testInfo: TestInfoImpl, step: TestStepInternal, body: (step: TestStepInfo) => T | Promise<T>, options: {box?: boolean, location?: Location, timeout?: number } = {}): Promise<T> {
     return await currentZone().with('stepZone', step).run(async () => {
       try {
         let result: Awaited<ReturnType<typeof raceAgainstDeadline<T>>> | undefined = undefined;

packages/playwright/src/matchers/expect.ts

@@ -385,7 +385,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
         const result = currentZone().with('stepZone', step).run(callback);
         if (result instanceof Promise) {
           const promise = result.then(finalizer).catch(reportStepError);
-          return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise);
+          return testInfo._floatingPromiseScope.wrapPromiseAPIResult(promise, stackFrames[0]);
         }
         finalizer();
         return result;

packages/playwright/src/reporters/base.ts

@@ -26,6 +26,7 @@ import { getEastAsianWidth } from '../utilsBundle';
 import type { ReporterV2 } from './reporterV2';
 import type { FullConfig, FullResult, Location, Suite, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter';
 import type { Colors } from '@isomorphic/colors';
+import type { TestAnnotation } from '../../types/test';
 
 export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
 export const kOutputSymbol = Symbol('output');
@@ -269,7 +270,6 @@ export class TerminalReporter implements ReporterV2 {
     if (full && summary.failuresToPrint.length && !this._omitFailures)
       this._printFailures(summary.failuresToPrint);
     this._printSlowTests();
-    this._printWarnings();
     this._printSummary(summaryMessage);
   }
 
@@ -289,31 +289,6 @@ 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 => {
-      const annotations = [...test.annotations, ...test.results.flatMap(r => r.annotations)];
-      return annotations.some(a => a.type === 'warning');
-    });
-    const encounteredWarnings = new Map<string, Array<TestCase>>();
-    for (const test of warningTests) {
-      for (const annotation of [...test.annotations, ...test.results.flatMap(r => r.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);
@@ -345,6 +320,7 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
   const header = formatTestHeader(screen, config, test, { indent: '  ', index, mode: 'error' });
   lines.push(screen.colors.red(header));
   for (const result of test.results) {
+    const warnings = [...result.annotations, ...test.annotations].filter(a => a.type === 'warning');
     const resultLines: string[] = [];
     const errors = formatResultFailure(screen, test, result, '    ');
     if (!errors.length)
@@ -354,6 +330,10 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
       resultLines.push(screen.colors.gray(separator(screen, `    Retry #${result.retry}`)));
     }
     resultLines.push(...errors.map(error => '\n' + error.message));
+    if (warnings.length) {
+      resultLines.push('');
+      resultLines.push(...formatTestWarning(screen, config, warnings));
+    }
     for (let i = 0; i < result.attachments.length; ++i) {
       const attachment = result.attachments[i];
       if (attachment.name.startsWith('_prompt') && attachment.path) {
@@ -396,6 +376,26 @@ export function formatFailure(screen: Screen, config: FullConfig, test: TestCase
   return lines.join('\n');
 }
 
+function formatTestWarning(screen: Screen, config: FullConfig, warnings: TestAnnotation[]): string[] {
+  warnings.sort((a, b) => {
+    const aLocationKey = a.location ? `${a.location.file}:${a.location.line}:${a.location.column}` : undefined;
+    const bLocationKey = b.location ? `${b.location.file}:${b.location.line}:${b.location.column}` : undefined;
+
+    if (!aLocationKey && !bLocationKey)
+      return 0;
+    if (!aLocationKey)
+      return 1;
+    if (!bLocationKey)
+      return -1;
+    return aLocationKey.localeCompare(bLocationKey);
+  });
+
+  return warnings.filter(w => !!w.description).map(w => {
+    const location = !!w.location ? `${relativeFilePath(screen, config, w.location.file)}:${w.location.line}:${w.location.column}: ` : '';
+    return `${screen.colors.yellow(`    Warning: ${location}${w.description}`)}`;
+  });
+}
+
 export function formatRetry(screen: Screen, result: TestResult) {
   const retryLines = [];
   if (result.retry) {

packages/playwright/src/util.ts

@@ -65,18 +65,6 @@ export function filteredStackTrace(rawStack: RawStack): StackFrame[] {
   return frames;
 }
 
-export function filteredLocation(rawStack: RawStack): Location | undefined {
-  const frame = filteredStackTrace(rawStack)[0] as StackFrame | undefined;
-  if (!frame)
-    return undefined;
-  return {
-    file: frame.file,
-    line: frame.line,
-    column: frame.column
-  };
-}
-
-
 export function serializeError(error: Error | any): TestInfoErrorImpl {
   if (error instanceof Error)
     return filterStackTrace(error);

packages/playwright/src/worker/floatingPromiseScope.ts

@@ -14,15 +14,17 @@
  * limitations under the License.
  */
 
+import type { Location } from '../../types/test';
+
 export class FloatingPromiseScope {
-  readonly _floatingCalls: Set<Promise<any>> = new Set();
+  readonly _floatingCalls: Map<Promise<any>, Location | undefined> = new Map();
 
   /**
    * 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> {
+  wrapPromiseAPIResult<T>(promise: Promise<T>, location: Location | undefined): Promise<T> {
     if (process.env.PW_DISABLE_FLOATING_PROMISES_WARNING)
       return promise;
 
@@ -41,7 +43,7 @@ export class FloatingPromiseScope {
       }
     });
 
-    this._floatingCalls.add(promise);
+    this._floatingCalls.set(promise, location);
 
     return promiseProxy;
   }
@@ -53,4 +55,8 @@ export class FloatingPromiseScope {
   hasFloatingPromises(): boolean {
     return this._floatingCalls.size > 0;
   }
+
+  floatingPromises(): Array<{ location: Location | undefined, promise: Promise<any> }> {
+    return Array.from(this._floatingCalls.entries()).map(([promise, location]) => ({ location, promise }));
+  }
 }

packages/playwright/src/worker/workerMain.ts

@@ -430,15 +430,25 @@ export class WorkerMain extends ProcessRunner {
         throw firstAfterHooksError;
     }).catch(() => {});  // Ignore the top-level error, it is already inside TestInfo.errors.
 
-    // Create warning if any of the async calls were not awaited in various stages.
-    if (!process.env.PW_DISABLE_FLOATING_PROMISES_WARNING && 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.` });
-      testInfo._floatingPromiseScope.clear();
-    }
-
-    if (testInfo._isFailure())
+    if (testInfo._isFailure()) {
       this._isStopped = true;
 
+      // Only if failed, create warning if any of the async calls were not awaited in various stages.
+      if (!process.env.PW_DISABLE_FLOATING_PROMISES_WARNING && testInfo._floatingPromiseScope.hasFloatingPromises()) {
+        // Dedupe by location
+        const annotationLocations = new Map<string | undefined, Location | undefined>(testInfo._floatingPromiseScope.floatingPromises().map(
+            ({ location }) => {
+              const locationKey = location ? `${location.file}:${location.line}:${location.column}` : undefined;
+              return [locationKey, location];
+            }));
+
+        testInfo.annotations.push(...[...annotationLocations.values()].map(location => ({
+          type: 'warning', description: `This async call was not awaited by the end of the test. This can cause flakiness. It is recommended to run ESLint with "@typescript-eslint/no-floating-promises" to verify.`, location
+        })));
+        testInfo._floatingPromiseScope.clear();
+      }
+    }
+
     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.

tests/playwright-test/reporter-base.spec.ts

@@ -486,5 +486,72 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       expect(text).toContain('› passes @bar1 @bar2 (');
       expect(text).toContain('› passes @baz1 @baz2 (');
     });
+
+    test('should show warnings on failing tests', async ({ runInlineTest }) => {
+      const result = await runInlineTest({
+        'a.spec.ts': `
+          import { test, expect } from '@playwright/test';
+          test('fail', async ({ page }, testInfo) => {
+            testInfo.annotations.push({ type: 'warning', description: 'foo' });
+            expect(page.locator('div')).toHaveText('A', { timeout: 100 });
+            throw new Error();
+          });
+        `,
+      });
+      expect(result.exitCode).toBe(1);
+      expect(result.passed).toBe(0);
+      expect(result.output).toContain('Warning: a.spec.ts:5:41: This async call was not awaited by the end of the test.');
+      expect(result.output).toContain('Warning: foo');
+    });
+
+    test('should not show warnings on passing tests', async ({ runInlineTest }) => {
+      const result = await runInlineTest({
+        'a.spec.ts': `
+          import { test, expect } from '@playwright/test';
+          test('success', async ({ page }, testInfo) => {
+            testInfo.annotations.push({ type: 'warning', description: 'foo' });
+          });
+        `,
+      });
+      expect(result.exitCode).toBe(0);
+      expect(result.passed).toBe(1);
+      expect(result.output).not.toContain('Warning: foo');
+    });
+
+    test('should properly sort warnings', async ({ runInlineTest }) => {
+      const result = await runInlineTest({
+        'external.js': `
+          import { expect } from '@playwright/test';
+          export const externalAsyncCall = (page) => {
+            expect(page.locator('div')).toHaveText('A', { timeout: 100 });
+          };
+        `,
+        'a.spec.ts': `
+          import { test, expect } from '@playwright/test';
+          import { externalAsyncCall } from './external.js';
+          test('fail a', async ({ page }, testInfo) => {
+            testInfo.annotations.push({ type: 'warning', description: 'foo' });
+            externalAsyncCall(page);
+            expect(page.locator('div')).toHaveText('A', { timeout: 100 });
+            testInfo.annotations.push({ type: 'warning', description: 'bar' });
+            throw new Error();
+          });
+        `,
+      });
+      expect(result.exitCode).toBe(1);
+      expect(result.passed).toBe(0);
+      expect(result.output).toContain('Warning: a.spec.ts:7:41: This async call was not awaited by the end of the test.');
+      expect(result.output).toContain('Warning: external.js:4:41: This async call was not awaited by the end of the test.');
+      expect(result.output).toContain('Warning: foo');
+      expect(result.output).toContain('Warning: bar');
+
+      const manualIndexFoo = result.output.indexOf('Warning: foo');
+      const manualIndexBar = result.output.indexOf('Warning: bar');
+      const externalIndex = result.output.indexOf('Warning: external.js:4:41');
+      const specIndex = result.output.indexOf('Warning: a.spec.ts:7:41');
+      expect(specIndex).toBeLessThan(externalIndex);
+      expect(externalIndex).toBeLessThan(manualIndexFoo);
+      expect(manualIndexFoo).toBeLessThan(manualIndexBar);
+    });
   });
 }