From c1a8dffa2d25a1eb5312e1546a4c220b5215f7ff Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 26 Sep 2019 17:10:58 -0700 Subject: [PATCH 1/7] Added functionality --- package.nls.json | 5 +- src/client/common/utils/localize.ts | 1 + .../virtualEnvs/condaInheritEnvPrompt.ts | 62 +++++++++++++++++++ src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 11 ++++ 5 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts diff --git a/package.nls.json b/package.nls.json index 0dc62bb90625..1c70d070fa5c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -121,6 +121,7 @@ "Experiments.inGroup": "User belongs to experiment group '{0}'", "Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters", "Interpreters.LoadingInterpreters": "Loading Python Interpreters", + "Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we suggest the \"terminal.integrated.inheritEnv\" setting to be changed to false. Would you like to update this setting?", "Logging.CurrentWorkingDirectory": "cwd:", "Common.doNotShowAgain": "Do not show again", "Common.reload": "Reload", @@ -367,6 +368,6 @@ "DataScience.untitledNotebookMessage": "Your changes will be lost if you don't save them.", "DataScience.untitledNotebookYes": "Save", "DataScience.untitledNotebookNo": "Cancel", - "DataScience.noInterpreter" : "No python selected", - "DataScience.notebookNotFound" : "python -m jupyter notebook --version is not running" + "DataScience.noInterpreter": "No python selected", + "DataScience.notebookNotFound": "python -m jupyter notebook --version is not running" } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index df6efc20df6f..883d86a28241 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -60,6 +60,7 @@ export namespace Experiments { export namespace Interpreters { export const loading = localize('Interpreters.LoadingInterpreters', 'Loading Python Interpreters'); export const refreshing = localize('Interpreters.RefreshingInterpreters', 'Refreshing Python Interpreters'); + export const condaInheritEnvMessage = localize('Interpreters.condaInheritEnvMessage', 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we suggest the \"terminal.integrated.inheritEnv\" setting to be changed to false. Would you like to update this setting?'); export const environmentPromptMessage = localize('Interpreters.environmentPromptMessage', 'We noticed a new virtual environment has been created. Do you want to select it for the workspace folder?'); export const selectInterpreterTip = localize('Interpreters.selectInterpreterTip', 'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar'); } diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts new file mode 100644 index 000000000000..b93851f3f433 --- /dev/null +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { inject, injectable } from 'inversify'; +import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; +import { IExtensionActivationService } from '../../activation/types'; +import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; +import { traceError } from '../../common/logger'; +import { IPersistentStateFactory } from '../../common/types'; +import { InteractiveShiftEnterBanner, Interpreters } from '../../common/utils/localize'; +import { sendTelemetryEvent } from '../../telemetry'; +import { EventName } from '../../telemetry/constants'; +import { IInterpreterService, InterpreterType } from '../contracts'; + +export const condaInheritEnvPromptKey = 'CONDA_INHERIT_ENV_PROMPT_KEY'; + +@injectable() +export class CondaInheritEnvPrompt implements IExtensionActivationService { + private terminalSettings!: WorkspaceConfiguration; + constructor( + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, + @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, + @inject(IApplicationShell) private readonly appShell: IApplicationShell, + @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory + ) { } + + public async activate(resource: Uri): Promise { + const interpreter = await this.interpreterService.getActiveInterpreter(resource); + if (!interpreter || interpreter.type !== InterpreterType.Conda) { + return; + } + this.terminalSettings = this.workspaceService.getConfiguration('terminal', resource); + const setting = this.terminalSettings.inspect('integrated.inheritEnv'); + if (!setting) { + traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `terminal.integrated.inheritEnv`'); + return; + } + if (setting.globalValue !== undefined || setting.workspaceValue !== undefined || setting.workspaceFolderValue !== undefined) { + return; + } + await this.promptAndUpdate(); + } + + public async promptAndUpdate() { + const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true); + if (!notificationPromptEnabled.value) { + return; + } + const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo()]; + const telemetrySelections: ['Yes', 'No'] = ['Yes', 'No']; + const selection = await this.appShell.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts); + sendTelemetryEvent(EventName.CONDA_INHERIT_ENV_PROMPT, undefined, { selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined }); + if (!selection) { + return; + } + if (selection === prompts[0]) { + await this.terminalSettings.update('integrated.inheritEnv', false, ConfigurationTarget.Global); + } else if (selection === prompts[1]) { + await notificationPromptEnabled.updateValue(false); + } + } +} diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 0cd6515c6c46..a8dbbce7bb99 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -30,6 +30,7 @@ export enum EventName { PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL = 'PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL', TERMINAL_SHELL_IDENTIFICATION = 'TERMINAL_SHELL_IDENTIFICATION', PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT = 'PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT', + CONDA_INHERIT_ENV_PROMPT = 'CONDA_INHERIT_ENV_PROMPT', INSIDERS_RELOAD_PROMPT = 'INSIDERS_RELOAD_PROMPT', INSIDERS_PROMPT = 'INSIDERS_PROMPT', OPT_INTO_INSIDERS_AGAIN_PROMPT = 'OPT_INTO_INSIDERS_AGAIN_PROMPT', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 568fcd1fbe2b..80b0956bb734 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -931,6 +931,17 @@ export interface IEventNamePropertyMapping { */ interpreters?: number; }; + /** + * Telemetry event sent with details when user clicks the prompt with the following message + * `Prompt message` :- 'We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we suggest the "terminal.integrated.inheritEnv" setting to be changed to false. Would you like to update this setting?' + */ + [EventName.CONDA_INHERIT_ENV_PROMPT]: { + /** + * `Yes` When 'Yes' option is selected + * `No` When 'No' option is selected + */ + selection: 'Yes' | 'No' | undefined; + }; /** * Telemetry event sent with details when user clicks a button in the virtual environment prompt. * `Prompt message` :- 'We noticed a new virtual environment has been created. Do you want to select it for the workspace folder?' From 72a12cc5d1ed2f61c1da2588697ce3b52b3f9b38 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 26 Sep 2019 17:14:02 -0700 Subject: [PATCH 2/7] News --- news/2 Fixes/7607.md | 1 + src/client/interpreter/serviceRegistry.ts | 2 ++ src/test/interpreters/serviceRegistry.unit.test.ts | 2 ++ 3 files changed, 5 insertions(+) create mode 100644 news/2 Fixes/7607.md diff --git a/news/2 Fixes/7607.md b/news/2 Fixes/7607.md new file mode 100644 index 000000000000..af6109dbd415 --- /dev/null +++ b/news/2 Fixes/7607.md @@ -0,0 +1 @@ +Added prompt to flip "inheritEnv" setting to false to fix conda activation issue diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 6e12603f717f..d1796e922717 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -74,6 +74,7 @@ import { WindowsStoreInterpreter } from './locators/services/windowsStoreInterpr import { WorkspaceVirtualEnvironmentsSearchPathProvider, WorkspaceVirtualEnvService } from './locators/services/workspaceVirtualEnvService'; import { WorkspaceVirtualEnvWatcherService } from './locators/services/workspaceVirtualEnvWatcherService'; import { IPipEnvServiceHelper, IPythonInPathCommandProvider } from './locators/types'; +import { CondaInheritEnvPrompt } from './virtualEnvs/condaInheritEnvPrompt'; import { VirtualEnvironmentManager } from './virtualEnvs/index'; import { IVirtualEnvironmentManager } from './virtualEnvs/types'; import { VirtualEnvironmentPrompt } from './virtualEnvs/virtualEnvPrompt'; @@ -135,6 +136,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IEnvironmentActivationService, EnvironmentActivationService); + serviceManager.addSingleton(IExtensionActivationService, CondaInheritEnvPrompt); serviceManager.addSingleton(WindowsStoreInterpreter, WindowsStoreInterpreter); serviceManager.addSingleton(InterpreterHashProvider, InterpreterHashProvider); serviceManager.addSingleton(InterpeterHashProviderFactory, InterpeterHashProviderFactory); diff --git a/src/test/interpreters/serviceRegistry.unit.test.ts b/src/test/interpreters/serviceRegistry.unit.test.ts index e8d7b2af517b..b3ed9a91b5fd 100644 --- a/src/test/interpreters/serviceRegistry.unit.test.ts +++ b/src/test/interpreters/serviceRegistry.unit.test.ts @@ -82,6 +82,7 @@ import { WorkspaceVirtualEnvWatcherService } from '../../client/interpreter/loca import { IPipEnvServiceHelper, IPythonInPathCommandProvider } from '../../client/interpreter/locators/types'; import { registerTypes } from '../../client/interpreter/serviceRegistry'; import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; +import { CondaInheritEnvPrompt } from '../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; import { VirtualEnvironmentPrompt } from '../../client/interpreter/virtualEnvs/virtualEnvPrompt'; import { ServiceManager } from '../../client/ioc/serviceManager'; @@ -142,6 +143,7 @@ suite('Interpreters - Service Registry', () => { [IInterpreterAutoSelectionService, InterpreterAutoSelectionService], [IEnvironmentActivationService, EnvironmentActivationService], + [IExtensionActivationService, CondaInheritEnvPrompt], [WindowsStoreInterpreter, WindowsStoreInterpreter], [InterpreterHashProvider, InterpreterHashProvider], From fa91672948df2926d73c790321a7a5a50237a348 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 30 Sep 2019 12:20:39 -0700 Subject: [PATCH 3/7] Temp --- .../virtualEnvs/condaInheritEnvPrompt.ts | 42 ++- .../condaInheritEnvPrompt.unit.test.ts | 298 ++++++++++++++++++ 2 files changed, 326 insertions(+), 14 deletions(-) create mode 100644 src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index b93851f3f433..08c2763fe1d5 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, optional } from 'inversify'; import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; -import { traceError } from '../../common/logger'; +import { traceDecorators, traceError } from '../../common/logger'; import { IPersistentStateFactory } from '../../common/types'; import { InteractiveShiftEnterBanner, Interpreters } from '../../common/utils/localize'; import { sendTelemetryEvent } from '../../telemetry'; @@ -21,21 +21,13 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IApplicationShell) private readonly appShell: IApplicationShell, - @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory + @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, + @optional() public hasPromptBeenShownInCurrentSession: boolean = false ) { } public async activate(resource: Uri): Promise { - const interpreter = await this.interpreterService.getActiveInterpreter(resource); - if (!interpreter || interpreter.type !== InterpreterType.Conda) { - return; - } - this.terminalSettings = this.workspaceService.getConfiguration('terminal', resource); - const setting = this.terminalSettings.inspect('integrated.inheritEnv'); - if (!setting) { - traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `terminal.integrated.inheritEnv`'); - return; - } - if (setting.globalValue !== undefined || setting.workspaceValue !== undefined || setting.workspaceFolderValue !== undefined) { + const show = await this.shouldShowPrompt(resource); + if (!show) { return; } await this.promptAndUpdate(); @@ -59,4 +51,26 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { await notificationPromptEnabled.updateValue(false); } } + + @traceDecorators.error('Failed to check whether to display prompt for conda inherit env setting') + public async shouldShowPrompt(resource: Uri): Promise { + if (this.hasPromptBeenShownInCurrentSession) { + return false; + } + const interpreter = await this.interpreterService.getActiveInterpreter(resource); + if (!interpreter || interpreter.type !== InterpreterType.Conda) { + return false; + } + this.terminalSettings = this.workspaceService.getConfiguration('terminal', resource); + const setting = this.terminalSettings.inspect('integrated.inheritEnv'); + if (!setting) { + traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `terminal.integrated.inheritEnv`'); + return false; + } + if (setting.globalValue !== undefined || setting.workspaceValue !== undefined || setting.workspaceFolderValue !== undefined) { + return false; + } + this.hasPromptBeenShownInCurrentSession = true; + return true; + } } diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts new file mode 100644 index 000000000000..98b69526e05b --- /dev/null +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -0,0 +1,298 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { assert, expect } from 'chai'; +import * as sinon from 'sinon'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import * as TypeMoq from 'typemoq'; +import { Uri, WorkspaceConfiguration } from 'vscode'; +import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types'; +import { PersistentStateFactory } from '../../../client/common/persistentState'; +import { IPersistentStateFactory } from '../../../client/common/types'; +import { IInterpreterService, InterpreterType } from '../../../client/interpreter/contracts'; +import { CondaInheritEnvPrompt } from '../../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; + +// tslint:disable:no-any + +// tslint:disable-next-line:max-func-body-length +suite('Conda Inherit Env Prompt', async () => { + let workspaceService: TypeMoq.IMock; + let appShell: TypeMoq.IMock; + let interpreterService: TypeMoq.IMock; + let persistentStateFactory: IPersistentStateFactory; + let condaInheritEnvPrompt: CondaInheritEnvPrompt; + function verifyAll() { + workspaceService.verifyAll(); + appShell.verifyAll(); + interpreterService.verifyAll(); + } + suite('Method shouldShowPrompt()', () => { + setup(() => { + workspaceService = TypeMoq.Mock.ofType(); + appShell = TypeMoq.Mock.ofType(); + interpreterService = TypeMoq.Mock.ofType(); + persistentStateFactory = mock(PersistentStateFactory); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + }); + test('Returns true otherwise', async () => { + const interpreter = { + type: InterpreterType.Pipenv + }; + const settings = { + globalValue: undefined, + workspaceValue: undefined, + workspaceFolderValue: undefined + }; + const workspaceConfig = TypeMoq.Mock.ofType(); + const resource = Uri.file('a'); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => interpreter as any) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.once()); + workspaceConfig + .setup(ws => ws.inspect('integrated.inheritEnv')) + .returns(() => settings as any); + const result = condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(false, 'Prompt should not be shown'); + }); + }); +}); + +// tslint:disable-next-line: max-func-body-length +// suite('Extension survey prompt - showSurvey()', () => { +// let notificationPromptEnabled: TypeMoq.IMock>; +// let experiments: TypeMoq.IMock; +// let appShell: TypeMoq.IMock; +// let browserService: TypeMoq.IMock; +// let random: TypeMoq.IMock; +// let persistentStateFactory: IPersistentStateFactory; +// let disableSurveyForTime: TypeMoq.IMock>; +// let doNotShowAgain: TypeMoq.IMock>; +// let extensionSurveyPrompt: ExtensionSurveyPrompt; +// let notificationPromptEnabled: TypeMoq.IMock>; +// setup(() => { +// appShell = TypeMoq.Mock.ofType(); +// browserService = TypeMoq.Mock.ofType(); +// random = TypeMoq.Mock.ofType(); +// persistentStateFactory = mock(PersistentStateFactory); +// disableSurveyForTime = TypeMoq.Mock.ofType>(); +// doNotShowAgain = TypeMoq.Mock.ofType>(); +// when(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).thenReturn(disableSurveyForTime.object); +// when(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).thenReturn(doNotShowAgain.object); +// experiments = TypeMoq.Mock.ofType(); +// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10); +// }); + +// test('Launch survey if \'Yes\' option is clicked', async () => { +// notificationPromptEnabled = TypeMoq.Mock.ofType>(); +// when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(notificationPromptEnabled.object); +// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; +// appShell +// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) +// .returns(() => Promise.resolve(LanguageService.bannerLabelYes())) +// .verifiable(TypeMoq.Times.once()); +// browserService +// .setup(s => s.launch(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.once()); +// disableSurveyForTime +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.once()); +// doNotShowAgain +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// await extensionSurveyPrompt.showSurvey(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).once(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); +// appShell.verifyAll(); +// browserService.verifyAll(); +// disableSurveyForTime.verifyAll(); +// doNotShowAgain.verifyAll(); +// }); + +// test('Do nothing if \'Maybe later\' option is clicked', async () => { +// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; +// appShell +// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) +// .returns(() => Promise.resolve(ExtensionSurveyBanner.maybeLater())) +// .verifiable(TypeMoq.Times.once()); +// browserService +// .setup(s => s.launch(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// disableSurveyForTime +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// doNotShowAgain +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// await extensionSurveyPrompt.showSurvey(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); +// appShell.verifyAll(); +// browserService.verifyAll(); +// disableSurveyForTime.verifyAll(); +// doNotShowAgain.verifyAll(); +// }); + +// test('Do nothing if no option is clicked', async () => { +// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; +// appShell +// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) +// .returns(() => Promise.resolve(undefined)) +// .verifiable(TypeMoq.Times.once()); +// browserService +// .setup(s => s.launch(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// disableSurveyForTime +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// doNotShowAgain +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// await extensionSurveyPrompt.showSurvey(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); +// appShell.verifyAll(); +// browserService.verifyAll(); +// disableSurveyForTime.verifyAll(); +// doNotShowAgain.verifyAll(); +// }); + +// test('Disable prompt if \'Do not show again\' option is clicked', async () => { +// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; +// appShell +// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) +// .returns(() => Promise.resolve(Common.doNotShowAgain())) +// .verifiable(TypeMoq.Times.once()); +// browserService +// .setup(s => s.launch(TypeMoq.It.isAny())) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// disableSurveyForTime +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.never()); +// doNotShowAgain +// .setup(d => d.updateValue(true)) +// .returns(() => Promise.resolve()) +// .verifiable(TypeMoq.Times.once()); +// await extensionSurveyPrompt.showSurvey(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); +// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).once(); +// appShell.verifyAll(); +// browserService.verifyAll(); +// disableSurveyForTime.verifyAll(); +// doNotShowAgain.verifyAll(); +// }); +// }); + +// // tslint:disable-next-line: max-func-body-length +// suite('Extension survey prompt - activate()', () => { +// let appShell: TypeMoq.IMock; +// let browserService: TypeMoq.IMock; +// let random: TypeMoq.IMock; +// let persistentStateFactory: IPersistentStateFactory; +// let shouldShowBanner: sinon.SinonStub; +// let showSurvey: sinon.SinonStub; +// let experiments: TypeMoq.IMock; +// let extensionSurveyPrompt: ExtensionSurveyPrompt; +// setup(() => { +// appShell = TypeMoq.Mock.ofType(); +// browserService = TypeMoq.Mock.ofType(); +// random = TypeMoq.Mock.ofType(); +// persistentStateFactory = mock(PersistentStateFactory); +// experiments = TypeMoq.Mock.ofType(); +// }); + +// teardown(() => { +// sinon.restore(); +// }); + +// test('If user is not in \'ShowExtensionPrompt\' experiment, send telemetry if in control group & return', async () => { +// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); +// shouldShowBanner.callsFake(() => false); +// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); +// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10); +// experiments +// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) +// .returns(() => false) +// .verifiable(TypeMoq.Times.once()); +// experiments +// .setup(exp => exp.sendTelemetryIfInExperiment(ShowExtensionSurveyPrompt.control)) +// .returns(() => undefined) +// .verifiable(TypeMoq.Times.once()); +// await extensionSurveyPrompt.activate(); +// assert.ok(shouldShowBanner.notCalled); +// experiments.verifyAll(); +// }); + +// test('No survey is shown if shouldShowBanner() returns false and user is in \'ShowExtensionPrompt\' experiment', async () => { +// const deferred = createDeferred(); +// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); +// shouldShowBanner.callsFake(() => false); +// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); +// showSurvey.callsFake(() => { +// deferred.resolve(true); +// return Promise.resolve(); +// }); +// // waitTimeToShowSurvey = 50 ms +// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10, 50); +// experiments +// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) +// .returns(() => true) +// .verifiable(TypeMoq.Times.once()); +// experiments +// .setup(exp => exp.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) +// .returns(() => undefined) +// .verifiable(TypeMoq.Times.never()); +// await extensionSurveyPrompt.activate(); +// assert.ok(shouldShowBanner.calledOnce); + +// const doesSurveyShowUp = await Promise.race([deferred.promise, sleep(100).then(() => false)]); +// assert.ok(showSurvey.notCalled); +// expect(doesSurveyShowUp).to.equal(false, 'Survey should not appear'); +// experiments.verifyAll(); +// }); + +// test('Survey is shown after waitTimeToShowSurvey if shouldShowBanner() returns true and user is in \'ShowExtensionPrompt\' experiment', async () => { +// const deferred = createDeferred(); +// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); +// shouldShowBanner.callsFake(() => true); +// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); +// showSurvey.callsFake(() => { +// deferred.resolve(true); +// return Promise.resolve(); +// }); +// // waitTimeToShowSurvey = 50 ms +// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10, 50); +// experiments +// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) +// .returns(() => true) +// .verifiable(TypeMoq.Times.once()); +// experiments +// .setup(exp => exp.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) +// .returns(() => undefined) +// .verifiable(TypeMoq.Times.never()); +// await extensionSurveyPrompt.activate(); +// assert.ok(shouldShowBanner.calledOnce); + +// const doesSurveyShowUp = await Promise.race([deferred.promise, sleep(200).then(() => false)]); +// expect(doesSurveyShowUp).to.equal(true, 'Survey should appear'); +// assert.ok(showSurvey.calledOnce); +// experiments.verifyAll(); +// }); +// }); From 0143daccfce49c9e2c70ed072163fa1ed776ba92 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 30 Sep 2019 15:36:02 -0700 Subject: [PATCH 4/7] Added tests --- .../virtualEnvs/condaInheritEnvPrompt.ts | 8 +- .../condaInheritEnvPrompt.unit.test.ts | 499 +++++++++--------- 2 files changed, 264 insertions(+), 243 deletions(-) diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index 08c2763fe1d5..a53356801a77 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable, optional } from 'inversify'; -import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; +import { ConfigurationTarget, Uri } from 'vscode'; import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IWorkspaceService } from '../../common/application/types'; import { traceDecorators, traceError } from '../../common/logger'; @@ -16,7 +16,6 @@ export const condaInheritEnvPromptKey = 'CONDA_INHERIT_ENV_PROMPT_KEY'; @injectable() export class CondaInheritEnvPrompt implements IExtensionActivationService { - private terminalSettings!: WorkspaceConfiguration; constructor( @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @@ -46,7 +45,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { return; } if (selection === prompts[0]) { - await this.terminalSettings.update('integrated.inheritEnv', false, ConfigurationTarget.Global); + await this.workspaceService.getConfiguration('terminal').update('integrated.inheritEnv', false, ConfigurationTarget.Global); } else if (selection === prompts[1]) { await notificationPromptEnabled.updateValue(false); } @@ -61,8 +60,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { if (!interpreter || interpreter.type !== InterpreterType.Conda) { return false; } - this.terminalSettings = this.workspaceService.getConfiguration('terminal', resource); - const setting = this.terminalSettings.inspect('integrated.inheritEnv'); + const setting = this.workspaceService.getConfiguration('terminal', resource).inspect('integrated.inheritEnv'); if (!setting) { traceError('WorkspaceConfiguration.inspect returns `undefined` for setting `terminal.integrated.inheritEnv`'); return false; diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index 98b69526e05b..0ed113f0c60d 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -5,29 +5,33 @@ import { assert, expect } from 'chai'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { instance, mock, verify, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; -import { Uri, WorkspaceConfiguration } from 'vscode'; +import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types'; import { PersistentStateFactory } from '../../../client/common/persistentState'; -import { IPersistentStateFactory } from '../../../client/common/types'; +import { IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; +import { InteractiveShiftEnterBanner, Interpreters } from '../../../client/common/utils/localize'; import { IInterpreterService, InterpreterType } from '../../../client/interpreter/contracts'; -import { CondaInheritEnvPrompt } from '../../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; +import { CondaInheritEnvPrompt, condaInheritEnvPromptKey } from '../../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; // tslint:disable:no-any -// tslint:disable-next-line:max-func-body-length +// tslint:disable:max-func-body-length suite('Conda Inherit Env Prompt', async () => { + const resource = Uri.file('a'); let workspaceService: TypeMoq.IMock; let appShell: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; let persistentStateFactory: IPersistentStateFactory; + let notificationPromptEnabled: TypeMoq.IMock>; let condaInheritEnvPrompt: CondaInheritEnvPrompt; function verifyAll() { workspaceService.verifyAll(); appShell.verifyAll(); interpreterService.verifyAll(); } + suite('Method shouldShowPrompt()', () => { setup(() => { workspaceService = TypeMoq.Mock.ofType(); @@ -36,20 +40,121 @@ suite('Conda Inherit Env Prompt', async () => { persistentStateFactory = mock(PersistentStateFactory); condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); }); - test('Returns true otherwise', async () => { + test('Returns false if active interpreter is not of type Conda', async () => { const interpreter = { type: InterpreterType.Pipenv }; + const workspaceConfig = TypeMoq.Mock.ofType(); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => Promise.resolve(interpreter) as any) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(false, 'Prompt should not be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false'); + verifyAll(); + }); + test('Returns false if no active interpreter is present', async () => { + const workspaceConfig = TypeMoq.Mock.ofType(); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(false, 'Prompt should not be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false'); + verifyAll(); + }); + test('Returns false if settings returned is `undefined`', async () => { + const interpreter = { + type: InterpreterType.Conda + }; + const workspaceConfig = TypeMoq.Mock.ofType(); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => Promise.resolve(interpreter) as any) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.once()); + workspaceConfig + .setup(ws => ws.inspect('integrated.inheritEnv')) + .returns(() => undefined); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(false, 'Prompt should not be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false'); + verifyAll(); + }); + [ + { + name: 'Returns false if globalValue `terminal.integrated.inheritEnv` setting is set', + settings: { + globalValue: true, + workspaceValue: undefined, + workspaceFolderValue: undefined + } + }, + { + name: 'Returns false if workspaceValue of `terminal.integrated.inheritEnv` setting is set', + settings: { + globalValue: undefined, + workspaceValue: true, + workspaceFolderValue: undefined + } + }, + { + name: 'Returns false if workspaceFolderValue of `terminal.integrated.inheritEnv` setting is set', + settings: { + globalValue: undefined, + workspaceValue: undefined, + workspaceFolderValue: false + } + } + ].forEach(testParams => { + test(testParams.name, async () => { + const interpreter = { + type: InterpreterType.Conda + }; + const workspaceConfig = TypeMoq.Mock.ofType(); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => Promise.resolve(interpreter) as any) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.once()); + workspaceConfig + .setup(ws => ws.inspect('integrated.inheritEnv')) + .returns(() => testParams.settings as any); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(false, 'Prompt should not be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false'); + verifyAll(); + }); + }); + test('Returns true otherwise', async () => { + const interpreter = { + type: InterpreterType.Conda + }; const settings = { globalValue: undefined, workspaceValue: undefined, workspaceFolderValue: undefined }; const workspaceConfig = TypeMoq.Mock.ofType(); - const resource = Uri.file('a'); interpreterService .setup(is => is.getActiveInterpreter(resource)) - .returns(() => interpreter as any) + .returns(() => Promise.resolve(interpreter) as any) .verifiable(TypeMoq.Times.once()); workspaceService .setup(ws => ws.getConfiguration('terminal', resource)) @@ -58,241 +163,159 @@ suite('Conda Inherit Env Prompt', async () => { workspaceConfig .setup(ws => ws.inspect('integrated.inheritEnv')) .returns(() => settings as any); - const result = condaInheritEnvPrompt.shouldShowPrompt(resource); - expect(result).to.equal(false, 'Prompt should not be shown'); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(true, 'Prompt should be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(true, 'Should be true'); + verifyAll(); }); }); -}); - -// tslint:disable-next-line: max-func-body-length -// suite('Extension survey prompt - showSurvey()', () => { -// let notificationPromptEnabled: TypeMoq.IMock>; -// let experiments: TypeMoq.IMock; -// let appShell: TypeMoq.IMock; -// let browserService: TypeMoq.IMock; -// let random: TypeMoq.IMock; -// let persistentStateFactory: IPersistentStateFactory; -// let disableSurveyForTime: TypeMoq.IMock>; -// let doNotShowAgain: TypeMoq.IMock>; -// let extensionSurveyPrompt: ExtensionSurveyPrompt; -// let notificationPromptEnabled: TypeMoq.IMock>; -// setup(() => { -// appShell = TypeMoq.Mock.ofType(); -// browserService = TypeMoq.Mock.ofType(); -// random = TypeMoq.Mock.ofType(); -// persistentStateFactory = mock(PersistentStateFactory); -// disableSurveyForTime = TypeMoq.Mock.ofType>(); -// doNotShowAgain = TypeMoq.Mock.ofType>(); -// when(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).thenReturn(disableSurveyForTime.object); -// when(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).thenReturn(doNotShowAgain.object); -// experiments = TypeMoq.Mock.ofType(); -// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10); -// }); - -// test('Launch survey if \'Yes\' option is clicked', async () => { -// notificationPromptEnabled = TypeMoq.Mock.ofType>(); -// when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(notificationPromptEnabled.object); -// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; -// appShell -// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) -// .returns(() => Promise.resolve(LanguageService.bannerLabelYes())) -// .verifiable(TypeMoq.Times.once()); -// browserService -// .setup(s => s.launch(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.once()); -// disableSurveyForTime -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.once()); -// doNotShowAgain -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// await extensionSurveyPrompt.showSurvey(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).once(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); -// appShell.verifyAll(); -// browserService.verifyAll(); -// disableSurveyForTime.verifyAll(); -// doNotShowAgain.verifyAll(); -// }); -// test('Do nothing if \'Maybe later\' option is clicked', async () => { -// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; -// appShell -// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) -// .returns(() => Promise.resolve(ExtensionSurveyBanner.maybeLater())) -// .verifiable(TypeMoq.Times.once()); -// browserService -// .setup(s => s.launch(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// disableSurveyForTime -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// doNotShowAgain -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// await extensionSurveyPrompt.showSurvey(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); -// appShell.verifyAll(); -// browserService.verifyAll(); -// disableSurveyForTime.verifyAll(); -// doNotShowAgain.verifyAll(); -// }); - -// test('Do nothing if no option is clicked', async () => { -// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; -// appShell -// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) -// .returns(() => Promise.resolve(undefined)) -// .verifiable(TypeMoq.Times.once()); -// browserService -// .setup(s => s.launch(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// disableSurveyForTime -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// doNotShowAgain -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// await extensionSurveyPrompt.showSurvey(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).never(); -// appShell.verifyAll(); -// browserService.verifyAll(); -// disableSurveyForTime.verifyAll(); -// doNotShowAgain.verifyAll(); -// }); - -// test('Disable prompt if \'Do not show again\' option is clicked', async () => { -// const prompts = [LanguageService.bannerLabelYes(), ExtensionSurveyBanner.maybeLater(), Common.doNotShowAgain()]; -// appShell -// .setup(a => a.showInformationMessage(ExtensionSurveyBanner.bannerMessage(), ...prompts)) -// .returns(() => Promise.resolve(Common.doNotShowAgain())) -// .verifiable(TypeMoq.Times.once()); -// browserService -// .setup(s => s.launch(TypeMoq.It.isAny())) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// disableSurveyForTime -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.never()); -// doNotShowAgain -// .setup(d => d.updateValue(true)) -// .returns(() => Promise.resolve()) -// .verifiable(TypeMoq.Times.once()); -// await extensionSurveyPrompt.showSurvey(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.disableSurveyForTime, false, anything())).never(); -// verify(persistentStateFactory.createGlobalPersistentState(extensionSurveyStateKeys.doNotShowAgain, false)).once(); -// appShell.verifyAll(); -// browserService.verifyAll(); -// disableSurveyForTime.verifyAll(); -// doNotShowAgain.verifyAll(); -// }); -// }); - -// // tslint:disable-next-line: max-func-body-length -// suite('Extension survey prompt - activate()', () => { -// let appShell: TypeMoq.IMock; -// let browserService: TypeMoq.IMock; -// let random: TypeMoq.IMock; -// let persistentStateFactory: IPersistentStateFactory; -// let shouldShowBanner: sinon.SinonStub; -// let showSurvey: sinon.SinonStub; -// let experiments: TypeMoq.IMock; -// let extensionSurveyPrompt: ExtensionSurveyPrompt; -// setup(() => { -// appShell = TypeMoq.Mock.ofType(); -// browserService = TypeMoq.Mock.ofType(); -// random = TypeMoq.Mock.ofType(); -// persistentStateFactory = mock(PersistentStateFactory); -// experiments = TypeMoq.Mock.ofType(); -// }); - -// teardown(() => { -// sinon.restore(); -// }); + suite('Method activate()', () => { + let shouldShowPrompt: sinon.SinonStub; + let promptAndUpdate: sinon.SinonStub; + setup(() => { + workspaceService = TypeMoq.Mock.ofType(); + appShell = TypeMoq.Mock.ofType(); + interpreterService = TypeMoq.Mock.ofType(); + persistentStateFactory = mock(PersistentStateFactory); + }); -// test('If user is not in \'ShowExtensionPrompt\' experiment, send telemetry if in control group & return', async () => { -// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); -// shouldShowBanner.callsFake(() => false); -// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); -// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10); -// experiments -// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) -// .returns(() => false) -// .verifiable(TypeMoq.Times.once()); -// experiments -// .setup(exp => exp.sendTelemetryIfInExperiment(ShowExtensionSurveyPrompt.control)) -// .returns(() => undefined) -// .verifiable(TypeMoq.Times.once()); -// await extensionSurveyPrompt.activate(); -// assert.ok(shouldShowBanner.notCalled); -// experiments.verifyAll(); -// }); + teardown(() => { + sinon.restore(); + }); -// test('No survey is shown if shouldShowBanner() returns false and user is in \'ShowExtensionPrompt\' experiment', async () => { -// const deferred = createDeferred(); -// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); -// shouldShowBanner.callsFake(() => false); -// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); -// showSurvey.callsFake(() => { -// deferred.resolve(true); -// return Promise.resolve(); -// }); -// // waitTimeToShowSurvey = 50 ms -// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10, 50); -// experiments -// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) -// .returns(() => true) -// .verifiable(TypeMoq.Times.once()); -// experiments -// .setup(exp => exp.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) -// .returns(() => undefined) -// .verifiable(TypeMoq.Times.never()); -// await extensionSurveyPrompt.activate(); -// assert.ok(shouldShowBanner.calledOnce); + test('Show prompt if shouldShowPrompt() returns true', async () => { + shouldShowPrompt = sinon.stub(CondaInheritEnvPrompt.prototype, 'shouldShowPrompt'); + shouldShowPrompt.callsFake(() => Promise.resolve(true)); + promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); + promptAndUpdate.callsFake(() => Promise.resolve(undefined)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + await condaInheritEnvPrompt.activate(resource); + assert.ok(shouldShowPrompt.calledOnce); + assert.ok(promptAndUpdate.calledOnce); + }); -// const doesSurveyShowUp = await Promise.race([deferred.promise, sleep(100).then(() => false)]); -// assert.ok(showSurvey.notCalled); -// expect(doesSurveyShowUp).to.equal(false, 'Survey should not appear'); -// experiments.verifyAll(); -// }); + test('Do not show prompt if shouldShowPrompt() returns false', async () => { + shouldShowPrompt = sinon.stub(CondaInheritEnvPrompt.prototype, 'shouldShowPrompt'); + shouldShowPrompt.callsFake(() => Promise.resolve(false)); + promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); + promptAndUpdate.callsFake(() => Promise.resolve(undefined)); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + await condaInheritEnvPrompt.activate(resource); + assert.ok(shouldShowPrompt.calledOnce); + assert.ok(promptAndUpdate.notCalled); + }); + }); -// test('Survey is shown after waitTimeToShowSurvey if shouldShowBanner() returns true and user is in \'ShowExtensionPrompt\' experiment', async () => { -// const deferred = createDeferred(); -// shouldShowBanner = sinon.stub(ExtensionSurveyPrompt.prototype, 'shouldShowBanner'); -// shouldShowBanner.callsFake(() => true); -// showSurvey = sinon.stub(ExtensionSurveyPrompt.prototype, 'showSurvey'); -// showSurvey.callsFake(() => { -// deferred.resolve(true); -// return Promise.resolve(); -// }); -// // waitTimeToShowSurvey = 50 ms -// extensionSurveyPrompt = new ExtensionSurveyPrompt(appShell.object, browserService.object, instance(persistentStateFactory), random.object, experiments.object, 10, 50); -// experiments -// .setup(exp => exp.inExperiment(ShowExtensionSurveyPrompt.enabled)) -// .returns(() => true) -// .verifiable(TypeMoq.Times.once()); -// experiments -// .setup(exp => exp.sendTelemetryIfInExperiment(TypeMoq.It.isAny())) -// .returns(() => undefined) -// .verifiable(TypeMoq.Times.never()); -// await extensionSurveyPrompt.activate(); -// assert.ok(shouldShowBanner.calledOnce); + suite('Method promptAndUpdate()', () => { + const prompts = [InteractiveShiftEnterBanner.bannerLabelYes(), InteractiveShiftEnterBanner.bannerLabelNo()]; + setup(() => { + workspaceService = TypeMoq.Mock.ofType(); + appShell = TypeMoq.Mock.ofType(); + interpreterService = TypeMoq.Mock.ofType(); + persistentStateFactory = mock(PersistentStateFactory); + notificationPromptEnabled = TypeMoq.Mock.ofType>(); + when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(notificationPromptEnabled.object); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + }); -// const doesSurveyShowUp = await Promise.race([deferred.promise, sleep(200).then(() => false)]); -// expect(doesSurveyShowUp).to.equal(true, 'Survey should appear'); -// assert.ok(showSurvey.calledOnce); -// experiments.verifyAll(); -// }); -// }); + test('Does not display prompt if it is disabled', async () => { + notificationPromptEnabled + .setup(n => n.value) + .returns(() => false) + .verifiable(TypeMoq.Times.once()); + appShell + .setup(a => a.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.never()); + await condaInheritEnvPrompt.promptAndUpdate(); + verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); + verifyAll(); + notificationPromptEnabled.verifyAll(); + }); + test('Do nothing if no option is selected', async () => { + const workspaceConfig = TypeMoq.Mock.ofType(); + notificationPromptEnabled + .setup(n => n.value) + .returns(() => true) + .verifiable(TypeMoq.Times.once()); + appShell + .setup(a => a.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal')) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + workspaceConfig + .setup(wc => wc.update('integrated.inheritEnv', false, ConfigurationTarget.Global)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.never()); + notificationPromptEnabled + .setup(n => n.updateValue(false)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.never()); + await condaInheritEnvPrompt.promptAndUpdate(); + verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); + verifyAll(); + workspaceConfig.verifyAll(); + notificationPromptEnabled.verifyAll(); + }); + test('Update terminal settings if `Yes` is selected', async () => { + const workspaceConfig = TypeMoq.Mock.ofType(); + notificationPromptEnabled + .setup(n => n.value) + .returns(() => true) + .verifiable(TypeMoq.Times.once()); + appShell + .setup(a => a.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts)) + .returns(() => Promise.resolve(InteractiveShiftEnterBanner.bannerLabelYes())) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal')) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.once()); + workspaceConfig + .setup(wc => wc.update('integrated.inheritEnv', false, ConfigurationTarget.Global)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.once()); + notificationPromptEnabled + .setup(n => n.updateValue(false)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.never()); + await condaInheritEnvPrompt.promptAndUpdate(); + verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); + verifyAll(); + workspaceConfig.verifyAll(); + notificationPromptEnabled.verifyAll(); + }); + test('Disable notification prompt if `No` is selected', async () => { + const workspaceConfig = TypeMoq.Mock.ofType(); + notificationPromptEnabled + .setup(n => n.value) + .returns(() => true) + .verifiable(TypeMoq.Times.once()); + appShell + .setup(a => a.showInformationMessage(Interpreters.condaInheritEnvMessage(), ...prompts)) + .returns(() => Promise.resolve(InteractiveShiftEnterBanner.bannerLabelNo())) + .verifiable(TypeMoq.Times.once()); + workspaceService + .setup(ws => ws.getConfiguration('terminal')) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + workspaceConfig + .setup(wc => wc.update('integrated.inheritEnv', false, ConfigurationTarget.Global)) + .returns(() => Promise.resolve()) + .verifiable(TypeMoq.Times.never()); + notificationPromptEnabled + .setup(n => n.updateValue(false)) + .returns(() => Promise.resolve(undefined)) + .verifiable(TypeMoq.Times.once()); + await condaInheritEnvPrompt.promptAndUpdate(); + verify(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).once(); + verifyAll(); + workspaceConfig.verifyAll(); + notificationPromptEnabled.verifyAll(); + }); + }); +}); From 4a0a7e2e881092b3fd095bfb7e04d8c50043b0ab Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 30 Sep 2019 16:13:31 -0700 Subject: [PATCH 5/7] Add missing test --- .../condaInheritEnvPrompt.unit.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index 0ed113f0c60d..cf502ba2ab00 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -40,6 +40,22 @@ suite('Conda Inherit Env Prompt', async () => { persistentStateFactory = mock(PersistentStateFactory); condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); }); + test('Returns false if prompt has already been shown in the current session', async () => { + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory), true); + const workspaceConfig = TypeMoq.Mock.ofType(); + interpreterService + .setup(is => is.getActiveInterpreter(resource)) + .returns(() => Promise.resolve(undefined) as any) + .verifiable(TypeMoq.Times.never()); + workspaceService + .setup(ws => ws.getConfiguration('terminal', resource)) + .returns(() => workspaceConfig.object) + .verifiable(TypeMoq.Times.never()); + const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); + expect(result).to.equal(true, 'Prompt should be shown'); + expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(true, 'Should be true'); + verifyAll(); + }); test('Returns false if active interpreter is not of type Conda', async () => { const interpreter = { type: InterpreterType.Pipenv From d75663e669a7a56fd56d19c1c10796eb0968e17b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 30 Sep 2019 17:00:01 -0700 Subject: [PATCH 6/7] Fixes --- .../interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index cf502ba2ab00..b72de1ff740a 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -52,7 +52,7 @@ suite('Conda Inherit Env Prompt', async () => { .returns(() => workspaceConfig.object) .verifiable(TypeMoq.Times.never()); const result = await condaInheritEnvPrompt.shouldShowPrompt(resource); - expect(result).to.equal(true, 'Prompt should be shown'); + expect(result).to.equal(false, 'Prompt should not be shown'); expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(true, 'Should be true'); verifyAll(); }); From d39e2c4e537a3095903f9a5f9a51622357ed5247 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 1 Oct 2019 00:50:02 -0700 Subject: [PATCH 7/7] Intialize prompt in background --- .../virtualEnvs/condaInheritEnvPrompt.ts | 6 +++ .../condaInheritEnvPrompt.unit.test.ts | 46 +++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts index a53356801a77..0dfec6eb5780 100644 --- a/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts +++ b/src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts @@ -25,6 +25,11 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { ) { } public async activate(resource: Uri): Promise { + this.initializeInBackground(resource).ignoreErrors(); + } + + @traceDecorators.error('Failed to intialize conda inherit env prompt') + public async initializeInBackground(resource: Uri): Promise { const show = await this.shouldShowPrompt(resource); if (!show) { return; @@ -32,6 +37,7 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService { await this.promptAndUpdate(); } + @traceDecorators.error('Failed to display conda inherit env prompt') public async promptAndUpdate() { const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true); if (!notificationPromptEnabled.value) { diff --git a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts index b72de1ff740a..8939ee595c57 100644 --- a/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts +++ b/src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts @@ -11,6 +11,7 @@ import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types'; import { PersistentStateFactory } from '../../../client/common/persistentState'; import { IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; +import { createDeferred, createDeferredFromPromise, sleep } from '../../../client/common/utils/async'; import { InteractiveShiftEnterBanner, Interpreters } from '../../../client/common/utils/localize'; import { IInterpreterService, InterpreterType } from '../../../client/interpreter/contracts'; import { CondaInheritEnvPrompt, condaInheritEnvPromptKey } from '../../../client/interpreter/virtualEnvs/condaInheritEnvPrompt'; @@ -185,8 +186,47 @@ suite('Conda Inherit Env Prompt', async () => { verifyAll(); }); }); - suite('Method activate()', () => { + let initializeInBackground: sinon.SinonStub; + setup(() => { + workspaceService = TypeMoq.Mock.ofType(); + appShell = TypeMoq.Mock.ofType(); + interpreterService = TypeMoq.Mock.ofType(); + persistentStateFactory = mock(PersistentStateFactory); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Invokes initializeInBackground() in the background', async () => { + const initializeInBackgroundDeferred = createDeferred(); + initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground'); + initializeInBackground.callsFake(() => initializeInBackgroundDeferred.promise); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + + const promise = condaInheritEnvPrompt.activate(resource); + const deferred = createDeferredFromPromise(promise); + await sleep(1); + + // Ensure activate() function has completed while initializeInBackground() is still not resolved + assert.equal(deferred.completed, true); + + initializeInBackgroundDeferred.resolve(); + await sleep(1); + assert.ok(initializeInBackground.calledOnce); + }); + + test('Ignores errors raised by initializeInBackground()', async () => { + initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground'); + initializeInBackground.rejects(new Error('Kaboom')); + condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); + await condaInheritEnvPrompt.activate(resource); + assert.ok(initializeInBackground.calledOnce); + }); + }); + + suite('Method initializeInBackground()', () => { let shouldShowPrompt: sinon.SinonStub; let promptAndUpdate: sinon.SinonStub; setup(() => { @@ -206,7 +246,7 @@ suite('Conda Inherit Env Prompt', async () => { promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); promptAndUpdate.callsFake(() => Promise.resolve(undefined)); condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); - await condaInheritEnvPrompt.activate(resource); + await condaInheritEnvPrompt.initializeInBackground(resource); assert.ok(shouldShowPrompt.calledOnce); assert.ok(promptAndUpdate.calledOnce); }); @@ -217,7 +257,7 @@ suite('Conda Inherit Env Prompt', async () => { promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate'); promptAndUpdate.callsFake(() => Promise.resolve(undefined)); condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, appShell.object, instance(persistentStateFactory)); - await condaInheritEnvPrompt.activate(resource); + await condaInheritEnvPrompt.initializeInBackground(resource); assert.ok(shouldShowPrompt.calledOnce); assert.ok(promptAndUpdate.notCalled); });