chore: surface syntax error in ui mode (#22982)

Fixes https://github.com/microsoft/playwright/issues/22863

packages/playwright-test/src/common/testLoader.ts

@@ -15,11 +15,12 @@
  */
 
 import path from 'path';
+import util from 'util';
 import type { TestError } from '../../reporter';
 import { isWorkerProcess, setCurrentlyLoadingFileSuite } from './globals';
 import { Suite } from './test';
 import { requireOrImport } from './transform';
-import { serializeError } from '../util';
+import { filterStackTrace } from '../util';
 import { startCollectingFileDeps, stopCollectingFileDeps } from './compilationCache';
 
 export const defaultTimeout = 30000;
@@ -44,7 +45,7 @@ export async function loadTestFile(file: string, rootDir: string, testErrors?: T
   } catch (e) {
     if (!testErrors)
       throw e;
-    testErrors.push(serializeError(e));
+    testErrors.push(serializeLoadError(file, e));
   } finally {
     stopCollectingFileDeps(file);
     setCurrentlyLoadingFileSuite(undefined);
@@ -72,3 +73,18 @@ export async function loadTestFile(file: string, rootDir: string, testErrors?: T
 
   return suite;
 }
+
+function serializeLoadError(file: string, error: Error | any): TestError {
+  if (error instanceof Error) {
+    const result: TestError = filterStackTrace(error);
+    // Babel parse errors have location.
+    const loc = (error as any).loc;
+    result.location = loc ? {
+      file,
+      line: loc.line || 0,
+      column: loc.column || 0,
+    } : undefined;
+    return result;
+  }
+  return { value: util.inspect(error) };
+}

packages/playwright-test/src/common/transform.ts

@@ -134,7 +134,7 @@ export function resolveHook(filename: string, specifier: string): string | undef
   }
 }
 
