From 1cedb1db4ea0033c7cfef0137acca0d807f49213 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Mar 2021 15:03:13 -0800 Subject: [PATCH 1/4] Remove prompt to install a linter --- news/1 Enhancements/15465.md | 1 + .../common/installer/productInstaller.ts | 252 +----------------- src/client/telemetry/constants.ts | 3 - src/client/telemetry/index.ts | 52 ---- .../common/installer/installer.unit.test.ts | 234 +--------------- src/test/linters/lint.provider.test.ts | 4 - 6 files changed, 5 insertions(+), 541 deletions(-) create mode 100644 news/1 Enhancements/15465.md diff --git a/news/1 Enhancements/15465.md b/news/1 Enhancements/15465.md new file mode 100644 index 000000000000..0826f67959d5 --- /dev/null +++ b/news/1 Enhancements/15465.md @@ -0,0 +1 @@ +Remove prompt to install a linter when none are available. diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 8f0eb08020db..30658d2feb8c 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -5,30 +5,24 @@ import { CancellationToken, OutputChannel, Uri } from 'vscode'; import '../../common/extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; -import { ILinterManager, LinterId } from '../../linters/types'; import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info'; -import { sendTelemetryEvent } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; -import { IApplicationShell, ICommandManager, IWorkspaceService } from '../application/types'; -import { Commands, STANDARD_OUTPUT_CHANNEL } from '../constants'; -import { LinterInstallationPromptVariants } from '../experiments/groups'; +import { IApplicationShell, IWorkspaceService } from '../application/types'; +import { STANDARD_OUTPUT_CHANNEL } from '../constants'; import { traceError, traceInfo } from '../logger'; import { IPlatformService } from '../platform/types'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types'; import { ITerminalServiceFactory } from '../terminal/types'; import { IConfigurationService, - IExperimentService, IInstaller, InstallerResponse, IOutputChannel, - IPersistentStateFactory, ProductInstallStatus, ModuleNamePurpose, Product, ProductType, } from '../types'; -import { Common, Installer, Linters } from '../utils/localize'; +import { Installer } from '../utils/localize'; import { isResource, noop } from '../utils/misc'; import { ProductNames } from './productNames'; import { @@ -294,244 +288,6 @@ export class FormatterInstaller extends BaseInstaller { return InstallerResponse.Ignore; } } - -export class LinterInstaller extends BaseInstaller { - // This is a hack, really we should be handling this in a service that - // controls the prompts we show. The issue here was that if we show - // a prompt to install pylint and flake8, and user selects flake8 - // we immediately show this prompt again saying install flake8, while the - // installation is on going. - private static promptSeen: boolean = false; - private readonly experimentsManager: IExperimentService; - private readonly linterManager: ILinterManager; - - constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) { - super(serviceContainer, outputChannel); - this.experimentsManager = serviceContainer.get(IExperimentService); - this.linterManager = serviceContainer.get(ILinterManager); - } - - public static reset() { - // Read notes where this is defined. - LinterInstaller.promptSeen = false; - } - - protected async promptToInstallImplementation( - product: Product, - resource?: Uri, - cancel?: CancellationToken, - ): Promise { - // This is a hack, really we should be handling this in a service that - // controls the prompts we show. The issue here was that if we show - // a prompt to install pylint and flake8, and user selects flake8 - // we immediately show this prompt again saying install flake8, while the - // installation is on going. - if (LinterInstaller.promptSeen) { - return InstallerResponse.Ignore; - } - - LinterInstaller.promptSeen = true; - - // Conditions to use experiment prompt: - // 1. There should be no linter set in any scope - // 2. The default linter should be pylint - - if (!this.isLinterSetInAnyScope() && product === Product.pylint) { - if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)) { - // We won't show a prompt, so tell the extension to treat as though user - // ignored the prompt. - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT, undefined, { - prompt: 'noPrompt', - }); - - const productName = ProductNames.get(product)!; - traceInfo(`Linter ${productName} is not installed.`); - - return InstallerResponse.Ignore; - } else if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)) { - return this.newPromptForInstallation(true, resource, cancel); - } else if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)) { - return this.newPromptForInstallation(false, resource, cancel); - } - } - - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT, undefined, { - prompt: 'old', - }); - 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 newPromptForInstallation(pylintFirst: boolean, resource?: Uri, cancel?: CancellationToken) { - const productName = ProductNames.get(Product.pylint)!; - - // User has already set to ignore this prompt - const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - if (this.getStoredResponse(disableLinterInstallPromptKey) === true) { - return InstallerResponse.Ignore; - } - - // Check if the linter settings has Pylint or flake8 pointing to executables. - // If the settings point to executables then we can't install. Defer to old Prompt. - if ( - !this.isExecutableAModule(Product.pylint, resource) || - !this.isExecutableAModule(Product.flake8, resource) - ) { - return this.oldPromptForInstallation(Product.pylint, resource, cancel); - } - - const installPylint = Linters.installPylint(); - const installFlake8 = Linters.installFlake8(); - const doNotShowAgain = Common.doNotShowAgain(); - - const options = pylintFirst - ? [installPylint, installFlake8, doNotShowAgain] - : [installFlake8, installPylint, doNotShowAgain]; - const message = Linters.installMessage(); - const prompt = pylintFirst ? 'pylintFirst' : 'flake8first'; - - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT, undefined, { - prompt, - }); - - const response = await this.appShell.showInformationMessage(message, ...options); - - if (response === installPylint) { - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT_ACTION, undefined, { - prompt, - action: 'installPylint', - }); - return this.install(Product.pylint, resource, cancel); - } else if (response === installFlake8) { - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT_ACTION, undefined, { - prompt, - action: 'installFlake8', - }); - await this.linterManager.setActiveLintersAsync([Product.flake8], resource); - return this.install(Product.flake8, resource, cancel); - } else if (response === doNotShowAgain) { - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT_ACTION, undefined, { - prompt, - action: 'disablePrompt', - }); - await this.setStoredResponse(disableLinterInstallPromptKey, true); - return InstallerResponse.Ignore; - } - - sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT_ACTION, undefined, { - prompt, - action: 'close', - }); - return InstallerResponse.Ignore; - } - - private async oldPromptForInstallation(product: Product, resource?: Uri, cancel?: CancellationToken) { - const isPylint = product === Product.pylint; - - const productName = ProductNames.get(product)!; - const install = Common.install(); - const doNotShowAgain = Common.doNotShowAgain(); - const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - const selectLinter = Linters.selectLinter(); - - if (isPylint && this.getStoredResponse(disableLinterInstallPromptKey) === true) { - return InstallerResponse.Ignore; - } - - const options = isPylint ? [selectLinter, doNotShowAgain] : [selectLinter]; - - let message = `Linter ${productName} is not installed.`; - if (this.isExecutableAModule(product, resource)) { - options.splice(0, 0, install); - } else { - const executable = this.getExecutableNameFromSettings(product, resource); - message = `Path to the ${productName} linter is invalid (${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); - } else 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; - } - - private isLinterSetInAnyScope() { - const config = this.workspaceService.getConfiguration('python'); - if (config) { - const keys = [ - 'linting.pylintEnabled', - 'linting.flake8Enabled', - 'linting.banditEnabled', - 'linting.mypyEnabled', - 'linting.pycodestyleEnabled', - 'linting.prospectorEnabled', - 'linting.pydocstyleEnabled', - 'linting.pylamaEnabled', - ]; - - const values = keys.map((key) => { - const value = config.inspect(key); - if (value) { - if (value.globalValue || value.workspaceValue || value.workspaceFolderValue) { - return 'linter set'; - } - } - return 'no info'; - }); - - return values.includes('linter set'); - } - - return false; - } - - /** - * 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, @@ -695,8 +451,6 @@ export class ProductInstaller implements IInstaller { switch (productType) { case ProductType.Formatter: return new FormatterInstaller(this.serviceContainer, this.outputChannel); - case ProductType.Linter: - return new LinterInstaller(this.serviceContainer, this.outputChannel); case ProductType.WorkspaceSymbols: return new CTagsInstaller(this.serviceContainer, this.outputChannel); case ProductType.TestFramework: diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index fe96b62ba88a..b1e46619646d 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -113,9 +113,6 @@ export enum EventName { SELECT_LINTER = 'LINTING.SELECT', - LINTER_NOT_INSTALLED_PROMPT = 'LINTER_NOT_INSTALLED_PROMPT', - LINTER_INSTALL_PROMPT = 'LINTER_INSTALL_PROMPT', - LINTER_INSTALL_PROMPT_ACTION = 'LINTER_INSTALL_PROMPT_ACTION', CONFIGURE_AVAILABLE_LINTER_PROMPT = 'CONFIGURE_AVAILABLE_LINTER_PROMPT', HASHED_PACKAGE_NAME = 'HASHED_PACKAGE_NAME', HASHED_PACKAGE_PERF = 'HASHED_PACKAGE_PERF', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 09ee984885c9..fd5420bc07c6 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -796,59 +796,7 @@ export interface IEventNamePropertyMapping { hashedName: string; }; [EventName.HASHED_PACKAGE_PERF]: never | undefined; - /** - * Telemetry event sent with details of selection in prompt - * `Prompt message` :- 'Linter ${productName} is not installed' - */ - [EventName.LINTER_NOT_INSTALLED_PROMPT]: { - /** - * Name of the linter - * - * @type {LinterId} - */ - tool?: LinterId; - /** - * `select` When 'Select linter' option is selected - * `disablePrompt` When 'Do not show again' option is selected - * `install` When 'Install' option is selected - * - * @type {('select' | 'disablePrompt' | 'install')} - */ - action: 'select' | 'disablePrompt' | 'install'; - }; - - /** - * Telemetry event sent before showing the linter prompt to install - * pylint or flake8. - */ - [EventName.LINTER_INSTALL_PROMPT]: { - /** - * Identify which prompt was shown. - * - * @type {('old' | 'noPrompt' | 'pylintFirst' | 'flake8first')} - */ - prompt: 'old' | 'noPrompt' | 'pylintFirst' | 'flake8first'; - }; - - /** - * Telemetry event sent after user had selected one of the options - * provided by the linter prompt. - */ - [EventName.LINTER_INSTALL_PROMPT_ACTION]: { - /** - * Identify which prompt was shown. - * - * @type {('pylintFirst' | 'flake8first')} - */ - prompt: 'pylintFirst' | 'flake8first'; - /** - * Which of the following actions did user select - * - * @type {('pylint' | 'flake8' | 'disablePrompt' | 'close')} - */ - action: 'installPylint' | 'installFlake8' | 'disablePrompt' | 'close'; - }; /** * Telemetry event sent when installing modules */ diff --git a/src/test/common/installer/installer.unit.test.ts b/src/test/common/installer/installer.unit.test.ts index 5be79c5a0293..a88a99b8a684 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -4,7 +4,7 @@ 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 { instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, OutputChannel, Uri, WorkspaceFolder } from 'vscode'; import { ApplicationShell } from '../../../client/common/application/applicationShell'; @@ -12,19 +12,13 @@ import { CommandManager } from '../../../client/common/application/commandManage 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 { LinterInstallationPromptVariants } from '../../../client/common/experiments/groups'; -import { ExperimentService } from '../../../client/common/experiments/service'; import '../../../client/common/extensions'; import { CTagsInstallationScript, CTagsInstaller, FormatterInstaller, - LinterInstaller, ProductInstaller, } from '../../../client/common/installer/productInstaller'; -import { ProductNames } from '../../../client/common/installer/productNames'; -import { LinterProductPathService } from '../../../client/common/installer/productPath'; import { ProductService } from '../../../client/common/installer/productService'; import { IInstallationChannelManager, @@ -44,7 +38,6 @@ import { ITerminalService, ITerminalServiceFactory } from '../../../client/commo import { IConfigurationService, IDisposableRegistry, - IExperimentService, InstallerResponse, IOutputChannel, IPersistentState, @@ -55,15 +48,11 @@ import { } from '../../../client/common/types'; import { createDeferred, Deferred } from '../../../client/common/utils/async'; import { getNamesAndValues } from '../../../client/common/utils/enum'; -import { Common, Linters } from '../../../client/common/utils/localize'; 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 { MockWorkspaceConfiguration } from '../../startPage/mockWorkspaceConfig'; use(chaiAsPromised); @@ -937,224 +926,3 @@ suite('Module Installer only', () => { }); }); }); - -[undefined, Uri.file('resource')].forEach((resource) => { - suite(`Test LinterInstaller with resource: ${resource}`, () => { - class LinterInstallerTest extends LinterInstaller { - public isModuleExecutable: boolean = true; - - public async promptToInstallImplementation(product: Product, uri?: Uri): Promise { - return super.promptToInstallImplementation(product, uri); - } - 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; - let outputChannel: TypeMoq.IMock; - 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); - outputChannel = TypeMoq.Mock.ofType(); - - 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), outputChannel.object); - }); - - teardown(() => { - sinon.restore(); - LinterInstaller.reset(); - }); - - test('Ensure 3 options for pylint', async () => { - const product = Product.pylint; - const options = ['Select Linter', 'Do not 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', 'Do not show again']; - const productName = ProductNames.get(product)!; - when( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).thenResolve('Select Linter' as any); - 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', 'Do not show again']; - const productName = ProductNames.get(product)!; - when( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).thenResolve('Install' as any); - 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)); - }); - - test('Linter should not call experiment if linter config is set', async () => { - when(workspaceService.getConfiguration('python')).thenReturn( - new MockWorkspaceConfiguration({ - 'linting.pylintEnabled': { - globalValue: true, - }, - }), - ); - - const product = Product.pylint; - const options = ['Select Linter', 'Do not show again']; - const productName = ProductNames.get(product)!; - - await installer.promptToInstallImplementation(product, resource); - - verify(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).never(); - verify( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).once(); - }); - - test('Do not show prompt if linter path is set', async () => { - when(workspaceService.getConfiguration('python')).thenReturn( - new MockWorkspaceConfiguration({ - 'linting.pylintPath': { - globalValue: 'path/to/something', - }, - }), - ); - when(productService.getProductType(Product.pylint)).thenReturn(ProductType.Linter); - when(productPathService.getExecutableNameFromSettings(Product.pylint, resource)).thenReturn( - 'path/to/something', - ); - when(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).thenResolve(true); - installer.isModuleExecutable = false; - - const product = Product.pylint; - const options = ['Select Linter', 'Do not show again']; - const productName = ProductNames.get(product)!; - await installer.promptToInstallImplementation(product, resource); - verify(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).once(); - verify( - appShell.showInformationMessage( - Linters.installMessage(), - Linters.installPylint(), - Linters.installFlake8(), - Common.doNotShowAgain(), - ), - ).never(); - verify( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).never(); - verify( - appShell.showErrorMessage( - `Path to the ${productName} linter is invalid (path/to/something)`, - options[0], - options[1], - ), - ).once(); - }); - - test('No-Prompt Experiment: Linter should not show any prompt', async () => { - const product = Product.pylint; - const options = ['Select Linter', 'Do not show again']; - const productName = ProductNames.get(product)!; - when(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).thenResolve(true); - - const response = await installer.promptToInstallImplementation(product, resource); - - verify(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).once(); - verify( - appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]), - ).never(); - expect(response).to.be.equal(InstallerResponse.Ignore); - }); - - test('pylint first Experiment: Linter should install pylint first and install flake8 next', async () => { - const product = Product.pylint; - when(experimentsService.inExperiment(LinterInstallationPromptVariants.pylintFirst)).thenResolve(true); - - await installer.promptToInstallImplementation(product, resource); - - verify(experimentsService.inExperiment(LinterInstallationPromptVariants.pylintFirst)).once(); - verify( - appShell.showInformationMessage( - Linters.installMessage(), - Linters.installPylint(), - Linters.installFlake8(), - Common.doNotShowAgain(), - ), - ).once(); - }); - - test('flake8 first Experiment: Linter should install flake8 first and install pylint next', async () => { - const product = Product.pylint; - when(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).thenResolve(true); - - await installer.promptToInstallImplementation(product, resource); - - verify(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).once(); - verify( - appShell.showInformationMessage( - Linters.installMessage(), - Linters.installFlake8(), - Linters.installPylint(), - Common.doNotShowAgain(), - ), - ).once(); - }); - }); -}); diff --git a/src/test/linters/lint.provider.test.ts b/src/test/linters/lint.provider.test.ts index 884c45c8b8ba..432b9290edf5 100644 --- a/src/test/linters/lint.provider.test.ts +++ b/src/test/linters/lint.provider.test.ts @@ -13,7 +13,6 @@ import { IFileSystem } from '../../client/common/platform/types'; import { GLOBAL_MEMENTO, IConfigurationService, - IInstaller, ILintingSettings, IMemento, IPersistentStateFactory, @@ -49,7 +48,6 @@ suite('Linting - Provider', () => { let document: TypeMoq.IMock; let fs: TypeMoq.IMock; let appShell: TypeMoq.IMock; - let linterInstaller: TypeMoq.IMock; let workspaceService: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; @@ -89,7 +87,6 @@ suite('Linting - Provider', () => { serviceManager.addSingletonInstance(IConfigurationService, configService.object); appShell = TypeMoq.Mock.ofType(); - linterInstaller = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); workspaceConfig = TypeMoq.Mock.ofType(); @@ -99,7 +96,6 @@ suite('Linting - Provider', () => { workspaceService.setup((w) => w.getConfiguration('python')).returns(() => workspaceConfig.object); serviceManager.addSingletonInstance(IApplicationShell, appShell.object); - serviceManager.addSingletonInstance(IInstaller, linterInstaller.object); serviceManager.addSingletonInstance(IWorkspaceService, workspaceService.object); serviceManager.add(IAvailableLinterActivator, AvailableLinterActivator); serviceManager.addSingleton( From 121e8259a55ad412cc4124a99542e15852650056 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 11 Mar 2021 15:09:22 -0800 Subject: [PATCH 2/4] Forgot to remove the experiment group lol --- src/client/common/experiments/groups.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 5b61fb4beb19..6a5c717bc4eb 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -53,13 +53,6 @@ export enum NativeTensorBoard { experiment = 'pythonTensorboardExperiment', } -// Experiment to show a prompt asking users to install or select linter -export enum LinterInstallationPromptVariants { - pylintFirst = 'pythonInstallPylintButtonFirst', - flake8First = 'pythonInstallFlake8ButtonFirst', - noPrompt = 'pythonNotDisplayLinterPrompt', -} - // Experiment to control which environment discovery mechanism can be used export enum DiscoveryVariants { discoverWithFileWatching = 'pythonDiscoveryModule', From 1c778d9a1dddcf1634cb4ccda84d9f7f86fcc01e Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 12 Mar 2021 09:41:05 -0800 Subject: [PATCH 3/4] Skip installing linter in single-workspace tests --- src/test/common/installer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 372608851a24..efcba7a81779 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -360,7 +360,7 @@ suite('Installer', () => { getNamesAndValues(Product).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) { + if (productType === ProductType.DataScience || productType === ProductType.Linter) { return this.skip(); } ioc.serviceManager.addSingletonInstance( From c7e2f171eb4bac8e2e73f65f9e65803d38ce7077 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 12 Mar 2021 09:48:03 -0800 Subject: [PATCH 4/4] Forgot the other test lol --- src/test/common/installer.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index efcba7a81779..56c181220231 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -323,9 +323,11 @@ suite('Installer', () => { } getNamesAndValues(Product).forEach((prod) => { test(`Ensure isInstalled for Product: '${prod.name}' executes the right command`, async function () { - if (new ProductService().getProductType(prod.value) === ProductType.DataScience) { + const productType = new ProductService().getProductType(prod.value); + if (productType === ProductType.DataScience || productType === ProductType.Linter) { return this.skip(); } + ioc.serviceManager.addSingletonInstance( IModuleInstaller, new MockModuleInstaller('one', false),