From 3bccf2fc197fef3be26eb5e60f50c5f592014eca Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 20 Mar 2024 21:13:52 -0700 Subject: [PATCH] chore: use uimode's test server api --- src/backend.ts | 84 ++++++----- src/debugHighlight.ts | 2 +- src/extension.ts | 92 ++++-------- src/listTests.d.ts | 2 +- src/oopReporter.ts | 2 +- src/playwrightTest.ts | 79 +++++----- src/reporterServer.ts | 4 +- src/reusedBrowser.ts | 7 +- src/testModel.ts | 40 +++--- src/testServerController.ts | 108 ++++++-------- src/testServerInterface.ts | 67 --------- src/testTree.ts | 2 +- {tests/mock => src/upstream}/events.ts | 0 src/{ => upstream}/reporter.d.ts | 0 src/upstream/teleEmitter.ts | 4 +- src/upstream/teleReceiver.ts | 35 ++--- src/upstream/testServerConnection.ts | 190 +++++++++++++++++++++++++ src/upstream/testServerInterface.ts | 102 +++++++++++++ src/upstream/testTree.ts | 8 +- tests/global-errors.spec.ts | 3 +- tests/list-files.spec.ts | 14 +- tests/mock/vscode.ts | 2 +- tests/run-tests.spec.ts | 3 +- 23 files changed, 515 insertions(+), 335 deletions(-) delete mode 100644 src/testServerInterface.ts rename {tests/mock => src/upstream}/events.ts (100%) rename src/{ => upstream}/reporter.d.ts (100%) create mode 100644 src/upstream/testServerConnection.ts create mode 100644 src/upstream/testServerInterface.ts diff --git a/src/backend.ts b/src/backend.ts index e09e09c19..760067237 100644 --- a/src/backend.ts +++ b/src/backend.ts @@ -20,59 +20,35 @@ import * as vscodeTypes from './vscodeTypes'; import EventEmitter from 'events'; import { WebSocketTransport } from './transport'; -export type BackendServerOptions = { +export type BackendServerOptions = { args: string[], cwd: string, envProvider: () => NodeJS.ProcessEnv, - clientFactory: () => T, dumpIO?: boolean, }; export class BackendServer { private _vscode: vscodeTypes.VSCode; - private _options: BackendServerOptions; + private _options: BackendServerOptions; + private _clientFactory: () => T; - constructor(vscode: vscodeTypes.VSCode, options: BackendServerOptions) { + constructor(vscode: vscodeTypes.VSCode, clientFactory: () => T, options: BackendServerOptions) { this._vscode = vscode; + this._clientFactory = clientFactory; this._options = options; } - async start(): Promise { - const node = await findNode(this._vscode, this._options.cwd); - const serverProcess = spawn(node, this._options.args, { - cwd: this._options.cwd, - stdio: 'pipe', - env: { - ...process.env, - ...this._options.envProvider(), - }, - }); - - const client = this._options.clientFactory(); - - serverProcess.stderr?.on('data', data => { - if (this._options.dumpIO) - console.log('[server err]', data.toString()); - }); - serverProcess.on('error', error => { - client._onErrorEvent.fire(error); - }); - return new Promise(fulfill => { - serverProcess.stdout?.on('data', async data => { - if (this._options.dumpIO) - console.log('[server out]', data.toString()); - const match = data.toString().match(/Listening on (.*)/); - if (!match) - return; - const wse = match[1]; - await client._connect(wse); - fulfill(client); - }); - serverProcess.on('exit', () => { - fulfill(null); - client._onCloseEvent.fire(); - }); + async startAndConnect(): Promise { + const client = this._clientFactory(); + const wsEndpoint = await startBackend(this._vscode, { + ...this._options, + onError: error => client._onErrorEvent.fire(error), + onClose: () => client._onCloseEvent.fire(), }); + if (!wsEndpoint) + return null; + await client._connect(wsEndpoint); + return client; } } @@ -143,3 +119,33 @@ export class BackendClient extends EventEmitter { this._transport.close(); } } + +export async function startBackend(vscode: vscodeTypes.VSCode, options: BackendServerOptions & { onError: (error: Error) => void, onClose: () => void }): Promise { + const node = await findNode(vscode, options.cwd); + const serverProcess = spawn(node, options.args, { + cwd: options.cwd, + stdio: 'pipe', + env: { + ...process.env, + ...options.envProvider(), + }, + }); + serverProcess.stderr?.on('data', data => { + if (options.dumpIO) + console.log('[server err]', data.toString()); + }); + serverProcess.on('error', options.onError); + serverProcess.on('close', options.onClose); + return new Promise(fulfill => { + serverProcess.stdout?.on('data', async data => { + if (options.dumpIO) + console.log('[server out]', data.toString()); + const match = data.toString().match(/Listening on (.*)/); + if (!match) + return; + const wse = match[1]; + fulfill(wse); + }); + serverProcess.on('exit', () => fulfill(null)); + }); +} diff --git a/src/debugHighlight.ts b/src/debugHighlight.ts index 52ec94f5c..a17786604 100644 --- a/src/debugHighlight.ts +++ b/src/debugHighlight.ts @@ -17,7 +17,7 @@ import { locatorForSourcePosition, pruneAstCaches } from './babelHighlightUtil'; import { debugSessionName } from './debugSessionName'; import { replaceActionWithLocator, locatorMethodRegex } from './methodNames'; -import type { Location } from './reporter'; +import type { Location } from './upstream/reporter'; import { ReusedBrowser } from './reusedBrowser'; import * as vscodeTypes from './vscodeTypes'; diff --git a/src/extension.ts b/src/extension.ts index e7289c651..ae77b9217 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -18,9 +18,8 @@ import path from 'path'; import StackUtils from 'stack-utils'; import { DebugHighlight } from './debugHighlight'; import { installBrowsers, installPlaywright } from './installer'; -import { MultiMap } from './multimap'; import { RunHooks, TestConfig, getPlaywrightInfo } from './playwrightTest'; -import * as reporterTypes from './reporter'; +import * as reporterTypes from './upstream/reporter'; import { ReusedBrowser } from './reusedBrowser'; import { SettingsModel } from './settingsModel'; import { SettingsView } from './settingsView'; @@ -85,7 +84,7 @@ export class Extension implements RunHooks { promise: Promise, finishedCallback: () => void } | undefined; - private _diagnostics: Record<'configErrors' | 'testErrors', vscodeTypes.DiagnosticCollection>; + private _diagnostics: vscodeTypes.DiagnosticCollection; private _treeItemObserver: TreeItemObserver; private _testServerController: TestServerController; private _watchQueue = Promise.resolve(); @@ -128,10 +127,7 @@ export class Extension implements RunHooks { 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 = { - testErrors: this._vscode.languages.createDiagnosticCollection('pw.testErrors.diagnostic'), - configErrors: this._vscode.languages.createDiagnosticCollection('pw.configErrors.diagnostic'), - }; + this._diagnostics = this._vscode.languages.createDiagnosticCollection('pw.testErrors.diagnostic'); this._treeItemObserver = new TreeItemObserver(this._vscode); this._watchSupport = new WatchSupport(this._vscode, this._testTree, watchData => this._watchesTriggered(watchData)); } @@ -229,7 +225,7 @@ export class Extension implements RunHooks { this._debugProfile, this._workspaceObserver, this._reusedBrowser, - ...Object.values(this._diagnostics), + this._diagnostics, this._treeItemObserver, this._testServerController, registerTerminalLinkProvider(this._vscode), @@ -315,44 +311,11 @@ export class Extension implements RunHooks { private _modelsUpdated() { this._workspaceObserver.reset(); this._updateVisibleEditorItems(); - this._reportConfigErrors(); + this._updateDiagnostics(); for (const testDir of this._models.testDirs()) this._workspaceObserver.addWatchFolder(testDir); } - private _reportConfigErrors() { - const configErrors = new MultiMap(); - for (const model of this._models.enabledModels()) { - const error = model.takeConfigError(); - if (error) - configErrors.set(model.config.configFile, error); - } - - 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(Math.max(error.location.line - 1, 0), Math.max(error.location.column - 1, 0)), - ); - 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 => { @@ -637,24 +600,13 @@ export class Extension implements RunHooks { const timer = setTimeout(async () => { delete this._filesPendingListTests; - const allErrors = new Set(); - const errorsByFile = new MultiMap(); for (const model of this._models.enabledModels()) { const filteredFiles = model.narrowDownFilesToEnabledProjects(files); if (!filteredFiles.size) continue; - const errors = await model.ensureTests([...filteredFiles]).catch(e => console.log(e)) || []; - for (const error of errors) { - if (!error.location || !error.message) - continue; - const key = error.location.file + ':' + error.location.line + ':' + error.message; - if (allErrors.has(key)) - continue; - allErrors.add(key); - errorsByFile.set(error.location?.file, error); - } + await model.ensureTests([...filteredFiles]).catch(e => console.log(e)); } - this._updateDiagnostics('testErrors', errorsByFile); + this._updateDiagnostics(); finishedCallback(); }, 0); @@ -672,21 +624,35 @@ export class Extension implements RunHooks { return this._filesPendingListTests.promise; } - private _updateDiagnostics(diagnosticsType: 'configErrors' | 'testErrors' , errorsByFile: MultiMap) { - const diagnosticsCollection = this._diagnostics[diagnosticsType]!; - diagnosticsCollection.clear(); - for (const [file, errors] of errorsByFile) { - const diagnostics: vscodeTypes.Diagnostic[] = []; - for (const error of errors) { - diagnostics.push({ + private _updateDiagnostics() { + this._diagnostics.clear(); + const diagnosticsByFile = new Map>(); + + const addError = (error: reporterTypes.TestError) => { + if (!error.location) + return; + let diagnostics = diagnosticsByFile.get(error.location.file); + if (!diagnostics) { + diagnostics = new Map(); + diagnosticsByFile.set(error.location.file, diagnostics); + } + const key = `${error.location?.line}:${error.location?.column}:${error.message}`; + if (!diagnostics.has(key)) { + diagnostics.set(key, { severity: this._vscode.DiagnosticSeverity.Error, source: 'playwright', range: new this._vscode.Range(Math.max(error.location!.line - 1, 0), Math.max(error.location!.column - 1, 0), error.location!.line, 0), message: error.message!, }); } - diagnosticsCollection.set(this._vscode.Uri.file(file), diagnostics); + }; + + for (const model of this._models.enabledModels()) { + for (const error of model.errors().values()) + addError(error); } + for (const [file, diagnostics] of diagnosticsByFile) + this._diagnostics.set(this._vscode.Uri.file(file), [...diagnostics.values()]); } private _errorInDebugger(errorStack: string, location: reporterTypes.Location) { diff --git a/src/listTests.d.ts b/src/listTests.d.ts index 5e5280e72..fa0cec6fe 100644 --- a/src/listTests.d.ts +++ b/src/listTests.d.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { TestError } from './reporter'; +import type { TestError } from './upstream/reporter'; // This matches the structs in packages/playwright-test/src/runner/runner.ts. diff --git a/src/oopReporter.ts b/src/oopReporter.ts index 9d454e467..ebe42a6d9 100644 --- a/src/oopReporter.ts +++ b/src/oopReporter.ts @@ -16,7 +16,7 @@ import { TeleReporterEmitter } from './upstream/teleEmitter'; import { WebSocketTransport } from './transport'; -import { FullResult } from './reporter'; +import { FullResult } from './upstream/reporter'; class TeleReporter extends TeleReporterEmitter { private _hasSender: boolean; diff --git a/src/playwrightTest.ts b/src/playwrightTest.ts index 11dd6ed65..fdcc6986e 100644 --- a/src/playwrightTest.ts +++ b/src/playwrightTest.ts @@ -23,7 +23,9 @@ import { findNode, spawnAsync } from './utils'; import * as vscodeTypes from './vscodeTypes'; import { SettingsModel } from './settingsModel'; import { TestServerController } from './testServerController'; -import * as reporterTypes from './reporter'; +import * as reporterTypes from './upstream/reporter'; +import { TeleReporterReceiver } from './upstream/teleReceiver'; +import { TestServerInterface } from './upstream/testServerInterface'; export type TestConfig = { workspaceFolder: string; @@ -35,15 +37,8 @@ export type TestConfig = { const pathSeparator = process.platform === 'win32' ? ';' : ':'; -export type PlaywrightTestRunOptions = { - headed?: boolean, - oneWorker?: boolean, - trace?: 'on' | 'off', - projects?: string[]; - grep?: string; - reuseContext?: boolean, - connectWsEndpoint?: string; -}; +type AllRunOptions = Parameters[0]; +export type PlaywrightTestRunOptions = Pick; export interface RunHooks { onWillRunTests(config: TestConfig, debug: boolean): Promise<{ connectWsEndpoint?: string }>; @@ -121,7 +116,7 @@ export class PlaywrightTest { const testServer = await this._options.testServerController.testServerFor(config); if (!testServer) throw new Error('Internal error: unable to connect to the test server'); - return await testServer.listFiles({ configFile: config.configFile }); + return await testServer.listFiles({}); } async runTests(config: TestConfig, projectNames: string[], locations: string[] | null, listener: reporterTypes.ReporterV2, parametrizedTestTitle: string | undefined, token: vscodeTypes.CancellationToken) { @@ -145,7 +140,7 @@ export class PlaywrightTest { grep: parametrizedTestTitle, projects: projectNames.length ? projectNames.filter(Boolean) : undefined, headed: showBrowser && !this._options.isUnderTest, - oneWorker: showBrowser, + workers: showBrowser ? 1 : undefined, trace, reuseContext: showBrowser, connectWsEndpoint: showBrowser ? externalOptions.connectWsEndpoint : undefined, @@ -179,10 +174,14 @@ export class PlaywrightTest { } private async _test(config: TestConfig, locations: string[], mode: 'list' | 'test', options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise { - if (this._useTestServer(config)) - await this._testWithServer(config, locations, mode, options, reporter, token); - else + if (this._useTestServer(config)) { + if (mode === 'test') + await this._testWithServer(config, locations, options, reporter, token); + else + await this._listWithServer(config, locations, reporter, token); + } else { await this._testWithCLI(config, locations, mode, options, reporter, token); + } } private async _testWithCLI(config: TestConfig, locations: string[], mode: 'list' | 'test', options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise { @@ -219,8 +218,8 @@ export class PlaywrightTest { if (options.headed) allArgs.push('--headed'); - if (options.oneWorker) - allArgs.push('--workers', '1'); + if (options.workers) + allArgs.push('--workers', String(options.workers)); if (options.trace) allArgs.push('--trace', options.trace); @@ -250,29 +249,41 @@ export class PlaywrightTest { await reporterServer.wireTestListener(mode, reporter, token); } - private async _testWithServer(config: TestConfig, locations: string[], mode: 'list' | 'test', options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise { + private async _listWithServer(config: TestConfig, locations: string[], reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise { const testServer = await this._options.testServerController.testServerFor(config); if (token?.isCancellationRequested) return; if (!testServer) return; const oopReporter = require.resolve('./oopReporter'); - if (mode === 'list') - testServer.listTests({ configFile: config.configFile, locations, reporter: oopReporter }); - if (mode === 'test') { - testServer.test({ configFile: config.configFile, locations, reporter: oopReporter, ...options }); - token.onCancellationRequested(() => { - testServer.stop({ configFile: config.configFile }); - }); - testServer.on('stdio', params => { - if (params.type === 'stdout') - reporter.onStdOut?.(unwrapString(params)); - if (params.type === 'stderr') - reporter.onStdErr?.(unwrapString(params)); - }); - } + const { report } = await testServer.listTests({ locations, serializer: oopReporter }); + const teleReceiver = new TeleReporterReceiver(reporter, { + mergeProjects: true, + mergeTestCases: true, + resolvePath: (rootDir: string, relativePath: string) => this._vscode.Uri.file(path.join(rootDir, relativePath)).fsPath, + }); + for (const message of report) + teleReceiver.dispatch(message); + } - await testServer.wireTestListener(mode, reporter, token); + private async _testWithServer(config: TestConfig, locations: string[], options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise { + const testServer = await this._options.testServerController.testServerFor(config); + if (token?.isCancellationRequested) + return; + if (!testServer) + return; + const oopReporter = require.resolve('./oopReporter'); + testServer.runTests({ locations, serializer: oopReporter, ...options }); + token.onCancellationRequested(() => { + testServer.stopTestsNoReply({}); + }); + testServer.onStdio(params => { + if (params.type === 'stdout') + reporter.onStdOut?.(unwrapString(params)); + if (params.type === 'stderr') + reporter.onStdErr?.(unwrapString(params)); + }); + await this._options.testServerController.wireTestListener(testServer, reporter, token); } async findRelatedTestFiles(config: TestConfig, files: string[]): Promise { @@ -313,7 +324,7 @@ export class PlaywrightTest { const testServer = await this._options.testServerController.testServerFor(config); if (!testServer) return { testFiles: files, errors: [{ message: 'Internal error: unable to connect to the test server' }] }; - return await testServer.findRelatedTestFiles({ configFile: config.configFile, files }); + return await testServer.findRelatedTestFiles({ files }); } async debugTests(vscode: vscodeTypes.VSCode, config: TestConfig, projectNames: string[], testDirs: string[], settingsEnv: NodeJS.ProcessEnv, locations: string[] | null, reporter: reporterTypes.ReporterV2, parametrizedTestTitle: string | undefined, token: vscodeTypes.CancellationToken) { diff --git a/src/reporterServer.ts b/src/reporterServer.ts index b8f84e177..6bf2bff06 100644 --- a/src/reporterServer.ts +++ b/src/reporterServer.ts @@ -20,7 +20,7 @@ import WebSocket, { WebSocketServer } from 'ws'; import { ConnectionTransport } from './transport'; import { createGuid } from './utils'; import * as vscodeTypes from './vscodeTypes'; -import * as reporterTypes from './reporter'; +import * as reporterTypes from './upstream/reporter'; import { TeleReporterReceiver } from './upstream/teleReceiver'; export class ReporterServer { @@ -97,7 +97,7 @@ export class ReporterServer { return; if (message.method === 'onEnd') transport.close(); - teleReceiver.dispatch(mode, message as any); + teleReceiver.dispatch(message as any); }; await new Promise(f => transport.onclose = f); diff --git a/src/reusedBrowser.ts b/src/reusedBrowser.ts index c395d93ca..a32f8ad1f 100644 --- a/src/reusedBrowser.ts +++ b/src/reusedBrowser.ts @@ -88,13 +88,12 @@ export class ReusedBrowser implements vscodeTypes.Disposable { PW_EXTENSION_MODE: '1', }); - const backendServer = new BackendServer(this._vscode, { + const backendServer = new BackendServer(this._vscode, () => new Backend(this._vscode), { args, cwd, - envProvider, - clientFactory: () => new Backend(this._vscode) + envProvider }); - const backend = await backendServer.start(); + const backend = await backendServer.startAndConnect(); if (!backend) return; backend.onClose(() => { diff --git a/src/testModel.ts b/src/testModel.ts index 07d353383..5b9be08a0 100644 --- a/src/testModel.ts +++ b/src/testModel.ts @@ -19,12 +19,13 @@ import { WorkspaceChange } from './workspaceObserver'; import * as vscodeTypes from './vscodeTypes'; import { resolveSourceMap } from './utils'; import { ProjectConfigWithFiles } from './listTests'; -import * as reporterTypes from './reporter'; +import * as reporterTypes from './upstream/reporter'; import { TeleSuite } from './upstream/teleReceiver'; import type { SettingsModel, WorkspaceSettings } from './settingsModel'; import path from 'path'; import { DisposableBase } from './disposableBase'; import type { TestServerController } from './testServerController'; +import { MultiMap } from './multimap'; export type TestEntry = reporterTypes.TestCase | reporterTypes.Suite; @@ -57,7 +58,7 @@ export class TestModel { private _envProvider: () => NodeJS.ProcessEnv; isEnabled = false; readonly tag: vscodeTypes.TestTag; - private _configError: reporterTypes.TestError | undefined; + private _errorByFile = new MultiMap(); constructor(vscode: vscodeTypes.VSCode, workspaceFolder: string, configFile: string, playwrightInfo: { cli: string, version: number }, options: TestModelOptions) { this._vscode = vscode; @@ -73,7 +74,7 @@ export class TestModel { this._projects.clear(); this._fileToSources.clear(); this._sourceToFile.clear(); - this._configError = undefined; + this._errorByFile.clear(); this._playwrightTest.reset(); } @@ -81,10 +82,8 @@ export class TestModel { return [...this._projects.values()]; } - takeConfigError(): reporterTypes.TestError | undefined { - const error = this._configError; - this._configError = undefined; - return error; + errors(): MultiMap { + return this._errorByFile; } projectMap(): Map { @@ -111,8 +110,8 @@ export class TestModel { async _listFiles() { const report = await this._playwrightTest.listFiles(this.config); - if (report.error) { - this._configError = report.error; + if (report.error?.location) { + this._errorByFile.set(report.error?.location.file, report.error); this._didUpdate.fire(); return; } @@ -206,7 +205,7 @@ export class TestModel { await this.listTests(changed); } - async ensureTests(files: string[]): Promise { + async ensureTests(files: string[]) { // Do not list tests if all files are already loaded, otherwise we // end up with update loop when updating tests for visible editors. let allFilesLoaded = true; @@ -225,25 +224,34 @@ export class TestModel { if (allFilesLoaded) return []; const { rootSuite, errors } = await this._playwrightTest.listTests(this.config, files); - this._updateProjects(rootSuite.suites, files); - return errors; + this._updateProjects(rootSuite.suites, files, errors); } - async listTests(files: string[]): Promise { + async listTests(files: string[]) { const { rootSuite, errors } = await this._playwrightTest.listTests(this.config, files); - this._updateProjects(rootSuite.suites, files); - return errors; + this._updateProjects(rootSuite.suites, files, errors); } - private _updateProjects(newProjectSuites: reporterTypes.Suite[], requestedFiles: string[]) { + private _updateProjects(newProjectSuites: reporterTypes.Suite[], requestedFiles: string[], errors: reporterTypes.TestError[]) { + for (const requestedFile of requestedFiles) + this._errorByFile.deleteAll(requestedFile); + for (const error of errors) { + if (error.location) + this._errorByFile.set(error.location.file, error); + } + for (const [projectName, project] of this._projects) { const files = projectFiles(project); const newProjectSuite = newProjectSuites.find(e => e.project()!.name === projectName); const filesToClear = new Set(requestedFiles); for (const fileSuite of newProjectSuite?.suites || []) { + // Do not show partial results in suites with errors. + if (this._errorByFile.has(fileSuite.location!.file)) + continue; filesToClear.delete(fileSuite.location!.file); files.set(fileSuite.location!.file, fileSuite); } + for (const file of filesToClear) { const fileSuite = files.get(file); if (fileSuite) { diff --git a/src/testServerController.ts b/src/testServerController.ts index 9f72ce41e..d3dd528f8 100644 --- a/src/testServerController.ts +++ b/src/testServerController.ts @@ -13,18 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import path from 'path'; -import { BackendClient, BackendServer } from './backend'; -import type { ConfigFindRelatedTestFilesReport } from './listTests'; +import { startBackend } from './backend'; import type { TestConfig } from './playwrightTest'; -import type { TestServerEvents, TestServerInterface } from './testServerInterface'; -import type * as vscodeTypes from './vscodeTypes'; -import type * as reporterTypes from './reporter'; import { TeleReporterReceiver } from './upstream/teleReceiver'; +import { TestServerConnection } from './upstream/testServerConnection'; +import type * as vscodeTypes from './vscodeTypes'; +import * as reporterTypes from './upstream/reporter'; +import path from 'path'; export class TestServerController implements vscodeTypes.Disposable { private _vscode: vscodeTypes.VSCode; - private _instancePromises = new Map>(); + private _connectionPromises = new Map>(); private _envProvider: () => NodeJS.ProcessEnv; constructor(vscode: vscodeTypes.VSCode, envProvider: () => NodeJS.ProcessEnv) { @@ -32,25 +31,25 @@ export class TestServerController implements vscodeTypes.Disposable { this._envProvider = envProvider; } - async testServerFor(config: TestConfig): Promise { - let instancePromise = this._instancePromises.get(config.configFile); - if (instancePromise) - return instancePromise; - instancePromise = this._createTestServer(config); - this._instancePromises.set(config.configFile, instancePromise); - return instancePromise; + testServerFor(config: TestConfig): Promise { + let connectionPromise = this._connectionPromises.get(config.configFile); + if (connectionPromise) + return connectionPromise; + connectionPromise = this._createTestServer(config); + this._connectionPromises.set(config.configFile, connectionPromise); + return connectionPromise; } disposeTestServerFor(configFile: string) { - const result = this._instancePromises.get(configFile); - this._instancePromises.delete(configFile); + const result = this._connectionPromises.get(configFile); + this._connectionPromises.delete(configFile); if (result) - result.then(server => server?.closeGracefully()); + result.then(server => server?.closeGracefully({})); } - private async _createTestServer(config: TestConfig): Promise { - const args = [config.cli, 'test-server']; - const testServerBackend = new BackendServer(this._vscode, { + private async _createTestServer(config: TestConfig): Promise { + const args = [config.cli, 'test-server', '-c', config.configFile]; + const wsEndpoint = await startBackend(this._vscode, { args, cwd: config.workspaceFolder, envProvider: () => { @@ -59,65 +58,42 @@ export class TestServerController implements vscodeTypes.Disposable { FORCE_COLOR: '1', }; }, - clientFactory: () => new TestServer(this._vscode), dumpIO: false, + onClose: () => { + this._connectionPromises.delete(config.configFile); + }, + onError: error => { + this._connectionPromises.delete(config.configFile); + }, }); - const testServer = await testServerBackend.start(); + if (!wsEndpoint) + return null; + const testServer = new TestServerConnection(wsEndpoint); + await testServer.connect(); return testServer; } - dispose() { - for (const instancePromise of this._instancePromises.values()) - instancePromise.then(server => server?.closeGracefully()); - } -} - -class TestServer extends BackendClient implements TestServerInterface, TestServerEvents { - override async initialize(): Promise { - } - - async listFiles(params: Parameters[0]) { - return await this.send('listFiles', params); - } - - async listTests(params: Parameters[0]) { - await this.send('listTests', params); - } - - findRelatedTestFiles(params: Parameters[0]): Promise { - return this.send('findRelatedTestFiles', params); - } - - async test(params: Parameters[0]) { - await this.send('test', params); - } - - async stop(params: Parameters[0]) { - await this.send('stop', {}); - } - - async closeGracefully() { - await this.send('closeGracefully', {}); - this.close(); - } - - async wireTestListener(mode: 'test' | 'list', reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) { + async wireTestListener(testServerConnection: TestServerConnection, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) { const teleReceiver = new TeleReporterReceiver(reporter, { mergeProjects: true, mergeTestCases: true, - resolvePath: (rootDir: string, relativePath: string) => this.vscode.Uri.file(path.join(rootDir, relativePath)).fsPath, + resolvePath: (rootDir: string, relativePath: string) => this._vscode.Uri.file(path.join(rootDir, relativePath)).fsPath, }); - return new Promise(f => { - const handler = (message: any) => { + return new Promise(resolve => { + const disposable = testServerConnection.onReport(message => { if (token.isCancellationRequested && message.method !== 'onEnd') return; - teleReceiver.dispatch(mode, message); + teleReceiver.dispatch(message); if (message.method === 'onEnd') { - this.off('report', handler); - f(); + disposable.dispose(); + resolve(); } - }; - this.on('report', handler); + }); }); } + + dispose() { + for (const instancePromise of this._connectionPromises.values()) + instancePromise.then(server => server?.closeGracefully({})); + } } diff --git a/src/testServerInterface.ts b/src/testServerInterface.ts deleted file mode 100644 index 31b8b4f40..000000000 --- a/src/testServerInterface.ts +++ /dev/null @@ -1,67 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * 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 type { TestError } from './reporter'; - -export interface TestServerInterface { - listFiles(params: { - configFile: string; - }): Promise<{ - projects: { - name: string; - testDir: string; - use: { testIdAttribute?: string }; - files: string[]; - }[]; - cliEntryPoint?: string; - error?: TestError; - }>; - - listTests(params: { - configFile: string; - locations: string[]; - reporter: string; - }): Promise; - - test(params: { - configFile: string; - locations: string[]; - reporter: string; - headed?: boolean; - oneWorker?: boolean; - trace?: 'on' | 'off'; - projects?: string[]; - grep?: string; - reuseContext?: boolean; - connectWsEndpoint?: string; - }): Promise; - - findRelatedTestFiles(params: { - configFile: string; - files: string[]; - }): Promise<{ testFiles: string[]; errors?: TestError[]; }>; - - stop(params: { - configFile: string; - }): Promise; - - closeGracefully(): Promise; -} - -export interface TestServerEvents { - on(event: 'report', listener: (params: any) => void): void; - on(event: 'stdio', listener: (params: { type: 'stdout' | 'stderr', text?: string, buffer?: string }) => void): void; -} diff --git a/src/testTree.ts b/src/testTree.ts index 16f91135e..2be606742 100644 --- a/src/testTree.ts +++ b/src/testTree.ts @@ -18,7 +18,7 @@ import path from 'path'; import { TestModelCollection } from './testModel'; import { createGuid } from './utils'; import * as vscodeTypes from './vscodeTypes'; -import * as reporterTypes from './reporter'; +import * as reporterTypes from './upstream/reporter'; import * as upstream from './upstream/testTree'; import { TeleSuite } from './upstream/teleReceiver'; import { DisposableBase } from './disposableBase'; diff --git a/tests/mock/events.ts b/src/upstream/events.ts similarity index 100% rename from tests/mock/events.ts rename to src/upstream/events.ts diff --git a/src/reporter.d.ts b/src/upstream/reporter.d.ts similarity index 100% rename from src/reporter.d.ts rename to src/upstream/reporter.d.ts diff --git a/src/upstream/teleEmitter.ts b/src/upstream/teleEmitter.ts index f4ee7205c..4e094abb5 100644 --- a/src/upstream/teleEmitter.ts +++ b/src/upstream/teleEmitter.ts @@ -20,10 +20,10 @@ import path from 'path'; import { createGuid } from '../utils'; -import type * as reporterTypes from '../reporter'; +import type * as reporterTypes from './reporter'; import type * as teleReceiver from './teleReceiver'; import { serializeRegexPatterns } from './teleReceiver'; -import type { ReporterV2 } from '../reporter'; +import type { ReporterV2 } from './reporter'; export type TeleReporterEmitterOptions = { omitOutput?: boolean; diff --git a/src/upstream/teleReceiver.ts b/src/upstream/teleReceiver.ts index 705ad1a8e..58d4eaa54 100644 --- a/src/upstream/teleReceiver.ts +++ b/src/upstream/teleReceiver.ts @@ -18,10 +18,10 @@ * limitations under the License. */ -import type { Annotation } from '../reporter'; -import type { FullProject, Metadata } from '../reporter'; -import type * as reporterTypes from '../reporter'; -import type { ReporterV2 } from '../reporter'; +import type { Annotation } from './reporter'; +import type { FullProject, Metadata } from './reporter'; +import type * as reporterTypes from './reporter'; +import type { ReporterV2 } from './reporter'; export type StringIntern = (s: string) => string; export type JsonLocation = reporterTypes.Location; @@ -139,7 +139,6 @@ export class TeleReporterReceiver { private _reporter: Partial; private _tests = new Map(); private _rootDir!: string; - private _listOnly = false; private _config!: reporterTypes.FullConfig; constructor(reporter: Partial, options: TeleReporterReceiverOptions) { @@ -148,11 +147,16 @@ export class TeleReporterReceiver { this._reporter = reporter; } - dispatch(mode: 'list' | 'test', message: JsonEvent): Promise | void { + reset() { + this._rootSuite.suites = []; + this._rootSuite.tests = []; + this._tests.clear(); + } + + dispatch(message: JsonEvent): Promise | void { const { method, params } = message; if (method === 'onConfigure') { this._onConfigure(params.config); - this._listOnly = mode === 'list'; return; } if (method === 'onProject') { @@ -209,23 +213,6 @@ export class TeleReporterReceiver { // Always update project in watch mode. projectSuite._project = this._parseProject(project); this._mergeSuitesInto(project.suites, projectSuite); - - // Remove deleted tests when listing. Empty suites will be auto-filtered - // in the UI layer. - if (this._listOnly) { - const testIds = new Set(); - const collectIds = (suite: JsonSuite) => { - suite.tests.map(t => t.testId).forEach(testId => testIds.add(testId)); - suite.suites.forEach(collectIds); - }; - project.suites.forEach(collectIds); - - const filterTests = (suite: TeleSuite) => { - suite.tests = suite.tests.filter(t => testIds.has(t.id)); - suite.suites.forEach(filterTests); - }; - filterTests(projectSuite); - } } private _onBegin() { diff --git a/src/upstream/testServerConnection.ts b/src/upstream/testServerConnection.ts new file mode 100644 index 000000000..6633e10fa --- /dev/null +++ b/src/upstream/testServerConnection.ts @@ -0,0 +1,190 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * 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 type { TestServerInterface, TestServerInterfaceEvents } from './testServerInterface'; +import * as events from './events'; + +import WebSocket from 'ws'; + +export class TestServerConnection implements TestServerInterface, TestServerInterfaceEvents { + readonly onClose: events.Event; + readonly onReport: events.Event; + readonly onStdio: events.Event<{ type: 'stderr' | 'stdout'; text?: string | undefined; buffer?: string | undefined; }>; + readonly onListChanged: events.Event; + readonly onTestFilesChanged: events.Event<{ testFiles: string[] }>; + readonly onLoadTraceRequested: events.Event<{ traceUrl: string }>; + + private _onCloseEmitter = new events.EventEmitter(); + private _onReportEmitter = new events.EventEmitter(); + private _onStdioEmitter = new events.EventEmitter<{ type: 'stderr' | 'stdout'; text?: string | undefined; buffer?: string | undefined; }>(); + private _onListChangedEmitter = new events.EventEmitter(); + private _onTestFilesChangedEmitter = new events.EventEmitter<{ testFiles: string[] }>(); + private _onLoadTraceRequestedEmitter = new events.EventEmitter<{ traceUrl: string }>(); + + private _lastId = 0; + private _ws: WebSocket; + private _callbacks = new Map void, reject: (arg: Error) => void }>(); + private _connectedPromise: Promise; + + constructor(wsURL: string) { + this.onClose = this._onCloseEmitter.event; + this.onReport = this._onReportEmitter.event; + this.onStdio = this._onStdioEmitter.event; + this.onListChanged = this._onListChangedEmitter.event; + this.onTestFilesChanged = this._onTestFilesChangedEmitter.event; + this.onLoadTraceRequested = this._onLoadTraceRequestedEmitter.event; + + this._ws = new WebSocket(wsURL); + this._ws.addEventListener('message', event => { + const message = JSON.parse(String(event.data)); + const { id, result, error, method, params } = message; + if (id) { + const callback = this._callbacks.get(id); + if (!callback) + return; + this._callbacks.delete(id); + if (error) + callback.reject(new Error(error)); + else + callback.resolve(result); + } else { + this._dispatchEvent(method, params); + } + }); + const pingInterval = setInterval(() => this._sendMessage('ping').catch(() => {}), 30000); + this._connectedPromise = new Promise((f, r) => { + this._ws.addEventListener('open', () => { + f(); + this._ws.send(JSON.stringify({ id: -1, method: 'ready' })); + }); + this._ws.addEventListener('error', r); + }); + this._ws.addEventListener('close', () => { + this._onCloseEmitter.fire(); + clearInterval(pingInterval); + }); + } + + connect() { + return this._connectedPromise; + } + + private async _sendMessage(method: string, params?: any): Promise { + const logForTest = (globalThis as any).__logForTest; + logForTest?.({ method, params }); + + await this._connectedPromise; + const id = ++this._lastId; + const message = { id, method, params }; + this._ws.send(JSON.stringify(message)); + return new Promise((resolve, reject) => { + this._callbacks.set(id, { resolve, reject }); + }); + } + + private _sendMessageNoReply(method: string, params?: any) { + this._sendMessage(method, params).catch(() => {}); + } + + private _dispatchEvent(method: string, params?: any) { + if (method === 'report') + this._onReportEmitter.fire(params); + else if (method === 'stdio') + this._onStdioEmitter.fire(params); + else if (method === 'listChanged') + this._onListChangedEmitter.fire(params); + else if (method === 'testFilesChanged') + this._onTestFilesChangedEmitter.fire(params); + else if (method === 'loadTraceRequested') + this._onLoadTraceRequestedEmitter.fire(params); + } + + async ping(params: Parameters[0]): ReturnType { + await this._sendMessage('ping'); + } + + async pingNoReply(params: Parameters[0]) { + this._sendMessageNoReply('ping'); + } + + async watch(params: Parameters[0]): ReturnType { + await this._sendMessage('watch', params); + } + + watchNoReply(params: Parameters[0]) { + this._sendMessageNoReply('watch', params); + } + + async open(params: Parameters[0]): ReturnType { + await this._sendMessage('open', params); + } + + openNoReply(params: Parameters[0]) { + this._sendMessageNoReply('open', params); + } + + async resizeTerminal(params: Parameters[0]): ReturnType { + await this._sendMessage('resizeTerminal', params); + } + + resizeTerminalNoReply(params: Parameters[0]) { + this._sendMessageNoReply('resizeTerminal', params); + } + + async checkBrowsers(params: Parameters[0]): ReturnType { + return await this._sendMessage('checkBrowsers'); + } + + async installBrowsers(params: Parameters[0]): ReturnType { + await this._sendMessage('installBrowsers'); + } + + async runGlobalSetup(params: Parameters[0]): ReturnType { + return await this._sendMessage('runGlobalSetup'); + } + + async runGlobalTeardown(params: Parameters[0]): ReturnType { + return await this._sendMessage('runGlobalTeardown'); + } + + async listFiles(params: Parameters[0]): ReturnType { + return await this._sendMessage('listFiles', params); + } + + async listTests(params: Parameters[0]): ReturnType { + return await this._sendMessage('listTests', params); + } + + async runTests(params: Parameters[0]): ReturnType { + return await this._sendMessage('runTests', params); + } + + async findRelatedTestFiles(params: Parameters[0]): ReturnType { + return await this._sendMessage('findRelatedTestFiles', params); + } + + async stopTests(params: Parameters[0]): ReturnType { + await this._sendMessage('stopTests'); + } + + stopTestsNoReply(params: Parameters[0]) { + this._sendMessageNoReply('stopTests'); + } + + async closeGracefully(params: Parameters[0]): ReturnType { + await this._sendMessage('closeGracefully'); + } +} diff --git a/src/upstream/testServerInterface.ts b/src/upstream/testServerInterface.ts new file mode 100644 index 000000000..5dce14d16 --- /dev/null +++ b/src/upstream/testServerInterface.ts @@ -0,0 +1,102 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * 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 type * as reporterTypes from './reporter'; +import type { Event } from '../vscodeTypes'; + +export interface TestServerInterface { + ping(params: {}): Promise; + + watch(params: { + fileNames: string[]; + }): Promise; + + open(params: { location: reporterTypes.Location }): Promise; + + resizeTerminal(params: { cols: number, rows: number }): Promise; + + checkBrowsers(params: {}): Promise<{ hasBrowsers: boolean }>; + + installBrowsers(params: {}): Promise; + + runGlobalSetup(params: {}): Promise; + + runGlobalTeardown(params: {}): Promise; + + listFiles(params: { + projects?: string[]; + }): Promise<{ + projects: { + name: string; + testDir: string; + use: { testIdAttribute?: string }; + files: string[]; + }[]; + cliEntryPoint?: string; + error?: reporterTypes.TestError; + }>; + + /** + * Returns list of teleReporter events. + */ + listTests(params: { + serializer?: string; + projects?: string[]; + locations?: string[]; + }): Promise<{ report: any[] }>; + + runTests(params: { + serializer?: string; + locations?: string[]; + grep?: string; + grepInvert?: string; + testIds?: string[]; + headed?: boolean; + workers?: number | string; + timeout?: number, + reporters?: string[], + trace?: 'on' | 'off'; + projects?: string[]; + reuseContext?: boolean; + connectWsEndpoint?: string; + }): Promise<{ status: reporterTypes.FullResult['status'] }>; + + findRelatedTestFiles(params: { + files: string[]; + }): Promise<{ testFiles: string[]; errors?: reporterTypes.TestError[]; }>; + + stopTests(params: {}): Promise; + + closeGracefully(params: {}): Promise; +} + +export interface TestServerInterfaceEvents { + onClose: Event; + onReport: Event; + onStdio: Event<{ type: 'stdout' | 'stderr', text?: string, buffer?: string }>; + onListChanged: Event; + onTestFilesChanged: Event<{ testFiles: string[] }>; + onLoadTraceRequested: Event<{ traceUrl: string }>; +} + +export interface TestServerInterfaceEventEmitters { + dispatchEvent(event: 'close', params: {}): void; + dispatchEvent(event: 'report', params: any): void; + dispatchEvent(event: 'stdio', params: { type: 'stdout' | 'stderr', text?: string, buffer?: string }): void; + dispatchEvent(event: 'listChanged', params: {}): void; + dispatchEvent(event: 'testFilesChanged', params: { testFiles: string[] }): void; + dispatchEvent(event: 'loadTraceRequested', params: { traceUrl: string }): void; +} diff --git a/src/upstream/testTree.ts b/src/upstream/testTree.ts index 7527cf668..b1bb78e30 100644 --- a/src/upstream/testTree.ts +++ b/src/upstream/testTree.ts @@ -19,7 +19,7 @@ */ export type TestItemStatus = 'none' | 'running' | 'scheduled' | 'passed' | 'failed' | 'skipped'; -import type * as reporterTypes from '../reporter'; +import type * as reporterTypes from './reporter'; export type TreeItemBase = { kind: 'root' | 'group' | 'case' | 'test', @@ -314,12 +314,6 @@ export class TestTree { visit(treeItem); return testIds; } - - locationToOpen(treeItem?: TreeItem) { - if (!treeItem) - return; - return treeItem.location.file + ':' + treeItem.location.line; - } } export function sortAndPropagateStatus(treeItem: TreeItem) { diff --git a/tests/global-errors.spec.ts b/tests/global-errors.spec.ts index 11bc6ab6a..749d2d388 100644 --- a/tests/global-errors.spec.ts +++ b/tests/global-errors.spec.ts @@ -32,8 +32,7 @@ test('should report duplicate test title', async ({ activate }) => { - tests - test.spec.ts `); - const diagnostics = vscode.languages.getDiagnostics(); - expect(diagnostics).toEqual([ + await expect.poll(() => vscode.languages.getDiagnostics()).toEqual([ { message: 'Error: duplicate test title \"one\", first declared in test.spec.ts:3', range: { start: { line: 4, character: 10 }, end: { line: 5, character: 0 } }, diff --git a/tests/list-files.spec.ts b/tests/list-files.spec.ts index 81ccb1ef9..4f1b23361 100644 --- a/tests/list-files.spec.ts +++ b/tests/list-files.spec.ts @@ -375,17 +375,25 @@ test('should ignore errors when listing files', async ({ activate }) => { `, }); + await enableConfigs(vscode, ['playwright.config.ts', 'playwright.config.js']); + await expect(testController).toHaveTestTree(` - tests - test.spec.ts `); - await enableConfigs(vscode, ['playwright.config.ts', 'playwright.config.js']); - expect(vscode).toHaveExecLog(` > playwright list-files -c playwright.config.js > playwright list-files -c playwright.config.ts `); - await expect.poll(() => vscode.errors).toEqual(['There are errors in Playwright configuration files.']); + await new Promise(f => setTimeout(f, 2000)); + await expect.poll(() => vscode.languages.getDiagnostics()).toEqual([ + { + message: 'Error: oh my', + range: { start: { line: 0, character: 6 }, end: { line: 1, character: 0 } }, + severity: 'Error', + source: 'playwright', + } + ]); }); diff --git a/tests/mock/vscode.ts b/tests/mock/vscode.ts index 95d1ff7be..653ec0a56 100644 --- a/tests/mock/vscode.ts +++ b/tests/mock/vscode.ts @@ -17,7 +17,7 @@ import fs from 'fs'; import glob from 'glob'; import path from 'path'; -import { Disposable, EventEmitter, Event } from './events'; +import { Disposable, EventEmitter, Event } from '../../src/upstream/events'; import minimatch from 'minimatch'; import { spawn } from 'child_process'; import which from 'which'; diff --git a/tests/run-tests.spec.ts b/tests/run-tests.spec.ts index b78b7b3f4..20f786099 100644 --- a/tests/run-tests.spec.ts +++ b/tests/run-tests.spec.ts @@ -484,7 +484,8 @@ test('should stop', async ({ activate, showBrowser }) => { await runPromise; }); -test('should tear down on stop', async ({ activate }) => { +test('should tear down on stop', async ({ activate, useTestServer }) => { + test.skip(useTestServer, 'New world will not teardown on stop'); const { testController } = await activate({ 'playwright.config.js': `module.exports = { testDir: 'tests',