-export function transformHook(code: string, filename: string, moduleUrl?: string): string {
+export function transformHook(preloadedCode: string, filename: string, moduleUrl?: string): string {
   const isTypeScript = filename.endsWith('.ts') || filename.endsWith('.tsx');
   const hasPreprocessor =
       process.env.PW_TEST_SOURCE_TRANSFORM &&
@@ -142,7 +142,7 @@ export function transformHook(code: string, filename: string, moduleUrl?: string
       process.env.PW_TEST_SOURCE_TRANSFORM_SCOPE.split(pathSeparator).some(f => filename.startsWith(f));
   const pluginsPrologue = babelPlugins;
   const pluginsEpilogue = hasPreprocessor ? [[process.env.PW_TEST_SOURCE_TRANSFORM!]] as BabelPlugin[] : [];
-  const hash = calculateHash(code, filename, !!moduleUrl, pluginsPrologue, pluginsEpilogue);
+  const hash = calculateHash(preloadedCode, filename, !!moduleUrl, pluginsPrologue, pluginsEpilogue);
   const { cachedCode, addToCache } = getFromCompilationCache(filename, hash, moduleUrl);
   if (cachedCode)
     return cachedCode;
@@ -151,17 +151,11 @@ export function transformHook(code: string, filename: string, moduleUrl?: string
   // Silence the annoying warning.
   process.env.BROWSERSLIST_IGNORE_OLD_DATA = 'true';
 
-  try {
-    const { babelTransform }: { babelTransform: BabelTransformFunction } = require('./babelBundle');
-    const { code, map } = babelTransform(filename, isTypeScript, !!moduleUrl, pluginsPrologue, pluginsEpilogue);
-    if (code)
-      addToCache!(code, map);
-    return code || '';
-  } catch (e) {
-    // Re-throw error with a playwright-test stack
-    // that could be filtered out.
-    throw new Error(e.message);
-  }
+  const { babelTransform }: { babelTransform: BabelTransformFunction } = require('./babelBundle');
+  const { code, map } = babelTransform(filename, isTypeScript, !!moduleUrl, pluginsPrologue, pluginsEpilogue);
+  if (code)
+    addToCache!(code, map);
+  return code || '';
 }
 
 function calculateHash(content: string, filePath: string, isModule: boolean, pluginsPrologue: BabelPlugin[], pluginsEpilogue: BabelPlugin[]): string {

packages/playwright-test/src/matchers/expect.ts

@@ -280,10 +280,12 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
           jestError.stack = jestError.name + ': ' + newMessage + '\n' + stringifyStackFrames(stackFrames).join('\n');
         }
 
-        const serializerError = serializeError(jestError);
-        step.complete({ error: serializerError });
+        const serializedError = serializeError(jestError);
+        // Serialized error has filtered stack trace.
+        jestError.stack = serializedError.stack;
+        step.complete({ error: serializedError });
         if (this._info.isSoft)
-          testInfo._failWithError(serializerError, false /* isHardError */);
+          testInfo._failWithError(serializedError, false /* isHardError */);
         else
           throw jestError;
       };

packages/playwright-test/src/runner/runner.ts

@@ -70,7 +70,7 @@ export class Runner {
     webServerPluginsForConfig(config).forEach(p => config.plugins.push({ factory: p }));
 
     const reporter = new InternalReporter(await createReporters(config, listOnly ? 'list' : 'run'));
-    const taskRunner = listOnly ? createTaskRunnerForList(config, reporter, 'in-process')
+    const taskRunner = listOnly ? createTaskRunnerForList(config, reporter, 'in-process', { failOnLoadErrors: true })
       : createTaskRunner(config, reporter);
 
     const testRun = new TestRun(config, reporter);

packages/playwright-test/src/runner/tasks.ts

@@ -102,9 +102,9 @@ function addRunTasks(taskRunner: TaskRunner<TestRun>, config: FullConfigInternal
   return taskRunner;
 }
 
-export function createTaskRunnerForList(config: FullConfigInternal, reporter: InternalReporter, mode: 'in-process' | 'out-of-process'): TaskRunner<TestRun> {
+export function createTaskRunnerForList(config: FullConfigInternal, reporter: InternalReporter, mode: 'in-process' | 'out-of-process', options: { failOnLoadErrors: boolean }): TaskRunner<TestRun> {
   const taskRunner = new TaskRunner<TestRun>(reporter, config.config.globalTimeout);
-  taskRunner.addTask('load tests', createLoadTask(mode, { filterOnly: false, failOnLoadErrors: false }));
+  taskRunner.addTask('load tests', createLoadTask(mode, { ...options, filterOnly: false }));
   taskRunner.addTask('report begin', async ({ reporter, rootSuite }) => {
     reporter.onBegin(config.config, rootSuite!);
     return () => reporter.onEnd();
@@ -172,7 +172,7 @@ function createLoadTask(mode: 'out-of-process' | 'in-process', options: { filter
     await loadFileSuites(testRun, mode, options.failOnLoadErrors ? errors : softErrors);
     testRun.rootSuite = await createRootSuite(testRun, options.failOnLoadErrors ? errors : softErrors, !!options.filterOnly);
     // Fail when no tests.
-    if (!testRun.rootSuite.allTests().length && !testRun.config.cliPassWithNoTests && !testRun.config.config.shard)
+    if (options.failOnLoadErrors && !testRun.rootSuite.allTests().length && !testRun.config.cliPassWithNoTests && !testRun.config.config.shard)
       throw new Error(`No tests found`);
   };
 }

packages/playwright-test/src/runner/uiMode.ts

@@ -149,7 +149,7 @@ class UIMode {
     const reporter = new InternalReporter([listReporter]);
     this._config.cliListOnly = true;
     this._config.testIdMatcher = undefined;
-    const taskRunner = createTaskRunnerForList(this._config, reporter, 'out-of-process');
+    const taskRunner = createTaskRunnerForList(this._config, reporter, 'out-of-process', { failOnLoadErrors: false });
     const testRun = new TestRun(this._config, reporter);
     clearCompilationCache();
     reporter.onConfigure(this._config);

packages/playwright-test/src/util.ts

@@ -29,13 +29,15 @@ import type { RawStack } from 'playwright-core/lib/utils';
 const PLAYWRIGHT_TEST_PATH = path.join(__dirname, '..');
 const PLAYWRIGHT_CORE_PATH = path.dirname(require.resolve('playwright-core/package.json'));
 
-export function filterStackTrace(e: Error) {
+export function filterStackTrace(e: Error): { message: string, stack: string } {
   if (process.env.PWDEBUGIMPL)
-    return;
+    return { message: e.message, stack: e.stack || '' };
+
   const stackLines = stringifyStackFrames(filteredStackTrace(e.stack?.split('\n') || []));
-  const message = e.message;
-  e.stack = `${e.name}: ${e.message}\n${stackLines.join('\n')}`;
-  e.message = message;
+  return {
+    message: e.message,
+    stack: `${e.name}: ${e.message}\n${stackLines.join('\n')}`
+  };
 }
 
 export function filteredStackTrace(rawStack: RawStack): StackFrame[] {
@@ -65,13 +67,8 @@ export function stringifyStackFrames(frames: StackFrame[]): string[] {
 }
 
 export function serializeError(error: Error | any): TestInfoError {
-  if (error instanceof Error) {
-    filterStackTrace(error);
-    return {
-      message: error.message,
-      stack: error.stack
-    };
-  }
+  if (error instanceof Error)
+    return filterStackTrace(error);
   return {
     value: util.inspect(error)
   };

tests/playwright-test/ui-mode-test-source.spec.ts

@@ -65,7 +65,7 @@ test('should show selected test in sources', async ({ runUITest }) => {
   ).toHaveText(`3    test('third', () => {});`);
 });
 
-test('should show syntax errors in file', async ({ runUITest }) => {
+test('should show top-level errors in file', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
       import { test } from '@playwright/test';
@@ -100,3 +100,31 @@ test('should show syntax errors in file', async ({ runUITest }) => {
     'Assignment to constant variable.'
   ]);
 });
+
+test('should show syntax errors in file', async ({ runUITest }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test } from '@playwright/test'&
+      test('first', () => {});
+      test('second', () => {});
+    `,
+  });
+  await expect.poll(dumpTestTree(page)).toBe(`
+      ◯ a.test.ts
+  `);
+
+  await page.getByTestId('test-tree').getByText('a.test.ts').click();
+  await expect(
+      page.getByTestId('source-code').locator('.source-tab-file-name')
+  ).toHaveText('a.test.ts');
+  await expect(
+      page.locator('.CodeMirror .source-line-running'),
+  ).toHaveText(`2      import { test } from '@playwright/test'&`);
+
+  await expect(
+      page.locator('.CodeMirror-linewidget')
+  ).toHaveText([
+    '                                              ',
+    /Missing semicolon./
+  ]);
+});