chore: add snippet to the error message (#21991)

docs/src/test-reporter-api/class-testerror.md

@@ -27,3 +27,9 @@ The value that was thrown. Set when anything except the [Error] (or its subclass
 - type: ?<[Location]>
 
 Error location in the source code.
+
+## property: TestError.snippet
+* since: v1.33
+- type: ?<[string]>
+
+Source code snippet with highlighted error.

packages/playwright-test/src/reporters/base.ts

@@ -337,7 +337,7 @@ export function formatResultFailure(config: FullConfig, test: TestCase, result:
   }
 
   for (const error of result.errors) {
-    const formattedError = formatError(config, error, highlightCode, test.location.file);
+    const formattedError = formatError(config, error, highlightCode);
     errorDetails.push({
       message: indent(formattedError.message, initialIndent),
       location: formattedError.location,
@@ -377,7 +377,7 @@ function formatTestHeader(config: FullConfig, test: TestCase, indent: string, in
   return separator(header);
 }
 
-export function formatError(config: FullConfig, error: TestError, highlightCode: boolean, file?: string): ErrorDetails {
+export function formatError(config: FullConfig, error: TestError, highlightCode: boolean): ErrorDetails {
   const message = error.message || error.value || '';
   const stack = error.stack;
   if (!stack && !error.location)
@@ -390,36 +390,52 @@ export function formatError(config: FullConfig, error: TestError, highlightCode:
   const parsedStack = stack ? prepareErrorStack(stack) : undefined;
   tokens.push(parsedStack?.message || message);
 
-  let location = error.location;
+  if (error.snippet) {
-  if (parsedStack && !location)
+    let snippet = error.snippet;
-    location = parsedStack.location;
+    if (!highlightCode)
-
+      snippet = stripAnsiEscapes(snippet);
-  if (location) {
+    tokens.push('');
-    try {
+    tokens.push(snippet);
-      const source = fs.readFileSync(location.file, 'utf8');
-      const codeFrame = codeFrameColumns(source, { start: location }, { highlightCode });
-      // 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, location.file)}:${location.line}`);
-      }
-      tokens.push('');
-      tokens.push(codeFrame);
-    } catch (e) {
-      // Failed to read the source file - that's ok.
-    }
   }
+
   if (parsedStack) {
     tokens.push('');
     tokens.push(colors.dim(parsedStack.stackLines.join('\n')));
   }
 
+  let location = error.location;
+  if (parsedStack && !location)
+    location = parsedStack.location;
+
   return {
     location,
     message: tokens.join('\n'),
   };
 }
 
+export function addSnippetToError(config: FullConfig, error: TestError, file?: string) {
+  let location = error.location;
+  if (error.stack && !location)
+    location = prepareErrorStack(error.stack).location;
+  if (!location)
+    return;
+
+  try {
+    const tokens = [];
+    const source = fs.readFileSync(location.file, 'utf8');
+    const codeFrame = codeFrameColumns(source, { start: location }, { highlightCode: true });
+    // Convert /var/folders to /private/var/folders on Mac.
+    if (!file || fs.realpathSync(file) !== location.file) {
+      tokens.push(colors.gray(`   at `) + `${relativeFilePath(config, location.file)}:${location.line}`);
+      tokens.push('');
+    }
+    tokens.push(codeFrame);
+    error.snippet = tokens.join('\n');
+  } catch (e) {
+    // Failed to read the source file - that's ok.
+  }
+}
+
 export function separator(text: string = ''): string {
   if (text)
     text += ' ';

packages/playwright-test/src/reporters/multiplexer.ts

@@ -16,6 +16,7 @@
 
 import type { FullConfig, TestCase, TestError, TestResult, FullResult, TestStep, Reporter } from '../../types/testReporter';
 import { Suite } from '../common/test';
+import { addSnippetToError } from './base';
 
 type StdIOChunk = {
   chunk: string | Buffer;
@@ -81,6 +82,7 @@ export class Multiplexer implements Reporter {
   }
 
   onTestEnd(test: TestCase, result: TestResult) {
+    this._addSnippetToTestErrors(test, result);
     for (const reporter of this._reporters)
       wrap(() => reporter.onTestEnd?.(test, result));
   }
@@ -105,6 +107,7 @@ export class Multiplexer implements Reporter {
       this._deferred.push({ error });
       return;
     }
+    addSnippetToError(this._config, error);
     for (const reporter of this._reporters)
       wrap(() => reporter.onError?.(error));
   }
@@ -115,9 +118,21 @@ export class Multiplexer implements Reporter {
   }
 
   onStepEnd(test: TestCase, result: TestResult, step: TestStep) {
+    this._addSnippetToStepError(test, step);
     for (const reporter of this._reporters)
       wrap(() => (reporter as any).onStepEnd?.(test, result, step));
   }
+
+  private _addSnippetToTestErrors(test: TestCase, result: TestResult) {
+    for (const error of result.errors)
+      addSnippetToError(this._config, error, test.location.file);
+  }
+
+  private _addSnippetToStepError(test: TestCase, step: TestStep) {
+    if (step.error)
+      addSnippetToError(this._config, step.error, test.location.file);
+  }
+
 }
 
 function wrap(callback: () => void) {

packages/playwright-test/types/testReporter.d.ts

@@ -584,6 +584,11 @@ export interface TestError {
    */
   message?: string;
 
+  /**
+   * Source code snippet with highlighted error.
+   */
+  snippet?: string;
+
   /**
    * Error stack. Set when [Error] (or its subclass) has been thrown.
    */

tests/playwright-test/reporter.spec.ts

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import { test, expect } from './playwright-test-fixtures';
+import { test, expect, stripAnsi } from './playwright-test-fixtures';
+import fs from 'fs';
 
 const smallReporterJS = `
 class Reporter {
@@ -58,6 +59,8 @@ class Reporter {
   onStepEnd(test, result, step) {
     if (step.error?.stack)
       step.error.stack = '<stack>';
+    if (step.error?.snippet)
+      step.error.snippet = '<snippet>';
     if (step.error?.message.includes('getaddrinfo'))
       step.error.message = '<message>';
     console.log('%%%% end', JSON.stringify(this.distillStep(step)));
@@ -257,7 +260,7 @@ test('should report expect steps', async ({ runInlineTest }) => {
     `begin {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`,
     `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`,
     `begin {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\"}`,
-    `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\",\"error\":{\"message\":\"\\u001b[2mexpect(\\u001b[22m\\u001b[31mreceived\\u001b[39m\\u001b[2m).\\u001b[22mtoBeTruthy\\u001b[2m()\\u001b[22m\\n\\nReceived: \\u001b[31mfalse\\u001b[39m\",\"stack\":\"<stack>\"}}`,
+    `end {\"title\":\"expect.toBeTruthy\",\"category\":\"expect\",\"error\":{\"message\":\"\\u001b[2mexpect(\\u001b[22m\\u001b[31mreceived\\u001b[39m\\u001b[2m).\\u001b[22mtoBeTruthy\\u001b[2m()\\u001b[22m\\n\\nReceived: \\u001b[31mfalse\\u001b[39m\",\"stack\":\"<stack>\",\"snippet\":\"<snippet>\"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `end {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`,
@@ -336,9 +339,9 @@ test('should report api steps', async ({ runInlineTest }) => {
     `begin {\"title\":\"locator.getByRole('button').click\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"locator.getByRole('button').click\",\"category\":\"pw:api\"}`,
     `begin {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api"}`,
-    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>"}}`,
+    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>","snippet":"<snippet>"}}`,
     `begin {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api"}`,
-    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>"}}`,
+    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>","snippet":"<snippet>"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"apiRequestContext.dispose\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"apiRequestContext.dispose\",\"category\":\"pw:api\"}`,
@@ -397,7 +400,7 @@ test('should report api step failure', async ({ runInlineTest }) => {
     `begin {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"page.setContent\",\"category\":\"pw:api\"}`,
     `begin {\"title\":\"page.click(input)\",\"category\":\"pw:api\"}`,
-    `end {\"title\":\"page.click(input)\",\"category\":\"pw:api\",\"error\":{\"message\":\"page.click: Timeout 1ms exceeded.\\n=========================== logs ===========================\\nwaiting for locator('input')\\n============================================================\",\"stack\":\"<stack>\"}}`,
+    `end {\"title\":\"page.click(input)\",\"category\":\"pw:api\",\"error\":{\"message\":\"page.click: Timeout 1ms exceeded.\\n=========================== logs ===========================\\nwaiting for locator('input')\\n============================================================\",\"stack\":\"<stack>\",\"snippet\":\"<snippet>\"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,
@@ -635,3 +638,89 @@ test('parallelIndex is presented in onTestEnd', async ({ runInlineTest }) => {
 
   expect(result.output).toContain('parallelIndex: 0');
 });
+
+test('test and step error should have code snippet', async ({ runInlineTest }) => {
+  const testErrorFile = test.info().outputPath('testError.txt');
+  const stepErrorFile = test.info().outputPath('stepError.txt');
+  const result = await runInlineTest({
+    'reporter.ts': `
+    import fs from 'fs';
+    class Reporter {
+      onStepEnd(test, result, step) {
+        console.log('\\n%%onStepEnd: ' + step.error?.snippet?.length);
+        fs.writeFileSync('${stepErrorFile.replace(/\\/g, '\\\\')}', step.error?.snippet);
+      }
+      onTestEnd(test, result) {
+        console.log('\\n%%onTestEnd: ' + result.error?.snippet?.length);
+        fs.writeFileSync('${testErrorFile.replace(/\\/g, '\\\\')}', result.error?.snippet);
+      }
+      onError(error) {
+        console.log('\\n%%onError: ' + error.snippet?.length);
+      }
+    }
+    module.exports = Reporter;`,
+    'playwright.config.ts': `
+      module.exports = {
+        reporter: './reporter',
+      };
+    `,
+    'a.spec.js': `
+      const { test, expect } = require('@playwright/test');
+      test('test', async () => {
+        await test.step('step', async () => {
+          expect(1).toBe(2);
+        });
+      });
+    `,
+  }, { 'reporter': '', 'workers': 1 });
+
+  expect(result.output).toContain('onTestEnd: 522');
+  expect(result.output).toContain('onStepEnd: 522');
+  expect(stripAnsi(fs.readFileSync(testErrorFile, 'utf8'))).toBe(`  3 |       test('test', async () => {
+  4 |         await test.step('step', async () => {
+> 5 |           expect(1).toBe(2);
+    |                     ^
+  6 |         });
+  7 |       });
+  8 |     `);
+  expect(stripAnsi(fs.readFileSync(stepErrorFile, 'utf8'))).toBe(`  3 |       test('test', async () => {
+  4 |         await test.step('step', async () => {
+> 5 |           expect(1).toBe(2);
+    |                     ^
+  6 |         });
+  7 |       });
+  8 |     `);
+});
+
+test('onError should have code snippet', async ({ runInlineTest }) => {
+  const errorFile = test.info().outputPath('error.txt');
+  const result = await runInlineTest({
+    'reporter.ts': `
+    import fs from 'fs';
+    class Reporter {
+      onError(error) {
+        console.log('\\n%%onError: ' + error.snippet?.length);
+        fs.writeFileSync('${errorFile.replace(/\\/g, '\\\\')}', error.snippet);
+      }
+    }
+    module.exports = Reporter;`,
+    'playwright.config.ts': `
+      module.exports = {
+        reporter: './reporter',
+      };
+    `,
+    'a.spec.js': `
+      const { test, expect } = require('@playwright/test');
+      throw new Error('test');
+    `,
+  }, { 'reporter': '', 'workers': 1 });
+
+  expect(result.output).toContain('onError: 396');
+  expect(stripAnsi(fs.readFileSync(errorFile, 'utf8'))).toBe(`   at a.spec.js:3
+
+  1 |
+  2 |       const { test, expect } = require('@playwright/test');
+> 3 |       throw new Error('test');
+    |             ^
+  4 |     `);
+});