From d9a8bb057dc4717436ad97ad7df26a3f2f314c5a Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 3 Feb 2022 17:14:12 -0800
Subject: [PATCH] fix(test-fail): allow unhandled expects in test.fail (#11850)

Previously, we would consider this a worker error, but
we make an exception for "expect()" calls.
---
 packages/playwright-test/src/workerRunner.ts |  6 +++++-
 tests/playwright-test/basic.spec.ts          | 16 ++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts
index 52420c9f97..f421ade8b7 100644
--- a/packages/playwright-test/src/workerRunner.ts
+++ b/packages/playwright-test/src/workerRunner.ts
@@ -94,7 +94,11 @@ export class WorkerRunner extends EventEmitter {
     // a test runner. In the latter case, the worker state could be messed up,
     // and continuing to run tests in the same worker is problematic. Therefore,
     // we turn this into a fatal error and restart the worker anyway.
-    if (this._currentTest && this._currentTest._test._type === 'test' && this._currentTest.expectedStatus !== 'failed') {
+    // The only exception is the expect() error that we still consider ok.
+    const isExpectError = (error instanceof Error) && !!(error as any).matcherResult;
+    const isCurrentTestExpectedToFail = this._currentTest?.expectedStatus === 'failed';
+    const shouldConsiderAsTestError = isExpectError || !isCurrentTestExpectedToFail;
+    if (this._currentTest && this._currentTest._test._type === 'test' && shouldConsiderAsTestError) {
       this._currentTest._failWithError(serializeError(error), true /* isHardError */);
     } else {
       // No current test - fatal error.
diff --git a/tests/playwright-test/basic.spec.ts b/tests/playwright-test/basic.spec.ts
index 074a409896..9eb9d96dde 100644
--- a/tests/playwright-test/basic.spec.ts
+++ b/tests/playwright-test/basic.spec.ts
@@ -426,3 +426,19 @@ test('should not reuse worker after unhandled rejection in test.fail', async ({
   expect(result.output).toContain(`Error: Oh my!`);
   expect(result.output).not.toContain(`Did not teardown test scope`);
 });
+
+test('should allow unhandled expects in test.fail', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      const { test } = pwt;
+      test('failing1', async ({}) => {
+        test.fail();
+        Promise.resolve().then(() => expect(1).toBe(2));
+        await new Promise(f => setTimeout(f, 100));
+      });
+    `
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+  expect(result.output).not.toContain(`Error: expect`);
+});