From 4e6e9fb57917b15958a833ebfe7dd2de11448832 Mon Sep 17 00:00:00 2001 From: Egor Blinov Date: Thu, 6 Jun 2019 16:28:55 +0300 Subject: [PATCH] Multiroot improvements (#443) * use Map to store jest instances * remove enbledWorkspaceFolders, update README, handle disabledWorkspaceFolders change on the fly * review fixes * Fix typo --- README.md | 4 +- package.json | 7 -- src/DebugCodeLens/DebugCodeLensProvider.ts | 2 +- src/JestExt.ts | 2 +- src/Settings/index.ts | 1 - src/extensionManager.ts | 79 ++++++++++++---------- tests/extensionManager.test.ts | 60 +++++++++++++++- 7 files changed, 106 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index e315eb5e3..c101f8a26 100644 --- a/README.md +++ b/README.md @@ -140,8 +140,8 @@ These are the [activation events](https://code.visualstudio.com/docs/extensionAP These are the things that will trigger the extension loading. If one of these applies, and you're not seeing the "Jest" in the bottom bar, reference the self-diagnosis below ### use extension in multiroot environment -vscode-jest supports multiroot feature, but if not all roots need jest, you can tune extension with `jest.enabledWorkspaceFolders` or `jest.disabledWorkspaceFolders` settings. -Both are arrays of strings with folder names. +vscode-jest supports multiroot feature, but if you want to turn it off for some workspace folders check out `jest.disabledWorkspaceFolders` configuration setting. +`jest.disabledWorkspaceFolders` is an array of strings with folder names. ### non-standard environments vscode-jest supports common jest configuration, such as when jest is in `root/node_modules/.bin/jest`, or for react-native `root/node_modules/react-native-scripts`. diff --git a/package.json b/package.json index 409f5d5f8..1870d030f 100644 --- a/package.json +++ b/package.json @@ -138,13 +138,6 @@ "default": false, "scope": "resource" }, - "jest.enabledWorkspaceFolders": { - "description": "Enabled workspace folders names in multiroot environment", - "type": "array", - "items": "string", - "default": [], - "scope": "window" - }, "jest.disabledWorkspaceFolders": { "description": "Disabled workspace folders names in multiroot environment", "type": "array", diff --git a/src/DebugCodeLens/DebugCodeLensProvider.ts b/src/DebugCodeLens/DebugCodeLensProvider.ts index 3417005dd..fdbc15266 100644 --- a/src/DebugCodeLens/DebugCodeLensProvider.ts +++ b/src/DebugCodeLens/DebugCodeLensProvider.ts @@ -12,7 +12,7 @@ export class DebugCodeLensProvider implements vscode.CodeLensProvider { private getJestExt: (uri: vscode.Uri) => JestExt onDidChange: vscode.EventEmitter - constructor(getJestExt: (uri: vscode.Uri) => JestExt, showWhenTestStateIn: TestState[]) { + constructor(getJestExt: (uri: vscode.Uri) => JestExt, showWhenTestStateIn: TestState[] = []) { this.getJestExt = getJestExt this._showWhenTestStateIn = showWhenTestStateIn this.onDidChange = new vscode.EventEmitter() diff --git a/src/JestExt.ts b/src/JestExt.ts index 22c096618..642497607 100644 --- a/src/JestExt.ts +++ b/src/JestExt.ts @@ -194,7 +194,7 @@ export class JestExt { if (!jestProcess.stopRequested()) { let msg = `Starting Jest in Watch mode failed too many times and has been stopped.` if (this.instanceSettings.multirootEnv) { - msg += `\nConsider either add this workspace folder to disabledWorkspaceFolders setting or add needed folders to enabledWorkspaceFolders` + msg += `\nConsider add this workspace folder to disabledWorkspaceFolders` } this.channel.appendLine(`${msg}\n see troubleshooting: ${messaging.TroubleShootingURL}`) this.channel.show(true) diff --git a/src/Settings/index.ts b/src/Settings/index.ts index b849135ff..17de6184f 100644 --- a/src/Settings/index.ts +++ b/src/Settings/index.ts @@ -20,7 +20,6 @@ export interface IPluginWindowSettings { showWhenTestStateIn: TestState[] } enableSnapshotPreviews?: boolean - enabledWorkspaceFolders: string[] disabledWorkspaceFolders: string[] } diff --git a/src/extensionManager.ts b/src/extensionManager.ts index 4b9efea72..00940b7bf 100644 --- a/src/extensionManager.ts +++ b/src/extensionManager.ts @@ -8,7 +8,7 @@ import { DebugConfigurationProvider } from './DebugConfigurationProvider' import { IPluginResourceSettings, IPluginWindowSettings } from './Settings' export class ExtensionManager { - private extByWorkspace: { [key: string]: JestExt } = {} + private extByWorkspace: Map = new Map() private context: vscode.ExtensionContext private commonPluginSettings: IPluginWindowSettings debugCodeLensProvider: DebugCodeLensProvider @@ -16,16 +16,16 @@ export class ExtensionManager { constructor(context: vscode.ExtensionContext) { this.context = context - - this.commonPluginSettings = getExtensionWindowSettings() - - this.debugCodeLensProvider = new DebugCodeLensProvider( - uri => this.getByDocUri(uri), - this.commonPluginSettings.debugCodeLens.enabled ? this.commonPluginSettings.debugCodeLens.showWhenTestStateIn : [] - ) this.debugConfigurationProvider = new DebugConfigurationProvider() - - vscode.workspace.workspaceFolders.forEach(this.register, this) + this.debugCodeLensProvider = new DebugCodeLensProvider(uri => this.getByDocUri(uri)) + this.applySettings(getExtensionWindowSettings()) + this.registerAll() + } + applySettings(settings: IPluginWindowSettings) { + this.commonPluginSettings = settings + const { debugCodeLens } = settings + this.debugCodeLensProvider.showWhenTestStateIn = debugCodeLens.enabled ? debugCodeLens.showWhenTestStateIn : [] + settings.disabledWorkspaceFolders.forEach(this.unregisterByName, this) } register(workspaceFolder: vscode.WorkspaceFolder) { if (!this.shouldStart(workspaceFolder.name)) { @@ -53,42 +53,52 @@ export class ExtensionManager { const failDiagnostics = vscode.languages.createDiagnosticCollection(`Jest (${workspaceFolder.name})`) - this.extByWorkspace[workspaceFolder.name] = new JestExt( - this.context, - workspaceFolder, - jestWorkspace, - channel, - pluginSettings, - this.debugCodeLensProvider, - this.debugConfigurationProvider, - failDiagnostics, - instanceSettings + this.extByWorkspace.set( + workspaceFolder.name, + new JestExt( + this.context, + workspaceFolder, + jestWorkspace, + channel, + pluginSettings, + this.debugCodeLensProvider, + this.debugConfigurationProvider, + failDiagnostics, + instanceSettings + ) ) } + registerAll() { + vscode.workspace.workspaceFolders.forEach(this.register, this) + } unregister(workspaceFolder: vscode.WorkspaceFolder) { this.unregisterByName(workspaceFolder.name) } unregisterByName(name: string) { - const extension = this.extByWorkspace[name] + const extension = this.extByWorkspace.get(name) if (extension) { extension.deactivate() - delete this.extByWorkspace[name] + this.extByWorkspace.delete(name) } } unregisterAll() { - Object.keys(this.extByWorkspace).forEach(key => this.unregisterByName(key)) + const keys = this.extByWorkspace.keys() + for (const key of keys) { + this.unregisterByName(key) + } } shouldStart(workspaceFolderName: string): boolean { - const { commonPluginSettings: { enabledWorkspaceFolders, disabledWorkspaceFolders } } = this - return ( - // start if enabledWorkspaceFolders contains current workspace - enabledWorkspaceFolders.includes(workspaceFolderName) || - // dont start if disabledWorkspaceFolders doesn't contain current workspace - !disabledWorkspaceFolders.includes(workspaceFolderName) - ) + const { commonPluginSettings: { disabledWorkspaceFolders } } = this + if (this.extByWorkspace.has(workspaceFolderName)) { + return false + } + if (disabledWorkspaceFolders.includes(workspaceFolderName)) { + return false + } + return true } getByName(workspaceFolderName: string) { - return this.extByWorkspace[workspaceFolderName] + return this.extByWorkspace.get(workspaceFolderName) } getByDocUri(uri: vscode.Uri) { const workspace = vscode.workspace.getWorkspaceFolder(uri) @@ -119,10 +129,8 @@ export class ExtensionManager { } onDidChangeConfiguration(e: vscode.ConfigurationChangeEvent) { if (e.affectsConfiguration('jest')) { - const updatedSettings = getExtensionWindowSettings() - this.debugCodeLensProvider.showWhenTestStateIn = updatedSettings.debugCodeLens.enabled - ? updatedSettings.debugCodeLens.showWhenTestStateIn - : [] + this.applySettings(getExtensionWindowSettings()) + this.registerAll() } vscode.workspace.workspaceFolders.forEach(workspaceFolder => { const jestExt = this.getByName(workspaceFolder.name) @@ -183,7 +191,6 @@ export function getExtensionWindowSettings(): IPluginWindowSettings { showWhenTestStateIn: config.get('debugCodeLens.showWhenTestStateIn'), }, enableSnapshotPreviews: config.get('enableSnapshotPreviews'), - enabledWorkspaceFolders: config.get('enabledWorkspaceFolders'), disabledWorkspaceFolders: config.get('disabledWorkspaceFolders'), } } diff --git a/tests/extensionManager.test.ts b/tests/extensionManager.test.ts index de9fc8618..7457d6c30 100644 --- a/tests/extensionManager.test.ts +++ b/tests/extensionManager.test.ts @@ -53,6 +53,7 @@ import * as vscode from 'vscode' import { ExtensionManager, getExtensionResourceSettings, getExtensionWindowSettings } from '../src/extensionManager' import { TestState } from '../src/DebugCodeLens' import { readFileSync } from 'fs' +import { IPluginWindowSettings } from '../src/Settings' describe('InstancesManager', () => { let extensionManager: ExtensionManager @@ -79,6 +80,51 @@ describe('InstancesManager', () => { }) }) + describe('applySettings()', () => { + it('should save settings to instance', () => { + const newSettings: IPluginWindowSettings = { + debugCodeLens: { + enabled: true, + showWhenTestStateIn: [], + }, + disabledWorkspaceFolders: [], + } + extensionManager.applySettings(newSettings) + expect((extensionManager as any).commonPluginSettings).toEqual(newSettings) + }) + it('should update debugCodeLensProvider instance', () => { + const newSettings: IPluginWindowSettings = { + debugCodeLens: { + enabled: true, + showWhenTestStateIn: [TestState.Fail], + }, + disabledWorkspaceFolders: ['workspaceFolder1'], + } + extensionManager.applySettings(newSettings) + expect((extensionManager as any).debugCodeLensProvider.showWhenTestStateIn).toEqual( + newSettings.debugCodeLens.showWhenTestStateIn + ) + }) + + it('should respect disabledWorkspaceFolders', () => { + registerInstance('workspaceFolder1') + registerInstance('workspaceFolder2') + + expect(extensionManager.getByName('workspaceFolder1')).toBeDefined() + expect(extensionManager.getByName('workspaceFolder2')).toBeDefined() + const newSettings: IPluginWindowSettings = { + debugCodeLens: { + enabled: true, + showWhenTestStateIn: [], + }, + disabledWorkspaceFolders: ['workspaceFolder1'], + } + extensionManager.applySettings(newSettings) + expect(extensionManager.getByName('workspaceFolder1')).toBeUndefined() + expect(extensionManager.getByName('workspaceFolder2')).toBeDefined() + }) + }) + describe('register()', () => { it('should register an instance', () => { registerInstance('workspaceFolder1') @@ -115,6 +161,19 @@ describe('InstancesManager', () => { }) }) + describe('shouldStart()', () => { + it('should check whether instance already started', () => { + registerInstance('workspaceFolder1') + expect(extensionManager.shouldStart('workspaceFolder1')).toEqual(false) + expect(extensionManager.shouldStart('workspaceFolder2')).toEqual(true) + }) + it('should check if folder is in disabledFolderNames', () => { + ;(extensionManager as any).commonPluginSettings.disabledWorkspaceFolders = ['workspaceFolder2'] + expect(extensionManager.shouldStart('workspaceFolder2')).toEqual(false) + expect(extensionManager.shouldStart('workspaceFolder3')).toEqual(true) + }) + }) + describe('getByName()', () => { it('should return extension', () => { registerInstance('workspaceFolder1') @@ -321,7 +380,6 @@ describe('InstancesManager', () => { showWhenTestStateIn: [TestState.Fail, TestState.Unknown], }, enableSnapshotPreviews: true, - enabledWorkspaceFolders: [], disabledWorkspaceFolders: [], }) })