chore: ensure error location is present (#22804)

Partial fix for https://github.com/microsoft/playwright/issues/22782

packages/playwright-test/src/cli.ts

@@ -21,17 +21,18 @@ import fs from 'fs';
 import path from 'path';
 import { Runner } from './runner/runner';
 import { stopProfiling, startProfiling } from 'playwright-core/lib/utils';
-import { experimentalLoaderOption, fileIsModule } from './util';
+import { experimentalLoaderOption, fileIsModule, serializeError } from './util';
 import { showHTMLReport } from './reporters/html';
 import { createMergedReport } from './reporters/merge';
 import { ConfigLoader, kDefaultConfigFiles, resolveConfigFile } from './common/configLoader';
 import type { ConfigCLIOverrides } from './common/ipc';
-import type { FullResult } from '../reporter';
+import type { FullResult, TestError } from '../reporter';
 import type { TraceMode } from '../types/test';
 import { builtInReporters, defaultReporter, defaultTimeout } from './common/config';
 import type { FullConfigInternal } from './common/config';
 import program from 'playwright-core/lib/cli/program';
 import type { ReporterDescription } from '..';
+import { prepareErrorStack } from './reporters/base';
 
 function addTestCommand(program: Command) {
   const command = program.command('test [test-filter...]');
@@ -149,20 +150,29 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
 
 async function listTestFiles(opts: { [key: string]: any }) {
   // Redefine process.stdout.write in case config decides to pollute stdio.
-  const write = process.stdout.write.bind(process.stdout);
+  const stdoutWrite = process.stdout.write.bind(process.stdout);
   process.stdout.write = (() => {}) as any;
+  process.stderr.write = (() => {}) as any;
   const configFileOrDirectory = opts.config ? path.resolve(process.cwd(), opts.config) : process.cwd();
   const resolvedConfigFile = resolveConfigFile(configFileOrDirectory)!;
   if (restartWithExperimentalTsEsm(resolvedConfigFile))
     return;
 
+  try {
     const configLoader = new ConfigLoader();
     const config = await configLoader.loadConfigFile(resolvedConfigFile);
     const runner = new Runner(config);
     const report = await runner.listTestFiles(opts.project);
-  write(JSON.stringify(report), () => {
+    stdoutWrite(JSON.stringify(report), () => {
       process.exit(0);
     });
+  } catch (e) {
+    const error: TestError = serializeError(e);
+    error.location = prepareErrorStack(e.stack).location;
+    stdoutWrite(JSON.stringify({ error }), () => {
+      process.exit(0);
+    });
+  }
 }
 
 async function mergeReports(reportDir: string | undefined, opts: { [key: string]: any }) {

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

@@ -15,11 +15,9 @@
  */
 
 import { colors, ms as milliseconds, parseStackTraceLine } from 'playwright-core/lib/utilsBundle';
-import fs from 'fs';
 import path from 'path';
 import type { FullConfig, TestCase, Suite, TestResult, TestError, FullResult, TestStep, Location, Reporter } from '../../types/testReporter';
 import type { SuitePrivate } from '../../types/reporterPrivate';
-import { codeFrameColumns } from '../common/babelBundle';
 import { monotonicTime } from 'playwright-core/lib/utils';
 import type { FullProject } from '../../types/test';
 export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
@@ -433,29 +431,6 @@ export function formatError(config: FullConfig, error: TestError, highlightCode:
   };
 }
 
-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/internalReporter.ts

@@ -14,11 +14,14 @@
  * limitations under the License.
  */
 
+import fs from 'fs';
+import { colors } from 'playwright-core/lib/utilsBundle';
+import { codeFrameColumns } from '../common/babelBundle';
 import type { FullConfig, TestCase, TestError, TestResult, FullResult, TestStep, Reporter } from '../../types/testReporter';
 import { Suite } from '../common/test';
 import type { FullConfigInternal } from '../common/config';
-import { addSnippetToError } from './base';
 import { Multiplexer } from './multiplexer';
+import { prepareErrorStack, relativeFilePath } from './base';
 
 type StdIOChunk = {
   chunk: string | Buffer;
@@ -96,7 +99,7 @@ export class InternalReporter {
       this._deferred.push({ error });
       return;
     }
-    addSnippetToError(this._config.config, error);
+    addLocationAndSnippetToError(this._config.config, error);
     this._multiplexer.onError(error);
   }
 
@@ -111,11 +114,34 @@ export class InternalReporter {
 
   private _addSnippetToTestErrors(test: TestCase, result: TestResult) {
     for (const error of result.errors)
-      addSnippetToError(this._config.config, error, test.location.file);
+      addLocationAndSnippetToError(this._config.config, error, test.location.file);
   }
 
   private _addSnippetToStepError(test: TestCase, step: TestStep) {
     if (step.error)
-      addSnippetToError(this._config.config, step.error, test.location.file);
+      addLocationAndSnippetToError(this._config.config, step.error, test.location.file);
+  }
+}
+
+function addLocationAndSnippetToError(config: FullConfig, error: TestError, file?: string) {
+  if (error.stack && !error.location)
+    error.location = prepareErrorStack(error.stack).location;
+  const location = error.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.
   }
 }

tests/playwright-test/list-files.spec.ts

@@ -0,0 +1,101 @@
+/**
+ * Copyright Microsoft Corporation. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test, expect } from './playwright-test-fixtures';
+
+test('should list files', async ({ runListFiles }) => {
+  const result = await runListFiles({
+    'playwright.config.ts': `
+      module.exports = { projects: [{ name: 'foo' }, { name: 'bar' }] };
+    `,
+    'a.test.js': ``
+  });
+  expect(result.exitCode).toBe(0);
+
+  const data = JSON.parse(result.output);
+  expect(data).toEqual({
+    projects: [
+      {
+        name: 'foo',
+        testDir: expect.stringContaining('list-files-should-list-files-playwright-test'),
+        use: {},
+        files: [
+          expect.stringContaining('a.test.js')
+        ]
+      },
+      {
+        name: 'bar',
+        testDir: expect.stringContaining('list-files-should-list-files-playwright-test'),
+        use: {},
+        files: [
+          expect.stringContaining('a.test.js')
+        ]
+      }
+    ]
+  });
+});
+
+test('should include testIdAttribute', async ({ runListFiles }) => {
+  const result = await runListFiles({
+    'playwright.config.ts': `
+      module.exports = {
+        use: { testIdAttribute: 'myid' }
+      };
+    `,
+    'a.test.js': ``
+  });
+  expect(result.exitCode).toBe(0);
+
+  const data = JSON.parse(result.output);
+  expect(data).toEqual({
+    projects: [
+      {
+        name: '',
+        testDir: expect.stringContaining('list-files-should-include-testIdAttribute-playwright-test'),
+        use: {
+          testIdAttribute: 'myid'
+        },
+        files: [
+          expect.stringContaining('a.test.js')
+        ]
+      },
+    ]
+  });
+});
+
+test('should report error', async ({ runListFiles }) => {
+  const result = await runListFiles({
+    'playwright.config.ts': `
+      const a = 1;
+      a = 2;
+    `,
+    'a.test.js': ``
+  });
+  expect(result.exitCode).toBe(0);
+
+  const data = JSON.parse(result.output);
+  expect(data).toEqual({
+    error: {
+      location: {
+        file: expect.stringContaining('playwright.config.ts'),
+        line: 3,
+        column: 8,
+      },
+      message: 'Assignment to constant variable.',
+      stack: expect.stringContaining('TypeError: Assignment to constant variable.'),
+    }
+  });
+});

tests/playwright-test/list-mode.spec.ts

@@ -172,3 +172,27 @@ test('should ignore .only', async ({ runInlineTest }) => {
     `Total: 2 tests in 1 file`
   ].join('\n'));
 });
+
+test('should report errors with location', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `module.exports = { reporter: './reporter' };`,
+    'reporter.ts': `
+      class Reporter {
+        onError(error) {
+          console.log('%% ' + JSON.stringify(error.location));
+        }
+      }
+      module.exports = Reporter;
+    `,
+    'a.test.js': `
+      const oh = '';
+      oh = 2;
+    `
+  }, { 'list': true });
+  expect(result.exitCode).toBe(1);
+  expect(JSON.parse(result.outputLines[0])).toEqual({
+    file: expect.stringContaining('a.test.js'),
+    line: 3,
+    column: 9,
+  });
+});

tests/playwright-test/playwright-test-fixtures.ts

@@ -162,6 +162,17 @@ async function runPlaywrightTest(childProcess: CommonFixtures['childProcess'], b
   };
 }
 
+async function runPlaywrightListFiles(childProcess: CommonFixtures['childProcess'], baseDir: string, env: NodeJS.ProcessEnv): Promise<{ output: string, exitCode: number }> {
+  const reportFile = path.join(baseDir, 'report.json');
+  // eslint-disable-next-line prefer-const
+  let { exitCode, output } = await runPlaywrightCommand(childProcess, baseDir, ['list-files'], {
+    PW_TEST_REPORTER: path.join(__dirname, '../../packages/playwright-test/lib/reporters/json.js'),
+    PLAYWRIGHT_JSON_OUTPUT_NAME: reportFile,
+    ...env,
+  });
+  return { exitCode, output };
+}
+
 function watchPlaywrightTest(childProcess: CommonFixtures['childProcess'], baseDir: string, env: NodeJS.ProcessEnv, options: RunOptions): TestChildProcess {
   const args = ['test', '--workers=2'];
   if (options.additionalArgs)
@@ -232,6 +243,7 @@ type Fixtures = {
   writeFiles: (files: Files) => Promise<string>;
   deleteFile: (file: string) => Promise<void>;
   runInlineTest: (files: Files, params?: Params, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<RunResult>;
+  runListFiles: (files: Files) => Promise<{ output: string, exitCode: number }>;
   runWatchTest: (files: Files, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise<TestChildProcess>;
   runTSC: (files: Files) => Promise<TSCResult>;
   nodeVersion: { major: number, minor: number, patch: number };
@@ -261,6 +273,15 @@ export const test = base
         await removeFolderAsync(cacheDir);
       },
 
+      runListFiles: async ({ childProcess }, use, testInfo: TestInfo) => {
+        const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-'));
+        await use(async (files: Files) => {
+          const baseDir = await writeFiles(testInfo, files, true);
+          return await runPlaywrightListFiles(childProcess, baseDir, { PWTEST_CACHE_DIR: cacheDir });
+        });
+        await removeFolderAsync(cacheDir);
+      },
+
       runWatchTest: async ({ childProcess }, use, testInfo: TestInfo) => {
         const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-'));
         let testProcess: TestChildProcess | undefined;

tests/playwright-test/reporter.spec.ts

@@ -62,6 +62,8 @@ class Reporter {
   onStepEnd(test, result, step) {
     if (step.error?.stack)
       step.error.stack = '<stack>';
+    if (step.error?.location)
+      step.error.location = '<location>';
     if (step.error?.snippet)
       step.error.snippet = '<snippet>';
     if (step.error?.message.includes('getaddrinfo'))
@@ -266,7 +268,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>\",\"snippet\":\"<snippet>\"}}`,
+    `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>\",\"location\":\"<location>\",\"snippet\":\"<snippet>\"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `end {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"Before Hooks\",\"category\":\"hook\"}`,
@@ -355,9 +357,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>","snippet":"<snippet>"}}`,
+    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>","location":"<location>","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>","snippet":"<snippet>"}}`,
+    `end {"title":"apiRequestContext.get(http://localhost2)","category":"pw:api","error":{"message":"<message>","stack":"<stack>","location":"<location>","snippet":"<snippet>"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"apiRequestContext.dispose\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"apiRequestContext.dispose\",\"category\":\"pw:api\"}`,
@@ -420,7 +422,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>\",\"snippet\":\"<snippet>\"}}`,
+    `end {\"title\":\"page.click(input)\",\"category\":\"pw:api\",\"error\":{\"message\":\"page.click: Timeout 1ms exceeded.\\n=========================== logs ===========================\\nwaiting for locator('input')\\n============================================================\",\"stack\":\"<stack>\",\"location\":\"<location>\",\"snippet\":\"<snippet>\"}}`,
     `begin {\"title\":\"After Hooks\",\"category\":\"hook\"}`,
     `begin {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,
     `end {\"title\":\"browserContext.close\",\"category\":\"pw:api\"}`,