diff --git a/CHANGELOG.md b/CHANGELOG.md index aa3b6c6f05d5..486e272292fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,70 @@ # Changelog +## 2020.11.2 (19 November 2020) + +### Enhancements + +1. Put linter prompt behind an experiment flag. + ([#14760](https://github.com/Microsoft/vscode-python/issues/14760)) + +### Thanks + +Thanks to the following projects which we fully rely on to provide some of +our features: + +- [debugpy](https://pypi.org/project/debugpy/) +- [isort](https://pypi.org/project/isort/) +- [jedi](https://pypi.org/project/jedi/) + and [parso](https://pypi.org/project/parso/) +- [Microsoft Python Language Server](https://github.com/microsoft/python-language-server) +- [Pylance](https://github.com/microsoft/pylance-release) +- [exuberant ctags](http://ctags.sourceforge.net/) (user-installed) +- [rope](https://pypi.org/project/rope/) (user-installed) + +Also thanks to the various projects we provide integrations with which help +make this extension useful: + +- Debugging support: + [Django](https://pypi.org/project/Django/), + [Flask](https://pypi.org/project/Flask/), + [gevent](https://pypi.org/project/gevent/), + [Jinja](https://pypi.org/project/Jinja/), + [Pyramid](https://pypi.org/project/pyramid/), + [PySpark](https://pypi.org/project/pyspark/), + [Scrapy](https://pypi.org/project/Scrapy/), + [Watson](https://pypi.org/project/Watson/) +- Formatting: + [autopep8](https://pypi.org/project/autopep8/), + [black](https://pypi.org/project/black/), + [yapf](https://pypi.org/project/yapf/) +- Interpreter support: + [conda](https://conda.io/), + [direnv](https://direnv.net/), + [pipenv](https://pypi.org/project/pipenv/), + [pyenv](https://github.com/pyenv/pyenv), + [venv](https://docs.python.org/3/library/venv.html#module-venv), + [virtualenv](https://pypi.org/project/virtualenv/) +- Linting: + [bandit](https://pypi.org/project/bandit/), + [flake8](https://pypi.org/project/flake8/), + [mypy](https://pypi.org/project/mypy/), + [prospector](https://pypi.org/project/prospector/), + [pylint](https://pypi.org/project/pylint/), + [pydocstyle](https://pypi.org/project/pydocstyle/), + [pylama](https://pypi.org/project/pylama/) +- Testing: + [nose](https://pypi.org/project/nose/), + [pytest](https://pypi.org/project/pytest/), + [unittest](https://docs.python.org/3/library/unittest.html#module-unittest) + +And finally thanks to the [Python](https://www.python.org/) development team and +community for creating a fantastic programming language and community to be a +part of! + ## 2020.11.1 (17 November 2020) +### Enhancements + 1. Replaced "pythonPath" debug configuration property with "python". ([#12462](https://github.com/Microsoft/vscode-python/issues/12462)) diff --git a/package-lock.json b/package-lock.json index d4f97a7cdb0b..aef232e04767 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2020.11.1", + "version": "2020.11.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index aedd3b928e7f..00e6d2258e65 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.", - "version": "2020.11.1", + "version": "2020.11.2", "featureFlags": { "usingNewInterpreterStorage": true }, diff --git a/package.nls.json b/package.nls.json index 655a8b8094fc..c7b36912e622 100644 --- a/package.nls.json +++ b/package.nls.json @@ -100,6 +100,10 @@ "Linter.enableLinter": "Enable {0}", "Linter.enablePylint": "You have a pylintrc file in your workspace. Do you want to enable pylint?", "Linter.replaceWithSelectedLinter": "Multiple linters are enabled in settings. Replace with '{0}'?", + "Linter.install": "Install a linter to get error reporting.", + "Linter.installPylint": "Install pylint", + "Linter.installFlake8": "Install flake8", + "Linter.selectLinter": "Select Linter", "Installer.noCondaOrPipInstaller": "There is no Conda or Pip installer available in the selected environment.", "Installer.noPipInstaller": "There is no Pip installer available in the selected environment.", "Installer.searchForHelp": "Search for help", diff --git a/src/client/activation/jedi/multiplexingActivator.ts b/src/client/activation/jedi/multiplexingActivator.ts index 70f5d664fc62..aec716c55c3f 100644 --- a/src/client/activation/jedi/multiplexingActivator.ts +++ b/src/client/activation/jedi/multiplexingActivator.ts @@ -1,6 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { CancellationToken, CompletionContext, @@ -13,9 +13,16 @@ import { } from 'vscode'; // tslint:disable-next-line: import-name import { IWorkspaceService } from '../../common/application/types'; +import { isTestExecution } from '../../common/constants'; import { JediLSP } from '../../common/experiments/groups'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, IExperimentService, Resource } from '../../common/types'; +import { + BANNER_NAME_PROPOSE_LS, + IConfigurationService, + IExperimentService, + IPythonExtensionBanner, + Resource +} from '../../common/types'; import { IServiceManager } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { JediExtensionActivator } from '../jedi'; @@ -38,7 +45,10 @@ export class MultiplexingJediLanguageServerActivator implements ILanguageServerA constructor( @inject(IServiceManager) private readonly manager: IServiceManager, - @inject(IExperimentService) experimentService: IExperimentService + @inject(IExperimentService) experimentService: IExperimentService, + @inject(IPythonExtensionBanner) + @named(BANNER_NAME_PROPOSE_LS) + private proposePylancePopup: IPythonExtensionBanner ) { // Check experiment service to see if using new Jedi LSP protocol this.realLanguageServerPromise = experimentService.inExperiment(JediLSP.experiment).then((inExperiment) => { @@ -56,6 +66,9 @@ export class MultiplexingJediLanguageServerActivator implements ILanguageServerA } public async start(resource: Resource, interpreter: PythonEnvironment | undefined): Promise { const realServer = await this.realLanguageServerPromise; + if (!isTestExecution()) { + this.proposePylancePopup.showBanner().ignoreErrors(); + } return realServer.start(resource, interpreter); } public activate(): void { diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index ca824afc08d8..f2ff5a655a8f 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -55,7 +55,9 @@ export enum DeprecatePythonPath { // Experiment to offer switch to Pylance language server export enum TryPylance { - experiment = 'tryPylance' + experiment = 'tryPylance', + jediPrompt1 = 'tryPylancePromptText1', + jediPrompt2 = 'tryPylancePromptText2' } // Experiment for the content of the tip being displayed on first extension launch: @@ -80,3 +82,10 @@ export enum JoinMailingListPromptVariants { export enum SendSelectionToREPL { experiment = 'pythonSendEntireLineToREPL' } + +// Experiment to show a prompt asking users to install or select linter +export enum LinterInstallationPromptVariants { + pylintFirst = 'pythonInstallPylintButtonFirst', + flake8First = 'pythonInstallFlake8ButtonFirst', + noPrompt = 'pythonNotDisplayLinterPrompt' +} diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index f26aef3f8a77..0f07402d6604 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -6,18 +6,20 @@ import { CancellationToken, OutputChannel, Uri } from 'vscode'; import '../../common/extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; -import { LinterId } from '../../linters/types'; +import { ILinterManager, LinterId } from '../../linters/types'; import { 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 { traceError } from '../logger'; +import { LinterInstallationPromptVariants } from '../experiments/groups'; +import { traceError, traceInfo } from '../logger'; import { IPlatformService } from '../platform/types'; import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types'; import { ITerminalServiceFactory } from '../terminal/types'; import { IConfigurationService, + IExperimentsManager, IInstaller, InstallerResponse, IOutputChannel, @@ -26,7 +28,7 @@ import { Product, ProductType } from '../types'; -import { Installer } from '../utils/localize'; +import { Common, Installer, Linters } from '../utils/localize'; import { isResource, noop } from '../utils/misc'; import { ProductNames } from './productNames'; import { @@ -39,14 +41,14 @@ import { export { Product } from '../types'; -export const CTagsInsllationScript = +export const CTagsInstallationScript = os.platform() === 'darwin' ? 'brew install ctags' : 'sudo apt-get install exuberant-ctags'; export abstract class BaseInstaller { private static readonly PromptPromises = new Map>(); protected readonly appShell: IApplicationShell; protected readonly configService: IConfigurationService; - private readonly workspaceService: IWorkspaceService; + protected readonly workspaceService: IWorkspaceService; private readonly productService: IProductService; constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) { @@ -168,8 +170,8 @@ export class CTagsInstaller extends BaseInstaller { .get(ITerminalServiceFactory) .getTerminalService(resource); terminalService - .sendCommand(CTagsInsllationScript, []) - .catch((ex) => traceError(`Failed to install ctags. Script sent '${CTagsInsllationScript}', ${ex}`)); + .sendCommand(CTagsInstallationScript, []) + .catch((ex) => traceError(`Failed to install ctags. Script sent '${CTagsInstallationScript}', ${ex}`)); } return InstallerResponse.Ignore; } @@ -230,24 +232,162 @@ export class FormatterInstaller extends BaseInstaller { } 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: IExperimentsManager; + private readonly linterManager: ILinterManager; + + constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) { + super(serviceContainer, outputChannel); + this.experimentsManager = serviceContainer.get(IExperimentsManager); + 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 (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 (this.experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)) { + return this.newPromptForInstallation(true, resource, cancel); + } else if (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 = 'Install'; - const disableInstallPrompt = 'Do not show again'; + const install = Common.install(); + const doNotShowAgain = Common.doNotShowAgain(); const disableLinterInstallPromptKey = `${productName}_DisableLinterInstallPrompt`; - const selectLinter = 'Select Linter'; + const selectLinter = Linters.selectLinter(); if (isPylint && this.getStoredResponse(disableLinterInstallPromptKey) === true) { return InstallerResponse.Ignore; } - const options = isPylint ? [selectLinter, disableInstallPrompt] : [selectLinter]; + const options = isPylint ? [selectLinter, doNotShowAgain] : [selectLinter]; let message = `Linter ${productName} is not installed.`; if (this.isExecutableAModule(product, resource)) { @@ -263,7 +403,7 @@ export class LinterInstaller extends BaseInstaller { action: 'install' }); return this.install(product, resource, cancel); - } else if (response === disableInstallPrompt) { + } else if (response === doNotShowAgain) { await this.setStoredResponse(disableLinterInstallPromptKey, true); sendTelemetryEvent(EventName.LINTER_NOT_INSTALLED_PROMPT, undefined, { tool: productName as LinterId, @@ -280,18 +420,34 @@ export class LinterInstaller extends BaseInstaller { 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 - * 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 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; } /** diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index cd474d1ffe6b..52ea07d25057 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -284,6 +284,11 @@ export namespace Linters { 'Linter.replaceWithSelectedLinter', "Multiple linters are enabled in settings. Replace with '{0}'?" ); + + export const installMessage = localize('Linter.install', 'Install a linter to get error reporting.'); + export const installPylint = localize('Linter.installPylint', 'Install pylint'); + export const installFlake8 = localize('Linter.installFlake8', 'Install flake8'); + export const selectLinter = localize('Linter.selectLinter', 'Select Linter'); } export namespace Installer { diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index 59096edc2271..e397748394ef 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -50,10 +50,6 @@ export class ProposePylanceBanner implements IPythonExtensionBanner { ) {} public get enabled(): boolean { - const lsType = this.configuration.getSettings().languageServer ?? LanguageServerType.Jedi; - if (lsType === LanguageServerType.Jedi || lsType === LanguageServerType.Node) { - return false; - } return this.persistentState.createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, true).value; } @@ -62,13 +58,13 @@ export class ProposePylanceBanner implements IPythonExtensionBanner { return; } - const show = await this.shouldShowBanner(); - if (!show) { + const message = await this.getPromptMessage(); + if (!message) { return; } const response = await this.appShell.showInformationMessage( - Pylance.proposePylanceMessage(), + message, Pylance.tryItNow(), Common.bannerLabelNo(), Pylance.remindMeLater() @@ -89,19 +85,36 @@ export class ProposePylanceBanner implements IPythonExtensionBanner { sendTelemetryEvent(EventName.LANGUAGE_SERVER_TRY_PYLANCE, undefined, { userAction }); } - public async shouldShowBanner(): Promise { - // Do not prompt if Pylance is already installed. - if (this.extensions.getExtension(PYLANCE_EXTENSION_ID)) { - return false; - } - // Only prompt for users in experiment. - const inExperiment = await this.experiments.inExperiment(TryPylance.experiment); - return inExperiment && this.enabled && !this.disabledInCurrentSession; - } - public async disable(): Promise { await this.persistentState .createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, false) .updateValue(false); } + + public async getPromptMessage(): Promise { + if (this.disabledInCurrentSession) { + return undefined; + } + + // Do not prompt if Pylance is already installed. + if (this.extensions.getExtension(PYLANCE_EXTENSION_ID)) { + return undefined; + } + + const lsType = this.configuration.getSettings().languageServer ?? LanguageServerType.Jedi; + + if (lsType === LanguageServerType.Jedi) { + if (await this.experiments.inExperiment(TryPylance.jediPrompt1)) { + return this.experiments.getExperimentValue(TryPylance.jediPrompt1); + } else if (await this.experiments.inExperiment(TryPylance.jediPrompt2)) { + return this.experiments.getExperimentValue(TryPylance.jediPrompt2); + } + } else if (lsType === LanguageServerType.Microsoft || lsType === LanguageServerType.None) { + if (await this.experiments.inExperiment(TryPylance.experiment)) { + return Pylance.proposePylanceMessage(); + } + } + + return undefined; + } } diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 9d9b21f48372..4ed66811e8a7 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -111,6 +111,8 @@ 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 64d8c5d68422..9fbb50d979aa 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -779,6 +779,39 @@ export interface IEventNamePropertyMapping { */ 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 86e478fc7340..573a4e69b167 100644 --- a/src/test/common/installer/installer.unit.test.ts +++ b/src/test/common/installer/installer.unit.test.ts @@ -6,7 +6,7 @@ import { assert, expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; -import { instance, mock, verify, when } from 'ts-mockito'; +import { anything, 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'; @@ -15,15 +15,18 @@ import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../ 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 { ExperimentsManager } from '../../../client/common/experiments/manager'; import '../../../client/common/extensions'; import { - CTagsInsllationScript, + 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, @@ -43,6 +46,7 @@ import { ITerminalService, ITerminalServiceFactory } from '../../../client/commo import { IConfigurationService, IDisposableRegistry, + IExperimentsManager, InstallerResponse, IOutputChannel, IPersistentState, @@ -53,11 +57,15 @@ 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); @@ -89,9 +97,6 @@ suite('Module Installer only', () => { promptDeferred = createDeferred(); serviceContainer = TypeMoq.Mock.ofType(); outputChannel = TypeMoq.Mock.ofType(); - if (new ProductService().getProductType(product.value) === ProductType.DataScience) { - return this.skip(); - } disposables = []; serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry), TypeMoq.It.isAny())) @@ -220,21 +225,21 @@ suite('Module Installer only', () => { .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) .returns(() => platformService.object); platformService.setup((p) => p.isWindows).returns(() => false); - const termianlService = TypeMoq.Mock.ofType(); + const terminalService = TypeMoq.Mock.ofType(); const terminalServiceFactory = TypeMoq.Mock.ofType(); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ITerminalServiceFactory))) .returns(() => terminalServiceFactory.object); terminalServiceFactory .setup((p) => p.getTerminalService(resource)) - .returns(() => termianlService.object); - termianlService - .setup((t) => t.sendCommand(CTagsInsllationScript, [])) + .returns(() => terminalService.object); + terminalService + .setup((t) => t.sendCommand(CTagsInstallationScript, [])) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); const response = await installer.install(product.value, resource); expect(response).to.be.equal(InstallerResponse.Ignore); - termianlService.verifyAll(); + terminalService.verifyAll(); }); test(`If platform is not Windows, for module installer ${product.name} (${ resource ? 'With a resource' : 'without a resource' @@ -244,21 +249,21 @@ suite('Module Installer only', () => { .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) .returns(() => platformService.object); platformService.setup((p) => p.isWindows).returns(() => false); - const termianlService = TypeMoq.Mock.ofType(); + const terminalService = TypeMoq.Mock.ofType(); const terminalServiceFactory = TypeMoq.Mock.ofType(); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(ITerminalServiceFactory))) .returns(() => terminalServiceFactory.object); terminalServiceFactory .setup((p) => p.getTerminalService(resource)) - .returns(() => termianlService.object); - termianlService - .setup((t) => t.sendCommand(CTagsInsllationScript, [])) + .returns(() => terminalService.object); + terminalService + .setup((t) => t.sendCommand(CTagsInstallationScript, [])) .returns(() => Promise.reject('Kaboom')) .verifiable(TypeMoq.Times.once()); const response = await installer.install(product.value, resource); expect(response).to.be.equal(InstallerResponse.Ignore); - termianlService.verifyAll(); + terminalService.verifyAll(); }); test(`If 'Yes' is selected on the install prompt for the the module installer ${ product.name @@ -792,115 +797,6 @@ suite('Module Installer only', () => { }); }); - suite('Test LinterInstaller.promptToInstallImplementation', () => { - class LinterInstallerTest extends LinterInstaller { - // tslint:disable-next-line:no-unnecessary-override - 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 true; - } - } - let installer: LinterInstallerTest; - 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); - const 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)); - - installer = new LinterInstallerTest(instance(serviceContainer), outputChannel.object); - }); - - teardown(() => { - sinon.restore(); - }); - - 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] - ) - // tslint:disable-next-line:no-any - ).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] - ) - // tslint:disable-next-line:no-any - ).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)); - }); - }); - suite('Test FormatterInstaller.promptToInstallImplementation', () => { class FormatterInstallerTest extends FormatterInstaller { // tslint:disable-next-line:no-unnecessary-override @@ -1061,3 +957,226 @@ suite('Module Installer only', () => { }); }); }); + +[undefined, Uri.file('resource')].forEach((resource) => { + suite(`Test LinterInstaller with resource: ${resource}`, () => { + class LinterInstallerTest extends LinterInstaller { + public isModuleExecutable: boolean = true; + // tslint:disable-next-line:no-unnecessary-override + 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 experimentsManager: IExperimentsManager; + 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); + experimentsManager = mock(ExperimentsManager); + 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(experimentsManager); + when(serviceContainer.get(IExperimentsManager)).thenReturn(exp); + when(experimentsManager.inExperiment(anything())).thenReturn(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]) + // tslint:disable-next-line:no-any + ).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]) + // tslint:disable-next-line:no-any + ).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(experimentsManager.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(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).thenReturn(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(experimentsManager.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(experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)).thenReturn(true); + + const response = await installer.promptToInstallImplementation(product, resource); + + verify(experimentsManager.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(experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)).thenReturn(true); + + await installer.promptToInstallImplementation(product, resource); + + verify(experimentsManager.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(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).thenReturn(true); + + await installer.promptToInstallImplementation(product, resource); + + verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).once(); + verify( + appShell.showInformationMessage( + Linters.installMessage(), + Linters.installFlake8(), + Linters.installPylint(), + Common.doNotShowAgain() + ) + ).once(); + }); + }); +}); diff --git a/src/test/startPage/mockWorkspaceConfig.ts b/src/test/startPage/mockWorkspaceConfig.ts index b449b864df6a..2bd49eaf6db5 100644 --- a/src/test/startPage/mockWorkspaceConfig.ts +++ b/src/test/startPage/mockWorkspaceConfig.ts @@ -33,7 +33,7 @@ export class MockWorkspaceConfiguration implements WorkspaceConfiguration { return this.values.has(section); } public inspect( - _section: string + section: string ): | { key: string; @@ -43,7 +43,7 @@ export class MockWorkspaceConfiguration implements WorkspaceConfiguration { workspaceFolderValue?: T | undefined; } | undefined { - return; + return this.values.get(section); } public update( section: string, diff --git a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts index 7dcfda29a19d..426a79c847f5 100644 --- a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts +++ b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts @@ -28,21 +28,38 @@ import * as Telemetry from '../../../client/telemetry'; import { EventName } from '../../../client/telemetry/constants'; interface IExperimentLsCombination { - inExperiment: boolean; + experiment?: TryPylance; lsType: LanguageServerType; shouldShowBanner: boolean; } const testData: IExperimentLsCombination[] = [ - { inExperiment: true, lsType: LanguageServerType.None, shouldShowBanner: true }, - { inExperiment: true, lsType: LanguageServerType.Microsoft, shouldShowBanner: true }, - { inExperiment: true, lsType: LanguageServerType.Node, shouldShowBanner: false }, - { inExperiment: true, lsType: LanguageServerType.Jedi, shouldShowBanner: false }, - { inExperiment: false, lsType: LanguageServerType.None, shouldShowBanner: false }, - { inExperiment: false, lsType: LanguageServerType.Microsoft, shouldShowBanner: false }, - { inExperiment: false, lsType: LanguageServerType.Node, shouldShowBanner: false }, - { inExperiment: false, lsType: LanguageServerType.Jedi, shouldShowBanner: false } + { experiment: undefined, lsType: LanguageServerType.None, shouldShowBanner: false }, + { experiment: undefined, lsType: LanguageServerType.Microsoft, shouldShowBanner: false }, + { experiment: undefined, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { experiment: undefined, lsType: LanguageServerType.Jedi, shouldShowBanner: false }, + + { experiment: TryPylance.experiment, lsType: LanguageServerType.None, shouldShowBanner: true }, + { experiment: TryPylance.experiment, lsType: LanguageServerType.Microsoft, shouldShowBanner: true }, + { experiment: TryPylance.experiment, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { experiment: TryPylance.experiment, lsType: LanguageServerType.Jedi, shouldShowBanner: false }, + + { experiment: TryPylance.jediPrompt1, lsType: LanguageServerType.None, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt1, lsType: LanguageServerType.Microsoft, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt1, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt1, lsType: LanguageServerType.Jedi, shouldShowBanner: true }, + + { experiment: TryPylance.jediPrompt2, lsType: LanguageServerType.None, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt2, lsType: LanguageServerType.Microsoft, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt2, lsType: LanguageServerType.Node, shouldShowBanner: false }, + { experiment: TryPylance.jediPrompt2, lsType: LanguageServerType.Jedi, shouldShowBanner: true } ]; +const expectedMessages = { + [TryPylance.experiment]: Pylance.proposePylanceMessage(), + [TryPylance.jediPrompt1]: 'Message for jediPrompt1', + [TryPylance.jediPrompt2]: 'Message for jediPrompt2' +}; + suite('Propose Pylance Banner', () => { let config: typemoq.IMock; let appShell: typemoq.IMock; @@ -51,7 +68,6 @@ suite('Propose Pylance Banner', () => { let sendTelemetryStub: sinon.SinonStub; let telemetryEvent: { eventName: EventName; properties: { userAction: string } } | undefined; - const message = Pylance.proposePylanceMessage(); const yes = Pylance.tryItNow(); const no = Common.bannerLabelNo(); const later = Pylance.remindMeLater(); @@ -81,43 +97,59 @@ suite('Propose Pylance Banner', () => { }); testData.forEach((t) => { - test(`${t.inExperiment ? 'In' : 'Not in'} experiment and "python.languageServer": "${t.lsType}" should ${ + test(`${t.experiment} experiment and "python.languageServer": "${t.lsType}" should ${ t.shouldShowBanner ? 'show' : 'not show' } banner`, async () => { settings.setup((x) => x.languageServer).returns(() => t.lsType); - const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.inExperiment, false); - const actual = await testBanner.shouldShowBanner(); - expect(actual).to.be.equal(t.shouldShowBanner, `shouldShowBanner() returned ${actual}`); + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.experiment, false); + const message = await testBanner.getPromptMessage(); + if (t.experiment) { + expect(message).to.be.equal( + t.shouldShowBanner ? expectedMessages[t.experiment] : undefined, + `getPromptMessage() returned ${message}` + ); + } else { + expect(message).to.be.equal(undefined, `message should be undefined`); + } }); }); testData.forEach((t) => { test(`When Pylance is installed, banner should not be shown when "python.languageServer": "${t.lsType}"`, async () => { settings.setup((x) => x.languageServer).returns(() => t.lsType); - const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.inExperiment, true); - const actual = await testBanner.shouldShowBanner(); - expect(actual).to.be.equal(false, `shouldShowBanner() returned ${actual}`); + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, t.experiment, true); + const message = await testBanner.getPromptMessage(); + expect(message).to.be.equal(undefined, `getPromptMessage() returned ${message}`); }); }); test('Do not show banner when it is disabled', async () => { + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Microsoft); appShell .setup((a) => a.showInformationMessage( - typemoq.It.isValue(message), + typemoq.It.isValue(expectedMessages[TryPylance.experiment]), typemoq.It.isValue(yes), typemoq.It.isValue(no), typemoq.It.isValue(later) ) ) .verifiable(typemoq.Times.never()); - const testBanner = preparePopup(false, appShell.object, appEnv.object, config.object, true, false); + const testBanner = preparePopup( + false, + appShell.object, + appEnv.object, + config.object, + TryPylance.experiment, + false + ); await testBanner.showBanner(); appShell.verifyAll(); }); test('Clicking No should disable the banner', async () => { + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Microsoft); appShell .setup((a) => a.showInformationMessage( - typemoq.It.isValue(message), + typemoq.It.isValue(expectedMessages[TryPylance.experiment]), typemoq.It.isValue(yes), typemoq.It.isValue(no), typemoq.It.isValue(later) @@ -127,7 +159,14 @@ suite('Propose Pylance Banner', () => { .verifiable(typemoq.Times.once()); appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.never()); - const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + const testBanner = preparePopup( + true, + appShell.object, + appEnv.object, + config.object, + TryPylance.experiment, + false + ); await testBanner.showBanner(); expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled when user clicked No'); @@ -140,10 +179,11 @@ suite('Propose Pylance Banner', () => { }); }); test('Clicking Later should disable banner in session', async () => { + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Microsoft); appShell .setup((a) => a.showInformationMessage( - typemoq.It.isValue(message), + typemoq.It.isValue(expectedMessages[TryPylance.experiment]), typemoq.It.isValue(yes), typemoq.It.isValue(no), typemoq.It.isValue(later) @@ -153,7 +193,14 @@ suite('Propose Pylance Banner', () => { .verifiable(typemoq.Times.once()); appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.never()); - const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + const testBanner = preparePopup( + true, + appShell.object, + appEnv.object, + config.object, + TryPylance.experiment, + false + ); await testBanner.showBanner(); expect(testBanner.enabled).to.be.equal( @@ -171,10 +218,11 @@ suite('Propose Pylance Banner', () => { }); }); test('Clicking Yes opens the extension marketplace entry', async () => { + settings.setup((x) => x.languageServer).returns(() => LanguageServerType.Microsoft); appShell .setup((a) => a.showInformationMessage( - typemoq.It.isValue(message), + typemoq.It.isValue(expectedMessages[TryPylance.experiment]), typemoq.It.isValue(yes), typemoq.It.isValue(no), typemoq.It.isValue(later) @@ -184,7 +232,14 @@ suite('Propose Pylance Banner', () => { .verifiable(typemoq.Times.once()); appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.once()); - const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + const testBanner = preparePopup( + true, + appShell.object, + appEnv.object, + config.object, + TryPylance.experiment, + false + ); await testBanner.showBanner(); expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled after opening store URL'); @@ -205,7 +260,7 @@ function preparePopup( appShell: IApplicationShell, appEnv: IApplicationEnvironment, config: IConfigurationService, - inExperiment: boolean, + experiment: TryPylance | undefined, pylanceInstalled: boolean ): ProposePylanceBanner { const myfactory = typemoq.Mock.ofType(); @@ -237,7 +292,12 @@ function preparePopup( }); const experiments = typemoq.Mock.ofType(); - experiments.setup((x) => x.inExperiment(TryPylance.experiment)).returns(() => Promise.resolve(inExperiment)); + Object.values(TryPylance).forEach((exp) => { + experiments.setup((x) => x.inExperiment(exp)).returns(() => Promise.resolve(exp === experiment)); + if (exp !== TryPylance.experiment) { + experiments.setup((x) => x.getExperimentValue(exp)).returns(() => Promise.resolve(expectedMessages[exp])); + } + }); const extensions = typemoq.Mock.ofType(); // tslint:disable-next-line: no-any