cherry-pick(#22254): revert(20509, 20596): expect.toPass is broken wi…

…th these Reverts #20509 and #20596 Fixes #22215
microsoft · Apr 11, 2023 · 710674a · 710674a
1 parent cb419e7
commit 710674a
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 106 deletions.
diff --git a/packages/playwright-test/src/matchers/matchers.ts b/packages/playwright-test/src/matchers/matchers.ts
@@ -19,8 +19,6 @@ 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 '../worker/testInfo';
 import { toBeTruthy } from './toBeTruthy';
 import { toEqual } from './toEqual';
 import { toExpectedTextValues, toMatchText } from './toMatchText';
@@ -328,22 +326,11 @@ 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 };
@@ -361,7 +348,5 @@ export async function toPass(
 
     return { message, pass: this.isNot };
   }
-  if (testStateBeforeToPassMatcher && testInfo)
-    testInfo._restoreErrorState(testStateBeforeToPassMatcher);
   return { pass: !this.isNot, message: () => '' };
 }
diff --git a/packages/playwright-test/src/worker/testInfo.ts b/packages/playwright-test/src/worker/testInfo.ts
@@ -25,12 +25,6 @@ import type { Annotation, FullConfigInternal, FullProjectInternal, Location } fr
 import { getContainedPath, normalizeAndSaveAttachment, sanitizeForFilePath, serializeError, trimLongString } from '../util';
 import type * as trace from '@trace/trace';
 
-export type TestInfoErrorState = {
-  status: TestStatus,
-  errors: TestInfoError[],
-  hasHardError: boolean,
-};
-
 interface TestStepInternal {
   complete(result: { error?: Error | TestInfoError }): void;
   title: string;
@@ -268,20 +262,6 @@ 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' | 'wallTime'>): Promise<T> {
     const step = this._addStep({ ...stepInfo, wallTime: Date.now() });
     try {

diff --git a/tests/playwright-test/expect-to-pass.spec.ts b/tests/playwright-test/expect-to-pass.spec.ts
@@ -35,17 +35,10 @@ 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(3);
+  expect(result.passed).toBe(2);
 });
 
 test('should respect timeout', async ({ runInlineTest }) => {
@@ -159,74 +152,12 @@ 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': `
-      import { test, expect } from '@playwright/test';
-      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(result.output).toContain('Received: "before-toPass"');
-  expect(result.output).toContain('Received: "after-toPass"');
-  expect(result.output).not.toContain('Received: "inside-toPass-1"');
-  expect(result.exitCode).toBe(1);
-  expect(result.failed).toBe(1);
-});
-
-test('should work with no.toPass and failing soft assertion', async ({ runInlineTest }) => {
-  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/20518' });
-
-  const result = await runInlineTest({
-    'a.spec.ts': `
-      import { test, expect } from '@playwright/test';
-      test('should work', async () => {
-        await test.expect(() => {
-          expect.soft(1).toBe(2);
-        }).not.toPass({ timeout: 1000 });
-      });
-    `
-  });
-  expect(result.exitCode).toBe(0);
-  expect(result.failed).toBe(0);
-  expect(result.passed).toBe(1);
-});
-
-test('should show only soft errors on last toPass pass', async ({ runInlineTest }) => {
-  const result = await runInlineTest({
-    'a.spec.ts': `
-      import { test, expect } from '@playwright/test';
-      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(result.output).not.toContain('Received: "inside-toPass-1"');
-  expect(result.output).not.toContain('Received: "inside-toPass-2"');
-  expect(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': `
       import { test, expect } from '@playwright/test';
       test('should respect soft', async () => {
-        await test.expect.soft(() => {
+        await expect.soft(() => {
           expect(1).toBe(3);
         }).toPass({ timeout: 1000 });
         expect.soft(2).toBe(3);