From 608e336dba8f5ba4581069ed208103960712b783 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Thu, 6 Jul 2023 10:48:12 -0700
Subject: [PATCH] fix(error): create a step for raw runtime error (#24057)

Fix https://github.com/microsoft/playwright/issues/23850
---
 .../playwright-test/src/worker/workerMain.ts  | 16 +++++++--
 .../playwright-test/playwright.trace.spec.ts  | 36 +++++++++++++++++++
 tests/playwright-test/reporter-html.spec.ts   |  2 +-
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/packages/playwright-test/src/worker/workerMain.ts b/packages/playwright-test/src/worker/workerMain.ts
index 5ad260ae93..7dc70cc8bb 100644
--- a/packages/playwright-test/src/worker/workerMain.ts
+++ b/packages/playwright-test/src/worker/workerMain.ts
@@ -23,7 +23,7 @@ import { ConfigLoader } from '../common/configLoader';
 import type { Suite, TestCase } from '../common/test';
 import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
 import { FixtureRunner } from './fixtureRunner';
-import { ManualPromise } from 'playwright-core/lib/utils';
+import { ManualPromise, captureLibraryStackTrace } from 'playwright-core/lib/utils';
 import { TestInfoImpl } from './testInfo';
 import { TimeoutManager, type TimeSlot } from './timeoutManager';
 import { ProcessRunner } from '../common/process';
@@ -369,13 +369,25 @@ export class WorkerMain extends ProcessRunner {
         return;
       }
 
-      await testInfo._runAndFailOnError(async () => {
+      const error = await testInfo._runAndFailOnError(async () => {
         // Now run the test itself.
         debugTest(`test function started`);
         const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]").
         await fn(testFunctionParams, testInfo);
         debugTest(`test function finished`);
       }, 'allowSkips');
+
+      // If there are no steps with errors in the test, but the test has an error - append artificial trace entry.
+      if (error && !testInfo._steps.some(s => !!s.error)) {
+        const frames = error.stack ? captureLibraryStackTrace(error.stack.split('\n')).frames : [];
+        const step = testInfo._addStep({
+          wallTime: Date.now(),
+          title: error.message || 'error',
+          category: 'hook',
+          location: frames[0],
+        });
+        step.complete({ error });
+      }
     });
 
     if (didFailBeforeAllForSuite) {
diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts
index 8931ea3224..a6a4a5d363 100644
--- a/tests/playwright-test/playwright.trace.spec.ts
+++ b/tests/playwright-test/playwright.trace.spec.ts
@@ -331,6 +331,7 @@ test('should not override trace file in afterAll', async ({ runInlineTest, serve
     '  fixture: page',
     '    browserContext.newPage',
     'page.goto',
+    'error',
     'After Hooks',
     '  fixture: page',
     '  fixture: context',
@@ -650,3 +651,38 @@ test('should expand expect.toPass', async ({ runInlineTest }, testInfo) => {
     '  fixture: context',
   ]);
 });
+
+test('should show non-expect error in trace', async ({ runInlineTest }, testInfo) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { use: { trace: { mode: 'on' } } };
+    `,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('fail', async ({ page }) => {
+        expect(1).toBe(1);
+        undefinedVariable1 = 'this throws an exception';
+        expect(1).toBe(2);
+      });
+    `,
+  }, { workers: 1 });
+
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  const trace = await parseTrace(testInfo.outputPath('test-results', 'a-fail', 'trace.zip'));
+  expect(trace.actionTree).toEqual([
+    'Before Hooks',
+    '  fixture: browser',
+    '    browserType.launch',
+    '  fixture: context',
+    '    browser.newContext',
+    '    tracing.start',
+    '  fixture: page',
+    '    browserContext.newPage',
+    'expect.toBe',
+    'undefinedVariable1 is not defined',
+    'After Hooks',
+    '  fixture: page',
+    '  fixture: context',
+  ]);
+});
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index f5d73671d2..33f9ddee00 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -780,7 +780,7 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       await showReport();
 
       await page.locator('text=sample').first().click();
-      await expect(page.locator('text=ouch')).toBeVisible();
+      await expect(page.locator('text=ouch')).toHaveCount(2);
       await page.locator('text=All').first().click();
 
       await page.locator('text=sample').nth(1).click();