diff --git a/CHANGELOG.md b/CHANGELOG.md index f06f56bae852..471dec73356f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 2020.7.1 (22 July 2020) + +1. Fix language server setting when provided an invalid value, send config event more consistently. + ([#13064](https://github.com/Microsoft/vscode-python/pull/13064)) +1. Add banner for pylance, and remove old LS experiment. + ([#12817](https://github.com/microsoft/vscode-python/pull/12817)) + ## 2020.7.0 (16 July 2020) ### Enhancements diff --git a/experiments.json b/experiments.json index 992b023270b0..02d843e969bc 100644 --- a/experiments.json +++ b/experiments.json @@ -65,18 +65,6 @@ "min": 0, "max": 0 }, - { - "name": "LS - enabled", - "salt": "LS", - "min": 0, - "max": 4 - }, - { - "name": "LS - control", - "salt": "LS", - "min": 20, - "max": 24 - }, { "name": "UseTerminalToGetActivatedEnvVars - experiment", "salt": "UseTerminalToGetActivatedEnvVars", diff --git a/package-lock.json b/package-lock.json index 5bcc48fd301d..fcd4b734e2e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2020.7.0", + "version": "2020.7.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 51814c4edd55..2197d57833a6 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.7.0", + "version": "2020.7.1", "featureFlags": { "usingNewInterpreterStorage": true }, @@ -1822,7 +1822,6 @@ "default": [], "items": { "enum": [ - "LS - enabled", "AlwaysDisplayTestExplorer - experiment", "ShowExtensionSurveyPrompt - enabled", "Reload - experiment", @@ -1836,6 +1835,7 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "EnableTrustedNotebooks", + "tryPylance", "All" ] }, @@ -1847,7 +1847,6 @@ "default": [], "items": { "enum": [ - "LS - enabled", "AlwaysDisplayTestExplorer - experiment", "ShowExtensionSurveyPrompt - enabled", "Reload - experiment", @@ -1861,6 +1860,7 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "EnableTrustedNotebooks", + "tryPylance", "All" ] }, diff --git a/package.nls.json b/package.nls.json index 7a1428eeab9b..8103c7b7e653 100644 --- a/package.nls.json +++ b/package.nls.json @@ -107,6 +107,9 @@ "python.snippet.launch.django.label": "Python: Django", "python.snippet.launch.flask.label": "Python: Flask", "python.snippet.launch.pyramid.label": "Python: Pyramid Application", + "LanguageService.proposePylanceMessage": "Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.", + "LanguageService.tryItNow": "Try it now", + "LanguageService.remindMeLater": "Remind me later", "LanguageService.bannerLabelYes": "Yes, take survey now", "LanguageService.bannerLabelNo": "No, thanks", "LanguageService.lsFailedToStart": "We encountered an issue starting the Language Server. Reverting to the alternative, Jedi. Check the Python output panel for details.", diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index f825a5235dc5..872c38c81b27 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -9,12 +9,10 @@ import { LSNotSupportedDiagnosticServiceId } from '../application/diagnostics/ch import { IDiagnosticsService } from '../application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; -import { LSControl, LSEnabled } from '../common/experiments/groups'; import { traceError } from '../common/logger'; import { IConfigurationService, IDisposableRegistry, - IExperimentsManager, IOutputChannel, IPersistentStateFactory, IPythonSettings, @@ -58,8 +56,7 @@ export class LanguageServerExtensionActivationService constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory, - @inject(IExperimentsManager) private readonly abExperiments: IExperimentsManager + @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory ) { this.workspaceService = this.serviceContainer.get(IWorkspaceService); this.interpreterService = this.serviceContainer.get(IInterpreterService); @@ -175,20 +172,8 @@ export class LanguageServerExtensionActivationService * @returns `true` if user is using jedi, `false` if user is using language server */ public useJedi(): boolean { - // Check if `languageServer` setting is missing (default configuration). - if (this.isJediUsingDefaultConfiguration(this.resource)) { - // If user is assigned to an experiment (i.e. use LS), return false. - if (this.abExperiments.inExperiment(LSEnabled)) { - return false; - } - // Send telemetry if user is in control group - this.abExperiments.sendTelemetryIfInExperiment(LSControl); - return true; // Do use Jedi as it is default. - } - // Configuration is non-default, so `languageServer` should be present. const configurationService = this.serviceContainer.get(IConfigurationService); const lstType = configurationService.getSettings(this.resource).languageServer; - this.sendTelemetryForChosenLanguageServer(lstType).ignoreErrors(); return lstType === LanguageServerType.Jedi; } @@ -253,6 +238,8 @@ export class LanguageServerExtensionActivationService break; } + this.sendTelemetryForChosenLanguageServer(serverType).ignoreErrors(); + await this.logStartup(serverType); let server = this.serviceContainer.get(ILanguageServerActivator, serverType); try { diff --git a/src/client/activation/languageServer/activator.ts b/src/client/activation/languageServer/activator.ts index 42194f36adbe..cc4bb9be9678 100644 --- a/src/client/activation/languageServer/activator.ts +++ b/src/client/activation/languageServer/activator.ts @@ -1,13 +1,15 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; +import { isTestExecution } from '../../common/constants'; import { traceDecorators } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; -import { IConfigurationService, Resource } from '../../common/types'; +import { BANNER_NAME_PROPOSE_LS, IConfigurationService, IPythonExtensionBanner, Resource } from '../../common/types'; +import { PythonInterpreter } from '../../pythonEnvironments/info'; import { LanguageServerActivatorBase } from '../common/activatorBase'; import { ILanguageServerDownloader, ILanguageServerFolderService, ILanguageServerManager } from '../types'; @@ -27,11 +29,21 @@ export class DotNetLanguageServerActivator extends LanguageServerActivatorBase { @inject(IFileSystem) fs: IFileSystem, @inject(ILanguageServerDownloader) lsDownloader: ILanguageServerDownloader, @inject(ILanguageServerFolderService) languageServerFolderService: ILanguageServerFolderService, - @inject(IConfigurationService) configurationService: IConfigurationService + @inject(IConfigurationService) configurationService: IConfigurationService, + @inject(IPythonExtensionBanner) + @named(BANNER_NAME_PROPOSE_LS) + private proposePylancePopup: IPythonExtensionBanner ) { super(manager, workspace, fs, lsDownloader, languageServerFolderService, configurationService); } + public async start(resource: Resource, interpreter?: PythonInterpreter): Promise { + if (!isTestExecution()) { + this.proposePylancePopup.showBanner().ignoreErrors(); + } + return super.start(resource, interpreter); + } + @traceDecorators.error('Failed to ensure language server is available') public async ensureLanguageServerIsAvailable(resource: Resource): Promise { const languageServerFolderPath = await this.ensureLanguageServerFileIsAvailable(resource, 'mscorlib.dll'); diff --git a/src/client/activation/node/languageServerFolderService.ts b/src/client/activation/node/languageServerFolderService.ts index 44922a052970..0ccb89b28fc3 100644 --- a/src/client/activation/node/languageServerFolderService.ts +++ b/src/client/activation/node/languageServerFolderService.ts @@ -8,14 +8,13 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { SemVer } from 'semver'; import { IWorkspaceService } from '../../common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../common/constants'; import { NugetPackage } from '../../common/nuget/types'; import { IConfigurationService, IExtensions, Resource } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { LanguageServerFolderService } from '../common/languageServerFolderService'; import { FolderVersionPair, ILanguageServerFolderService, NodeLanguageServerFolder } from '../types'; -export const PylanceExtensionName = 'ms-python.vscode-pylance'; - class FallbackNodeLanguageServerFolderService extends LanguageServerFolderService { constructor(serviceContainer: IServiceContainer) { super(serviceContainer, NodeLanguageServerFolder); @@ -101,7 +100,7 @@ export class NodeLanguageServerFolderService implements ILanguageServerFolderSer return undefined; } - const extension = this.extensions.getExtension(PylanceExtensionName); + const extension = this.extensions.getExtension(PYLANCE_EXTENSION_ID); if (!extension) { return undefined; } diff --git a/src/client/activation/none/activator.ts b/src/client/activation/none/activator.ts index 8d808ae87ce0..5266e8ab39c1 100644 --- a/src/client/activation/none/activator.ts +++ b/src/client/activation/none/activator.ts @@ -1,6 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import { CancellationToken, CodeLens, @@ -20,7 +20,8 @@ import { TextDocument, WorkspaceEdit } from 'vscode'; -import { Resource } from '../../common/types'; +import { isTestExecution } from '../../common/constants'; +import { BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner, Resource } from '../../common/types'; import { PythonInterpreter } from '../../pythonEnvironments/info'; import { ILanguageServerActivator } from '../types'; @@ -33,8 +34,16 @@ import { ILanguageServerActivator } from '../types'; */ @injectable() export class NoLanguageServerExtensionActivator implements ILanguageServerActivator { - // tslint:disable-next-line: no-empty - public async start(_resource: Resource, _interpreter?: PythonInterpreter): Promise {} + constructor( + @inject(IPythonExtensionBanner) + @named(BANNER_NAME_PROPOSE_LS) + private proposePylancePopup: IPythonExtensionBanner + ) {} + public async start(_resource: Resource, _interpreter?: PythonInterpreter): Promise { + if (!isTestExecution()) { + this.proposePylancePopup.showBanner().ignoreErrors(); + } + } // tslint:disable-next-line: no-empty public dispose(): void {} // tslint:disable-next-line: no-empty diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index f5bec7c65f35..0f1fa5497f67 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -12,7 +12,7 @@ import { import { DataScienceSurveyBanner } from '../datascience/dataScienceSurveyBanner'; import { InteractiveShiftEnterBanner } from '../datascience/shiftEnterBanner'; import { IServiceManager } from '../ioc/types'; -import { ProposeLanguageServerBanner } from '../languageServices/proposeLanguageServerBanner'; +import { ProposePylanceBanner } from '../languageServices/proposeLanguageServerBanner'; import { AATesting } from './aaTesting'; import { ExtensionActivationManager } from './activationManager'; import { LanguageServerExtensionActivationService } from './activationService'; @@ -86,7 +86,7 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp serviceManager.addSingleton( IPythonExtensionBanner, - ProposeLanguageServerBanner, + ProposePylanceBanner, BANNER_NAME_PROPOSE_LS ); serviceManager.addSingleton( diff --git a/src/client/common/application/applicationEnvironment.ts b/src/client/common/application/applicationEnvironment.ts index a16082e0b848..3d01d85f4b35 100644 --- a/src/client/common/application/applicationEnvironment.ts +++ b/src/client/common/application/applicationEnvironment.ts @@ -88,4 +88,7 @@ export class ApplicationEnvironment implements IApplicationEnvironment { const version = parse(this.packageJson.version); return !version || version.prerelease.length > 0 ? 'insiders' : 'stable'; } + public get uriScheme(): string { + return vscode.env.uriScheme; + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index f1d7f38900c6..2459ca1aded7 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -1024,6 +1024,10 @@ export interface IApplicationEnvironment { * The version of the editor. */ readonly vscodeVersion: string; + /** + * The custom uri scheme the editor registers to in the operating system. + */ + readonly uriScheme: string; } export const IWebPanelMessageListener = Symbol('IWebPanelMessageListener'); diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 57c865fccfa2..d0066f58b378 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -231,11 +231,12 @@ export class PythonSettings implements IPythonSettings { pythonSettings.get('autoUpdateLanguageServer', true) )!; - let ls = pythonSettings.get('languageServer'); - if (!ls) { + let ls = pythonSettings.get('languageServer') ?? LanguageServerType.Jedi; + ls = systemVariables.resolveAny(ls); + if (!Object.values(LanguageServerType).includes(ls)) { ls = LanguageServerType.Jedi; } - this.languageServer = systemVariables.resolveAny(ls)!; + this.languageServer = ls; // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.jediPath = systemVariables.resolveAny(pythonSettings.get('jediPath'))!; diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index fb4ccf0e9ef9..6096285fa170 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -15,6 +15,7 @@ export const PYTHON_ALLFILES = [{ language: PYTHON_LANGUAGE }]; export const PVSC_EXTENSION_ID = 'ms-python.python'; export const CODE_RUNNER_EXTENSION_ID = 'formulahendry.code-runner'; +export const PYLANCE_EXTENSION_ID = 'ms-python.vscode-pylance'; export const AppinsightsKey = 'AIF-d9b70cd4-b9f9-4d70-929b-a071c400b217'; export namespace Commands { diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 3757a03d54a0..6daf346c23f8 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -1,6 +1,3 @@ -export const LSControl = 'LS - control'; -export const LSEnabled = 'LS - enabled'; - // Experiment to check whether to always display the test explorer. export enum AlwaysDisplayTestExplorerGroups { control = 'AlwaysDisplayTestExplorer - control', @@ -99,3 +96,8 @@ export enum RemoveKernelToolbarInInteractiveWindow { export enum EnableTrustedNotebooks { experiment = 'EnableTrustedNotebooks' } + +// Experiment to offer switch to Pylance language server +export enum TryPylance { + experiment = 'tryPylance' +} diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 99557821e85c..d7fab4ddc24e 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -525,7 +525,7 @@ export interface IPythonExtensionBanner { readonly enabled: boolean; showBanner(): Promise; } -export const BANNER_NAME_PROPOSE_LS: string = 'ProposeLS'; +export const BANNER_NAME_PROPOSE_LS: string = 'ProposePylance'; export const BANNER_NAME_DS_SURVEY: string = 'DSSurveyBanner'; export const BANNER_NAME_INTERACTIVE_SHIFTENTER: string = 'InteractiveShiftEnterBanner'; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 27642897ae80..324fc2e2f914 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -101,6 +101,13 @@ export namespace AttachProcess { } export namespace LanguageService { + export const proposePylanceMessage = localize( + 'LanguageService.proposePylanceMessage', + 'Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.' + ); + export const tryItNow = localize('LanguageService.tryItNow', 'Try it now'); + export const remindMeLater = localize('LanguageService.remindMeLater', 'Remind me later'); + export const bannerLabelYes = localize('LanguageService.bannerLabelYes', 'Yes, take survey now'); export const bannerLabelNo = localize('LanguageService.bannerLabelNo', 'No, thanks'); export const lsFailedToStart = localize( diff --git a/src/client/languageServices/proposeLanguageServerBanner.ts b/src/client/languageServices/proposeLanguageServerBanner.ts index 55645f90f220..dfc792c48e47 100644 --- a/src/client/languageServices/proposeLanguageServerBanner.ts +++ b/src/client/languageServices/proposeLanguageServerBanner.ts @@ -4,22 +4,27 @@ 'use strict'; import { inject, injectable } from 'inversify'; -import { ConfigurationTarget } from 'vscode'; import { LanguageServerType } from '../activation/types'; -import { IApplicationShell } from '../common/application/types'; +import { IApplicationEnvironment, IApplicationShell } from '../common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../common/constants'; +import { TryPylance } from '../common/experiments/groups'; import '../common/extensions'; -import { IConfigurationService, IPersistentStateFactory, IPythonExtensionBanner } from '../common/types'; -import { getRandomBetween } from '../common/utils/random'; +import { + IConfigurationService, + IExperimentService, + IExtensions, + IPersistentStateFactory, + IPythonExtensionBanner +} from '../common/types'; +import { LanguageService } from '../common/utils/localize'; -// persistent state names, exported to make use of in testing -export enum ProposeLSStateKeys { - ShowBanner = 'ProposeLSBanner' +export function getPylanceExtensionUri(appEnv: IApplicationEnvironment): string { + return `${appEnv.uriScheme}:extension/${PYLANCE_EXTENSION_ID}`; } -enum ProposeLSLabelIndex { - Yes, - No, - Later +// persistent state names, exported to make use of in testing +export enum ProposeLSStateKeys { + ShowBanner = 'TryPylanceBanner' } /* @@ -30,46 +35,24 @@ and will show as soon as it is instructed to do so, if a random sample function enables the popup for this user. */ @injectable() -export class ProposeLanguageServerBanner implements IPythonExtensionBanner { - private initialized?: boolean; +export class ProposePylanceBanner implements IPythonExtensionBanner { private disabledInCurrentSession: boolean = false; - private sampleSizePerHundred: number; - private bannerMessage: string = - 'Try out Preview of our new Python Language Server to get richer and faster IntelliSense completions, and syntax errors as you type.'; - private bannerLabels: string[] = ['Try it now', 'No thanks', 'Remind me Later']; constructor( @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IApplicationEnvironment) private appEnv: IApplicationEnvironment, @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory, @inject(IConfigurationService) private configuration: IConfigurationService, - sampleSizePerOneHundredUsers: number = 10 - ) { - this.sampleSizePerHundred = sampleSizePerOneHundredUsers; - this.initialize(); - } - - public initialize() { - if (this.initialized) { - return; - } - this.initialized = true; - - // Don't even bother adding handlers if banner has been turned off. - if (!this.enabled) { - return; - } + @inject(IExperimentService) private experiments: IExperimentService, + @inject(IExtensions) readonly extensions: IExtensions + ) {} - // we only want 10% of folks that use Jedi to see this survey. - const randomSample: number = getRandomBetween(0, 100); - if (randomSample >= this.sampleSizePerHundred) { - this.disable().ignoreErrors(); - return; - } - } public get enabled(): boolean { - // Banner is deactivated. - return false; - // return this.persistentState.createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, true).value; + 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; } public async showBanner(): Promise { @@ -82,30 +65,31 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { return; } - const response = await this.appShell.showInformationMessage(this.bannerMessage, ...this.bannerLabels); - switch (response) { - case this.bannerLabels[ProposeLSLabelIndex.Yes]: { - await this.enableLanguageServer(); - await this.disable(); - break; - } - case this.bannerLabels[ProposeLSLabelIndex.No]: { - await this.disable(); - break; - } - case this.bannerLabels[ProposeLSLabelIndex.Later]: { - this.disabledInCurrentSession = true; - break; - } - default: { - // Disable for the current session. - this.disabledInCurrentSession = true; - } + const response = await this.appShell.showInformationMessage( + LanguageService.proposePylanceMessage(), + LanguageService.tryItNow(), + LanguageService.bannerLabelNo(), + LanguageService.remindMeLater() + ); + + if (response === LanguageService.tryItNow()) { + this.appShell.openUrl(getPylanceExtensionUri(this.appEnv)); + await this.disable(); + } else if (response === LanguageService.bannerLabelNo()) { + await this.disable(); + } else { + this.disabledInCurrentSession = true; } } public async shouldShowBanner(): Promise { - return Promise.resolve(this.enabled && !this.disabledInCurrentSession); + // 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 { @@ -113,13 +97,4 @@ export class ProposeLanguageServerBanner implements IPythonExtensionBanner { .createGlobalPersistentState(ProposeLSStateKeys.ShowBanner, false) .updateValue(false); } - - public async enableLanguageServer(): Promise { - await this.configuration.updateSetting( - 'languageServer', - LanguageServerType.Microsoft, - undefined, - ConfigurationTarget.Global - ); - } } diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index c72a700f03f7..add675bb2e76 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -7,19 +7,13 @@ import * as path from 'path'; // @ts-ignore import * as pidusage from 'pidusage'; import { CancellationToken, CancellationTokenSource, CompletionItemKind, Disposable, SymbolKind, Uri } from 'vscode'; -import { isTestExecution } from '../common/constants'; import '../common/extensions'; import { IS_WINDOWS } from '../common/platform/constants'; import { IFileSystem } from '../common/platform/types'; import * as internalPython from '../common/process/internal/python'; import * as internalScripts from '../common/process/internal/scripts'; import { IPythonExecutionFactory } from '../common/process/types'; -import { - BANNER_NAME_PROPOSE_LS, - IConfigurationService, - IPythonExtensionBanner, - IPythonSettings -} from '../common/types'; +import { IConfigurationService, IPythonSettings } from '../common/types'; import { createDeferred, Deferred } from '../common/utils/async'; import { swallowExceptions } from '../common/utils/decorators'; import { StopWatch } from '../common/utils/stopWatch'; @@ -157,7 +151,6 @@ export class JediProxy implements Disposable { private pidUsageFailures = { timer: new StopWatch(), counter: 0 }; private lastCmdIdProcessed?: number; private lastCmdIdProcessedForPidUsage?: number; - private proposeNewLanguageServerPopup: IPythonExtensionBanner; private readonly disposables: Disposable[] = []; private timer?: NodeJS.Timer | number; @@ -174,12 +167,6 @@ export class JediProxy implements Disposable { this.startLanguageServer() .then(() => this.initialized.resolve()) .ignoreErrors(); - - this.proposeNewLanguageServerPopup = serviceContainer.get( - IPythonExtensionBanner, - BANNER_NAME_PROPOSE_LS - ); - this.checkJediMemoryFootprint().ignoreErrors(); } @@ -332,9 +319,6 @@ export class JediProxy implements Disposable { private async startLanguageServer(): Promise { const newAutoComletePaths = await this.buildAutoCompletePaths(); this.additionalAutoCompletePaths = newAutoComletePaths; - if (!isTestExecution()) { - await this.proposeNewLanguageServerPopup.showBanner(); - } return this.restartLanguageServer(); } private restartLanguageServer(): Promise { diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index ae0c34aa5a6d..fea8949e050e 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -16,13 +16,11 @@ import { import { LSNotSupportedDiagnosticServiceId } from '../../client/application/diagnostics/checks/lsNotSupported'; import { IDiagnostic, IDiagnosticsService } from '../../client/application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; -import { LSControl, LSEnabled } from '../../client/common/experiments/groups'; import { IPlatformService } from '../../client/common/platform/types'; import { IConfigurationService, IDisposable, IDisposableRegistry, - IExperimentsManager, IOutputChannel, IPersistentState, IPersistentStateFactory, @@ -53,7 +51,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let interpreterChangedHandler!: Function; @@ -67,7 +64,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); const langFolderServiceMock = TypeMoq.Mock.ofType(); const folderVer: FolderVersionPair = { path: '', @@ -179,16 +175,10 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => activator.object) .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - await activationService.activate(undefined); activator.verifyAll(); serviceContainer.verifyAll(); - experiments.verifyAll(); } async function testReloadMessage(settingName: string): Promise { @@ -205,8 +195,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); @@ -244,8 +233,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, true); @@ -255,8 +243,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, false); @@ -267,8 +254,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator); @@ -278,8 +264,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator); @@ -294,8 +279,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await testActivation(activationService, activator, false, LanguageServerType.None); }); @@ -316,8 +300,7 @@ suite('Language Server Activation - ActivationService', () => { const activator = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); @@ -392,8 +375,7 @@ suite('Language Server Activation - ActivationService', () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const ls1 = await activationService.get(folder1.uri, interpreter1); const ls2 = await activationService.get(folder1.uri, interpreter2); @@ -463,8 +445,7 @@ suite('Language Server Activation - ActivationService', () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.activate(folder1.uri); await interpreterChangedHandler(); @@ -486,8 +467,7 @@ suite('Language Server Activation - ActivationService', () => { const activatorJedi = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const diagnostics: IDiagnostic[] = []; lsNotSupportedDiagnosticService @@ -562,17 +542,12 @@ suite('Language Server Activation - ActivationService', () => { .setup((w) => w.getWorkspaceFolderIdentifier(resource, '')) .returns(() => resource!.fsPath) .verifiable(TypeMoq.Times.atLeastOnce()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(resource); activator.verifyAll(); serviceContainer.verifyAll(); workspaceService.verifyAll(); - experiments.verifyAll(); } test('Activator is disposed if activated workspace is removed and LS is "Microsoft"', async () => { pythonSettings.setup((p) => p.languageServer).returns(() => LanguageServerType.Microsoft); @@ -584,8 +559,7 @@ suite('Language Server Activation - ActivationService', () => { .verifiable(TypeMoq.Times.once()); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); workspaceService.verifyAll(); expect(workspaceFoldersChangedHandler).not.to.be.equal(undefined, 'Handler not set'); @@ -624,8 +598,7 @@ suite('Language Server Activation - ActivationService', () => { const activator1 = TypeMoq.Mock.ofType(); const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; @@ -642,15 +615,10 @@ suite('Language Server Activation - ActivationService', () => { .setup((a) => a.start(folder1.uri, undefined)) .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(folder1.uri); activator1.verifyAll(); activator1.verify((a) => a.activate(), TypeMoq.Times.once()); serviceContainer.verifyAll(); - experiments.verifyAll(); const activator2 = TypeMoq.Mock.ofType(); serviceContainer @@ -667,16 +635,11 @@ suite('Language Server Activation - ActivationService', () => { .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.never()); activator2.setup((a) => a.activate()).verifiable(TypeMoq.Times.never()); - experiments - .setup((ex) => ex.inExperiment(TypeMoq.It.isAny())) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); await activationService.activate(folder2.uri); serviceContainer.verifyAll(); activator1.verifyAll(); activator1.verify((a) => a.activate(), TypeMoq.Times.exactly(2)); activator2.verifyAll(); - experiments.verifyAll(); }); } } @@ -693,7 +656,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; setup(() => { @@ -706,7 +668,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); const e = new EventEmitter(); interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); @@ -785,8 +746,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -805,8 +765,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Microsoft); @@ -825,8 +784,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -845,8 +803,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); await activationService.sendTelemetryForChosenLanguageServer(LanguageServerType.Jedi); @@ -854,204 +811,6 @@ suite('Language Server Activation - ActivationService', () => { }); }); - suite('Test useJedi()', () => { - let serviceContainer: TypeMoq.IMock; - let pythonSettings: TypeMoq.IMock; - let appShell: TypeMoq.IMock; - let cmdManager: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let lsNotSupportedDiagnosticService: TypeMoq.IMock; - let stateFactory: TypeMoq.IMock; - let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; - let workspaceConfig: TypeMoq.IMock; - let interpreterService: TypeMoq.IMock; - setup(() => { - serviceContainer = TypeMoq.Mock.ofType(); - appShell = TypeMoq.Mock.ofType(); - workspaceService = TypeMoq.Mock.ofType(); - cmdManager = TypeMoq.Mock.ofType(); - platformService = TypeMoq.Mock.ofType(); - stateFactory = TypeMoq.Mock.ofType(); - state = TypeMoq.Mock.ofType>(); - const configService = TypeMoq.Mock.ofType(); - pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); - interpreterService = TypeMoq.Mock.ofType(); - const e = new EventEmitter(); - interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); - const langFolderServiceMock = TypeMoq.Mock.ofType(); - const folderVer: FolderVersionPair = { - path: '', - version: new SemVer('1.2.3') - }; - lsNotSupportedDiagnosticService = TypeMoq.Mock.ofType(); - workspaceService.setup((w) => w.hasWorkspaceFolders).returns(() => false); - workspaceService.setup((w) => w.workspaceFolders).returns(() => []); - configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); - langFolderServiceMock - .setup((l) => l.getCurrentLanguageServerDirectory()) - .returns(() => Promise.resolve(folderVer)); - stateFactory - .setup((f) => - f.createGlobalPersistentState( - TypeMoq.It.isValue('SWITCH_LS'), - TypeMoq.It.isAny(), - TypeMoq.It.isAny() - ) - ) - .returns(() => state.object); - state.setup((s) => s.value).returns(() => undefined); - state.setup((s) => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - workspaceConfig = TypeMoq.Mock.ofType(); - workspaceService - .setup((ws) => ws.getConfiguration('python', TypeMoq.It.isAny())) - .returns(() => workspaceConfig.object); - const output = TypeMoq.Mock.ofType(); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) - .returns(() => output.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) - .returns(() => workspaceService.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IConfigurationService))) - .returns(() => configService.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(ICommandManager))).returns(() => cmdManager.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterService))) - .returns(() => interpreterService.object); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(ILanguageServerFolderService))) - .returns(() => langFolderServiceMock.object); - serviceContainer - .setup((s) => - s.get( - TypeMoq.It.isValue(IDiagnosticsService), - TypeMoq.It.isValue(LSNotSupportedDiagnosticServiceId) - ) - ) - .returns(() => lsNotSupportedDiagnosticService.object); - }); - - test('If default value (Jedi) of language server is being used, and LSEnabled experiment is enabled, then return false', async () => { - const settings = {}; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => true) - .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) - .returns(() => undefined) - .verifiable(TypeMoq.Times.never()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal(false, 'LS should be enabled'); - - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - experiments.verifyAll(); - }); - - test('If default value of languageServer (Jedi) is being used, and LSEnabled experiment is disabled, then send telemetry if user is in Experiment LSControl and useJedi() should be true)', async () => { - const settings = {}; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => false) - .verifiable(TypeMoq.Times.once()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(LSControl)) - .returns(() => undefined) - .verifiable(TypeMoq.Times.once()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal(true, 'Return value should be true'); - - pythonSettings.verifyAll(); - experiments.verifyAll(); - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - }); - - suite( - 'If default value of languageServer (Jedi) is not being used, then no experiments are used, and python settings value is returned', - async () => { - [ - { - testName: 'Returns false when python settings value is Microsoft', - pythonSettingsValue: LanguageServerType.Microsoft, - expectedResult: false - }, - { - testName: 'Returns true when python settings value is Jedi', - pythonSettingsValue: LanguageServerType.Jedi, - expectedResult: true - } - ].forEach((testParams) => { - test(testParams.testName, async () => { - const settings = { workspaceFolderValue: true }; - experiments - .setup((ex) => ex.inExperiment(LSEnabled)) - .returns(() => false) - .verifiable(TypeMoq.Times.never()); - experiments - .setup((ex) => ex.sendTelemetryIfInExperiment(LSControl)) - .returns(() => undefined) - .verifiable(TypeMoq.Times.never()); - workspaceConfig - .setup((c) => c.inspect('languageServer')) - .returns(() => settings as any) - .verifiable(TypeMoq.Times.once()); - pythonSettings - .setup((p) => p.languageServer) - .returns(() => testParams.pythonSettingsValue) - .verifiable(TypeMoq.Times.once()); - - const activationService = new LanguageServerExtensionActivationService( - serviceContainer.object, - stateFactory.object, - experiments.object - ); - const result = activationService.useJedi(); - expect(result).to.equal( - testParams.expectedResult, - `Return value should be ${testParams.expectedResult}` - ); - - pythonSettings.verifyAll(); - experiments.verifyAll(); - workspaceService.verifyAll(); - workspaceConfig.verifyAll(); - }); - }); - } - ); - }); - suite('Function isJediUsingDefaultConfiguration()', () => { let serviceContainer: TypeMoq.IMock; let pythonSettings: TypeMoq.IMock; @@ -1062,7 +821,6 @@ suite('Language Server Activation - ActivationService', () => { let lsNotSupportedDiagnosticService: TypeMoq.IMock; let stateFactory: TypeMoq.IMock; let state: TypeMoq.IMock>; - let experiments: TypeMoq.IMock; let workspaceConfig: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; setup(() => { @@ -1075,7 +833,6 @@ suite('Language Server Activation - ActivationService', () => { state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); - experiments = TypeMoq.Mock.ofType(); interpreterService = TypeMoq.Mock.ofType(); const e = new EventEmitter(); interpreterService.setup((i) => i.onDidChangeInterpreter).returns(() => e.event); @@ -1166,8 +923,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const result = activationService.isJediUsingDefaultConfiguration(Uri.parse('a')); expect(result).to.equal(expectedResult); @@ -1187,8 +943,7 @@ suite('Language Server Activation - ActivationService', () => { const activationService = new LanguageServerExtensionActivationService( serviceContainer.object, - stateFactory.object, - experiments.object + stateFactory.object ); const result = activationService.isJediUsingDefaultConfiguration(Uri.parse('a')); expect(result).to.equal(false, 'Return value should be false'); diff --git a/src/test/activation/languageServer/activator.unit.test.ts b/src/test/activation/languageServer/activator.unit.test.ts index 87fa327568a7..1b393fa95488 100644 --- a/src/test/activation/languageServer/activator.unit.test.ts +++ b/src/test/activation/languageServer/activator.unit.test.ts @@ -21,9 +21,10 @@ import { PythonSettings } from '../../../client/common/configSettings'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; +import { IConfigurationService, IPythonExtensionBanner, IPythonSettings } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../client/constants'; +import { ProposePylanceBanner } from '../../../client/languageServices/proposeLanguageServerBanner'; import { sleep } from '../../core'; // tslint:disable:max-func-body-length @@ -37,6 +38,7 @@ suite('Language Server - Activator', () => { let lsFolderService: ILanguageServerFolderService; let configuration: IConfigurationService; let settings: IPythonSettings; + let banner: IPythonExtensionBanner; setup(() => { manager = mock(DotNetLanguageServerManager); workspaceService = mock(WorkspaceService); @@ -45,6 +47,7 @@ suite('Language Server - Activator', () => { lsFolderService = mock(DotNetLanguageServerFolderService); configuration = mock(ConfigurationService); settings = mock(PythonSettings); + banner = mock(ProposePylanceBanner); when(configuration.getSettings(anything())).thenReturn(instance(settings)); activator = new DotNetLanguageServerActivator( instance(manager), @@ -52,7 +55,8 @@ suite('Language Server - Activator', () => { instance(fs), instance(lsDownloader), instance(lsFolderService), - instance(configuration) + instance(configuration), + instance(banner) ); }); test('Manager must be started without any workspace', async () => { diff --git a/src/test/activation/node/languageServerFolderService.unit.test.ts b/src/test/activation/node/languageServerFolderService.unit.test.ts index 115bdecc7eda..b944dde8ffb6 100644 --- a/src/test/activation/node/languageServerFolderService.unit.test.ts +++ b/src/test/activation/node/languageServerFolderService.unit.test.ts @@ -9,10 +9,10 @@ import { Extension, Uri, WorkspaceConfiguration } from 'vscode'; import { ILanguageServerFolder, ILSExtensionApi, - NodeLanguageServerFolderService, - PylanceExtensionName + NodeLanguageServerFolderService } from '../../../client/activation/node/languageServerFolderService'; import { IWorkspaceService } from '../../../client/common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; import { IConfigurationService, IExtensions, IPythonSettings } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -115,7 +115,7 @@ suite('Node Language Server Folder Service', () => { test('lsExtension not installed', async () => { pythonSettings.setup((p) => p.downloadLanguageServer).returns(() => true); workspaceConfiguration.setup((wc) => wc.get('packageName')).returns(() => undefined); - extensions.setup((e) => e.getExtension(PylanceExtensionName)).returns(() => undefined); + extensions.setup((e) => e.getExtension(PYLANCE_EXTENSION_ID)).returns(() => undefined); const folderService = new TestService( serviceContainer.object, @@ -148,7 +148,7 @@ suite('Node Language Server Folder Service', () => { extension.setup((e) => e.exports).returns(() => extensionApi); pythonSettings.setup((p) => p.downloadLanguageServer).returns(() => true); workspaceConfiguration.setup((wc) => wc.get('packageName')).returns(() => undefined); - extensions.setup((e) => e.getExtension(PylanceExtensionName)).returns(() => extension.object); + extensions.setup((e) => e.getExtension(PYLANCE_EXTENSION_ID)).returns(() => extension.object); folderService = new TestService( serviceContainer.object, configService.object, diff --git a/src/test/activation/serviceRegistry.unit.test.ts b/src/test/activation/serviceRegistry.unit.test.ts index 18e2bc853ba3..51219665453e 100644 --- a/src/test/activation/serviceRegistry.unit.test.ts +++ b/src/test/activation/serviceRegistry.unit.test.ts @@ -57,7 +57,7 @@ import { DataScienceSurveyBanner } from '../../client/datascience/dataScienceSur import { InteractiveShiftEnterBanner } from '../../client/datascience/shiftEnterBanner'; import { ServiceManager } from '../../client/ioc/serviceManager'; import { IServiceManager } from '../../client/ioc/types'; -import { ProposeLanguageServerBanner } from '../../client/languageServices/proposeLanguageServerBanner'; +import { ProposePylanceBanner } from '../../client/languageServices/proposeLanguageServerBanner'; // tslint:disable:max-func-body-length @@ -101,7 +101,7 @@ suite('Unit Tests - Language Server Activation Service Registry', () => { verify( serviceManager.addSingleton( IPythonExtensionBanner, - ProposeLanguageServerBanner, + ProposePylanceBanner, BANNER_NAME_PROPOSE_LS ) ).once(); diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index a07d1082fb8b..919459206484 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -193,6 +193,31 @@ suite('Python Settings', async () => { config.verifyAll(); }); + function testLanguageServer(languageServer: LanguageServerType, expectedValue: LanguageServerType) { + test(languageServer, () => { + expected.pythonPath = 'python3'; + expected.languageServer = languageServer; + initializeConfig(expected); + config + .setup((c) => c.get('languageServer')) + .returns(() => expected.languageServer) + .verifiable(TypeMoq.Times.once()); + + settings.update(config.object); + + expect(settings.languageServer).to.be.equal(expectedValue); + config.verifyAll(); + }); + } + + suite('languageServer settings', async () => { + Object.values(LanguageServerType).forEach(async (languageServer) => { + testLanguageServer(languageServer, languageServer); + }); + + testLanguageServer('invalid' as LanguageServerType, LanguageServerType.Jedi); + }); + function testExperiments(enabled: boolean) { expected.pythonPath = 'python3'; // tslint:disable-next-line:no-any diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index b0ac5a5220e6..974befd219b0 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -151,6 +151,7 @@ import { TerminalActivationProviders } from '../../client/common/terminal/types'; import { + BANNER_NAME_PROPOSE_LS, GLOBAL_MEMENTO, IAsyncDisposableRegistry, IBrowserService, @@ -170,6 +171,7 @@ import { IOutputChannel, IPathUtils, IPersistentStateFactory, + IPythonExtensionBanner, IPythonSettings, IsWindows, ProductType, @@ -365,6 +367,7 @@ import { InterpreterVersionService } from '../../client/interpreter/interpreterV import { registerInterpreterTypes } from '../../client/interpreter/serviceRegistry'; import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; +import { ProposePylanceBanner } from '../../client/languageServices/proposeLanguageServerBanner'; import { CacheableLocatorPromiseCache } from '../../client/pythonEnvironments/discovery/locators/services/cacheableLocatorService'; import { InterpreterType, PythonInterpreter } from '../../client/pythonEnvironments/info'; import { registerForIOC } from '../../client/pythonEnvironments/legacyIOC'; @@ -761,6 +764,11 @@ export class DataScienceIocContainer extends UnitTestIocContainer { LanguageServerType.Microsoft ); this.serviceManager.add(ILanguageServerManager, DotNetLanguageServerManager); + this.serviceManager.add( + IPythonExtensionBanner, + ProposePylanceBanner, + BANNER_NAME_PROPOSE_LS + ); } else if (languageServerType === LanguageServerType.Node) { this.serviceManager.add( ILanguageServerActivator, diff --git a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts index 42e34a2106a0..db4911814901 100644 --- a/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts +++ b/src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts @@ -3,103 +3,202 @@ 'use strict'; -// tslint:disable:no-any max-func-body-length +import { expect } from 'chai'; +import * as typemoq from 'typemoq'; +import { Extension } from 'vscode'; +import { LanguageServerType } from '../../../client/activation/types'; +import { IApplicationEnvironment, IApplicationShell } from '../../../client/common/application/types'; +import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; +import { TryPylance } from '../../../client/common/experiments/groups'; +import { + IConfigurationService, + IExperimentService, + IExtensions, + IPersistentState, + IPersistentStateFactory, + IPythonSettings +} from '../../../client/common/types'; +import { LanguageService } from '../../../client/common/utils/localize'; +import { + getPylanceExtensionUri, + ProposeLSStateKeys, + ProposePylanceBanner +} from '../../../client/languageServices/proposeLanguageServerBanner'; -// import { expect } from 'chai'; -// import * as typemoq from 'typemoq'; -// import { IApplicationShell } from '../../../client/common/application/types'; -// import { IConfigurationService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; -// import { -// ProposeLanguageServerBanner, -// ProposeLSStateKeys -// } from '../../../client/languageServices/proposeLanguageServerBanner'; +interface IExperimentLsCombination { + inExperiment: boolean; + 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 } +]; -// suite('Propose New Language Server Banner', () => { -// let config: typemoq.IMock; -// let appShell: typemoq.IMock; -// const message = -// 'Try out Preview of our new Python Language Server to get richer and faster IntelliSense completions, and syntax errors as you type.'; -// const yes = 'Try it now'; -// const no = 'No thanks'; -// const later = 'Remind me Later'; +suite('Propose Pylance Banner', () => { + let config: typemoq.IMock; + let appShell: typemoq.IMock; + let appEnv: typemoq.IMock; + let settings: typemoq.IMock; -// setup(() => { -// config = typemoq.Mock.ofType(); -// appShell = typemoq.Mock.ofType(); -// }); -// test('Is debugger enabled upon creation?', () => { -// const enabledValue: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabledValue, 100, appShell.object, config.object); -// expect(testBanner.enabled).to.be.equal(true, 'Sampling 100/100 should always enable the banner.'); -// }); -// test('Do not show banner when it is disabled', () => { -// appShell -// .setup((a) => -// a.showInformationMessage( -// typemoq.It.isValue(message), -// typemoq.It.isValue(yes), -// typemoq.It.isValue(no), -// typemoq.It.isValue(later) -// ) -// ) -// .verifiable(typemoq.Times.never()); -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 0, appShell.object, config.object); -// testBanner.showBanner().ignoreErrors(); -// }); -// test('shouldShowBanner must return false when Banner is implicitly disabled by sampling', () => { -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 0, appShell.object, config.object); -// expect(testBanner.enabled).to.be.equal(false, 'We implicitly disabled the banner, it should never show.'); -// }); -// test('shouldShowBanner must return false when Banner is explicitly disabled', async () => { -// const enabled: boolean = true; -// const testBanner: ProposeLanguageServerBanner = preparePopup(enabled, 100, appShell.object, config.object); + const message = LanguageService.proposePylanceMessage(); + const yes = LanguageService.tryItNow(); + const no = LanguageService.bannerLabelNo(); + const later = LanguageService.remindMeLater(); -// expect(await testBanner.shouldShowBanner()).to.be.equal( -// true, -// '100% sample size should always make the banner enabled.' -// ); -// await testBanner.disable(); -// expect(await testBanner.shouldShowBanner()).to.be.equal( -// false, -// 'Explicitly disabled banner shouldShowBanner != false.' -// ); -// }); -// }); + setup(() => { + config = typemoq.Mock.ofType(); + settings = typemoq.Mock.ofType(); + config.setup((x) => x.getSettings(typemoq.It.isAny())).returns(() => settings.object); + appShell = typemoq.Mock.ofType(); + appEnv = typemoq.Mock.ofType(); + appEnv.setup((x) => x.uriScheme).returns(() => 'scheme'); + }); + testData.forEach((t) => { + test(`${t.inExperiment ? 'In' : 'Not in'} 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}`); + }); + }); + 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}`); + }); + }); + test('Do not show banner when it is disabled', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + 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); + await testBanner.showBanner(); + appShell.verifyAll(); + }); + test('Clicking No should disable the banner', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => no) + .verifiable(typemoq.Times.once()); + appShell.setup((a) => a.openUrl(getPylanceExtensionUri(appEnv.object))).verifiable(typemoq.Times.never()); -// function preparePopup( -// enabledValue: boolean, -// sampleValue: number, -// appShell: IApplicationShell, -// config: IConfigurationService -// ): ProposeLanguageServerBanner { -// const myfactory: typemoq.IMock = typemoq.Mock.ofType(); -// const val: typemoq.IMock> = typemoq.Mock.ofType>(); -// val.setup((a) => a.updateValue(typemoq.It.isValue(true))).returns(() => { -// enabledValue = true; -// return Promise.resolve(); -// }); -// val.setup((a) => a.updateValue(typemoq.It.isValue(false))).returns(() => { -// enabledValue = false; -// return Promise.resolve(); -// }); -// val.setup((a) => a.value).returns(() => { -// return enabledValue; -// }); -// myfactory -// .setup((a) => -// a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(true)) -// ) -// .returns(() => { -// return val.object; -// }); -// myfactory -// .setup((a) => -// a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(false)) -// ) -// .returns(() => { -// return val.object; -// }); -// return new ProposeLanguageServerBanner(appShell, myfactory.object, config, sampleValue); -// } + const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled when user clicked No'); + appShell.verifyAll(); + }); + test('Clicking Later should disable banner in session', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => later) + .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); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal( + true, + 'Banner should not be permanently disabled when user clicked Later' + ); + appShell.verifyAll(); + }); + test('Clicking Yes opens the extension marketplace entry', async () => { + appShell + .setup((a) => + a.showInformationMessage( + typemoq.It.isValue(message), + typemoq.It.isValue(yes), + typemoq.It.isValue(no), + typemoq.It.isValue(later) + ) + ) + .returns(async () => yes) + .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); + await testBanner.showBanner(); + expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled after opening store URL'); + appShell.verifyAll(); + }); +}); + +function preparePopup( + enabledValue: boolean, + appShell: IApplicationShell, + appEnv: IApplicationEnvironment, + config: IConfigurationService, + inExperiment: boolean, + pylanceInstalled: boolean +): ProposePylanceBanner { + const myfactory = typemoq.Mock.ofType(); + const val = typemoq.Mock.ofType>(); + val.setup((a) => a.updateValue(typemoq.It.isValue(true))).returns(() => { + enabledValue = true; + return Promise.resolve(); + }); + val.setup((a) => a.updateValue(typemoq.It.isValue(false))).returns(() => { + enabledValue = false; + return Promise.resolve(); + }); + val.setup((a) => a.value).returns(() => { + return enabledValue; + }); + myfactory + .setup((a) => + a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(true)) + ) + .returns(() => { + return val.object; + }); + myfactory + .setup((a) => + a.createGlobalPersistentState(typemoq.It.isValue(ProposeLSStateKeys.ShowBanner), typemoq.It.isValue(false)) + ) + .returns(() => { + return val.object; + }); + + const experiments = typemoq.Mock.ofType(); + experiments.setup((x) => x.inExperiment(TryPylance.experiment)).returns(() => Promise.resolve(inExperiment)); + + const extensions = typemoq.Mock.ofType(); + // tslint:disable-next-line: no-any + const extension = typemoq.Mock.ofType>(); + extensions + .setup((x) => x.getExtension(PYLANCE_EXTENSION_ID)) + .returns(() => (pylanceInstalled ? extension.object : undefined)); + return new ProposePylanceBanner(appShell, appEnv, myfactory.object, config, experiments.object, extensions.object); +}