From 13ed1dee503bf8a8ce39c094d35f2a20baba648f Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Mon, 25 Oct 2021 14:17:27 -0700
Subject: [PATCH] fix(test runner): do not mask uncaught error in beforeEach
 (#9764)

---
 packages/playwright-test/src/workerRunner.ts |  8 ++++--
 tests/playwright-test/hooks.spec.ts          | 28 +++++++++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts
index 47c3af4218..c745624a3e 100644
--- a/packages/playwright-test/src/workerRunner.ts
+++ b/packages/playwright-test/src/workerRunner.ts
@@ -435,8 +435,12 @@ export class WorkerRunner extends EventEmitter {
         if (testInfo.status === 'passed')
           testInfo.status = 'skipped';
       } else {
-        testInfo.status = 'failed';
-        testInfo.error = serializeError(error);
+        if (testInfo.status === 'passed')
+          testInfo.status = 'failed';
+        // Do not overwrite any uncaught error that happened first.
+        // This is typical if you have some expect() that fails in beforeEach.
+        if (!('error' in testInfo))
+          testInfo.error = serializeError(error);
       }
       // Continue running afterEach hooks even after the failure.
     }
diff --git a/tests/playwright-test/hooks.spec.ts b/tests/playwright-test/hooks.spec.ts
index ce0866480b..4688f44e0b 100644
--- a/tests/playwright-test/hooks.spec.ts
+++ b/tests/playwright-test/hooks.spec.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { test, expect } from './playwright-test-fixtures';
+import { test, expect, stripAscii } from './playwright-test-fixtures';
 
 test('hooks should work with fixtures', async ({ runInlineTest }) => {
   const { results } = await runInlineTest({
@@ -517,3 +517,29 @@ test('afterEach should get the test status right away', async ({ runInlineTest }
     '%%timing out: timedOut',
   ]);
 });
+
+test('uncaught error in beforeEach should not be masked by another error', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const test = pwt.test.extend({
+        foo: async ({}, use) => {
+          let cb;
+          await use(new Promise((f, r) => cb = r));
+          cb(new Error('Oh my!'));
+        },
+      });
+      test.beforeEach(async ({ foo }, testInfo) => {
+        setTimeout(() => {
+          expect(1).toBe(2);
+        }, 0);
+        await foo;
+      });
+      test('passing', () => {
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(stripAscii(result.output)).toContain('Expected: 2');
+  expect(stripAscii(result.output)).toContain('Received: 1');
+});