diff --git a/src/extension.ts b/src/extension.ts index 0f39d5d66..676ce8067 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -67,7 +67,7 @@ export class Extension implements RunHooks { private _debugHighlight: DebugHighlight; private _isUnderTest: boolean; private _reusedBrowser: ReusedBrowser; - private _traceViewer: TraceViewer; + private _currentTraceViewer?: TraceViewer; private _settingsModel: SettingsModel; private _settingsView!: SettingsView; private _diagnostics: vscodeTypes.DiagnosticCollection; @@ -102,7 +102,6 @@ export class Extension implements RunHooks { this._settingsModel = new SettingsModel(vscode, context); this._models = new TestModelCollection(vscode, context); this._reusedBrowser = new ReusedBrowser(this._vscode, this._settingsModel, this._envProvider.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(() => {}); @@ -228,6 +227,7 @@ export class Extension implements RunHooks { this._reusedBrowser, this._diagnostics, this._treeItemObserver, + this._models, registerTerminalLinkProvider(this._vscode), ]; const fileSystemWatchers = [ @@ -485,7 +485,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._currentTraceViewer?.currentFile()) this._showTrace(testItem); if (result.status === test.expectedStatus) { @@ -537,7 +537,7 @@ export class Extension implements RunHooks { if (isDebug) { await model.debugTests(items, testListener, testRun.token); } else { - await this._traceViewer.willRunTests(model.config); + await this._enabledTraceView()?.willRunTests(); await model.runTests(items, testListener, testRun.token); } } @@ -756,18 +756,25 @@ export class Extension implements RunHooks { 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._enabledTraceView()?.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._currentTraceViewer?.isStarted()) + return; + this._enabledTraceView()?.open(traceUrl); + } + + private _enabledTraceView() { + const traceViewer = this._models.selectedModel()?.enabledTraceViewer(); + if (traceViewer !== this._currentTraceViewer) + this._currentTraceViewer?.close(); + this._currentTraceViewer = traceViewer; + return traceViewer; } private _queueCommand(callback: () => Promise, defaultValue: T): Promise { diff --git a/src/testModel.ts b/src/testModel.ts index 80595250d..1b74ae93c 100644 --- a/src/testModel.ts +++ b/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; @@ -55,7 +56,7 @@ type Watch = { include: readonly vscodeTypes.TestItem[] | undefined; }; -export class TestModel { +export class TestModel extends DisposableBase { private _vscode: vscodeTypes.VSCode; readonly config: TestConfig; private _projects = new Map(); @@ -79,8 +80,10 @@ export class TestModel { private _ranGlobalSetup = false; private _startedDevServer = false; private _useLegacyCLIDriver: boolean; + private _spawnTraceViewer: SpawnTraceViewer; constructor(vscode: vscodeTypes.VSCode, workspaceFolder: string, configFile: string, playwrightInfo: { cli: string, version: number }, options: TestModelOptions) { + super(); this._vscode = vscode; this._options = options; this.config = { ...playwrightInfo, workspaceFolder, configFile }; @@ -89,6 +92,22 @@ export class TestModel { this._didUpdate = new vscode.EventEmitter(); this.onUpdated = this._didUpdate.event; this.tag = new this._vscode.TestTag(this.config.configFile); + this._spawnTraceViewer = new SpawnTraceViewer(vscode, options.envProvider, this.config); + + this._disposables.push( + this._spawnTraceViewer, + options.settingsModel.showTrace.onChange(value => { + if (!value) + this._spawnTraceViewer.close(); + }), + ); + } + + enabledTraceViewer() { + if (!this._options.settingsModel.showTrace.get()) + return; + if (this._spawnTraceViewer.checkVersion()) + return this._spawnTraceViewer; } reset() { @@ -102,6 +121,12 @@ export class TestModel { this._playwrightTest.reset(); this._watches.clear(); this._ranGlobalSetup = false; + this._spawnTraceViewer.close(); + } + + dispose() { + this.reset(); + super.dispose(); } projects(): TestProject[] { @@ -665,7 +690,10 @@ export class TestModelCollection extends DisposableBase { const configSettings = (workspaceSettings.configs || []).find(c => c.relativeConfigFile === path.relative(model.config.workspaceFolder, model.config.configFile)); model.isEnabled = configSettings?.enabled || (this._models.length === 1 && !configSettings); await this._loadModelIfNeeded(model); - this._disposables.push(model.onUpdated(() => this._didUpdate.fire())); + this._disposables.push( + model, + model.onUpdated(() => this._didUpdate.fire()) + ); this._didUpdate.fire(); } @@ -709,12 +737,14 @@ export class TestModelCollection extends DisposableBase { clear() { this.dispose(); - for (const model of this._models) - model.reset(); - this._models = []; this._didUpdate.fire(); } + dispose() { + super.dispose(); + this._models = []; + } + enabledModels(): TestModel[] { return this._models.filter(m => m.isEnabled); } diff --git a/src/traceViewer.ts b/src/traceViewer.ts index 1e2c188c2..0599dd099 100644 --- a/src/traceViewer.ts +++ b/src/traceViewer.ts @@ -16,68 +16,53 @@ 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; - 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 +80,30 @@ export class TraceViewer implements vscodeTypes.Disposable { }); traceViewerProcess.on('error', error => { this._vscode.window.showErrorMessage(error.message); - this.close().catch(() => {}); + this.close(); }); } - 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; + } + + dispose() { + this.close(); } }