microsoft · jakebailey · Nov 19, 2020 · Nov 11, 2020 · Nov 12, 2020 · Nov 12, 2020
diff --git 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<void> {
         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
@@ -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:

diff --git 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<boolean>(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<boolean> {
-        // 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<void> {
         await this.persistentState
             .createGlobalPersistentState<boolean>(ProposeLSStateKeys.ShowBanner, false)
             .updateValue(false);
     }
+
+    public async getPromptMessage(): Promise<string | undefined> {
+        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<string>(TryPylance.jediPrompt1);
+            } else if (await this.experiments.inExperiment(TryPylance.jediPrompt2)) {
+                return this.experiments.getExperimentValue<string>(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/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<IConfigurationService>;
     let appShell: typemoq.IMock<IApplicationShell>;
@@ -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<IPersistentStateFactory>();
@@ -237,7 +292,12 @@ function preparePopup(
         });
 
     const experiments = typemoq.Mock.ofType<IExperimentService>();
-    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<IExtensions>();
     // tslint:disable-next-line: no-any