From a4b527bad87582882e22b0b28b06d6269ff67c08 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 12 May 2021 13:48:46 -0700 Subject: [PATCH 01/10] Add telemetry info --- src/client/telemetry/constants.ts | 3 +++ src/client/telemetry/index.ts | 33 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index c14cbed6d3b3..244a5a42b031 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -128,6 +128,9 @@ export enum EventName { JEDI_LANGUAGE_SERVER_TELEMETRY = 'JEDI_LANGUAGE_SERVER.EVENT', JEDI_LANGUAGE_SERVER_REQUEST = 'JEDI_LANGUAGE_SERVER.REQUEST', + JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED = 'JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED', + JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION = 'JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION', + TENSORBOARD_SESSION_LAUNCH = 'TENSORBOARD.SESSION_LAUNCH', TENSORBOARD_SESSION_DURATION = 'TENSORBOARD.SESSION_DURATION', TENSORBOARD_SESSION_DAEMON_STARTUP_DURATION = 'TENSORBOARD.SESSION_DAEMON_STARTUP_DURATION', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index a6aa451997bf..ad02c2c95d1d 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1750,6 +1750,39 @@ export interface IEventNamePropertyMapping { terminal: TerminalShellType; }; + /** + * Telemetry event sent when the notification about the Jupyter extension not being installed is displayed. + * Since this notification will only be displayed after an action that requires the Jupyter extension, + * the telemetry event will include the action the user took, under the `entrypoint` property. + * + * Note: While #16102 is being worked on, the `entrypoint` enum will be updated as we add ways to display this notification. + */ + [EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED]: { + /** + * Action that the user took to trigger the notification. + * + * Note: While #16102 is being worked on, this enum will be updated as we add ways to display this notification. + */ + entrypoint: + | 'startpage_create_blank_notebook' + | 'startpage_create_jupyter_notebook' + | 'startpage_sample_notebook' + | 'startpage_use_interactive_window'; + }; + + /** + * Telemetry event sent when the notification about the Jupyter extension not being installed is closed. + */ + [EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION]: { + /** + * Action selected by the user in response to the notification: install Jupyter, + * close the notification using the close button, or "Do not show again". + * + * @type {('install' | 'closed' | 'do_not_show_again')} + */ + action: 'install' | 'closed' | 'do_not_show_again'; + }; + [Telemetry.WebviewStyleUpdate]: never | undefined; [Telemetry.WebviewMonacoStyleUpdate]: never | undefined; [Telemetry.WebviewStartup]: { type: string }; From c7dce831e4a8405199cd15542b9057638413e78f Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 13 May 2021 07:24:48 -0700 Subject: [PATCH 02/10] Use enum for the telemetry --- src/client/jupyter/types.ts | 8 ++++++++ src/client/telemetry/index.ts | 15 ++++----------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/client/jupyter/types.ts b/src/client/jupyter/types.ts index 5eb58c7cf2b2..86d9586e4017 100644 --- a/src/client/jupyter/types.ts +++ b/src/client/jupyter/types.ts @@ -45,3 +45,11 @@ enum ColumnType { // eslint-disable-next-line @typescript-eslint/no-explicit-any type IRowsResponse = any[]; + +// Note: While #16102 is being worked on, this enum will be updated as we add ways to display this notification. +export enum JupyterNotInstalledOrigin { + StartPageCreateBlankNotebook = 'startpage_create_blank_notebook', + StartPageCreateJupyterNotebook = 'startpage_create_jupyter_notebook', + StartPageCreateSampleNotebook = 'startpage_sample_notebook', + StartPageUseInteractiveWindow = 'startpage_use_interactive_window', +} diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index ad02c2c95d1d..c9889f54121f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -27,6 +27,7 @@ import { import { TestProvider } from '../testing/types'; import { EventName, PlatformErrors } from './constants'; import type { LinterTrigger, TestTool } from './types'; +import { JupyterNotInstalledOrigin } from '../jupyter/types'; /** * Checks whether telemetry is supported. @@ -1754,20 +1755,12 @@ export interface IEventNamePropertyMapping { * Telemetry event sent when the notification about the Jupyter extension not being installed is displayed. * Since this notification will only be displayed after an action that requires the Jupyter extension, * the telemetry event will include the action the user took, under the `entrypoint` property. - * - * Note: While #16102 is being worked on, the `entrypoint` enum will be updated as we add ways to display this notification. */ [EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED]: { /** * Action that the user took to trigger the notification. - * - * Note: While #16102 is being worked on, this enum will be updated as we add ways to display this notification. */ - entrypoint: - | 'startpage_create_blank_notebook' - | 'startpage_create_jupyter_notebook' - | 'startpage_sample_notebook' - | 'startpage_use_interactive_window'; + entrypoint: JupyterNotInstalledOrigin; }; /** @@ -1778,9 +1771,9 @@ export interface IEventNamePropertyMapping { * Action selected by the user in response to the notification: install Jupyter, * close the notification using the close button, or "Do not show again". * - * @type {('install' | 'closed' | 'do_not_show_again')} + * @type {('Install' | 'Do not show again' | undefined)} */ - action: 'install' | 'closed' | 'do_not_show_again'; + selection: 'Install' | 'Do not show again' | undefined; }; [Telemetry.WebviewStyleUpdate]: never | undefined; From 253266cd8a7c61d450d17428eecc47abad79c841 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Thu, 13 May 2021 07:25:17 -0700 Subject: [PATCH 03/10] Add prompt as a standalone function --- src/client/common/utils/localize.ts | 5 ++ src/client/jupyter/jupyterNotInstalled.ts | 57 +++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 src/client/jupyter/jupyterNotInstalled.ts diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index d266f9805eef..80fd801ba783 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -141,6 +141,11 @@ export namespace Jupyter { 'Jupyter.extensionRequired', 'The Jupyter extension is required to perform that task. Click Yes to open the Jupyter extension installation page.', ); + + export const jupyterExtensionNotInstalled = localize( + 'Jupyter.extensionNotInstalled', + "This feature is available in the Jupyter extension, which isn't currently installed.", + ); } export namespace TensorBoard { diff --git a/src/client/jupyter/jupyterNotInstalled.ts b/src/client/jupyter/jupyterNotInstalled.ts new file mode 100644 index 000000000000..aa1c24dce703 --- /dev/null +++ b/src/client/jupyter/jupyterNotInstalled.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { IApplicationShell, ICommandManager, IJupyterExtensionDependencyManager } from '../common/application/types'; +import { JUPYTER_EXTENSION_ID } from '../common/constants'; +import { IPersistentStateFactory } from '../common/types'; +import { Common, Jupyter } from '../common/utils/localize'; +import { sendTelemetryEvent } from '../telemetry'; +import { EventName } from '../telemetry/constants'; +import { JupyterNotInstalledOrigin } from './types'; + +export const jupyterExtensionNotInstalledKey = 'jupyterExtensionNotInstalledKey'; + +export function shouldShowJupypterExtensionNotInstalledPrompt( + depsManager: IJupyterExtensionDependencyManager, + persistentState: IPersistentStateFactory, +): boolean { + const doNotShowAgain = persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false); + + if (doNotShowAgain.value) { + return false; + } + + const isInstalled = depsManager.isJupyterExtensionInstalled; + + return !isInstalled; +} + +export async function jupyterNotInstalledPrompt( + entrypoint: JupyterNotInstalledOrigin, + appShell: IApplicationShell, + persistentState: IPersistentStateFactory, + commandManager: ICommandManager, +): Promise { + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, undefined, { entrypoint }); + + const prompts = [Common.install(), Common.doNotShowAgain()]; + const telemetrySelections: ['Install', 'Do not show again'] = ['Install', 'Do not show again']; + + const selection = await appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), ...prompts); + + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION, undefined, { + selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined, + }); + + if (!selection) { + return; + } + + if (selection === Common.install()) { + // Install the Jupyter extension + commandManager.executeCommand('extension.open', JUPYTER_EXTENSION_ID); + } else if (selection === Common.doNotShowAgain()) { + // Never show this prompt again + await persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false).updateValue(true); + } +} From 3ec463ee95f068386037eedeb08adf76f6d4cb6e Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 15:35:29 -0700 Subject: [PATCH 04/10] Remove "Install" from the prompt --- src/client/telemetry/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c9889f54121f..d5b8c714b127 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1768,12 +1768,12 @@ export interface IEventNamePropertyMapping { */ [EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION]: { /** - * Action selected by the user in response to the notification: install Jupyter, + * Action selected by the user in response to the notification: * close the notification using the close button, or "Do not show again". * - * @type {('Install' | 'Do not show again' | undefined)} + * @type {('Do not show again' | undefined)} */ - selection: 'Install' | 'Do not show again' | undefined; + selection: 'Do not show again' | undefined; }; [Telemetry.WebviewStyleUpdate]: never | undefined; From 5febf45e3484f3422f27fb9b3cff19fcb87e05fc Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 15:35:41 -0700 Subject: [PATCH 05/10] Make it a class --- src/client/jupyter/jupyterNotInstalled.ts | 71 ++++++++++++----------- src/client/jupyter/types.ts | 6 ++ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/client/jupyter/jupyterNotInstalled.ts b/src/client/jupyter/jupyterNotInstalled.ts index aa1c24dce703..328fa23ca955 100644 --- a/src/client/jupyter/jupyterNotInstalled.ts +++ b/src/client/jupyter/jupyterNotInstalled.ts @@ -1,57 +1,58 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { IApplicationShell, ICommandManager, IJupyterExtensionDependencyManager } from '../common/application/types'; -import { JUPYTER_EXTENSION_ID } from '../common/constants'; +import { injectable, inject } from 'inversify'; +import { IApplicationShell, IJupyterExtensionDependencyManager } from '../common/application/types'; import { IPersistentStateFactory } from '../common/types'; import { Common, Jupyter } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; -import { JupyterNotInstalledOrigin } from './types'; +import { IJupyterNotInstalledNotificationHelper, JupyterNotInstalledOrigin } from './types'; export const jupyterExtensionNotInstalledKey = 'jupyterExtensionNotInstalledKey'; -export function shouldShowJupypterExtensionNotInstalledPrompt( - depsManager: IJupyterExtensionDependencyManager, - persistentState: IPersistentStateFactory, -): boolean { - const doNotShowAgain = persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false); +@injectable() +export class JupyterNotInstalledNotificationHelper implements IJupyterNotInstalledNotificationHelper { + constructor( + @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory, + @inject(IJupyterExtensionDependencyManager) private depsManager: IJupyterExtensionDependencyManager, + ) {} - if (doNotShowAgain.value) { - return false; - } + public shouldShowJupypterExtensionNotInstalledPrompt(): boolean { + const doNotShowAgain = this.persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false); - const isInstalled = depsManager.isJupyterExtensionInstalled; + if (doNotShowAgain.value) { + return false; + } - return !isInstalled; -} + const isInstalled = this.depsManager.isJupyterExtensionInstalled; -export async function jupyterNotInstalledPrompt( - entrypoint: JupyterNotInstalledOrigin, - appShell: IApplicationShell, - persistentState: IPersistentStateFactory, - commandManager: ICommandManager, -): Promise { - sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, undefined, { entrypoint }); + return !isInstalled; + } - const prompts = [Common.install(), Common.doNotShowAgain()]; - const telemetrySelections: ['Install', 'Do not show again'] = ['Install', 'Do not show again']; + public async jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise { + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, undefined, { entrypoint }); - const selection = await appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), ...prompts); + const prompts = [Common.doNotShowAgain()]; + const telemetrySelections: ['Do not show again'] = ['Do not show again']; - sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION, undefined, { - selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined, - }); + const selection = await this.appShell.showInformationMessage( + Jupyter.jupyterExtensionNotInstalled(), + ...prompts, + ); - if (!selection) { - return; - } + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION, undefined, { + selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined, + }); + + if (!selection) { + return; + } - if (selection === Common.install()) { - // Install the Jupyter extension - commandManager.executeCommand('extension.open', JUPYTER_EXTENSION_ID); - } else if (selection === Common.doNotShowAgain()) { // Never show this prompt again - await persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false).updateValue(true); + await this.persistentState + .createGlobalPersistentState(jupyterExtensionNotInstalledKey, false) + .updateValue(true); } } diff --git a/src/client/jupyter/types.ts b/src/client/jupyter/types.ts index 86d9586e4017..14d0c868adbf 100644 --- a/src/client/jupyter/types.ts +++ b/src/client/jupyter/types.ts @@ -53,3 +53,9 @@ export enum JupyterNotInstalledOrigin { StartPageCreateSampleNotebook = 'startpage_sample_notebook', StartPageUseInteractiveWindow = 'startpage_use_interactive_window', } + +export const IJupyterNotInstalledNotificationHelper = Symbol('IJupyterNotInstalledNotificationHelper'); +export interface IJupyterNotInstalledNotificationHelper { + shouldShowJupypterExtensionNotInstalledPrompt(): boolean; + jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise; +} From e2b40f7a83156b52a341252eb66c4247e0887f7e Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 15:35:50 -0700 Subject: [PATCH 06/10] Register singleton --- src/client/common/serviceRegistry.ts | 6 ++++++ src/test/common/moduleInstaller.test.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 8c8dc9aff5e9..160123873d9f 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -120,6 +120,8 @@ import { } from './types'; import { IMultiStepInputFactory, MultiStepInputFactory } from './utils/multiStepInput'; import { Random } from './utils/random'; +import { JupyterNotInstalledNotificationHelper } from '../jupyter/jupyterNotInstalled'; +import { IJupyterNotInstalledNotificationHelper } from '../jupyter/types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingletonInstance(IsWindows, IS_WINDOWS); @@ -141,6 +143,10 @@ export function registerTypes(serviceManager: IServiceManager) { IJupyterExtensionDependencyManager, JupyterExtensionDependencyManager, ); + serviceManager.addSingleton( + IJupyterNotInstalledNotificationHelper, + JupyterNotInstalledNotificationHelper, + ); serviceManager.addSingleton(ICommandManager, CommandManager); serviceManager.addSingleton(IConfigurationService, ConfigurationService); serviceManager.addSingleton(IWorkspaceService, WorkspaceService); diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 1190cfca61c0..7892ac657978 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -134,6 +134,8 @@ import { MockModuleInstaller } from '../mocks/moduleInstaller'; import { MockProcessService } from '../mocks/proc'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; import { closeActiveWindows, initializeTest } from '../initialize'; +import { JupyterNotInstalledNotificationHelper } from '../../client/jupyter/jupyterNotInstalled'; +import { IJupyterNotInstalledNotificationHelper } from '../../client/jupyter/types'; chaiUse(chaiAsPromised); @@ -245,6 +247,10 @@ suite('Module Installer', () => { IJupyterExtensionDependencyManager, JupyterExtensionDependencyManager, ); + ioc.serviceManager.addSingleton( + IJupyterNotInstalledNotificationHelper, + JupyterNotInstalledNotificationHelper, + ); ioc.serviceManager.addSingleton(IBrowserService, BrowserService); ioc.serviceManager.addSingleton(IHttpClient, HttpClient); ioc.serviceManager.addSingleton(IFileDownloader, FileDownloader); From 20fbb7d7deed7eda79828470a9705744d56ed8a0 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 15:37:30 -0700 Subject: [PATCH 07/10] Rename file to a long but descriptive name --- src/client/common/serviceRegistry.ts | 2 +- ...NotInstalled.ts => jupyterNotInstalledNotificationHelper.ts} | 0 src/test/common/moduleInstaller.test.ts | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/client/jupyter/{jupyterNotInstalled.ts => jupyterNotInstalledNotificationHelper.ts} (100%) diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 160123873d9f..bd6e00bd6083 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -120,7 +120,7 @@ import { } from './types'; import { IMultiStepInputFactory, MultiStepInputFactory } from './utils/multiStepInput'; import { Random } from './utils/random'; -import { JupyterNotInstalledNotificationHelper } from '../jupyter/jupyterNotInstalled'; +import { JupyterNotInstalledNotificationHelper } from '../jupyter/jupyterNotInstalledNotificationHelper'; import { IJupyterNotInstalledNotificationHelper } from '../jupyter/types'; export function registerTypes(serviceManager: IServiceManager) { diff --git a/src/client/jupyter/jupyterNotInstalled.ts b/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts similarity index 100% rename from src/client/jupyter/jupyterNotInstalled.ts rename to src/client/jupyter/jupyterNotInstalledNotificationHelper.ts diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 7892ac657978..6d1b4b504b11 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -134,7 +134,7 @@ import { MockModuleInstaller } from '../mocks/moduleInstaller'; import { MockProcessService } from '../mocks/proc'; import { UnitTestIocContainer } from '../testing/serviceRegistry'; import { closeActiveWindows, initializeTest } from '../initialize'; -import { JupyterNotInstalledNotificationHelper } from '../../client/jupyter/jupyterNotInstalled'; +import { JupyterNotInstalledNotificationHelper } from '../../client/jupyter/jupyterNotInstalledNotificationHelper'; import { IJupyterNotInstalledNotificationHelper } from '../../client/jupyter/types'; chaiUse(chaiAsPromised); From f3faa3059e94ec5753219aadf92e57d0dd92d13a Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 16:26:28 -0700 Subject: [PATCH 08/10] Unit tests --- ...otInstalledNotificationHelper.unit.test.ts | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts diff --git a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts new file mode 100644 index 000000000000..90415cb5cb9b --- /dev/null +++ b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { IApplicationShell, IJupyterExtensionDependencyManager } from '../../client/common/application/types'; +import { IPersistentStateFactory, IPersistentState } from '../../client/common/types'; +import { Jupyter, Common } from '../../client/common/utils/localize'; +import { + jupyterExtensionNotInstalledKey, + JupyterNotInstalledNotificationHelper, +} from '../../client/jupyter/jupyterNotInstalledNotificationHelper'; +import { JupyterNotInstalledOrigin } from '../../client/jupyter/types'; + +suite('Jupyter not installed notification helper', () => { + let appShell: IApplicationShell; + let persistentStateFactory: IPersistentStateFactory; + let jupyterExtDependencyManager: IJupyterExtensionDependencyManager; + + setup(() => { + appShell = mock(); + persistentStateFactory = mock(); + jupyterExtDependencyManager = mock(); + }); + + test('Notification check should return false if the Jupyter extension is installed', () => { + const persistentState = mock>(); + when(persistentState.value).thenReturn(undefined); + + when( + persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), + ).thenReturn(instance(persistentState)); + + when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(true); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + instance(appShell), + instance(persistentStateFactory), + instance(jupyterExtDependencyManager), + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, false); + verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); + verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).once(); + }); + + test('Notification check should return false if the doNotShowAgain persistent value is set', () => { + const persistentState = mock>(); + when(persistentState.value).thenReturn(true); + + when( + persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), + ).thenReturn(instance(persistentState)); + + when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(true); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + instance(appShell), + instance(persistentStateFactory), + instance(jupyterExtDependencyManager), + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, false); + verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); + verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).never(); + }); + + test('Notification check should return true if the doNotShowAgain persistent value is not set and the Jupyter extension is not installed', () => { + const persistentState = mock>(); + when(persistentState.value).thenReturn(undefined); + + when( + persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), + ).thenReturn(instance(persistentState)); + + when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(false); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + instance(appShell), + instance(persistentStateFactory), + instance(jupyterExtDependencyManager), + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, true); + verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); + verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).once(); + }); + + test('Selecting "Do not show again" should set the doNotShowAgain persistent value', async () => { + const persistentState = mock>(); + + when( + persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), + ).thenReturn(instance(persistentState)); + + when( + appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), Common.doNotShowAgain()), + ).thenReturn(Promise.resolve(Common.doNotShowAgain())); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + instance(appShell), + instance(persistentStateFactory), + instance(jupyterExtDependencyManager), + ); + + await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + + verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false)).once(); + verify(persistentState.updateValue(true)).once(); + }); + + test('Selecting "Do not show again" should make the prompt check return false', async () => { + const persistentState = mock>(); + + when( + persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), + ).thenReturn(instance(persistentState)); + + when( + appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), Common.doNotShowAgain()), + ).thenReturn(Promise.resolve(Common.doNotShowAgain())); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + instance(appShell), + instance(persistentStateFactory), + instance(jupyterExtDependencyManager), + ); + + await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, false); + }); +}); From 96e22dbfabaebe73e3b13b37f59f7dbefc9730f7 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 25 May 2021 16:41:02 -0700 Subject: [PATCH 09/10] Add to package.nls.json --- package.nls.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.nls.json b/package.nls.json index 01c38a8b7111..c3aa71c3d06c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -233,6 +233,7 @@ "StartPage.folderDesc": "- Open a
Folder

- Open a
Workspace
", "StartPage.badWebPanelFormatString": "

{0} is not a valid file name

", "Jupyter.extensionRequired": "The Jupyter extension is required to perform that task. Click Yes to open the Jupyter extension installation page.", + "Jupyter.extensionNotInstalled": "This feature is available in the Jupyter extension, which isn't currently installed.", "TensorBoard.missingSourceFile": "We could not locate the requested source file on disk. Please manually specify the file.", "TensorBoard.selectMissingSourceFile": "Choose File", "TensorBoard.selectMissingSourceFileDescription": "The source file's contents may not match the original contents in the trace.", From 4fc76bdbe9ab9efb1434aa8f43cab0054b139880 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 26 May 2021 09:10:42 -0700 Subject: [PATCH 10/10] Use sinon for tests --- ...otInstalledNotificationHelper.unit.test.ts | 159 ++++++++++-------- 1 file changed, 85 insertions(+), 74 deletions(-) diff --git a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts index 90415cb5cb9b..f82b9cb0c8a6 100644 --- a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts +++ b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts @@ -2,9 +2,9 @@ // Licensed under the MIT License. import * as assert from 'assert'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import * as sinon from 'sinon'; import { IApplicationShell, IJupyterExtensionDependencyManager } from '../../client/common/application/types'; -import { IPersistentStateFactory, IPersistentState } from '../../client/common/types'; +import { IPersistentStateFactory } from '../../client/common/types'; import { Jupyter, Common } from '../../client/common/utils/localize'; import { jupyterExtensionNotInstalledKey, @@ -13,126 +13,137 @@ import { import { JupyterNotInstalledOrigin } from '../../client/jupyter/types'; suite('Jupyter not installed notification helper', () => { - let appShell: IApplicationShell; - let persistentStateFactory: IPersistentStateFactory; - let jupyterExtDependencyManager: IJupyterExtensionDependencyManager; - - setup(() => { - appShell = mock(); - persistentStateFactory = mock(); - jupyterExtDependencyManager = mock(); + teardown(() => { + sinon.restore(); }); test('Notification check should return false if the Jupyter extension is installed', () => { - const persistentState = mock>(); - when(persistentState.value).thenReturn(undefined); - - when( - persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), - ).thenReturn(instance(persistentState)); - - when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(true); + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ value: undefined }); + + // Need to define 'isJupyterExtensionInstalled' for it to be stubbed. + const jupyterExtDependencyManager = { + isJupyterExtensionInstalled: false, + } as IJupyterExtensionDependencyManager; + const isJupyterExtensionInstalledStub = sinon.stub().returns(true); + sinon.stub(jupyterExtDependencyManager, 'isJupyterExtensionInstalled').get(isJupyterExtensionInstalledStub); const notificationHelper = new JupyterNotInstalledNotificationHelper( - instance(appShell), - instance(persistentStateFactory), - instance(jupyterExtDependencyManager), + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + jupyterExtDependencyManager, ); const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); assert.strictEqual(result, false); - verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); - verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).once(); + sinon.assert.calledOnce(createGlobalPersistentStateStub); + sinon.assert.calledWith(createGlobalPersistentStateStub, jupyterExtensionNotInstalledKey, sinon.match.bool); + sinon.assert.calledOnce(isJupyterExtensionInstalledStub); }); test('Notification check should return false if the doNotShowAgain persistent value is set', () => { - const persistentState = mock>(); - when(persistentState.value).thenReturn(true); - - when( - persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), - ).thenReturn(instance(persistentState)); + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ value: true }); - when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(true); + const jupyterExtDependencyManager = { + isJupyterExtensionInstalled: false, + } as IJupyterExtensionDependencyManager; + const isJupyterExtensionInstalledStub = sinon.stub().returns(false); + sinon.stub(jupyterExtDependencyManager, 'isJupyterExtensionInstalled').get(isJupyterExtensionInstalledStub); const notificationHelper = new JupyterNotInstalledNotificationHelper( - instance(appShell), - instance(persistentStateFactory), - instance(jupyterExtDependencyManager), + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + jupyterExtDependencyManager, ); const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); assert.strictEqual(result, false); - verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); - verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).never(); + sinon.assert.calledOnce(createGlobalPersistentStateStub); + sinon.assert.calledWith(createGlobalPersistentStateStub, jupyterExtensionNotInstalledKey, sinon.match.bool); + sinon.assert.notCalled(isJupyterExtensionInstalledStub); }); test('Notification check should return true if the doNotShowAgain persistent value is not set and the Jupyter extension is not installed', () => { - const persistentState = mock>(); - when(persistentState.value).thenReturn(undefined); + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ value: undefined }); - when( - persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), - ).thenReturn(instance(persistentState)); - - when(jupyterExtDependencyManager.isJupyterExtensionInstalled).thenReturn(false); + const jupyterExtDependencyManager = { + isJupyterExtensionInstalled: false, + } as IJupyterExtensionDependencyManager; + const isJupyterExtensionInstalledStub = sinon.stub().returns(false); + sinon.stub(jupyterExtDependencyManager, 'isJupyterExtensionInstalled').get(isJupyterExtensionInstalledStub); const notificationHelper = new JupyterNotInstalledNotificationHelper( - instance(appShell), - instance(persistentStateFactory), - instance(jupyterExtDependencyManager), + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + (jupyterExtDependencyManager as unknown) as IJupyterExtensionDependencyManager, ); const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); assert.strictEqual(result, true); - verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything())).once(); - verify(jupyterExtDependencyManager.isJupyterExtensionInstalled).once(); + sinon.assert.calledOnce(createGlobalPersistentStateStub); + sinon.assert.calledWith(createGlobalPersistentStateStub, jupyterExtensionNotInstalledKey, sinon.match.bool); + sinon.assert.calledOnce(isJupyterExtensionInstalledStub); }); test('Selecting "Do not show again" should set the doNotShowAgain persistent value', async () => { - const persistentState = mock>(); - - when( - persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), - ).thenReturn(instance(persistentState)); + const updateValueStub = sinon.stub(); + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ updateValue: updateValueStub }); - when( - appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), Common.doNotShowAgain()), - ).thenReturn(Promise.resolve(Common.doNotShowAgain())); + const showInformationMessageStub = sinon.stub().returns(Promise.resolve(Common.doNotShowAgain)); const notificationHelper = new JupyterNotInstalledNotificationHelper( - instance(appShell), - instance(persistentStateFactory), - instance(jupyterExtDependencyManager), + ({ showInformationMessage: showInformationMessageStub } as unknown) as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); - verify(persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false)).once(); - verify(persistentState.updateValue(true)).once(); + sinon.assert.calledOnce(createGlobalPersistentStateStub); + sinon.assert.calledOnce(showInformationMessageStub); + sinon.assert.calledWith( + showInformationMessageStub, + Jupyter.jupyterExtensionNotInstalled(), + Common.doNotShowAgain(), + ); + sinon.assert.calledOnce(updateValueStub); + sinon.assert.calledWith(updateValueStub, true); }); test('Selecting "Do not show again" should make the prompt check return false', async () => { - const persistentState = mock>(); - - when( - persistentStateFactory.createGlobalPersistentState(jupyterExtensionNotInstalledKey, anything()), - ).thenReturn(instance(persistentState)); - - when( - appShell.showInformationMessage(Jupyter.jupyterExtensionNotInstalled(), Common.doNotShowAgain()), - ).thenReturn(Promise.resolve(Common.doNotShowAgain())); + const persistentState: { value: boolean | undefined; updateValue: (v: boolean) => void } = { + value: undefined, + updateValue(v: boolean) { + this.value = v; + }, + }; + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns(persistentState); + + const showInformationMessageStub = sinon.stub().returns(Promise.resolve(Common.doNotShowAgain)); const notificationHelper = new JupyterNotInstalledNotificationHelper( - instance(appShell), - instance(persistentStateFactory), - instance(jupyterExtDependencyManager), + ({ showInformationMessage: showInformationMessageStub } as unknown) as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); assert.strictEqual(result, false);