fix: swallow expect.soft errors inside successful toPass matcher

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

@@ -24,6 +24,12 @@ import { TimeoutManager } from './timeoutManager';
 import type { Annotation, FullConfigInternal, FullProjectInternal, TestStepInternal } from './types';
 import { getContainedPath, normalizeAndSaveAttachment, sanitizeForFilePath, serializeError, trimLongString } from '../util';
 
+export type TestInfoState = {
+  status: TestStatus,
+  errors: TestInfoError[],
+  hasHardError: boolean,
+};
+
 export class TestInfoImpl implements TestInfo {
   private _onStepBegin: (payload: StepBeginPayload) => void;
   private _onStepEnd: (payload: StepEndPayload) => void;
@@ -246,6 +252,20 @@ export class TestInfoImpl implements TestInfo {
     this.errors.push(error);
   }
 
+  _saveTestState(): TestInfoState {
+    return {
+      hasHardError: this._hasHardError,
+      status: this.status,
+      errors: this.errors.slice(),
+    };
+  }
+
+  _restoreTestState(state: TestInfoState) {
+    this.status = state.status;
+    this.errors = state.errors.slice();
+    this._hasHardError = state.hasHardError;
+  }
+
   async _runAsStep<T>(cb: () => Promise<T>, stepInfo: Omit<TestStepInternal, 'complete'>): Promise<T> {
     const step = this._addStep(stepInfo);
     try {

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

@@ -19,6 +19,8 @@ import type { FrameExpectOptions } from 'playwright-core/lib/client/types';
 import { colors } from 'playwright-core/lib/utilsBundle';
 import type { Expect } from '../common/types';
 import { expectTypes, callLogText } from '../util';
+import { currentTestInfo } from '../common/globals';
+import type { TestInfoState } from '../common/testInfo';
 import { toBeTruthy } from './toBeTruthy';
 import { toEqual } from './toEqual';
 import { toExpectedTextValues, toMatchText } from './toMatchText';
@@ -317,11 +319,24 @@ export async function toPass(
     timeout?: number,
   } = {},
 ) {
+  const testInfo = currentTestInfo();
+  if (!testInfo)
+    throw new Error(`toPass() must be called during the test`);
+
   const timeout = options.timeout !== undefined ? options.timeout : 0;
 
+  // Soft expects might mark test as failing.
+  // We want to revert this later if the matcher is actually passing.
+  // See https://github.com/microsoft/playwright/issues/20437
+  let testStateBeforeToPassMatcher: undefined|TestInfoState;
   const result = await pollAgainstTimeout<Error|undefined>(async () => {
     try {
+      if (testStateBeforeToPassMatcher)
+        testInfo._restoreTestState(testStateBeforeToPassMatcher);
+      testStateBeforeToPassMatcher = testInfo._saveTestState();
       await callback();
+      if (testInfo.errors.length !== testStateBeforeToPassMatcher.errors.length)
+        return { continuePolling: !this.isNot, result: testInfo.errors[testInfo.errors.length - 1] };
       return { continuePolling: this.isNot, result: undefined };
     } catch (e) {
       return { continuePolling: !this.isNot, result: e };
@@ -339,5 +354,6 @@ export async function toPass(
 
     return { message, pass: this.isNot };
   }
-  return { pass: !this.isNot, message: () => '' };
+  const pass = !this.isNot;
+  return { pass, message: () => '' };
 }

tests/playwright-test/expect-to-pass.spec.ts

@@ -35,10 +35,17 @@ test('should retry predicate', async ({ runInlineTest }) => {
         }).toPass();
         expect(i).toBe(3);
       });
+      test('should retry expect.soft assertions', async () => {
+        let i = 0;
+        await test.expect(() => {
+          expect.soft(++i).toBe(3);
+        }).toPass();
+        expect(i).toBe(3);
+      });
     `
   });
   expect(result.exitCode).toBe(0);
-  expect(result.passed).toBe(2);
+  expect(result.passed).toBe(3);
 });
 
 test('should respect timeout', async ({ runInlineTest }) => {
@@ -152,6 +159,48 @@ test('should use custom message', async ({ runInlineTest }) => {
   expect(result.failed).toBe(1);
 });
 
+test('should swallow all soft errors inside toPass matcher, if successful', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      const { test } = pwt;
+      test('should respect soft', async () => {
+        expect.soft('before-toPass').toBe('zzzz');
+        let i = 0;
+        await test.expect(() => {
+          ++i;
+          expect.soft('inside-toPass-' + i).toBe('inside-toPass-2');
+        }).toPass({ timeout: 1000 });
+        expect.soft('after-toPass').toBe('zzzz');
+      });
+    `
+  });
+  expect(stripAnsi(result.output)).toContain('Received: "before-toPass"');
+  expect(stripAnsi(result.output)).toContain('Received: "after-toPass"');
+  expect(stripAnsi(result.output)).not.toContain('Received: "inside-toPass-1"');
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+});
+
+test('should show only soft errors on last toPass pass', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      const { test } = pwt;
+      test('should respect soft', async () => {
+        let i = 0;
+        await test.expect(() => {
+          ++i;
+          expect.soft('inside-toPass-' + i).toBe('0');
+        }).toPass({ timeout: 1000, intervals: [100, 100, 100000] });
+      });
+    `
+  });
+  expect(stripAnsi(result.output)).not.toContain('Received: "inside-toPass-1"');
+  expect(stripAnsi(result.output)).not.toContain('Received: "inside-toPass-2"');
+  expect(stripAnsi(result.output)).toContain('Received: "inside-toPass-3"');
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+});
+
 test('should work with soft', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'a.spec.ts': `