From ca3cdf89b3f62607bfa3df1e3a17f3408595882e Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 5 Jun 2024 10:32:15 +0200 Subject: [PATCH 1/2] fix: run tests with correct CWD --- README.md | 1 + package.json | 6 ++++ src/api.ts | 85 +++++++++++++++++++++++++++--------------------- src/config.ts | 1 + src/extension.ts | 46 ++++++++++++++++---------- 5 files changed, 84 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 8b3a894d..1e0f4a47 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ These options are resolved relative to the [workspace file](https://code.visuals `process.env` - `vitest.debugExclude`: Excludes files matching specified glob patterns from debugging. Default: `[\"/**\", \"**/node_modules/**\"]` +- `vitest.maximumConfigs`: The maximum amount of configs that Vitest extension can load. If exceeded, the extension will show a warning suggesting to use a workspace config file. Default: `3` ## FAQs (Frequently Asked Questions) diff --git a/package.json b/package.json index 59621126..639ce57a 100644 --- a/package.json +++ b/package.json @@ -135,6 +135,12 @@ "default": false, "scope": "resource" }, + "vitest.maximumConfigs": { + "description": "The maximum amount of configs that Vitest extension can load. If exceeded, the extension will show a warning suggesting to use a workspace config file.", + "type": "number", + "default": 3, + "scope": "window" + }, "vitest.debugExclude": { "markdownDescription": "Automatically skip files covered by these glob patterns.", "type": "array", diff --git a/src/api.ts b/src/api.ts index 3e8b0899..dd6a7e72 100644 --- a/src/api.ts +++ b/src/api.ts @@ -49,18 +49,21 @@ export class VitestAPI { constructor( private readonly api: VitestFolderAPI[], - private readonly meta: ResolvedMeta, ) { - meta.process.on('error', (error) => { - if (!this.disposing) - showVitestError('Vitest process failed', error) + this.processes.forEach((process) => { + process.on('error', (error) => { + if (!this.disposing) + showVitestError('Vitest process failed', error) + }) }) } onUnexpectedExit(callback: (code: number | null) => void) { - this.meta.process.on('exit', (code) => { - if (!this.disposing) - callback(code) + this.processes.forEach((process) => { + process.on('exit', (code) => { + if (!this.disposing) + callback(code) + }) }) } @@ -75,29 +78,16 @@ export class VitestAPI { async dispose() { this.disposing = true try { - this.forEach(api => api.dispose()) - this.meta.packages.forEach((pkg) => { - delete require.cache[pkg.vitestPackageJsonPath] - }) - if (!this.meta.process.closed) { - try { - await this.meta.rpc.close() - log.info('[API]', `Vitest process ${this.meta.process.id} closed successfully`) - } - catch (err) { - log.error('[API]', 'Failed to close Vitest process', err) - } - const promise = new Promise((resolve) => { - this.meta.process.once('exit', () => resolve()) - }) - this.meta.process.close() - await promise - } + await Promise.all(this.api.map(api => api.dispose())) } finally { this.disposing = false } } + + private get processes() { + return this.api.map(api => api.process) + } } const WEAKMAP_API_FOLDER = new WeakMap() @@ -116,13 +106,17 @@ export class VitestFolderAPI extends VitestReporter { } get processId() { - return this.meta.process.id + return this.process.id } get prefix() { return this.pkg.prefix } + get process() { + return this.meta.process + } + get version() { return this.pkg.version } @@ -170,9 +164,26 @@ export class VitestFolderAPI extends VitestReporter { await this.collectPromise } - dispose() { + async dispose() { WEAKMAP_API_FOLDER.delete(this) this.handlers.clearListeners() + this.meta.packages.forEach((pkg) => { + delete require.cache[pkg.vitestPackageJsonPath] + }) + if (!this.meta.process.closed) { + try { + await this.meta.rpc.close() + log.info('[API]', `Vitest process ${this.meta.process.id} closed successfully`) + } + catch (err) { + log.error('[API]', 'Failed to close Vitest process', err) + } + const promise = new Promise((resolve) => { + this.meta.process.once('exit', () => resolve()) + }) + this.meta.process.close() + await promise + } } async cancelRun() { @@ -201,11 +212,12 @@ export class VitestFolderAPI extends VitestReporter { } export async function resolveVitestAPI(showWarning: boolean, packages: VitestPackage[]) { - const vitest = await createVitestProcess(showWarning, packages) - const apis = packages.map(pkg => - new VitestFolderAPI(pkg, vitest), - ) - return new VitestAPI(apis, vitest) + const promises = packages.map(async (pkg) => { + const vitest = await createVitestProcess(showWarning, [pkg]) + return new VitestFolderAPI(pkg, vitest) + }) + const apis = await Promise.all(promises) + return new VitestAPI(apis) } export interface ResolvedMeta { @@ -243,7 +255,7 @@ async function createChildVitestProcess(showWarning: boolean, meta: VitestPackag : undefined const env = getConfig().env || {} const execPath = getConfig().nodeExecutable || await findNode(vscode.workspace.workspaceFile?.fsPath || vscode.workspace.workspaceFolders![0].uri.fsPath) - log.info('[API]', `Starting Vitest process with Node.js: ${execPath}`) + log.info('[API]', `Running Vitest: ${meta.map(x => `v${x.version} (${relative(dirname(x.cwd), x.id)})`).join(', ')} with Node.js: ${execPath}`) const vitest = fork( workerPath, { @@ -260,7 +272,7 @@ async function createChildVitestProcess(showWarning: boolean, meta: VitestPackag NODE_ENV: env.NODE_ENV ?? process.env.NODE_ENV ?? 'test', }, stdio: 'overlapped', - cwd: pnp ? dirname(pnp) : undefined, + cwd: pnp ? dirname(pnp) : meta[0].cwd, }, ) @@ -340,12 +352,11 @@ async function createChildVitestProcess(showWarning: boolean, meta: VitestPackag }) } +// TODO: packages should be a single package export async function createVitestProcess(showWarning: boolean, packages: VitestPackage[]): Promise { - log.info('[API]', `Running Vitest: ${packages.map(x => `v${x.version} (${relative(x.cwd, x.id)})`).join(', ')}`) - const vitest = await createChildVitestProcess(showWarning, packages) - log.info('[API]', `Vitest process ${vitest.pid} created`) + log.info('[API]', `Vitest ${packages.map(x => `v${x.version} (${relative(dirname(x.cwd), x.id)})`).join(', ')} process ${vitest.pid} created`) const { handlers, api } = createVitestRpc({ on: listener => vitest.on('message', listener), diff --git a/src/config.ts b/src/config.ts index b3f5a391..43a13eec 100644 --- a/src/config.ts +++ b/src/config.ts @@ -47,6 +47,7 @@ export function getConfig(workspaceFolder?: WorkspaceFolder) { workspaceConfig: resolvePath(workspaceConfig), rootConfig: resolvePath(rootConfigFile), configSearchPatternExclude, + maximumConfigs: get('maximumConfigs', 3), nodeExecutable: resolvePath(nodeExecutable), disableWorkspaceWarning: get('disableWorkspaceWarning', false), debuggerPort: get('debuggerPort') || undefined, diff --git a/src/extension.ts b/src/extension.ts index b45a703a..19722dc2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -65,24 +65,34 @@ class VitestExtension { const configFiles = vitest.filter(x => x.configFile && !x.workspaceFile) - // TODO: hard limit on the number of config files - - if (configFiles.length > 3 && configFiles.every(c => getConfig(c.folder).disableWorkspaceWarning !== true)) { - vscode.window.showWarningMessage( - `Vitest found ${configFiles.length} config files. For better performance, consider using a workspace configuration.`, - 'Create vitest.workspace.js', - 'Disable notification', - ).then((result) => { - if (result === 'Create vitest.workspace.js') - createVitestWorkspaceFile(configFiles).catch(noop) - - if (result === 'Disable notification') { - configFiles.forEach((c) => { - const rootConfig = vscode.workspace.getConfiguration('vitest', c.folder) - rootConfig.update('disableWorkspaceWarning', true) - }) - } - }) + const maximumConfigs = getConfig().maximumConfigs ?? 3 + + if (configFiles.length > maximumConfigs) { + const warningMessage = `Vitest found multiple config files. The extension will use only the first ${maximumConfigs} due to performance concerns. Consider using a workspace configuration to group your configs.` + // remove all but the first 3 + const discardedConfigs = configFiles.splice(maximumConfigs) + + if (configFiles.every(c => getConfig(c.folder).disableWorkspaceWarning !== true)) { + vscode.window.showWarningMessage( + warningMessage, + 'Create vitest.workspace.js', + 'Disable notification', + ).then((result) => { + if (result === 'Create vitest.workspace.js') + createVitestWorkspaceFile(configFiles).catch(noop) + + if (result === 'Disable notification') { + configFiles.forEach((c) => { + const rootConfig = vscode.workspace.getConfiguration('vitest', c.folder) + rootConfig.update('disableWorkspaceWarning', true) + }) + } + }) + } + else { + log.info(warningMessage) + log.info(`Discarded config files: ${discardedConfigs.map(x => x.workspaceFile || x.configFile).join(', ')}`) + } } const folders = new Set(vitest.map(x => x.folder)) From 0bbc55e498ea5a9df59e29830d4dc39272a0afb1 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 5 Jun 2024 10:37:47 +0200 Subject: [PATCH 2/2] chore: more disposes --- src/api.ts | 18 ++++++++++++++++-- src/debug/api.ts | 4 ++++ src/process.ts | 1 + 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/api.ts b/src/api.ts index dd6a7e72..de8f5f76 100644 --- a/src/api.ts +++ b/src/api.ts @@ -46,23 +46,32 @@ export class VitestReporter { export class VitestAPI { private disposing = false + private _disposes: (() => void)[] = [] constructor( private readonly api: VitestFolderAPI[], ) { this.processes.forEach((process) => { - process.on('error', (error) => { + const warn = (error: any) => { if (!this.disposing) showVitestError('Vitest process failed', error) + } + process.on('error', warn) + this._disposes.push(() => { + process.off('error', warn) }) }) } onUnexpectedExit(callback: (code: number | null) => void) { this.processes.forEach((process) => { - process.on('exit', (code) => { + const onExit = (code: number | null) => { if (!this.disposing) callback(code) + } + process.on('exit', onExit) + this._disposes.push(() => { + process.off('exit', onExit) }) }) } @@ -78,6 +87,7 @@ export class VitestAPI { async dispose() { this.disposing = true try { + this._disposes.forEach(dispose => dispose()) await Promise.all(this.api.map(api => api.dispose())) } finally { @@ -390,6 +400,10 @@ class VitestChildProvess implements VitestProcess { this.child.once(event, listener) } + off(event: string, listener: (...args: any[]) => void) { + this.child.off(event, listener) + } + close() { this.child.kill() } diff --git a/src/debug/api.ts b/src/debug/api.ts index 6cc4bcfb..931b8bb6 100644 --- a/src/debug/api.ts +++ b/src/debug/api.ts @@ -215,4 +215,8 @@ class VitestWebSocketProcess implements VitestProcess { once(event: string, listener: (...args: any[]) => void) { this.ws.once(event, listener) } + + off(event: string, listener: (...args: any[]) => void) { + this.ws.off(event, listener) + } } diff --git a/src/process.ts b/src/process.ts index 3acc0f14..acaa05bd 100644 --- a/src/process.ts +++ b/src/process.ts @@ -3,5 +3,6 @@ export interface VitestProcess { id: number closed: boolean on: (event: string, listener: (...args: any[]) => void) => void + off: (event: string, listener: (...args: any[]) => void) => void once: (event: string, listener: (...args: any[]) => void) => void }