chore: render successful toPass as such (#23411)

Fixes https://github.com/microsoft/playwright/issues/23302

packages/playwright-test/src/matchers/expect.ts

@@ -249,13 +249,13 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
 
       const defaultTitle = `expect${this._info.isPoll ? '.poll' : ''}${this._info.isSoft ? '.soft' : ''}${this._info.isNot ? '.not' : ''}.${matcherName}${argsSuffix}`;
       const wallTime = Date.now();
-      const step = testInfo._addStep({
+      const step = matcherName !== 'toPass' ? testInfo._addStep({
         location: stackFrames[0],
         category: 'expect',
         title: trimLongString(customMessage || defaultTitle, 1024),
         params: args[0] ? { expected: args[0] } : undefined,
         wallTime
-      });
+      }) : undefined;
 
       const reportStepError = (jestError: Error) => {
         const message = jestError.message;
@@ -283,7 +283,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
         const serializedError = serializeError(jestError);
         // Serialized error has filtered stack trace.
         jestError.stack = serializedError.stack;
-        step.complete({ error: serializedError });
+        step?.complete({ error: serializedError });
         if (this._info.isSoft)
           testInfo._failWithError(serializedError, false /* isHardError */);
         else
@@ -291,7 +291,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
       };
 
       const finalizer = () => {
-        step.complete({});
+        step?.complete({});
       };
 
       // Process the async matchers separately to preserve the zones in the stacks.

packages/playwright-test/src/matchers/matchers.ts

@@ -18,12 +18,13 @@ import type { Locator, Page, APIResponse } from 'playwright-core';
 import type { FrameExpectOptions } from 'playwright-core/lib/client/types';
 import { colors } from 'playwright-core/lib/utilsBundle';
 import type { Expect } from '../../types/test';
-import { expectTypes, callLogText } from '../util';
+import { expectTypes, callLogText, filteredStackTrace } from '../util';
 import { toBeTruthy } from './toBeTruthy';
 import { toEqual } from './toEqual';
 import { toExpectedTextValues, toMatchText } from './toMatchText';
-import { constructURLBasedOnBaseURL, isTextualMimeType, pollAgainstTimeout } from 'playwright-core/lib/utils';
+import { captureRawStack, constructURLBasedOnBaseURL, isTextualMimeType, pollAgainstTimeout } from 'playwright-core/lib/utils';
 import { currentTestInfo } from '../common/globals';
+import type { TestStepInternal } from '../worker/testInfo';
 
 interface LocatorEx extends Locator {
   _expect(expression: string, options: Omit<FrameExpectOptions, 'expectedValue'> & { expectedValue?: any }): Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }>;
@@ -341,27 +342,43 @@ export async function toPass(
   const testInfo = currentTestInfo();
   const timeout = options.timeout !== undefined ? options.timeout : 0;
 
-  const result = await pollAgainstTimeout<Error|undefined>(async () => {
-    if (testInfo && currentTestInfo() !== testInfo)
-      return { continuePolling: false, result: undefined };
-    try {
-      await callback();
-      return { continuePolling: this.isNot, result: undefined };
-    } catch (e) {
-      return { continuePolling: !this.isNot, result: e };
+  const rawStack = captureRawStack();
+  const stackFrames = filteredStackTrace(rawStack);
+
+  const runWithOrWithoutStep = async (callback: (step: TestStepInternal | undefined) => Promise<{ pass: boolean; message: () => string; }>) => {
+    if (!testInfo)
+      return await callback(undefined);
+    return await testInfo._runAsStep({
+      title: 'expect.toPass',
+      category: 'expect',
+      location: stackFrames[0],
+      insulateChildErrors: true,
+    }, callback);
+  };
+
+  return await runWithOrWithoutStep(async (step: TestStepInternal | undefined) => {
+    const result = await pollAgainstTimeout<Error|undefined>(async () => {
+      if (testInfo && currentTestInfo() !== testInfo)
+        return { continuePolling: false, result: undefined };
+      try {
+        await callback();
+        return { continuePolling: this.isNot, result: undefined };
+      } catch (e) {
+        return { continuePolling: !this.isNot, result: e };
+      }
+    }, timeout, options.intervals || [100, 250, 500, 1000]);
+
+    if (result.timedOut) {
+      const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`;
+      const message = result.result ? [
+        result.result.message,
+        '',
+        `Call Log:`,
+        `- ${timeoutMessage}`,
+      ].join('\n') : timeoutMessage;
+      step?.complete({ error: { message } });
+      return { message: () => message, pass: this.isNot };
     }
-  }, timeout, options.intervals || [100, 250, 500, 1000]);
-
-  if (result.timedOut) {
-    const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`;
-    const message = () => result.result ? [
-      result.result.message,
-      '',
-      `Call Log:`,
-      `- ${timeoutMessage}`,
-    ].join('\n') : timeoutMessage;
-
-    return { message, pass: this.isNot };
-  }
-  return { pass: !this.isNot, message: () => '' };
+    return { pass: !this.isNot, message: () => '' };
+  });
 }

packages/playwright-test/src/worker/testInfo.ts

@@ -39,6 +39,7 @@ export interface TestStepInternal {
   apiName?: string;
   params?: Record<string, any>;
   error?: TestInfoError;
+  insulateChildErrors?: boolean;
 }
 
 export class TestInfoImpl implements TestInfo {
@@ -252,7 +253,7 @@ export class TestInfoImpl implements TestInfo {
         } else if (result.error) {
           // Internal API step reported an error.
           error = result.error;
-        } else {
+        } else if (!data.insulateChildErrors) {
           // One of the child steps failed (probably soft expect).
           // Report this step as failed to make it easier to spot.
           error = step.steps.map(s => s.error).find(e => !!e);

tests/playwright-test/test-step.spec.ts

@@ -951,7 +951,7 @@ test('should not mark page.close as failed when page.click fails', async ({ runI
   ]);
 });
 
-test('should nest page.continue insize page.goto steps', async ({ runInlineTest }) => {
+test('should nest page.continue inside page.goto steps', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'reporter.ts': stepHierarchyReporter,
     'playwright.config.ts': `module.exports = { reporter: './reporter', };`,
@@ -1033,3 +1033,98 @@ test('should nest page.continue insize page.goto steps', async ({ runInlineTest
     },
   ]);
 });
+
+test('should not propagate errors from within toPass', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepHierarchyReporter,
+    'playwright.config.ts': `module.exports = { reporter: './reporter', };`,
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('pass', async () => {
+        let i = 0;
+        await expect(() => {
+          expect(i++).toBe(2);
+        }).toPass();
+      });
+    `
+  }, { reporter: '' });
+
+  expect(result.exitCode).toBe(0);
+  const objects = result.outputLines.map(line => JSON.parse(line));
+  expect(objects).toEqual([
+    {
+      title: 'Before Hooks',
+      category: 'hook',
+    },
+    {
+      title: 'expect.toPass',
+      category: 'expect',
+      location: { file: 'a.test.ts', line: 'number', column: 'number' },
+      steps: [
+        {
+          category: 'expect',
+          error: '<error>',
+          location: { file: 'a.test.ts', line: 'number', column: 'number' },
+          title: 'expect.toBe',
+        },
+        {
+          category: 'expect',
+          error: '<error>',
+          location: { file: 'a.test.ts', line: 'number', column: 'number' },
+          title: 'expect.toBe',
+        },
+        {
+          category: 'expect',
+          location: { file: 'a.test.ts', line: 'number', column: 'number' },
+          title: 'expect.toBe',
+        },
+      ],
+    },
+    {
+      title: 'After Hooks',
+      category: 'hook',
+    },
+  ]);
+});
+
+test('should show final toPass error', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepHierarchyReporter,
+    'playwright.config.ts': `module.exports = { reporter: './reporter', };`,
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('fail', async () => {
+        await expect(() => {
+          expect(true).toBe(false);
+        }).toPass({ timeout: 1 });
+      });
+    `
+  }, { reporter: '' });
+
+  expect(result.exitCode).toBe(1);
+  const objects = result.outputLines.map(line => JSON.parse(line));
+  expect(objects).toEqual([
+    {
+      title: 'Before Hooks',
+      category: 'hook',
+    },
+    {
+      title: 'expect.toPass',
+      category: 'expect',
+      error: '<error>',
+      location: { file: 'a.test.ts', line: 'number', column: 'number' },
+      steps: [
+        {
+          category: 'expect',
+          error: '<error>',
+          location: { file: 'a.test.ts', line: 'number', column: 'number' },
+          title: 'expect.toBe',
+        },
+      ],
+    },
+    {
+      title: 'After Hooks',
+      category: 'hook',
+    },
+  ]);
+});