fix: swallow expect.soft errors inside successful toPass matcher (#20509)

Fixes #20437

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 TestInfoErrorState = {
+  status: TestStatus,
+  errors: TestInfoError[],
+  hasHardError: boolean,
+};
+
 export class TestInfoImpl implements TestInfo {
   private _onStepBegin: (payload: StepBeginPayload) => void;
   private _onStepEnd: (payload: StepEndPayload) => void;
@@ -248,6 +254,20 @@ export class TestInfoImpl implements TestInfo {
     this.errors.push(error);
   }
 
+  _saveErrorState(): TestInfoErrorState {
+    return {
+      hasHardError: this._hasHardError,
+      status: this.status,
+      errors: this.errors.slice(),
+    };
+  }
+
+  _restoreErrorState(state: TestInfoErrorState) {
+    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 { TestInfoErrorState } from '../common/testInfo';
 import { toBeTruthy } from './toBeTruthy';
 import { toEqual } from './toEqual';
 import { toExpectedTextValues, toMatchText } from './toMatchText';
@@ -317,11 +319,22 @@ export async function toPass(
     timeout?: number,
   } = {},
 ) {
+  const testInfo = currentTestInfo();
+
   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|TestInfoErrorState;
   const result = await pollAgainstTimeout<Error|undefined>(async () => {
     try {
+      if (testStateBeforeToPassMatcher && testInfo)
+        testInfo._restoreErrorState(testStateBeforeToPassMatcher);
+      testStateBeforeToPassMatcher = testInfo?._saveErrorState();
       await callback();
+      if (testInfo && testStateBeforeToPassMatcher && 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 };

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,50 @@ 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 }) => {
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/20437' });
+
+  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': `