feat(trace-viewer): refactor trace viewer (#509)

src/extension.ts

@@ -27,7 +27,6 @@ import { TestTree } from './testTree';
 import { NodeJSNotFoundError, ansiToHtml, getPlaywrightInfo } from './utils';
 import * as vscodeTypes from './vscodeTypes';
 import { WorkspaceChange, WorkspaceObserver } from './workspaceObserver';
-import { TraceViewer } from './traceViewer';
 import { registerTerminalLinkProvider } from './terminalLinkProvider';
 import { RunHooks, TestConfig } from './playwrightTestTypes';
 
@@ -67,7 +66,6 @@ export class Extension implements RunHooks {
   private _debugHighlight: DebugHighlight;
   private _isUnderTest: boolean;
   private _reusedBrowser: ReusedBrowser;
-  private _traceViewer: TraceViewer;
   private _settingsModel: SettingsModel;
   private _settingsView!: SettingsView;
   private _diagnostics: vscodeTypes.DiagnosticCollection;
@@ -112,7 +110,6 @@ export class Extension implements RunHooks {
       onStdOut: this._debugHighlight.onStdOut.bind(this._debugHighlight),
       requestWatchRun: this._runWatchedTests.bind(this),
     });
-    this._traceViewer = new TraceViewer(this._vscode, this._settingsModel, this._envProvider.bind(this));
     this._testController = vscode.tests.createTestController('playwright', 'Playwright');
     this._testController.resolveHandler = item => this._resolveChildren(item);
     this._testController.refreshHandler = () => this._rebuildModels(true).then(() => {});
@@ -486,7 +483,7 @@ export class Extension implements RunHooks {
         // if trace viewer is currently displaying the trace file about to be replaced, it needs to be refreshed
         const prevTrace = (testItem as any)[traceUrlSymbol];
         (testItem as any)[traceUrlSymbol] = trace;
-        if (enqueuedSingleTest || prevTrace === this._traceViewer.currentFile())
+        if (enqueuedSingleTest || prevTrace === this._traceViewer()?.currentFile())
           this._showTrace(testItem);
 
         if (result.status === test.expectedStatus) {
@@ -538,7 +535,7 @@ export class Extension implements RunHooks {
     if (isDebug) {
       await model.debugTests(items, testListener, testRun.token);
     } else {
-      await this._traceViewer.willRunTests(model.config);
+      await this._traceViewer()?.willRunTests();
       await model.runTests(items, testListener, testRun.token);
     }
   }
@@ -755,20 +752,27 @@ export class Extension implements RunHooks {
     this._treeItemSelected(testItem);
   }
 
+  traceViewerInfoForTest() {
+    return this._traceViewer()?.infoForTest();
+  }
+
   private _showTrace(testItem: vscodeTypes.TestItem) {
     const traceUrl = (testItem as any)[traceUrlSymbol];
-    const testModel = this._models.selectedModel();
-    if (testModel)
-      this._traceViewer.open(traceUrl, testModel.config);
+    if (traceUrl)
+      this._traceViewer()?.open(traceUrl);
   }
 
   private _treeItemSelected(treeItem: vscodeTypes.TreeItem | null) {
     if (!treeItem)
       return;
     const traceUrl = (treeItem as any)[traceUrlSymbol] || '';
-    const testModel = this._models.selectedModel();
-    if (testModel)
-      this._traceViewer.open(traceUrl, testModel.config);
+    if (!traceUrl && !this._traceViewer()?.isStarted())
+      return;
+    this._traceViewer()?.open(traceUrl);
+  }
+
+  private _traceViewer() {
+    return this._models.selectedModel()?.traceViewer();
   }
 
   private _queueCommand<T>(callback: () => Promise<T>, defaultValue: T): Promise<T> {

src/testModel.ts

@@ -30,6 +30,7 @@ import type { PlaywrightTestRunOptions, RunHooks, TestConfig } from './playwrigh
 import { PlaywrightTestCLI } from './playwrightTestCLI';
 import { upstreamTreeItem } from './testTree';
 import { collectTestIds } from './upstream/testTree';
+import { SpawnTraceViewer } from './traceViewer';
 
 export type TestEntry = reporterTypes.TestCase | reporterTypes.Suite;
 
@@ -79,6 +80,7 @@ export class TestModel extends DisposableBase {
   private _startedDevServer = false;
   private _useLegacyCLIDriver: boolean;
   private _collection: TestModelCollection;
+  private _spawnTraceViewer: SpawnTraceViewer;
 
   constructor(collection: TestModelCollection, workspaceFolder: string, configFile: string, playwrightInfo: { cli: string, version: number }) {
     super();
@@ -89,6 +91,24 @@ export class TestModel extends DisposableBase {
     this._useLegacyCLIDriver = playwrightInfo.version < 1.44;
     this._playwrightTest =  this._useLegacyCLIDriver ? new PlaywrightTestCLI(this._vscode, this, collection.embedder) : new PlaywrightTestServer(this._vscode, this, collection.embedder);
     this.tag = new this._vscode.TestTag(this.config.configFile);
+    this._spawnTraceViewer = new SpawnTraceViewer(this._vscode, this._embedder.envProvider, this.config);
+
+    this._disposables = [
+      this._embedder.settingsModel.showTrace.onChange(() => this._closeTraceViewerIfNeeded()),
+      this._collection.onUpdated(() => this._closeTraceViewerIfNeeded()),
+    ];
+  }
+
+  traceViewer() {
+    if (!this._embedder.settingsModel.showTrace.get())
+      return;
+    if (this._spawnTraceViewer.checkVersion())
+      return this._spawnTraceViewer;
+  }
+
+  _closeTraceViewerIfNeeded() {
+    if (this._collection.selectedModel() !== this || !this._embedder.settingsModel.showTrace.get())
+      this._spawnTraceViewer.close();
   }
 
   async _loadModelIfNeeded(configSettings: ConfigSettings | undefined) {
@@ -127,6 +147,7 @@ export class TestModel extends DisposableBase {
     this._playwrightTest.reset();
     this._watches.clear();
     this._ranGlobalSetup = false;
+    this._spawnTraceViewer.close();
   }
 
   projects(): TestProject[] {

src/traceViewer.ts

@@ -16,68 +16,54 @@
 
 import { ChildProcess, spawn } from 'child_process';
 import type { TestConfig } from './playwrightTestTypes';
-import { SettingsModel } from './settingsModel';
 import { findNode } from './utils';
 import * as vscodeTypes from './vscodeTypes';
 
-export class TraceViewer implements vscodeTypes.Disposable {
+export type TraceViewer = SpawnTraceViewer;
+
+export class SpawnTraceViewer {
   private _vscode: vscodeTypes.VSCode;
   private _envProvider: () => NodeJS.ProcessEnv;
-  private _disposables: vscodeTypes.Disposable[] = [];
   private _traceViewerProcess: ChildProcess | undefined;
-  private _settingsModel: SettingsModel;
   private _currentFile?: string;
+  private _config: TestConfig;
+  private _serverUrlPrefixForTest?: string;
 
-  constructor(vscode: vscodeTypes.VSCode, settingsModel: SettingsModel, envProvider: () => NodeJS.ProcessEnv) {
+  constructor(vscode: vscodeTypes.VSCode, envProvider: () => NodeJS.ProcessEnv, config: TestConfig) {
     this._vscode = vscode;
     this._envProvider = envProvider;
-    this._settingsModel = settingsModel;
+    this._config = config;
+  }
 
-    this._disposables.push(settingsModel.showTrace.onChange(value => {
-      if (!value && this._traceViewerProcess)
-        this.close().catch(() => {});
-    }));
+  isStarted() {
+    return !!this._traceViewerProcess;
   }
 
   currentFile() {
     return this._currentFile;
   }
 
-  async willRunTests(config: TestConfig) {
-    if (this._settingsModel.showTrace.get())
-      await this._startIfNeeded(config);
+  async willRunTests() {
+    await this._startIfNeeded();
   }
 
-  async open(file: string, config: TestConfig) {
-    if (!this._settingsModel.showTrace.get())
-      return;
-    if (!this._checkVersion(config))
-      return;
-    if (!file && !this._traceViewerProcess)
-      return;
-    await this._startIfNeeded(config);
+  async open(file: string) {
+    await this._startIfNeeded();
     this._traceViewerProcess?.stdin?.write(file + '\n');
     this._currentFile = file;
   }
 
-  dispose() {
-    this.close().catch(() => {});
-    for (const d of this._disposables)
-      d.dispose();
-    this._disposables = [];
-  }
-
-  private async _startIfNeeded(config: TestConfig) {
-    const node = await findNode(this._vscode, config.workspaceFolder);
+  private async _startIfNeeded() {
+    const node = await findNode(this._vscode, this._config.workspaceFolder);
     if (this._traceViewerProcess)
       return;
-    const allArgs = [config.cli, 'show-trace', `--stdin`];
+    const allArgs = [this._config.cli, 'show-trace', `--stdin`];
     if (this._vscode.env.remoteName) {
       allArgs.push('--host', '0.0.0.0');
       allArgs.push('--port', '0');
     }
     const traceViewerProcess = spawn(node, allArgs, {
-      cwd: config.workspaceFolder,
+      cwd: this._config.workspaceFolder,
       stdio: 'pipe',
       detached: true,
       env: {
@@ -95,26 +81,44 @@ export class TraceViewer implements vscodeTypes.Disposable {
     });
     traceViewerProcess.on('error', error => {
       this._vscode.window.showErrorMessage(error.message);
-      this.close().catch(() => {});
+      this.close();
     });
+    if (this._vscode.isUnderTest) {
+      traceViewerProcess.stdout?.on('data', data => {
+        const match = data.toString().match(/Listening on (.*)/);
+        if (match)
+          this._serverUrlPrefixForTest = match[1];
+      });
+    }
   }
 
-  private _checkVersion(
-    config: TestConfig,
-    message: string = this._vscode.l10n.t('this feature')
-  ): boolean {
+  checkVersion() {
     const version = 1.35;
-    if (config.version < 1.35) {
+    if (this._config.version < version) {
+      const message = this._vscode.l10n.t('this feature');
       this._vscode.window.showWarningMessage(
-          this._vscode.l10n.t('Playwright v{0}+ is required for {1} to work, v{2} found', version, message, config.version)
+          this._vscode.l10n.t('Playwright v{0}+ is required for {1} to work, v{2} found', version, message, this._config.version)
       );
       return false;
     }
     return true;
   }
 
-  async close() {
+  close() {
     this._traceViewerProcess?.stdin?.end();
     this._traceViewerProcess = undefined;
+    this._currentFile = undefined;
+    this._serverUrlPrefixForTest = undefined;
+  }
+
+  infoForTest() {
+    if (!this._serverUrlPrefixForTest)
+      return;
+    return {
+      type: 'spawn',
+      serverUrlPrefix: this._serverUrlPrefixForTest,
+      testConfigFile: this._config.configFile,
+      traceFile: this.currentFile(),
+    };
   }
 }

src/vscodeTypes.ts

@@ -57,4 +57,6 @@ export type {
   TerminalLink,
 } from 'vscode';
 
-export type VSCode = typeof import('vscode');
+export type VSCode = typeof import('vscode') & {
+  isUnderTest?: boolean;
+};

tests/mock/vscode.ts

@@ -498,6 +498,7 @@ export class TestController {
   private _didCreateTestRun = new EventEmitter<TestRun>();
   readonly onDidCreateTestRun = this._didCreateTestRun.event;
 
+  refreshHandler: (item: TestItem | null) => Promise<void>;
   resolveHandler: (item: TestItem | null) => Promise<void>;
 
   constructor(readonly vscode: VSCode, id: string, label: string) {