From 77f7303290e30fdb41be7a8eaf8e335ccbd46f84 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Mon, 18 Sep 2023 15:11:01 -0700 Subject: [PATCH] Remove old linter and formatter prompts and commands (#21979) --- package.json | 33 -- package.nls.json | 3 - src/client/common/application/commands.ts | 3 - src/client/common/constants.ts | 3 - .../common/installer/productInstaller.ts | 162 +--------- src/client/extensionActivation.ts | 3 - .../linters/errorHandlers/errorHandler.ts | 6 +- .../linters/errorHandlers/notInstalled.ts | 29 -- src/client/linters/errorHandlers/standard.ts | 18 ++ src/client/linters/linterCommands.ts | 114 ------- src/test/common/installer.test.ts | 12 +- .../installer.invalidPath.unit.test.ts | 5 +- .../common/installer/installer.unit.test.ts | 297 +----------------- .../installer/productInstaller.unit.test.ts | 264 +--------------- .../common/installer/productPath.unit.test.ts | 7 +- src/test/common/productsToTest.ts | 25 ++ src/test/linters/lint.multilinter.test.ts | 126 -------- src/test/linters/linterCommands.unit.test.ts | 182 ----------- 18 files changed, 64 insertions(+), 1228 deletions(-) delete mode 100644 src/client/linters/errorHandlers/notInstalled.ts delete mode 100644 src/client/linters/linterCommands.ts create mode 100644 src/test/common/productsToTest.ts delete mode 100644 src/test/linters/lint.multilinter.test.ts delete mode 100644 src/test/linters/linterCommands.unit.test.ts diff --git a/package.json b/package.json index da2a3755c3052..d98bf6aa6b0b7 100644 --- a/package.json +++ b/package.json @@ -365,11 +365,6 @@ "command": "python.createEnvironment-button", "title": "%python.command.python.createEnvironment.title%" }, - { - "category": "Python", - "command": "python.enableLinting", - "title": "%python.command.python.enableLinting.title%" - }, { "category": "Python", "command": "python.enableSourceMapSupport", @@ -431,21 +426,11 @@ "icon": "$(run-errors)", "title": "%python.command.testing.rerunFailedTests.title%" }, - { - "category": "Python", - "command": "python.runLinting", - "title": "%python.command.python.runLinting.title%" - }, { "category": "Python", "command": "python.setInterpreter", "title": "%python.command.python.setInterpreter.title%" }, - { - "category": "Python", - "command": "python.setLinter", - "title": "%python.command.python.setLinter.title%" - }, { "category": "Python Refactor", "command": "python.sortImports", @@ -1808,12 +1793,6 @@ "title": "%python.command.python.createTerminal.title%", "when": "!virtualWorkspace && shellExecutionSupported" }, - { - "category": "Python", - "command": "python.enableLinting", - "title": "%python.command.python.enableLinting.title%", - "when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python" - }, { "category": "Python", "command": "python.enableSourceMapSupport", @@ -1886,24 +1865,12 @@ "title": "%python.command.testing.rerunFailedTests.title%", "when": "!virtualWorkspace && shellExecutionSupported" }, - { - "category": "Python", - "command": "python.runLinting", - "title": "%python.command.python.runLinting.title%", - "when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python" - }, { "category": "Python", "command": "python.setInterpreter", "title": "%python.command.python.setInterpreter.title%", "when": "!virtualWorkspace && shellExecutionSupported" }, - { - "category": "Python", - "command": "python.setLinter", - "title": "%python.command.python.setLinter.title%", - "when": "!virtualWorkspace && shellExecutionSupported && editorLangId == python" - }, { "category": "Python Refactor", "command": "python.sortImports", diff --git a/package.nls.json b/package.nls.json index b8e82b150e76d..d260c98b0b5ab 100644 --- a/package.nls.json +++ b/package.nls.json @@ -18,9 +18,6 @@ "python.command.python.execSelectionInTerminal.title": "Run Selection/Line in Python Terminal", "python.command.python.execSelectionInDjangoShell.title": "Run Selection/Line in Django Shell", "python.command.python.reportIssue.title": "Report Issue...", - "python.command.python.setLinter.title": "Select Linter", - "python.command.python.enableLinting.title": "Enable/Disable Linting", - "python.command.python.runLinting.title": "Run Linting", "python.command.python.enableSourceMapSupport.title": "Enable Source Map Support For Extension Debugging", "python.command.python.clearCacheAndReload.title": "Clear Cache and Reload Window", "python.command.python.analysis.restartLanguageServer.title": "Restart Language Server", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 2a44044401012..d8944fe2b0570 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -23,8 +23,6 @@ interface ICommandNameWithoutArgumentTypeMapping { [Commands.ClearWorkspaceInterpreter]: []; [Commands.Set_Interpreter]: []; [Commands.Set_ShebangInterpreter]: []; - [Commands.Run_Linter]: []; - [Commands.Enable_Linter]: []; ['workbench.action.showCommands']: []; ['workbench.action.debug.continue']: []; ['workbench.action.debug.stepOver']: []; @@ -35,7 +33,6 @@ interface ICommandNameWithoutArgumentTypeMapping { ['editor.action.formatDocument']: []; ['editor.action.rename']: []; [Commands.ViewOutput]: []; - [Commands.Set_Linter]: []; [Commands.Start_REPL]: []; [Commands.Enable_SourceMap_Support]: []; [Commands.Exec_Selection_In_Terminal]: []; diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index bea0ef9e235c7..f18fd20bb3dc5 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -40,7 +40,6 @@ export namespace Commands { export const Create_Environment_Button = 'python.createEnvironment-button'; export const Create_Terminal = 'python.createTerminal'; export const Debug_In_Terminal = 'python.debugInTerminal'; - export const Enable_Linter = 'python.enableLinting'; export const Enable_SourceMap_Support = 'python.enableSourceMapSupport'; export const Exec_In_Terminal = 'python.execInTerminal'; export const Exec_In_Terminal_Icon = 'python.execInTerminal-icon'; @@ -56,9 +55,7 @@ export namespace Commands { export const PickLocalProcess = 'python.pickLocalProcess'; export const RefreshTensorBoard = 'python.refreshTensorBoard'; export const ReportIssue = 'python.reportIssue'; - export const Run_Linter = 'python.runLinting'; export const Set_Interpreter = 'python.setInterpreter'; - export const Set_Linter = 'python.setLinter'; export const Set_ShebangInterpreter = 'python.setShebangInterpreter'; export const Sort_Imports = 'python.sortImports'; export const Start_REPL = 'python.startREPL'; diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 526369f9e9adb..fba860aaa3839 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -6,12 +6,10 @@ import { CancellationToken, l10n, Uri } from 'vscode'; import '../extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; -import { LinterId } from '../../linters/types'; import { EnvironmentType, ModuleInstallerType, PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../application/types'; -import { Commands } from '../constants'; +import { IApplicationShell, IWorkspaceService } from '../application/types'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types'; import { IConfigurationService, @@ -22,7 +20,7 @@ import { Product, ProductType, } from '../types'; -import { Common, Linters } from '../utils/localize'; +import { Common } from '../utils/localize'; import { isResource, noop } from '../utils/misc'; import { translateProductToModule } from './moduleInstaller'; import { ProductNames } from './productNames'; @@ -225,158 +223,6 @@ abstract class BaseInstaller implements IBaseInstaller { } } -const doNotDisplayFormatterPromptStateKey = 'FORMATTER_NOT_INSTALLED_KEY'; - -export class FormatterInstaller extends BaseInstaller { - protected async promptToInstallImplementation( - product: Product, - resource?: Uri, - cancel?: CancellationToken, - _flags?: ModuleInstallFlags, - ): Promise { - const neverShowAgain = this.persistentStateFactory.createGlobalPersistentState( - doNotDisplayFormatterPromptStateKey, - false, - ); - - if (neverShowAgain.value) { - return InstallerResponse.Ignore; - } - - // Hard-coded on purpose because the UI won't necessarily work having - // another formatter. - const formatters = [Product.autopep8, Product.black, Product.yapf]; - const formatterNames = formatters.map((formatter) => ProductNames.get(formatter)!); - const productName = ProductNames.get(product)!; - formatterNames.splice(formatterNames.indexOf(productName), 1); - const useOptions = formatterNames.map((name) => l10n.t('Use {0}', name)); - const yesChoice = Common.bannerLabelYes; - - const options = [...useOptions, Common.doNotShowAgain]; - let message = l10n.t('Formatter {0} is not installed. Install?', productName); - if (this.isExecutableAModule(product, resource)) { - options.splice(0, 0, yesChoice); - } else { - const executable = this.getExecutableNameFromSettings(product, resource); - message = l10n.t('Path to the {0} formatter is invalid ({1})', productName, executable); - } - - const item = await this.appShell.showErrorMessage(message, ...options); - if (item === yesChoice) { - return this.install(product, resource, cancel); - } - - if (item === Common.doNotShowAgain) { - neverShowAgain.updateValue(true); - return InstallerResponse.Ignore; - } - - if (typeof item === 'string') { - for (const formatter of formatters) { - const formatterName = ProductNames.get(formatter)!; - - if (item.endsWith(formatterName)) { - await this.configService.updateSetting('formatting.provider', formatterName, resource); - return this.install(formatter, resource, cancel); - } - } - } - - return InstallerResponse.Ignore; - } -} - -export class LinterInstaller extends BaseInstaller { - constructor(protected serviceContainer: IServiceContainer) { - super(serviceContainer); - } - - protected async promptToInstallImplementation( - product: Product, - resource?: Uri, - cancel?: CancellationToken, - _flags?: ModuleInstallFlags, - ): Promise { - return this.oldPromptForInstallation(product, resource, cancel); - } - - /** - * For installers that want to avoid prompting the user over and over, they can make use of a - * persisted true/false value representing user responses to 'stop showing this prompt'. This method - * gets the persisted value given the installer-defined key. - * - * @param key Key to use to get a persisted response value, each installer must define this for themselves. - * @returns Boolean: The current state of the stored response key given. - */ - protected getStoredResponse(key: string): boolean { - const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createGlobalPersistentState(key, undefined); - return state.value === true; - } - - private async oldPromptForInstallation(product: Product, resource?: Uri, cancel?: CancellationToken) { - const productName = ProductNames.get(product)!; - const { install } = Common; - const { doNotShowAgain } = Common; - const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - const { selectLinter } = Linters; - - if (this.getStoredResponse(disableLinterInstallPromptKey) === true) { - return InstallerResponse.Ignore; - } - - const options = [selectLinter, doNotShowAgain]; - - let message = l10n.t('Linter {0} is not installed.', productName); - if (this.isExecutableAModule(product, resource)) { - options.splice(0, 0, install); - } else { - const executable = this.getExecutableNameFromSettings(product, resource); - message = l10n.t('Path to the {0} linter is invalid ({1})', productName, executable); - } - const response = await this.appShell.showErrorMessage(message, ...options); - if (response === install) { - sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, { - tool: productName as LinterId, - action: 'install', - }); - return this.install(product, resource, cancel); - } - if (response === doNotShowAgain) { - await this.setStoredResponse(disableLinterInstallPromptKey, true); - sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, { - tool: productName as LinterId, - action: 'disablePrompt', - }); - return InstallerResponse.Ignore; - } - - if (response === selectLinter) { - sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, { action: 'select' }); - const commandManager = this.serviceContainer.get(ICommandManager); - await commandManager.executeCommand(Commands.Set_Linter); - } - return InstallerResponse.Ignore; - } - - /** - * For installers that want to avoid prompting the user over and over, they can make use of a - * persisted true/false value representing user responses to 'stop showing this prompt'. This - * method will set that persisted value given the installer-defined key. - * - * @param key Key to use to get a persisted response value, each installer must define this for themselves. - * @param value Boolean value to store for the user - if they choose to not be prompted again for instance. - * @returns Boolean: The current state of the stored response key given. - */ - private async setStoredResponse(key: string, value: boolean): Promise { - const factory = this.serviceContainer.get(IPersistentStateFactory); - const state = factory.createGlobalPersistentState(key, undefined); - if (state && state.value !== value) { - await state.updateValue(value); - } - } -} - export class TestFrameworkInstaller extends BaseInstaller { protected async promptToInstallImplementation( product: Product, @@ -687,10 +533,6 @@ export class ProductInstaller implements IInstaller { private createInstaller(product: Product): IBaseInstaller { const productType = this.productService.getProductType(product); switch (productType) { - case ProductType.Formatter: - return new FormatterInstaller(this.serviceContainer); - case ProductType.Linter: - return new LinterInstaller(this.serviceContainer); case ProductType.TestFramework: return new TestFrameworkInstaller(this.serviceContainer); case ProductType.DataScience: diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 8dcea063676a9..c82bddef3c208 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -28,7 +28,6 @@ import { IDebugConfigurationService, IDynamicDebugConfigurationService } from '. import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry'; import { IInterpreterService } from './interpreter/contracts'; import { getLanguageConfiguration } from './language/languageConfiguration'; -import { LinterCommands } from './linters/linterCommands'; import { registerTypes as lintersRegisterTypes } from './linters/serviceRegistry'; import { PythonFormattingEditProvider } from './providers/formatProvider'; import { ReplProvider } from './providers/replProvider'; @@ -168,8 +167,6 @@ async function activateLegacy(ext: ExtensionState): Promise { serviceManager.get(ICodeExecutionManager).registerCommands(); - disposables.push(new LinterCommands(serviceManager)); - if ( pythonSettings && pythonSettings.formatting && diff --git a/src/client/linters/errorHandlers/errorHandler.ts b/src/client/linters/errorHandlers/errorHandler.ts index dc884e97739c0..af28dd61c3a40 100644 --- a/src/client/linters/errorHandlers/errorHandler.ts +++ b/src/client/linters/errorHandlers/errorHandler.ts @@ -3,17 +3,13 @@ import { ExecutionInfo, Product } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { IErrorHandler } from '../types'; import { BaseErrorHandler } from './baseErrorHandler'; -import { NotInstalledErrorHandler } from './notInstalled'; import { StandardErrorHandler } from './standard'; export class ErrorHandler implements IErrorHandler { private handler: BaseErrorHandler; constructor(product: Product, serviceContainer: IServiceContainer) { - // Create chain of handlers. - const standardErrorHandler = new StandardErrorHandler(product, serviceContainer); - this.handler = new NotInstalledErrorHandler(product, serviceContainer); - this.handler.setNextHandler(standardErrorHandler); + this.handler = new StandardErrorHandler(product, serviceContainer); } public handleError(error: Error, resource: Uri, execInfo: ExecutionInfo): Promise { diff --git a/src/client/linters/errorHandlers/notInstalled.ts b/src/client/linters/errorHandlers/notInstalled.ts deleted file mode 100644 index 8c598ae5ece2a..0000000000000 --- a/src/client/linters/errorHandlers/notInstalled.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Uri } from 'vscode'; -import { IPythonExecutionFactory } from '../../common/process/types'; -import { ExecutionInfo } from '../../common/types'; -import { traceError, traceLog, traceWarn } from '../../logging'; -import { ILinterManager } from '../types'; -import { BaseErrorHandler } from './baseErrorHandler'; - -export class NotInstalledErrorHandler extends BaseErrorHandler { - public async handleError(error: Error, resource: Uri, execInfo: ExecutionInfo): Promise { - const pythonExecutionService = await this.serviceContainer - .get(IPythonExecutionFactory) - .create({ resource }); - const isModuleInstalled = await pythonExecutionService.isModuleInstalled(execInfo.moduleName!); - if (isModuleInstalled) { - return this.nextHandler ? this.nextHandler.handleError(error, resource, execInfo) : false; - } - - this.installer - .promptToInstall(this.product, resource) - .catch((ex) => traceError('NotInstalledErrorHandler.promptToInstall', ex)); - - const linterManager = this.serviceContainer.get(ILinterManager); - const info = linterManager.getLinterInfo(execInfo.product!); - const customError = `Linter '${info.id}' is not installed. Please install it or select another linter".`; - traceLog(`\n${customError}\n${error}`); - traceWarn(customError, error); - return true; - } -} diff --git a/src/client/linters/errorHandlers/standard.ts b/src/client/linters/errorHandlers/standard.ts index f6e04b50ff19c..6367da7abe4aa 100644 --- a/src/client/linters/errorHandlers/standard.ts +++ b/src/client/linters/errorHandlers/standard.ts @@ -18,6 +18,24 @@ export class StandardErrorHandler extends BaseErrorHandler { const info = linterManager.getLinterInfo(execInfo.product!); traceError(`There was an error in running the linter ${info.id}`, error); + if (info.id === LinterId.PyLint) { + traceError('Support for "pylint" is moved to ms-python.pylint extension.'); + traceError( + 'Please install the extension from: https://marketplace.visualstudio.com/items?itemName=ms-python.pylint', + ); + } else if (info.id === LinterId.Flake8) { + traceError('Support for "flake8" is moved to ms-python.flake8 extension.'); + traceError( + 'Please install the extension from: https://marketplace.visualstudio.com/items?itemName=ms-python.flake8', + ); + } else if (info.id === LinterId.MyPy) { + traceError('Support for "mypy" is moved to ms-python.mypy-type-checker extension.'); + traceError( + 'Please install the extension from: https://marketplace.visualstudio.com/items?itemName=ms-python.mypy-type-checker', + ); + } + traceError(`If the error is due to missing ${info.id}, please install ${info.id} using pip manually.`); + traceError('Learn more here: https://aka.ms/AAlgvkb'); traceLog(`Linting with ${info.id} failed.`); traceLog(error.toString()); diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts deleted file mode 100644 index cc35e80f26b17..0000000000000 --- a/src/client/linters/linterCommands.ts +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { DiagnosticCollection, Disposable, l10n, QuickPickOptions, Uri } from 'vscode'; -import { IApplicationShell, ICommandManager, IDocumentManager } from '../common/application/types'; -import { Commands } from '../common/constants'; -import { IDisposable } from '../common/types'; -import { Common } from '../common/utils/localize'; -import { IServiceContainer } from '../ioc/types'; -import { sendTelemetryEvent } from '../telemetry'; -import { EventName } from '../telemetry/constants'; -import { ILinterManager, ILintingEngine, LinterId } from './types'; - -export class LinterCommands implements IDisposable { - private disposables: Disposable[] = []; - - private linterManager: ILinterManager; - - private readonly appShell: IApplicationShell; - - private readonly documentManager: IDocumentManager; - - constructor(private serviceContainer: IServiceContainer) { - this.linterManager = this.serviceContainer.get(ILinterManager); - this.appShell = this.serviceContainer.get(IApplicationShell); - this.documentManager = this.serviceContainer.get(IDocumentManager); - - const commandManager = this.serviceContainer.get(ICommandManager); - commandManager.registerCommand(Commands.Set_Linter, this.setLinterAsync.bind(this)); - commandManager.registerCommand(Commands.Enable_Linter, this.enableLintingAsync.bind(this)); - commandManager.registerCommand(Commands.Run_Linter, this.runLinting.bind(this)); - } - - public dispose(): void { - this.disposables.forEach((disposable) => disposable.dispose()); - } - - public async setLinterAsync(): Promise { - const linters = this.linterManager.getAllLinterInfos(); - const suggestions = linters.map((x) => x.id).sort(); - const linterList = ['Disable Linting', ...suggestions]; - const activeLinters = await this.linterManager.getActiveLinters(this.settingsUri); - - let current: string; - switch (activeLinters.length) { - case 0: - current = 'none'; - break; - case 1: - current = activeLinters[0].id; - break; - default: - current = 'multiple selected'; - break; - } - - const quickPickOptions: QuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: `current: ${current}`, - }; - - const selection = await this.appShell.showQuickPick(linterList, quickPickOptions); - if (selection !== undefined) { - if (selection === 'Disable Linting') { - await this.linterManager.enableLintingAsync(false); - sendTelemetryEvent(EventName.SELECT_LINTER, undefined, { enabled: false }); - } else { - const index = linters.findIndex((x) => x.id === selection); - if (activeLinters.length > 1) { - const response = await this.appShell.showWarningMessage( - l10n.t("Multiple linters are enabled in settings. Replace with '{0}'?", selection), - Common.bannerLabelYes, - Common.bannerLabelNo, - ); - if (response !== Common.bannerLabelYes) { - return; - } - } - await this.linterManager.setActiveLintersAsync([linters[index].product], this.settingsUri); - sendTelemetryEvent(EventName.SELECT_LINTER, undefined, { tool: selection as LinterId, enabled: true }); - } - } - } - - public async enableLintingAsync(): Promise { - const options = ['Enable', 'Disable']; - const current = (await this.linterManager.isLintingEnabled(this.settingsUri)) ? options[0] : options[1]; - - const quickPickOptions: QuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: `current: ${current}`, - }; - - const selection = await this.appShell.showQuickPick(options, quickPickOptions); - - if (selection !== undefined) { - const enable: boolean = selection === options[0]; - await this.linterManager.enableLintingAsync(enable, this.settingsUri); - } - } - - public runLinting(): Promise { - const engine = this.serviceContainer.get(ILintingEngine); - return engine.lintOpenPythonFiles('manual'); - } - - private get settingsUri(): Uri | undefined { - return this.documentManager.activeTextEditor ? this.documentManager.activeTextEditor.document.uri : undefined; - } -} diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 7ff0ee81c27f1..5c1842a2c97cb 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -87,7 +87,6 @@ import { ProductType, } from '../../client/common/types'; import { createDeferred } from '../../client/common/utils/async'; -import { getNamesAndValues } from '../../client/common/utils/enum'; import { IMultiStepInputFactory, MultiStepInputFactory } from '../../client/common/utils/multiStepInput'; import { Random } from '../../client/common/utils/random'; import { ImportTracker } from '../../client/telemetry/importTracker'; @@ -105,6 +104,7 @@ import { } from '../../client/interpreter/configuration/types'; import { PythonPathUpdaterService } from '../../client/interpreter/configuration/pythonPathUpdaterService'; import { PythonPathUpdaterServiceFactory } from '../../client/interpreter/configuration/pythonPathUpdaterServiceFactory'; +import { getProductsForInstallerTests } from './productsToTest'; suite('Installer', () => { let ioc: UnitTestIocContainer; @@ -276,7 +276,8 @@ suite('Installer', () => { await installer.isInstalled(product, resource); await checkInstalledDef.promise; } - getNamesAndValues(Product).forEach((prod) => { + + getProductsForInstallerTests().forEach((prod) => { test(`Ensure isInstalled for Product: '${prod.name}' executes the right command`, async function () { if ( new ProductService().getProductType(prod.value) === ProductType.DataScience || @@ -293,7 +294,7 @@ suite('Installer', () => { new MockModuleInstaller('two', true), ); ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); - if (prod.value === Product.unittest || prod.value === Product.isort) { + if (prod.value === Product.unittest) { return undefined; } await testCheckingIfProductIsInstalled(prod.value); @@ -316,7 +317,8 @@ suite('Installer', () => { await installer.install(product); await checkInstalledDef.promise; } - getNamesAndValues(Product).forEach((prod) => { + + getProductsForInstallerTests().forEach((prod) => { test(`Ensure install for Product: '${prod.name}' executes the right command in IModuleInstaller`, async function () { const productType = new ProductService().getProductType(prod.value); if (productType === ProductType.DataScience || productType === ProductType.Python) { @@ -331,7 +333,7 @@ suite('Installer', () => { new MockModuleInstaller('two', true), ); ioc.serviceManager.addSingletonInstance(ITerminalHelper, instance(mock(TerminalHelper))); - if (prod.value === Product.unittest || prod.value === Product.isort) { + if (prod.value === Product.unittest) { return undefined; } await testInstallingProduct(prod.value); diff --git a/src/test/common/installer/installer.invalidPath.unit.test.ts b/src/test/common/installer/installer.invalidPath.unit.test.ts index 7e8392204600a..b6738759f0d77 100644 --- a/src/test/common/installer/installer.invalidPath.unit.test.ts +++ b/src/test/common/installer/installer.invalidPath.unit.test.ts @@ -14,10 +14,10 @@ import { ProductInstaller } from '../../../client/common/installer/productInstal import { ProductService } from '../../../client/common/installer/productService'; import { IProductPathService, IProductService } from '../../../client/common/installer/types'; import { IPersistentState, IPersistentStateFactory, Product, ProductType } from '../../../client/common/types'; -import { getNamesAndValues } from '../../../client/common/utils/enum'; import { IInterpreterService } from '../../../client/interpreter/contracts'; import { IServiceContainer } from '../../../client/ioc/types'; import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; +import { getProductsForInstallerTests } from '../productsToTest'; use(chaiAsPromised); @@ -26,7 +26,7 @@ suite('Module Installer - Invalid Paths', () => { ['moduleName', path.join('users', 'dev', 'tool', 'executable')].forEach((pathToExecutable) => { const isExecutableAModule = path.basename(pathToExecutable) === pathToExecutable; - getNamesAndValues(Product).forEach((product) => { + getProductsForInstallerTests().forEach((product) => { let installer: ProductInstaller; let serviceContainer: TypeMoq.IMock; let app: TypeMoq.IMock; @@ -78,7 +78,6 @@ suite('Module Installer - Invalid Paths', () => { }); switch (product.value) { - case Product.isort: case Product.unittest: { return; } diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts index 38b9d9174472e..69a5f3678f698 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -6,24 +6,11 @@ import { assert, expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, Uri, WorkspaceFolder } from 'vscode'; -import { ApplicationShell } from '../../../client/common/application/applicationShell'; -import { CommandManager } from '../../../client/common/application/commandManager'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../client/common/application/types'; -import { WorkspaceService } from '../../../client/common/application/workspace'; -import { ConfigurationService } from '../../../client/common/configuration/service'; -import { Commands } from '../../../client/common/constants'; -import { ExperimentService } from '../../../client/common/experiments/service'; +import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types'; import '../../../client/common/extensions'; -import { - FormatterInstaller, - LinterInstaller, - ProductInstaller, -} from '../../../client/common/installer/productInstaller'; -import { ProductNames } from '../../../client/common/installer/productNames'; -import { LinterProductPathService } from '../../../client/common/installer/productPath'; +import { ProductInstaller } from '../../../client/common/installer/productInstaller'; import { ProductService } from '../../../client/common/installer/productService'; import { IInstallationChannelManager, @@ -39,9 +26,7 @@ import { IPythonExecutionService, } from '../../../client/common/process/types'; import { - IConfigurationService, IDisposableRegistry, - IExperimentService, InstallerResponse, IPersistentState, IPersistentStateFactory, @@ -49,20 +34,17 @@ import { ProductType, } from '../../../client/common/types'; import { createDeferred, Deferred } from '../../../client/common/utils/async'; -import { getNamesAndValues } from '../../../client/common/utils/enum'; import { IInterpreterService } from '../../../client/interpreter/contracts'; -import { ServiceContainer } from '../../../client/ioc/container'; import { IServiceContainer } from '../../../client/ioc/types'; -import { LinterManager } from '../../../client/linters/linterManager'; -import { ILinterManager } from '../../../client/linters/types'; import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; import { sleep } from '../../common'; +import { getProductsForInstallerTests } from '../productsToTest'; use(chaiAsPromised); suite('Module Installer only', () => { [undefined, Uri.file('resource')].forEach((resource) => { - getNamesAndValues(Product) + getProductsForInstallerTests() .concat([{ name: 'Unknown product', value: 404 }]) .forEach((product) => { @@ -183,9 +165,6 @@ suite('Module Installer only', () => { }); return; } - case Product.isort: { - return; - } case Product.unittest: { test(`Ensure resource info is passed into the module installer ${product.name} (${ resource ? 'With a resource' : 'without a resource' @@ -638,273 +617,5 @@ suite('Module Installer only', () => { workspaceService.verifyAll(); }); }); - - suite('Test FormatterInstaller.promptToInstallImplementation', () => { - class FormatterInstallerTest extends FormatterInstaller { - public async promptToInstallImplementation(product: Product, uri?: Uri): Promise { - return super.promptToInstallImplementation(product, uri); - } - - // eslint-disable-next-line class-methods-use-this - protected getStoredResponse(_key: string) { - return false; - } - - // eslint-disable-next-line class-methods-use-this - protected isExecutableAModule(_product: Product, _resource?: Uri) { - return true; - } - } - let installer: FormatterInstallerTest; - let appShell: IApplicationShell; - let configService: IConfigurationService; - let workspaceService: IWorkspaceService; - let productService: IProductService; - let cmdManager: ICommandManager; - setup(() => { - const serviceContainer = mock(ServiceContainer); - appShell = mock(ApplicationShell); - configService = mock(ConfigurationService); - workspaceService = mock(WorkspaceService); - productService = mock(ProductService); - cmdManager = mock(CommandManager); - - when(serviceContainer.get(IApplicationShell)).thenReturn(instance(appShell)); - when(serviceContainer.get(IConfigurationService)).thenReturn( - instance(configService), - ); - when(serviceContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); - when(serviceContainer.get(IProductService)).thenReturn(instance(productService)); - when(serviceContainer.get(ICommandManager)).thenReturn(instance(cmdManager)); - - installer = new FormatterInstallerTest(instance(serviceContainer)); - }); - - teardown(() => { - sinon.restore(); - }); - - test('If nothing is selected, return Ignore as response', async () => { - const product = Product.autopep8; - when( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).thenReturn((undefined as unknown) as Thenable); - - const response = await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).once(); - expect(response).to.equal(InstallerResponse.Ignore); - }); - - test('If `Yes` is selected, install product', async () => { - const product = Product.autopep8; - const install = sinon.stub(FormatterInstaller.prototype, 'install'); - install.resolves(InstallerResponse.Installed); - - when( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).thenReturn(('Yes' as unknown) as Thenable); - const response = await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).once(); - expect(response).to.equal(InstallerResponse.Installed); - assert.ok(install.calledOnceWith(product, resource, undefined)); - }); - - test('If `Use black` is selected, install black formatter', async () => { - const product = Product.autopep8; - const install = sinon.stub(FormatterInstaller.prototype, 'install'); - install.resolves(InstallerResponse.Installed); - - when( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).thenReturn(('Use black' as unknown) as Thenable); - when(configService.updateSetting('formatting.provider', 'black', resource)).thenResolve(); - - const response = await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).once(); - expect(response).to.equal(InstallerResponse.Installed); - verify(configService.updateSetting('formatting.provider', 'black', resource)).once(); - assert.ok(install.calledOnceWith(Product.black, resource, undefined)); - }); - - test('If `Use yapf` is selected, install black formatter', async () => { - const product = Product.autopep8; - const install = sinon.stub(FormatterInstaller.prototype, 'install'); - install.resolves(InstallerResponse.Installed); - - when( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).thenReturn(('Use yapf' as unknown) as Thenable); - when(configService.updateSetting('formatting.provider', 'yapf', resource)).thenResolve(); - - const response = await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage( - `Formatter autopep8 is not installed. Install?`, - 'Yes', - 'Use black', - 'Use yapf', - ), - ).once(); - expect(response).to.equal(InstallerResponse.Installed); - verify(configService.updateSetting('formatting.provider', 'yapf', resource)).once(); - assert.ok(install.calledOnceWith(Product.yapf, resource, undefined)); - }); - }); - }); -}); - -[undefined, Uri.file('resource')].forEach((resource) => { - suite(`Test LinterInstaller with resource: ${resource}`, () => { - class LinterInstallerTest extends LinterInstaller { - public isModuleExecutable = true; - - public async promptToInstallImplementation(product: Product, uri?: Uri): Promise { - return super.promptToInstallImplementation(product, uri); - } - - // eslint-disable-next-line class-methods-use-this - protected getStoredResponse(_key: string) { - return false; - } - - protected isExecutableAModule(_product: Product, _resource?: Uri) { - return this.isModuleExecutable; - } - } - - let installer: LinterInstallerTest; - let appShell: IApplicationShell; - let configService: IConfigurationService; - let workspaceService: IWorkspaceService; - let productService: IProductService; - let cmdManager: ICommandManager; - let experimentsService: IExperimentService; - let linterManager: ILinterManager; - let serviceContainer: IServiceContainer; - let productPathService: IProductPathService; - setup(() => { - serviceContainer = mock(ServiceContainer); - appShell = mock(ApplicationShell); - configService = mock(ConfigurationService); - workspaceService = mock(WorkspaceService); - productService = mock(ProductService); - cmdManager = mock(CommandManager); - experimentsService = mock(ExperimentService); - linterManager = mock(LinterManager); - productPathService = mock(LinterProductPathService); - - when(serviceContainer.get(IApplicationShell)).thenReturn(instance(appShell)); - when(serviceContainer.get(IConfigurationService)).thenReturn( - instance(configService), - ); - when(serviceContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); - when(serviceContainer.get(IProductService)).thenReturn(instance(productService)); - when(serviceContainer.get(ICommandManager)).thenReturn(instance(cmdManager)); - - const exp = instance(experimentsService); - when(serviceContainer.get(IExperimentService)).thenReturn(exp); - when(experimentsService.inExperiment(anything())).thenResolve(false); - - when(serviceContainer.get(ILinterManager)).thenReturn(instance(linterManager)); - when(serviceContainer.get(IProductPathService, ProductType.Linter)).thenReturn( - instance(productPathService), - ); - - installer = new LinterInstallerTest(instance(serviceContainer)); - }); - - teardown(() => { - sinon.restore(); - }); - - test('Ensure 3 options for pylint', async () => { - const product = Product.pylint; - const options = ['Select Linter', "Don't show again"]; - const productName = ProductNames.get(product)!; - - await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).once(); - }); - test('Ensure select linter command is invoked', async () => { - const product = Product.pylint; - const options = ['Select Linter', "Don't show again"]; - const productName = ProductNames.get(product)!; - when( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).thenResolve(('Select Linter' as unknown) as void); - when(cmdManager.executeCommand(Commands.Set_Linter)).thenResolve(undefined); - - const response = await installer.promptToInstallImplementation(product, resource); - - verify( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).once(); - verify(cmdManager.executeCommand(Commands.Set_Linter)).once(); - expect(response).to.be.equal(InstallerResponse.Ignore); - }); - test('If install button is selected, install linter and return response', async () => { - const product = Product.pylint; - const options = ['Select Linter', "Don't show again"]; - const productName = ProductNames.get(product)!; - when( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).thenResolve(('Install' as unknown) as void); - when(cmdManager.executeCommand(Commands.Set_Linter)).thenResolve(undefined); - const install = sinon.stub(LinterInstaller.prototype, 'install'); - install.resolves(InstallerResponse.Installed); - - const response = await installer.promptToInstallImplementation(product, resource); - - expect(response).to.be.equal(InstallerResponse.Installed); - assert.ok(install.calledOnceWith(product, resource, undefined)); - }); }); }); diff --git a/src/test/common/installer/productInstaller.unit.test.ts b/src/test/common/installer/productInstaller.unit.test.ts index 66e0cc0058700..2934d613f88fa 100644 --- a/src/test/common/installer/productInstaller.unit.test.ts +++ b/src/test/common/installer/productInstaller.unit.test.ts @@ -3,26 +3,15 @@ 'use strict'; -import * as assert from 'assert'; import { expect } from 'chai'; -import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; import { IApplicationShell } from '../../../client/common/application/types'; -import { DataScienceInstaller, FormatterInstaller } from '../../../client/common/installer/productInstaller'; -import { ProductNames } from '../../../client/common/installer/productNames'; -import { - IInstallationChannelManager, - IModuleInstaller, - InterpreterUri, - IProductPathService, - IProductService, -} from '../../../client/common/installer/types'; -import { InstallerResponse, IPersistentStateFactory, Product, ProductType } from '../../../client/common/types'; -import { Common } from '../../../client/common/utils/localize'; +import { DataScienceInstaller } from '../../../client/common/installer/productInstaller'; +import { IInstallationChannelManager, IModuleInstaller, InterpreterUri } from '../../../client/common/installer/types'; +import { InstallerResponse, Product } from '../../../client/common/types'; import { Architecture } from '../../../client/common/utils/platform'; import { IServiceContainer } from '../../../client/ioc/types'; import { EnvironmentType, ModuleInstallerType, PythonEnvironment } from '../../../client/pythonEnvironments/info'; -import { MockMemento } from '../../mocks/mementos'; class AlwaysInstalledDataScienceInstaller extends DataScienceInstaller { // eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this @@ -89,250 +78,3 @@ suite('DataScienceInstaller install', async () => { expect(result).to.equal(InstallerResponse.Installed, 'Should be Installed'); }); }); - -suite('Formatter installer', async () => { - let serviceContainer: TypeMoq.IMock; - // let outputChannel: TypeMoq.IMock; - let appShell: TypeMoq.IMock; - let persistentStateFactory: TypeMoq.IMock; - let productPathService: TypeMoq.IMock; - // let isExecutableAsModuleStub: sinon.SinonStub; - - // constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) { - // this.appShell = serviceContainer.get(IApplicationShell); - // this.configService = serviceContainer.get(IConfigurationService); - // this.workspaceService = serviceContainer.get(IWorkspaceService); - // this.productService = serviceContainer.get(IProductService); - // this.persistentStateFactory = serviceContainer.get(IPersistentStateFactory); - // } - - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - // outputChannel = TypeMoq.Mock.ofType(); - appShell = TypeMoq.Mock.ofType(); - persistentStateFactory = TypeMoq.Mock.ofType(); - productPathService = TypeMoq.Mock.ofType(); - - const installStub = sinon.stub(FormatterInstaller.prototype, 'install'); - installStub.returns(Promise.resolve(InstallerResponse.Installed)); - - const productService = TypeMoq.Mock.ofType(); - productService.setup((p) => p.getProductType(TypeMoq.It.isAny())).returns(() => ProductType.Formatter); - - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPersistentStateFactory))) - .returns(() => persistentStateFactory.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IProductService))).returns(() => productService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IProductPathService), ProductType.Formatter)) - .returns(() => productPathService.object); - }); - - teardown(() => { - sinon.restore(); - }); - - // - if black not installed, offer autopep8 and yapf options - // - if autopep8 not installed, offer black and yapf options - // - if yapf not installed, offer black and autopep8 options - // - if not executable as a module, display error message - // - if never show again was set to true earlier, ignore - // if never show again is selected, ignore - - test('If black is not installed, offer autopep8 and yapf as options', async () => { - const messageOptions = [ - Common.bannerLabelYes, - `Use ${ProductNames.get(Product.autopep8)!}`, - `Use ${ProductNames.get(Product.yapf)!}`, - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.bannerLabelYes)) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.isExecutableAModule(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value: false, - updateValue: () => Promise.resolve(), - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - const result = await formatterInstaller.promptToInstall(Product.black); - - appShell.verifyAll(); - productPathService.verifyAll(); - assert.strictEqual(result, InstallerResponse.Installed); - }); - - test('If autopep8 is not installed, offer black and yapf as options', async () => { - const messageOptions = [ - Common.bannerLabelYes, - - 'Use {0}'.format(ProductNames.get(Product.black)!), - 'Use {0}'.format(ProductNames.get(Product.yapf)!), - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.bannerLabelYes)) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.isExecutableAModule(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value: false, - updateValue: () => Promise.resolve(), - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - const result = await formatterInstaller.promptToInstall(Product.autopep8); - - appShell.verifyAll(); - productPathService.verifyAll(); - assert.strictEqual(result, InstallerResponse.Installed); - }); - - test('If yapf is not installed, offer autopep8 and black as options', async () => { - const messageOptions = [ - Common.bannerLabelYes, - `Use ${ProductNames.get(Product.autopep8)!}`, - `Use ${ProductNames.get(Product.black)!}`, - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.bannerLabelYes)) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.isExecutableAModule(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value: false, - updateValue: () => Promise.resolve(), - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - const result = await formatterInstaller.promptToInstall(Product.yapf); - - appShell.verifyAll(); - productPathService.verifyAll(); - assert.strictEqual(result, InstallerResponse.Installed); - }); - - test('If the formatter is not executable as a module, display an error message', async () => { - const messageOptions = [ - `Use ${ProductNames.get(Product.autopep8)!}`, - `Use ${ProductNames.get(Product.yapf)!}`, - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.bannerLabelYes)) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.isExecutableAModule(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.getExecutableNameFromSettings(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => 'foo'); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value: false, - updateValue: () => Promise.resolve(), - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - await formatterInstaller.promptToInstall(Product.black); - - appShell.verifyAll(); - productPathService.verifyAll(); - }); - - test('If "Do not show again" has been selected earlier, do not display the prompt', async () => { - const messageOptions = [ - Common.bannerLabelYes, - `Use ${ProductNames.get(Product.autopep8)!}`, - `Use ${ProductNames.get(Product.yapf)!}`, - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.bannerLabelYes)) - .verifiable(TypeMoq.Times.never()); - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value: true, - updateValue: () => Promise.resolve(), - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - const result = await formatterInstaller.promptToInstall(Product.black); - - appShell.verifyAll(); - assert.strictEqual(result, InstallerResponse.Ignore); - }); - - test('If "Do not show again" is selected, do not install the formatter and do not show the prompt again', async () => { - let value = false; - const messageOptions = [ - Common.bannerLabelYes, - `Use ${ProductNames.get(Product.autopep8)!}`, - `Use ${ProductNames.get(Product.yapf)!}`, - Common.doNotShowAgain, - ]; - - appShell - .setup((a) => a.showErrorMessage(TypeMoq.It.isAnyString(), ...messageOptions)) - .returns(() => Promise.resolve(Common.doNotShowAgain)) - .verifiable(TypeMoq.Times.once()); - productPathService - .setup((p) => p.isExecutableAModule(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - - persistentStateFactory - .setup((p) => p.createGlobalPersistentState(TypeMoq.It.isAnyString(), false)) - .returns(() => ({ - value, - updateValue: (newValue) => { - value = newValue; - return Promise.resolve(); - }, - storage: new MockMemento(), - })); - - const formatterInstaller = new FormatterInstaller(serviceContainer.object); - const result = await formatterInstaller.promptToInstall(Product.black); - const resultTwo = await formatterInstaller.promptToInstall(Product.black); - - appShell.verifyAll(); - productPathService.verifyAll(); - assert.strictEqual(result, InstallerResponse.Ignore); - assert.strictEqual(resultTwo, InstallerResponse.Ignore); - }); -}); diff --git a/src/test/common/installer/productPath.unit.test.ts b/src/test/common/installer/productPath.unit.test.ts index 1e64ca63e117a..0f627289da70d 100644 --- a/src/test/common/installer/productPath.unit.test.ts +++ b/src/test/common/installer/productPath.unit.test.ts @@ -26,18 +26,18 @@ import { Product, ProductType, } from '../../../client/common/types'; -import { getNamesAndValues } from '../../../client/common/utils/enum'; import { IFormatterHelper } from '../../../client/formatters/types'; import { IServiceContainer } from '../../../client/ioc/types'; import { ILinterInfo, ILinterManager } from '../../../client/linters/types'; import { ITestsHelper } from '../../../client/testing/common/types'; import { ITestingSettings } from '../../../client/testing/configuration/types'; +import { getProductsForInstallerTests } from '../productsToTest'; use(chaiAsPromised); suite('Product Path', () => { [undefined, Uri.file('resource')].forEach((resource) => { - getNamesAndValues(Product).forEach((product) => { + getProductsForInstallerTests().forEach((product) => { class TestBaseProductPathsService extends BaseProductPathsService { public getExecutableNameFromSettings(_: Product, _resource?: Uri): string { return ''; @@ -75,9 +75,6 @@ suite('Product Path', () => { .returns(() => new ProductService()); }); - if (product.value === Product.isort) { - return; - } suite('Method isExecutableAModule()', () => { test('Returns true if User has customized the executable name', () => { productInstaller.translateProductToModuleName = () => 'moduleName'; diff --git a/src/test/common/productsToTest.ts b/src/test/common/productsToTest.ts new file mode 100644 index 0000000000000..7fc06863f67ce --- /dev/null +++ b/src/test/common/productsToTest.ts @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Product } from '../../client/common/types'; +import { getNamesAndValues } from '../../client/common/utils/enum'; + +export function getProductsForInstallerTests(): { name: string; value: Product }[] { + return getNamesAndValues(Product).filter( + (p) => + ![ + 'pylint', + 'flake8', + 'pycodestyle', + 'pylama', + 'prospector', + 'pydocstyle', + 'yapf', + 'autopep8', + 'mypy', + 'isort', + 'black', + 'bandit', + ].includes(p.name), + ); +} diff --git a/src/test/linters/lint.multilinter.test.ts b/src/test/linters/lint.multilinter.test.ts deleted file mode 100644 index dba263e784794..0000000000000 --- a/src/test/linters/lint.multilinter.test.ts +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import * as assert from 'assert'; -import * as path from 'path'; -import { ConfigurationTarget, DiagnosticCollection, Uri, window, workspace } from 'vscode'; -import { LanguageServerType } from '../../client/activation/types'; -import { ICommandManager } from '../../client/common/application/types'; -import { Product } from '../../client/common/installer/productInstaller'; -import { PythonToolExecutionService } from '../../client/common/process/pythonToolService'; -import { ExecutionResult, IPythonToolExecutionService, SpawnOptions } from '../../client/common/process/types'; -import { ExecutionInfo, IConfigurationService } from '../../client/common/types'; -import { ILinterManager } from '../../client/linters/types'; -import { deleteFile, IExtensionTestApi, PythonSettingKeys, rootWorkspaceUri } from '../common'; -import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST } from '../initialize'; - -const workspaceUri = Uri.file(path.join(__dirname, '..', '..', '..', 'src', 'test')); -const pythonFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'linting'); - -// Mocked out python tool execution (all we need is mocked linter return values). -class MockPythonToolExecService extends PythonToolExecutionService { - // Mocked samples of linter messages from flake8 and pylint: - public flake8Msg = - '1,1,W,W391:blank line at end of file\ns:142:13), :1\n1,7,E,E999:SyntaxError: invalid syntax\n'; - - public pylintMsg = `[ - { - "type": "error", - "module": "print", - "obj": "", - "line": 1, - "column": 0, - "path": "print.py", - "symbol": "syntax-error", - "message": "Missing parentheses in call to 'print'. Did you mean print(x)? (, line 1)", - "message-id": "E0001" - } -]`; - - // Depending on moduleName being exec'd, return the appropriate sample. - public async execForLinter( - executionInfo: ExecutionInfo, - _options: SpawnOptions, - _resource: Uri, - ): Promise> { - let msg = this.flake8Msg; - if (executionInfo.moduleName === 'pylint') { - msg = this.pylintMsg; - } - return { stdout: msg }; - } -} - -suite('Linting - Multiple Linters Enabled Test', () => { - let api: IExtensionTestApi; - let configService: IConfigurationService; - let linterManager: ILinterManager; - - suiteSetup(async () => { - api = await initialize(); - configService = api.serviceContainer.get(IConfigurationService); - linterManager = api.serviceContainer.get(ILinterManager); - }); - setup(async () => { - await initializeTest(); - await resetSettings(); - - // We only want to return some valid strings from linters, we don't care if they - // are being returned by actual linters (we aren't testing linters here, only how - // our code responds to those linters). - api.serviceManager.rebind(IPythonToolExecutionService, MockPythonToolExecService); - }); - suiteTeardown(closeActiveWindows); - teardown(async () => { - await closeActiveWindows(); - await resetSettings(); - await deleteFile(path.join(workspaceUri.fsPath, '.pylintrc')); - await deleteFile(path.join(workspaceUri.fsPath, '.pydocstyle')); - - // Restore the execution service as it was... - api.serviceManager.rebind(IPythonToolExecutionService, PythonToolExecutionService); - }); - - async function resetSettings() { - // Don't run these updates in parallel, as they are updating the same file. - const target = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace; - - await configService.updateSetting('linting.enabled', true, rootWorkspaceUri, target); - await configService.updateSetting('linting.lintOnSave', false, rootWorkspaceUri, target); - - linterManager.getAllLinterInfos().forEach(async (x) => { - await configService.updateSetting(makeSettingKey(x.product), false, rootWorkspaceUri, target); - }); - } - - function makeSettingKey(product: Product): PythonSettingKeys { - return `linting.${linterManager.getLinterInfo(product).enabledSettingName}` as PythonSettingKeys; - } - - test('Multiple linters', async () => { - await closeActiveWindows(); - const document = await workspace.openTextDocument(path.join(pythonFilesPath, 'print.py')); - await window.showTextDocument(document); - await configService.updateSetting( - 'languageServer', - LanguageServerType.Jedi, - undefined, - ConfigurationTarget.Workspace, - ); - await configService.updateSetting('linting.enabled', true, workspaceUri); - await configService.updateSetting('linting.pylintEnabled', true, workspaceUri); - await configService.updateSetting('linting.flake8Enabled', true, workspaceUri); - - const commands = api.serviceContainer.get(ICommandManager); - - const collection = (await commands.executeCommand('python.runLinting')) as DiagnosticCollection; - assert.notStrictEqual(collection, undefined, 'python.runLinting did not return valid diagnostics collection.'); - - const messages = collection!.get(document.uri); - assert.notStrictEqual(messages!.length, 0, 'No diagnostic messages.'); - assert.notStrictEqual(messages!.filter((x) => x.source === 'pylint').length, 0, 'No pylint messages.'); - assert.notStrictEqual(messages!.filter((x) => x.source === 'flake8').length, 0, 'No flake8 messages.'); - }); -}); diff --git a/src/test/linters/linterCommands.unit.test.ts b/src/test/linters/linterCommands.unit.test.ts deleted file mode 100644 index b3d5c4693832c..0000000000000 --- a/src/test/linters/linterCommands.unit.test.ts +++ /dev/null @@ -1,182 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import { expect } from 'chai'; -import { anything, capture, deepEqual, instance, mock, verify, when } from 'ts-mockito'; -import { DiagnosticCollection } from 'vscode'; -import { ApplicationShell } from '../../client/common/application/applicationShell'; -import { CommandManager } from '../../client/common/application/commandManager'; -import { DocumentManager } from '../../client/common/application/documentManager'; -import { IApplicationShell, ICommandManager, IDocumentManager } from '../../client/common/application/types'; -import { Commands } from '../../client/common/constants'; -import { Product } from '../../client/common/types'; -import { ServiceContainer } from '../../client/ioc/container'; -import { LinterCommands } from '../../client/linters/linterCommands'; -import { LinterManager } from '../../client/linters/linterManager'; -import { LintingEngine } from '../../client/linters/lintingEngine'; -import { ILinterInfo, ILinterManager, ILintingEngine } from '../../client/linters/types'; - -suite('Linting - Linter Commands', () => { - let linterCommands: LinterCommands; - let manager: ILinterManager; - let shell: IApplicationShell; - let docManager: IDocumentManager; - let cmdManager: ICommandManager; - let lintingEngine: ILintingEngine; - setup(() => { - const svcContainer = mock(ServiceContainer); - manager = mock(LinterManager); - shell = mock(ApplicationShell); - docManager = mock(DocumentManager); - cmdManager = mock(CommandManager); - lintingEngine = mock(LintingEngine); - when(svcContainer.get(ILinterManager)).thenReturn(instance(manager)); - when(svcContainer.get(IApplicationShell)).thenReturn(instance(shell)); - when(svcContainer.get(IDocumentManager)).thenReturn(instance(docManager)); - when(svcContainer.get(ICommandManager)).thenReturn(instance(cmdManager)); - when(svcContainer.get(ILintingEngine)).thenReturn(instance(lintingEngine)); - linterCommands = new LinterCommands(instance(svcContainer)); - }); - - test('Commands are registered', () => { - verify(cmdManager.registerCommand(Commands.Set_Linter, anything())).once(); - verify(cmdManager.registerCommand(Commands.Enable_Linter, anything())).once(); - verify(cmdManager.registerCommand(Commands.Run_Linter, anything())).once(); - }); - - test('Run Linting method will lint all open files', async () => { - when(lintingEngine.lintOpenPythonFiles('manual')).thenResolve(('Hello' as unknown) as DiagnosticCollection); - - const result = await linterCommands.runLinting(); - - expect(result).to.be.equal('Hello'); - }); - - async function testEnableLintingWithCurrentState( - currentState: boolean, - selectedState: 'Enable' | 'Disable' | undefined, - ) { - when(manager.isLintingEnabled(anything())).thenResolve(currentState); - const expectedQuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: `current: ${currentState ? 'Enable' : 'Disable'}`, - }; - when(shell.showQuickPick(anything(), anything())).thenReturn(Promise.resolve(selectedState)); - - await linterCommands.enableLintingAsync(); - - verify(shell.showQuickPick(anything(), anything())).once(); - const options = capture(shell.showQuickPick).last()[0]; - const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(options).to.deep.equal(['Enable', 'Disable']); - expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); - - if (selectedState) { - verify(manager.enableLintingAsync(selectedState === 'Enable', anything())).once(); - } else { - verify(manager.enableLintingAsync(anything(), anything())).never(); - } - } - test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select nothing", async () => { - await testEnableLintingWithCurrentState(true, undefined); - }); - - test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select 'Enable'", async () => { - await testEnableLintingWithCurrentState(true, 'Enable'); - }); - - test("Enable linting should check if linting is enabled, and display current state of 'Enable' and select 'Disable'", async () => { - await testEnableLintingWithCurrentState(true, 'Disable'); - }); - - test("Enable linting should check if linting is enabled, and display current state of 'Disable' and select 'Enable'", async () => { - await testEnableLintingWithCurrentState(true, 'Enable'); - }); - - test("Enable linting should check if linting is enabled, and display current state of 'Disable' and select 'Disable'", async () => { - await testEnableLintingWithCurrentState(true, 'Disable'); - }); - - test('Set Linter should display a quickpick', async () => { - when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(anything())).thenResolve([]); - when(shell.showQuickPick(anything(), anything())).thenResolve(); - const expectedQuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: 'current: none', - }; - - await linterCommands.setLinterAsync(); - - verify(shell.showQuickPick(anything(), anything())); - const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); - }); - - test('Set Linter should display a quickpick and currently active linter when only one is enabled', async () => { - const linterId = 'Hello World'; - const activeLinters: ILinterInfo[] = [({ id: linterId } as unknown) as ILinterInfo]; - when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(anything())).thenResolve(activeLinters); - when(shell.showQuickPick(anything(), anything())).thenResolve(); - const expectedQuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: `current: ${linterId}`, - }; - - await linterCommands.setLinterAsync(); - - verify(shell.showQuickPick(anything(), anything())).once(); - const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); - }); - - test('Set Linter should display a quickpick and with message about multiple linters being enabled', async () => { - const activeLinters: ILinterInfo[] = ([{ id: 'linterId' }, { id: 'linterId2' }] as unknown) as ILinterInfo[]; - when(manager.getAllLinterInfos()).thenReturn([]); - when(manager.getActiveLinters(anything())).thenResolve(activeLinters); - when(shell.showQuickPick(anything(), anything())).thenResolve(); - const expectedQuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: 'current: multiple selected', - }; - - await linterCommands.setLinterAsync(); - - verify(shell.showQuickPick(anything(), anything())); - const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); - }); - - test('Selecting a linter should display warning message about multiple linters', async () => { - const linters: ILinterInfo[] = ([ - { id: '1' }, - { id: '2' }, - { id: '3', product: 'Three' }, - ] as unknown) as ILinterInfo[]; - const activeLinters: ILinterInfo[] = ([{ id: '1' }, { id: '3' }] as unknown) as ILinterInfo[]; - when(manager.getAllLinterInfos()).thenReturn(linters); - when(manager.getActiveLinters(anything())).thenResolve(activeLinters); - when(shell.showQuickPick(anything(), anything())).thenReturn(Promise.resolve('3')); - when(shell.showWarningMessage(anything(), 'Yes', 'No')).thenReturn(Promise.resolve('Yes')); - const expectedQuickPickOptions = { - matchOnDetail: true, - matchOnDescription: true, - placeHolder: 'current: multiple selected', - }; - - await linterCommands.setLinterAsync(); - - verify(shell.showQuickPick(anything(), anything())).once(); - verify(shell.showWarningMessage(anything(), 'Yes', 'No')).once(); - const quickPickOptions = capture(shell.showQuickPick).last()[1]; - expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions); - verify(manager.setActiveLintersAsync(deepEqual([('Three' as unknown) as Product]), anything())).once(); - }); -});