From 058f98d3dd2eba4e61ac70407b3d7dc2712d958f Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 7 Jan 2022 11:06:47 -0800
Subject: [PATCH] fix(test runner): revert error location from top frame
 (#11250)

This does not play nicely with some internal Playwright errors, so
it needs more tweaks.
---
 .../playwright-test/src/reporters/base.ts     | 24 ++++++++++++-------
 .../playwright-test/src/reporters/json.ts     |  6 ++---
 tests/playwright-test/reporter-base.spec.ts   |  9 ++++---
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts
index 676b28e87a..a1edf86aa3 100644
--- a/packages/playwright-test/src/reporters/base.ts
+++ b/packages/playwright-test/src/reporters/base.ts
@@ -346,18 +346,17 @@ export function formatError(config: FullConfig, error: TestError, highlightCode:
   const tokens = [''];
   let location: Location | undefined;
   if (stack) {
-    const parsed = prepareErrorStack(stack);
+    const parsed = prepareErrorStack(stack, file);
     tokens.push(parsed.message);
     location = parsed.location;
     if (location) {
       try {
-        // Stack will have /private/var/folders instead of /var/folders on Mac.
-        const realFile = fs.realpathSync(location.file);
-        const source = fs.readFileSync(realFile, 'utf8');
+        const source = fs.readFileSync(location.file, 'utf8');
         const codeFrame = codeFrameColumns(source, { start: location }, { highlightCode });
-        if (!file || file !== realFile) {
+        // Convert /var/folders to /private/var/folders on Mac.
+        if (!file || fs.realpathSync(file) !== location.file) {
           tokens.push('');
-          tokens.push(colors.gray(`   at `) + `${relativeFilePath(config, realFile)}:${location.line}`);
+          tokens.push(colors.gray(`   at `) + `${relativeFilePath(config, location.file)}:${location.line}`);
         }
         tokens.push('');
         tokens.push(codeFrame);
@@ -388,11 +387,15 @@ function indent(lines: string, tab: string) {
   return lines.replace(/^(?=.+$)/gm, tab);
 }
 
-export function prepareErrorStack(stack: string): {
+export function prepareErrorStack(stack: string, file?: string): {
   message: string;
   stackLines: string[];
   location?: Location;
 } {
+  if (file) {
+    // Stack will have /private/var/folders instead of /var/folders on Mac.
+    file = fs.realpathSync(file);
+  }
   const lines = stack.split('\n');
   let firstStackLine = lines.findIndex(line => line.startsWith('    at '));
   if (firstStackLine === -1)
@@ -402,8 +405,11 @@ export function prepareErrorStack(stack: string): {
   let location: Location | undefined;
   for (const line of stackLines) {
     const parsed = stackUtils.parseLine(line);
-    if (parsed && parsed.file) {
-      location = { file: path.join(process.cwd(), parsed.file), column: parsed.column || 0, line: parsed.line || 0 };
+    if (!parsed || !parsed.file)
+      continue;
+    const resolvedFile = path.join(process.cwd(), parsed.file);
+    if (!file || resolvedFile === file) {
+      location = { file: resolvedFile, column: parsed.column || 0, line: parsed.line || 0 };
       break;
     }
   }
diff --git a/packages/playwright-test/src/reporters/json.ts b/packages/playwright-test/src/reporters/json.ts
index 9f5f525ae2..6c1c41cdec 100644
--- a/packages/playwright-test/src/reporters/json.ts
+++ b/packages/playwright-test/src/reporters/json.ts
@@ -229,12 +229,12 @@ class JSONReporter implements Reporter {
       annotations: test.annotations,
       expectedStatus: test.expectedStatus,
       projectName: test.titlePath()[1],
-      results: test.results.map(r => this._serializeTestResult(r)),
+      results: test.results.map(r => this._serializeTestResult(r, test)),
       status: test.outcome(),
     };
   }
 
-  private _serializeTestResult(result: TestResult): JSONReportTestResult {
+  private _serializeTestResult(result: TestResult, test: TestCase): JSONReportTestResult {
     const steps = result.steps.filter(s => s.category === 'test.step');
     const jsonResult: JSONReportTestResult = {
       workerIndex: result.workerIndex,
@@ -253,7 +253,7 @@ class JSONReporter implements Reporter {
       })),
     };
     if (result.error?.stack)
-      jsonResult.errorLocation = prepareErrorStack(result.error.stack).location;
+      jsonResult.errorLocation = prepareErrorStack(result.error.stack, test.location.file).location;
     return jsonResult;
   }
 
diff --git a/tests/playwright-test/reporter-base.spec.ts b/tests/playwright-test/reporter-base.spec.ts
index 42fc23b94b..ccbde11dfb 100644
--- a/tests/playwright-test/reporter-base.spec.ts
+++ b/tests/playwright-test/reporter-base.spec.ts
@@ -86,7 +86,7 @@ test('should print an error in a codeframe', async ({ runInlineTest }) => {
   expect(result.output).toContain(`>  7 |         const error = new Error('my-message');`);
 });
 
-test('should print codeframe from a helper', async ({ runInlineTest }) => {
+test('should not print codeframe from a helper', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'helper.ts': `
       export function ohMy() {
@@ -106,10 +106,9 @@ test('should print codeframe from a helper', async ({ runInlineTest }) => {
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
   expect(result.output).toContain('Error: oh my');
-  expect(result.output).toContain(`   at helper.ts:5`);
-  expect(result.output).toContain(`  4 |       export function ohMy() {`);
-  expect(result.output).toContain(`> 5 |         throw new Error('oh my');`);
-  expect(result.output).toContain(`    |               ^`);
+  expect(result.output).toContain(`   7 |       test('foobar', async ({}) => {`);
+  expect(result.output).toContain(`>  8 |         ohMy();`);
+  expect(result.output).toContain(`     |         ^`);
 });
 
 test('should print slow tests', async ({ runInlineTest }) => {