fix: report config file errors to the user (#401)

src/extension.ts

@@ -86,7 +86,7 @@ export class Extension {
     promise: Promise<void>,
     finishedCallback: () => void
   } | undefined;
-  private _diagnostics: vscodeTypes.DiagnosticCollection;
+  private _diagnostics: Record<'configErrors' | 'testErrors', vscodeTypes.DiagnosticCollection>;
   private _treeItemObserver: TreeItemObserver;
 
   constructor(vscode: vscodeTypes.VSCode) {
@@ -120,7 +120,10 @@ export class Extension {
     this._debugHighlight = new DebugHighlight(vscode, this._reusedBrowser);
     this._debugHighlight.onErrorInDebugger(e => this._errorInDebugger(e.error, e.location));
     this._workspaceObserver = new WorkspaceObserver(this._vscode, changes => this._workspaceChanged(changes));
-    this._diagnostics = this._vscode.languages.createDiagnosticCollection('pw.diagnostics');
+    this._diagnostics = {
+      testErrors: this._vscode.languages.createDiagnosticCollection('pw.testErrors.diagnostic'),
+      configErrors: this._vscode.languages.createDiagnosticCollection('pw.configErrors.diagnostic'),
+    };
     this._treeItemObserver = new TreeItemObserver(this._vscode);
   }
 
@@ -199,7 +202,7 @@ export class Extension {
       this._testController,
       this._workspaceObserver,
       this._reusedBrowser,
-      this._diagnostics,
+      ...Object.values(this._diagnostics),
       this._treeItemObserver,
     ];
     await this._rebuildModel(true);
@@ -234,6 +237,7 @@ export class Extension {
     // Reuse already created run profiles in order to retain their 'selected' status.
     const usedProfiles = new Set<vscodeTypes.TestRunProfile>();
 
+    const configErrors = new MultiMap<string, TestError>();
     for (const configFileUri of configFiles) {
       const configFilePath = configFileUri.fsPath;
       // TODO: parse .gitignore
@@ -270,7 +274,11 @@ export class Extension {
       const model = new TestModel(this._vscode, this._playwrightTest, workspaceFolderPath, configFileUri.fsPath, playwrightInfo, this._envProvider.bind(this));
       this._models.push(model);
       this._testTree.addModel(model);
-      await model.listFiles();
+      const configError = await model.listFiles();
+      if (configError) {
+        configErrors.set(configError.location?.file ?? configFilePath, configError);
+        continue;
+      }
 
       for (const project of model.projects.values()) {
         await this._createRunProfile(project, usedProfiles);
@@ -291,9 +299,37 @@ export class Extension {
     this._testTree.finishedLoading();
     await this._updateVisibleEditorItems();
 
+    this._reportConfigErrorsToUser(configErrors);
+
     return configFiles;
   }
 
+  private _reportConfigErrorsToUser(configErrors: MultiMap<string, TestError>) {
+    this._updateDiagnostics('configErrors', configErrors);
+    if (!configErrors.size)
+      return;
+    (async () => {
+      const showDetails = this._vscode.l10n.t('Show details');
+      const choice = await this._vscode.window.showErrorMessage(this._vscode.l10n.t('There are errors in Playwright configuration files.'), showDetails);
+      if (choice === showDetails) {
+        // Show the document to the user.
+        const document = await this._vscode.workspace.openTextDocument([...configErrors.keys()][0]);
+        await this._vscode.window.showTextDocument(document);
+        const error = [...configErrors.values()][0];
+        // Reveal the error line.
+        if (error?.location) {
+          const range = new this._vscode.Range(
+              new this._vscode.Position(Math.max(error.location.line - 4, 0), 0),
+              new this._vscode.Position(error.location.line - 1, error.location.column - 1),
+          );
+          this._vscode.window.activeTextEditor?.revealRange(range);
+        }
+        // Focus problems view.
+        await this._vscode.commands.executeCommand('workbench.action.problems.focus');
+      }
+    })();
+  }
+
   private _envProvider(): NodeJS.ProcessEnv {
     const env = this._vscode.workspace.getConfiguration('playwright').get('env', {});
     return Object.fromEntries(Object.entries(env).map(entry => {
@@ -642,7 +678,7 @@ located next to Run / Debug Tests toolbar buttons.`);
             errorsByFile.set(error.location?.file, error);
           }
         }
-        this._updateDiagnostics(errorsByFile);
+        this._updateDiagnostics('testErrors', errorsByFile);
         finishedCallback();
       }, 0);
 
@@ -660,8 +696,9 @@ located next to Run / Debug Tests toolbar buttons.`);
     return this._filesPendingListTests.promise;
   }
 
-  private _updateDiagnostics(errorsByFile: MultiMap<string, TestError>) {
-    this._diagnostics.clear();
+  private _updateDiagnostics(diagnosticsType: 'configErrors' | 'testErrors' , errorsByFile: MultiMap<string, TestError>) {
+    const diagnosticsCollection = this._diagnostics[diagnosticsType]!;
+    diagnosticsCollection.clear();
     for (const [file, errors] of errorsByFile) {
       const diagnostics: vscodeTypes.Diagnostic[] = [];
       for (const error of errors) {
@@ -672,7 +709,7 @@ located next to Run / Debug Tests toolbar buttons.`);
           message: error.message!,
         });
       }
-      this._diagnostics.set(this._vscode.Uri.file(file), diagnostics);
+      diagnosticsCollection.set(this._vscode.Uri.file(file), diagnostics);
     }
   }
 

src/listTests.d.ts

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import type { TestError } from './reporter';
+
 // This matches the structs in packages/playwright-test/src/runner/runner.ts.
 
 export type ProjectConfigWithFiles = {
@@ -25,4 +27,5 @@ export type ProjectConfigWithFiles = {
 
 export type ConfigListFilesReport = {
   projects: ProjectConfigWithFiles[];
+  error?: TestError
 };

src/playwrightTest.ts

@@ -88,27 +88,26 @@ export class PlaywrightTest {
     return { cli, version: v };
   }
 
-  async listFiles(config: TestConfig): Promise<ConfigListFilesReport | null> {
+  async listFiles(config: TestConfig): Promise<ConfigListFilesReport> {
     const configFolder = path.dirname(config.configFile);
     const configFile = path.basename(config.configFile);
     const allArgs = [config.cli, 'list-files', '-c', configFile];
     {
       // For tests.
       this._log(`${escapeRegex(path.relative(config.workspaceFolder, configFolder))}> playwright list-files -c ${configFile}`);
     }
-    const output = await this._runNode(allArgs, configFolder);
     try {
+      const output = await this._runNode(allArgs, configFolder);
       const result = JSON.parse(output) as ConfigListFilesReport;
-      if ((result as any).error) {
-        // TODO: show errors as diagnostics.
-        if (!this._isUnderTest)
-          console.error(`Error while listing files: ${allArgs.join(' ')}`, (result as any).error);
-        return null;
-      }
       return result;
-    } catch (e) {
-      console.error(e);
-      return null;
+    } catch (error: any) {
+      return {
+        error: {
+          location: { file: configFile, line: 0, column: 0 },
+          message: error.message,
+        },
+        projects: [],
+      };
     }
   }
 

src/testModel.ts

@@ -85,10 +85,10 @@ export class TestModel {
     this.onUpdated = this._didUpdate.event;
   }
 
-  async listFiles() {
+  async listFiles(): Promise<TestError | undefined> {
     const report = await this._playwrightTest.listFiles(this.config);
-    if (!report)
-      return;
+    if (report.error)
+      return report.error;
 
     // Resolve files to sources when using source maps.
     for (const project of report.projects) {

tests/list-files.spec.ts

@@ -384,4 +384,5 @@ test('should ignore errors when listing files', async ({ activate }) => {
     > playwright list-files -c playwright.config.js
     > playwright list-files -c playwright.config.ts
   `);
+  expect(vscode.errors).toEqual(['There are errors in Playwright configuration files.']);
 });

tests/mock/vscode.ts

@@ -743,6 +743,7 @@ export class VSCode {
   readonly testControllers: TestController[] = [];
   readonly fsWatchers = new Set<FileSystemWatcher>();
   readonly warnings: string[] = [];
+  readonly errors: string[] = [];
   readonly context: { subscriptions: any[]; extensionUri: Uri; };
   readonly extensions: any[] = [];
   private _webviewProviders = new Map<string, any>();
@@ -804,6 +805,7 @@ export class VSCode {
     this.window.onDidChangeActiveColorTheme = () => disposable;
     this.window.createTextEditorDecorationType = () => ++lastDecorationTypeId;
     this.window.showWarningMessage = (message: string) => this.warnings.push(message);
+    this.window.showErrorMessage = (message: string) => this.errors.push(message);
     this.window.visibleTextEditors = [];
     this.window.registerTreeDataProvider = () => disposable;
     this.window.registerWebviewViewProvider = (name: string, provider: any) => {