From 99ca4217dddc4c32c1ea663e5b80e6fa0febe515 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Tue, 18 May 2021 14:10:10 -0700 Subject: [PATCH 1/5] Make Jupyter an optional dependency (#16267) * News entry * Move Jupyter to the optional dependencies step * Update news/1 Enhancements/16102.md Co-authored-by: Kartik Raj Co-authored-by: Kartik Raj --- .github/actions/build-vsix/action.yml | 6 +----- gulpfile.js | 17 +---------------- news/1 Enhancements/16102.md | 1 + package.json | 1 - 4 files changed, 3 insertions(+), 22 deletions(-) create mode 100644 news/1 Enhancements/16102.md diff --git a/.github/actions/build-vsix/action.yml b/.github/actions/build-vsix/action.yml index 537013baea63..f2adde82f246 100644 --- a/.github/actions/build-vsix/action.yml +++ b/.github/actions/build-vsix/action.yml @@ -44,11 +44,7 @@ runs: run: npm run updateBuildNumber -- --buildNumber $GITHUB_RUN_ID shell: bash - - name: Update extension dependencies - run: npm run addExtensionDependencies - shell: bash - - - name: Update Optional extension dependencies + - name: Update optional extension dependencies run: npm run addExtensionPackDependencies shell: bash diff --git a/gulpfile.js b/gulpfile.js index 478deee0cc9f..3b5717479855 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -77,32 +77,17 @@ gulp.task('webpack', async () => { await buildWebPackForDevOrProduction('./build/webpack/webpack.extension.config.js', 'extension'); }); -gulp.task('addExtensionDependencies', async () => { - await addExtensionDependencies(); -}); - gulp.task('addExtensionPackDependencies', async () => { await buildLicense(); await addExtensionPackDependencies(); }); -async function addExtensionDependencies() { - // Update the package.json to add extension dependencies at build time so that - // extension dependencies need not be installed during development - const packageJsonContents = await fsExtra.readFile('package.json', 'utf-8'); - const packageJson = JSON.parse(packageJsonContents); - packageJson.extensionDependencies = ['ms-toolsai.jupyter'].concat( - packageJson.extensionDependencies ? packageJson.extensionDependencies : [], - ); - await fsExtra.writeFile('package.json', JSON.stringify(packageJson, null, 4), 'utf-8'); -} - async function addExtensionPackDependencies() { // Update the package.json to add extension pack dependencies at build time so that // extension dependencies need not be installed during development const packageJsonContents = await fsExtra.readFile('package.json', 'utf-8'); const packageJson = JSON.parse(packageJsonContents); - packageJson.extensionPack = ['ms-python.vscode-pylance'].concat( + packageJson.extensionPack = ['ms-toolsai.jupyter', 'ms-python.vscode-pylance'].concat( packageJson.extensionPack ? packageJson.extensionPack : [], ); await fsExtra.writeFile('package.json', JSON.stringify(packageJson, null, 4), 'utf-8'); diff --git a/news/1 Enhancements/16102.md b/news/1 Enhancements/16102.md new file mode 100644 index 000000000000..728d3f03fecd --- /dev/null +++ b/news/1 Enhancements/16102.md @@ -0,0 +1 @@ +Move the Jupyter extension from being a hard dependency to an optional one. diff --git a/package.json b/package.json index 78aeb691b3fa..06dc42c78b70 100644 --- a/package.json +++ b/package.json @@ -2076,7 +2076,6 @@ "format-check": "prettier --check 'src/**/*.ts' 'src/**/*.tsx' 'build/**/*.js' '.github/**/*.yml' gulpfile.js", "format-fix": "prettier --write 'src/**/*.ts' 'src/**/*.tsx' 'build/**/*.js' '.github/**/*.yml' gulpfile.js", "clean": "gulp clean", - "addExtensionDependencies": "gulp addExtensionDependencies", "addExtensionPackDependencies": "gulp addExtensionPackDependencies", "updateBuildNumber": "gulp updateBuildNumber", "verifyBundle": "gulp verifyBundle", From 7668b02c5e9475a87aeda5eb747f1927aebe3569 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Thu, 20 May 2021 09:48:30 -0700 Subject: [PATCH 2/5] License wording update (#16278) * Wording * License wording --- build/license-header.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/build/license-header.txt b/build/license-header.txt index d1575272d318..2a8122642cb2 100644 --- a/build/license-header.txt +++ b/build/license-header.txt @@ -1,5 +1,9 @@ -PLEASE NOTE: This Python extension for Visual Studio Code has a hard dependency on the Jupyter extension for Visual Studio Code which is installed automatically alongside it. The Python extension for Visual Studio Code also holds an optional dependency on the Pylance extension for Visual Studio Code, which is also installed automatically but is separately licensed. +PLEASE NOTE: This is the license for the Python extension for Visual Studio Code. The Python extension automatically installs other extensions as optional dependencies, which can be uninstalled at any time. These extensions have separate licenses: -All the source code for the Python extension for Visual Studio Code is available under the MIT License (given below) as is the source code for the Jupyter extension for Visual Studio Code. But the optional Pylance extension for Visual Studio Code is only available in binary form and it is not licensed under the MIT License. The Pylance extension for Visual Studio Code is licensed under a Microsoft proprietary license, the terms of which are available here: https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license. + - The Jupyter extension is released under an MIT License: + https://marketplace.visualstudio.com/items/ms-toolsai.jupyter/license + + - The Pylance extension is only available in binary form and is released under a Microsoft proprietary license, the terms of which are available here: + https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license ------------------------------------------------------------------------------ From d294a987452d58ec1f37b39a679e9430946d3107 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Mon, 31 May 2021 12:56:18 -0700 Subject: [PATCH 3/5] Add a "Jupyter not installed" notification helper (#16321) * Add telemetry info * Use enum for the telemetry * Add prompt as a standalone function * Remove "Install" from the prompt * Make it a class * Register singleton * Rename file to a long but descriptive name * Unit tests * Add to package.nls.json * Use sinon for tests --- package.nls.json | 1 + src/client/common/serviceRegistry.ts | 6 + src/client/common/utils/localize.ts | 5 + .../jupyterNotInstalledNotificationHelper.ts | 58 +++++++ src/client/jupyter/types.ts | 14 ++ src/client/telemetry/constants.ts | 3 + src/client/telemetry/index.ts | 26 +++ src/test/common/moduleInstaller.test.ts | 6 + ...otInstalledNotificationHelper.unit.test.ts | 151 ++++++++++++++++++ 9 files changed, 270 insertions(+) create mode 100644 src/client/jupyter/jupyterNotInstalledNotificationHelper.ts create mode 100644 src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts 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.", diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 8c8dc9aff5e9..bd6e00bd6083 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/jupyterNotInstalledNotificationHelper'; +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/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/jupyterNotInstalledNotificationHelper.ts b/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts new file mode 100644 index 000000000000..328fa23ca955 --- /dev/null +++ b/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +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 { IJupyterNotInstalledNotificationHelper, JupyterNotInstalledOrigin } from './types'; + +export const jupyterExtensionNotInstalledKey = 'jupyterExtensionNotInstalledKey'; + +@injectable() +export class JupyterNotInstalledNotificationHelper implements IJupyterNotInstalledNotificationHelper { + constructor( + @inject(IApplicationShell) private appShell: IApplicationShell, + @inject(IPersistentStateFactory) private persistentState: IPersistentStateFactory, + @inject(IJupyterExtensionDependencyManager) private depsManager: IJupyterExtensionDependencyManager, + ) {} + + public shouldShowJupypterExtensionNotInstalledPrompt(): boolean { + const doNotShowAgain = this.persistentState.createGlobalPersistentState(jupyterExtensionNotInstalledKey, false); + + if (doNotShowAgain.value) { + return false; + } + + const isInstalled = this.depsManager.isJupyterExtensionInstalled; + + return !isInstalled; + } + + public async jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise { + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, undefined, { entrypoint }); + + const prompts = [Common.doNotShowAgain()]; + const telemetrySelections: ['Do not show again'] = ['Do not show again']; + + const selection = await this.appShell.showInformationMessage( + Jupyter.jupyterExtensionNotInstalled(), + ...prompts, + ); + + sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_ACTION, undefined, { + selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined, + }); + + if (!selection) { + return; + } + + // Never show this prompt again + await this.persistentState + .createGlobalPersistentState(jupyterExtensionNotInstalledKey, false) + .updateValue(true); + } +} diff --git a/src/client/jupyter/types.ts b/src/client/jupyter/types.ts index 5eb58c7cf2b2..14d0c868adbf 100644 --- a/src/client/jupyter/types.ts +++ b/src/client/jupyter/types.ts @@ -45,3 +45,17 @@ 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', +} + +export const IJupyterNotInstalledNotificationHelper = Symbol('IJupyterNotInstalledNotificationHelper'); +export interface IJupyterNotInstalledNotificationHelper { + shouldShowJupypterExtensionNotInstalledPrompt(): boolean; + jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise; +} 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..d5b8c714b127 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. @@ -1750,6 +1751,31 @@ 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. + */ + [EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED]: { + /** + * Action that the user took to trigger the notification. + */ + entrypoint: JupyterNotInstalledOrigin; + }; + + /** + * 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: + * close the notification using the close button, or "Do not show again". + * + * @type {('Do not show again' | undefined)} + */ + selection: 'Do not show again' | undefined; + }; + [Telemetry.WebviewStyleUpdate]: never | undefined; [Telemetry.WebviewMonacoStyleUpdate]: never | undefined; [Telemetry.WebviewStartup]: { type: string }; diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 1190cfca61c0..6d1b4b504b11 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/jupyterNotInstalledNotificationHelper'; +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); diff --git a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts new file mode 100644 index 000000000000..f82b9cb0c8a6 --- /dev/null +++ b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { IApplicationShell, IJupyterExtensionDependencyManager } from '../../client/common/application/types'; +import { IPersistentStateFactory } 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', () => { + teardown(() => { + sinon.restore(); + }); + + test('Notification check should return false if the Jupyter extension is installed', () => { + 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( + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + jupyterExtDependencyManager, + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, false); + 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 createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ value: true }); + + const jupyterExtDependencyManager = { + isJupyterExtensionInstalled: false, + } as IJupyterExtensionDependencyManager; + const isJupyterExtensionInstalledStub = sinon.stub().returns(false); + sinon.stub(jupyterExtDependencyManager, 'isJupyterExtensionInstalled').get(isJupyterExtensionInstalledStub); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + jupyterExtDependencyManager, + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, false); + 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 createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ value: undefined }); + + const jupyterExtDependencyManager = { + isJupyterExtensionInstalled: false, + } as IJupyterExtensionDependencyManager; + const isJupyterExtensionInstalledStub = sinon.stub().returns(false); + sinon.stub(jupyterExtDependencyManager, 'isJupyterExtensionInstalled').get(isJupyterExtensionInstalledStub); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + {} as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + (jupyterExtDependencyManager as unknown) as IJupyterExtensionDependencyManager, + ); + + const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + + assert.strictEqual(result, true); + 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 updateValueStub = sinon.stub(); + const createGlobalPersistentStateStub = sinon + .stub() + .withArgs(jupyterExtensionNotInstalledKey, sinon.match.bool) + .returns({ updateValue: updateValueStub }); + + const showInformationMessageStub = sinon.stub().returns(Promise.resolve(Common.doNotShowAgain)); + + const notificationHelper = new JupyterNotInstalledNotificationHelper( + ({ showInformationMessage: showInformationMessageStub } as unknown) as IApplicationShell, + ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, + {} as IJupyterExtensionDependencyManager, + ); + await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + + 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: { 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( + ({ 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); + }); +}); From b8997c80d93e747d4bb32da042847cd1883bccf4 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Mon, 7 Jun 2021 14:27:30 -0700 Subject: [PATCH 4/5] Use the same "Jupyter is not installed" message everywhere (#16372) * rename to showJupyterNotInstalledPrompt * Replace existing prompt with new prompt * Remove Jupyter check from command manager --- .../common/application/commandManager.ts | 15 ++++----------- src/client/common/application/types.ts | 1 - src/client/common/utils/localize.ts | 5 ----- .../jupyterExtensionDependencyManager.ts | 18 ++---------------- .../jupyterNotInstalledNotificationHelper.ts | 2 +- src/client/jupyter/types.ts | 2 +- ...NotInstalledNotificationHelper.unit.test.ts | 4 ++-- 7 files changed, 10 insertions(+), 37 deletions(-) diff --git a/src/client/common/application/commandManager.ts b/src/client/common/application/commandManager.ts index 498eec10f59a..b0ddb4d60198 100644 --- a/src/client/common/application/commandManager.ts +++ b/src/client/common/application/commandManager.ts @@ -1,17 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { injectable } from 'inversify'; import { commands, Disposable, TextEditor, TextEditorEdit } from 'vscode'; import { ICommandNameArgumentTypeMapping } from './commands'; -import { ICommandManager, IJupyterExtensionDependencyManager } from './types'; +import { ICommandManager } from './types'; @injectable() export class CommandManager implements ICommandManager { - constructor( - @inject(IJupyterExtensionDependencyManager) - private jupyterExtensionDependencyManager: IJupyterExtensionDependencyManager, - ) {} + constructor() {} /** * Registers a command that can be invoked via a keyboard shortcut, @@ -73,11 +70,7 @@ export class CommandManager implements ICommandManager { E extends keyof ICommandNameArgumentTypeMapping, U extends ICommandNameArgumentTypeMapping[E] >(command: E, ...rest: U): Thenable { - if (command.includes('jupyter') && !this.jupyterExtensionDependencyManager.isJupyterExtensionInstalled) { - return this.jupyterExtensionDependencyManager.installJupyterExtension(this); - } else { - return commands.executeCommand(command, ...rest); - } + return commands.executeCommand(command, ...rest); } /** diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index a9d66cb1a7cd..6a279c903c4f 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -489,7 +489,6 @@ export interface ICommandManager { export const IJupyterExtensionDependencyManager = Symbol('IJupyterExtensionDependencyManager'); export interface IJupyterExtensionDependencyManager { readonly isJupyterExtensionInstalled: boolean; - installJupyterExtension(commandManager: ICommandManager): Promise; } export const IDocumentManager = Symbol('IDocumentManager'); diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 80fd801ba783..6614f8017284 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -137,11 +137,6 @@ export namespace Pylance { } export namespace Jupyter { - export const jupyterExtensionRequired = localize( - '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.", diff --git a/src/client/jupyter/jupyterExtensionDependencyManager.ts b/src/client/jupyter/jupyterExtensionDependencyManager.ts index 0db458eac051..defd5ea38241 100644 --- a/src/client/jupyter/jupyterExtensionDependencyManager.ts +++ b/src/client/jupyter/jupyterExtensionDependencyManager.ts @@ -1,27 +1,13 @@ import { inject, injectable } from 'inversify'; -import { IApplicationShell, ICommandManager, IJupyterExtensionDependencyManager } from '../common/application/types'; +import { IJupyterExtensionDependencyManager } from '../common/application/types'; import { JUPYTER_EXTENSION_ID } from '../common/constants'; import { IExtensions } from '../common/types'; -import { Common, Jupyter } from '../common/utils/localize'; @injectable() export class JupyterExtensionDependencyManager implements IJupyterExtensionDependencyManager { - constructor( - @inject(IExtensions) private extensions: IExtensions, - @inject(IApplicationShell) private appShell: IApplicationShell, - ) {} + constructor(@inject(IExtensions) private extensions: IExtensions) {} public get isJupyterExtensionInstalled(): boolean { return this.extensions.getExtension(JUPYTER_EXTENSION_ID) !== undefined; } - - public async installJupyterExtension(commandManager: ICommandManager): Promise { - const yes = Common.bannerLabelYes(); - const no = Common.bannerLabelNo(); - const answer = await this.appShell.showErrorMessage(Jupyter.jupyterExtensionRequired(), yes, no); - if (answer === yes) { - commandManager.executeCommand('extension.open', JUPYTER_EXTENSION_ID); - } - return undefined; - } } diff --git a/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts b/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts index 328fa23ca955..c3c7ec578702 100644 --- a/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts +++ b/src/client/jupyter/jupyterNotInstalledNotificationHelper.ts @@ -31,7 +31,7 @@ export class JupyterNotInstalledNotificationHelper implements IJupyterNotInstall return !isInstalled; } - public async jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise { + public async showJupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise { sendTelemetryEvent(EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, undefined, { entrypoint }); const prompts = [Common.doNotShowAgain()]; diff --git a/src/client/jupyter/types.ts b/src/client/jupyter/types.ts index 14d0c868adbf..dec66cdcb729 100644 --- a/src/client/jupyter/types.ts +++ b/src/client/jupyter/types.ts @@ -57,5 +57,5 @@ export enum JupyterNotInstalledOrigin { export const IJupyterNotInstalledNotificationHelper = Symbol('IJupyterNotInstalledNotificationHelper'); export interface IJupyterNotInstalledNotificationHelper { shouldShowJupypterExtensionNotInstalledPrompt(): boolean; - jupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise; + showJupyterNotInstalledPrompt(entrypoint: JupyterNotInstalledOrigin): Promise; } diff --git a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts index f82b9cb0c8a6..6323e30aa09c 100644 --- a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts +++ b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts @@ -110,7 +110,7 @@ suite('Jupyter not installed notification helper', () => { ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); sinon.assert.calledOnce(createGlobalPersistentStateStub); sinon.assert.calledOnce(showInformationMessageStub); @@ -142,7 +142,7 @@ suite('Jupyter not installed notification helper', () => { ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.jupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); From a7c4fa27733a37e865d8f30c69ea9494bfa419d2 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Tue, 8 Jun 2021 15:28:10 -0700 Subject: [PATCH 5/5] Update the start page to use the prompt (#16417) * Update copy * Update origin key * Show prompt if jupyter not installed & should show * Add tests for this functionality only * Update news entry * Remove comments * follow-up from the merge * Add singletons for startpage functional tests * Missing one symbol * Update src/client/common/startPage/startPage.ts Co-authored-by: Don Jayamanne * Add logging Co-authored-by: Don Jayamanne --- news/1 Enhancements/16102.md | 2 +- package.nls.json | 2 +- src/client/common/startPage/startPage.ts | 94 +++++++--- src/client/common/utils/localize.ts | 2 +- src/client/jupyter/types.ts | 8 +- src/startPage-ui/startPage/startPage.tsx | 2 +- ...otInstalledNotificationHelper.unit.test.ts | 4 +- src/test/startPage/startPage.unit.test.ts | 164 +++++++++++++++++- src/test/startPage/startPageIocContainer.ts | 17 ++ 9 files changed, 255 insertions(+), 40 deletions(-) diff --git a/news/1 Enhancements/16102.md b/news/1 Enhancements/16102.md index 728d3f03fecd..3f3cb719476b 100644 --- a/news/1 Enhancements/16102.md +++ b/news/1 Enhancements/16102.md @@ -1 +1 @@ -Move the Jupyter extension from being a hard dependency to an optional one. +Move the Jupyter extension from being a hard dependency to an optional one, and display an informational prompt if Jupyter commands try to be executed from the Start Page. diff --git a/package.nls.json b/package.nls.json index c3aa71c3d06c..0e1ee2047ffd 100644 --- a/package.nls.json +++ b/package.nls.json @@ -222,7 +222,7 @@ "StartPage.createAPythonFile": "Create a Python File", "StartPage.pythonFileDescription": "- Create a
new file
with a .py extension", "StartPage.openInteractiveWindow": "Use the Interactive Window to develop Python Scripts", - "StartPage.interactiveWindowDesc": "- You can create cells on a Python file by typing \"#%%\"
- Use \"
Shift + Enter
\" to run a cell, the output will be shown in the interactive window", + "StartPage.interactiveWindowDesc": "- You can create cells on a Python file by typing \"#%%\". Make sure you have the Jupyter extension installed.
- Use \"
Shift + Enter
\" to run a cell, the output will be shown in the interactive window", "StartPage.releaseNotes": "Take a look at our Release Notes to learn more about the latest features.", "StartPage.mailingList": "Sign up for tips and tutorials through our mailing list.", "StartPage.tutorialAndDoc": "Explore more features in our Tutorials or check Documentation for tips and troubleshooting.", diff --git a/src/client/common/startPage/startPage.ts b/src/client/common/startPage/startPage.ts index 3c9b339f8c2c..c75708676065 100644 --- a/src/client/common/startPage/startPage.ts +++ b/src/client/common/startPage/startPage.ts @@ -3,24 +3,27 @@ 'use strict'; -import { inject, injectable } from 'inversify'; +import { inject, injectable, named } from 'inversify'; import * as path from 'path'; import { ConfigurationTarget, EventEmitter, UIKind, Uri, ViewColumn } from 'vscode'; import { IExtensionSingleActivationService } from '../../activation/types'; import { EXTENSION_ROOT_DIR } from '../../constants'; +import { IJupyterNotInstalledNotificationHelper, JupyterNotInstalledOrigin } from '../../jupyter/types'; import { sendTelemetryEvent } from '../../telemetry'; import { IApplicationEnvironment, IApplicationShell, ICommandManager, IDocumentManager, + IJupyterExtensionDependencyManager, IWebviewPanelProvider, IWorkspaceService, } from '../application/types'; -import { CommandSource } from '../constants'; +import { CommandSource, STANDARD_OUTPUT_CHANNEL } from '../constants'; import { IFileSystem } from '../platform/types'; -import { IConfigurationService, IExtensionContext, Resource } from '../types'; +import { IConfigurationService, IExtensionContext, IOutputChannel, Resource } from '../types'; import * as localize from '../utils/localize'; +import { Jupyter } from '../utils/localize'; import { StopWatch } from '../utils/stopWatch'; import { Telemetry } from './constants'; import { StartPageMessageListener } from './startPageMessageListener'; @@ -62,6 +65,10 @@ export class StartPage extends WebviewPanelHost @inject(IApplicationShell) private appShell: IApplicationShell, @inject(IExtensionContext) private readonly context: IExtensionContext, @inject(IApplicationEnvironment) private appEnvironment: IApplicationEnvironment, + @inject(IJupyterNotInstalledNotificationHelper) + private notificationHelper: IJupyterNotInstalledNotificationHelper, + @inject(IJupyterExtensionDependencyManager) private depsManager: IJupyterExtensionDependencyManager, + @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel, ) { super( configuration, @@ -128,6 +135,9 @@ export class StartPage extends WebviewPanelHost } public async onMessage(message: string, payload: unknown): Promise { + const shouldShowJupyterNotInstalledPrompt = await this.notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); + const isJupyterInstalled = this.depsManager.isJupyterExtensionInstalled; + switch (message) { case StartPageMessages.Started: this.webviewDidLoad = true; @@ -140,19 +150,29 @@ export class StartPage extends WebviewPanelHost break; } case StartPageMessages.OpenBlankNotebook: { - sendTelemetryEvent(Telemetry.StartPageOpenBlankNotebook); - this.setTelemetryFlags(); - - const savedVersion: string | undefined = this.context.globalState.get(EXTENSION_VERSION_MEMENTO); - - if (savedVersion) { - await this.commandManager.executeCommand( - 'jupyter.opennotebook', - undefined, - CommandSource.commandPalette, - ); + if (!isJupyterInstalled) { + this.output.appendLine(Jupyter.jupyterExtensionNotInstalled()); + + if (shouldShowJupyterNotInstalledPrompt) { + await this.notificationHelper.showJupyterNotInstalledPrompt( + JupyterNotInstalledOrigin.StartPageOpenBlankNotebook, + ); + } } else { - this.openSampleNotebook().ignoreErrors(); + sendTelemetryEvent(Telemetry.StartPageOpenBlankNotebook); + this.setTelemetryFlags(); + + const savedVersion: string | undefined = this.context.globalState.get(EXTENSION_VERSION_MEMENTO); + + if (savedVersion) { + await this.commandManager.executeCommand( + 'jupyter.opennotebook', + undefined, + CommandSource.commandPalette, + ); + } else { + this.openSampleNotebook().ignoreErrors(); + } } break; } @@ -168,15 +188,25 @@ export class StartPage extends WebviewPanelHost break; } case StartPageMessages.OpenInteractiveWindow: { - sendTelemetryEvent(Telemetry.StartPageOpenInteractiveWindow); - this.setTelemetryFlags(); - - const doc2 = await this.documentManager.openTextDocument({ - language: 'python', - content: `#%%\nprint("${localize.StartPage.helloWorld()}")`, - }); - await this.documentManager.showTextDocument(doc2, 1, true); - await this.commandManager.executeCommand('jupyter.runallcells', Uri.parse('')); + if (!isJupyterInstalled) { + this.output.appendLine(Jupyter.jupyterExtensionNotInstalled()); + + if (shouldShowJupyterNotInstalledPrompt) { + await this.notificationHelper.showJupyterNotInstalledPrompt( + JupyterNotInstalledOrigin.StartPageOpenInteractiveWindow, + ); + } + } else { + sendTelemetryEvent(Telemetry.StartPageOpenInteractiveWindow); + this.setTelemetryFlags(); + + const doc2 = await this.documentManager.openTextDocument({ + language: 'python', + content: `#%%\nprint("${localize.StartPage.helloWorld()}")`, + }); + await this.documentManager.showTextDocument(doc2, 1, true); + await this.commandManager.executeCommand('jupyter.runallcells', doc2.uri); + } break; } case StartPageMessages.OpenCommandPalette: @@ -192,10 +222,20 @@ export class StartPage extends WebviewPanelHost await this.commandManager.executeCommand('workbench.action.quickOpen', '>Create New Blank Notebook'); break; case StartPageMessages.OpenSampleNotebook: - sendTelemetryEvent(Telemetry.StartPageOpenSampleNotebook); - this.setTelemetryFlags(); + if (!isJupyterInstalled) { + this.output.appendLine(Jupyter.jupyterExtensionNotInstalled()); + + if (shouldShowJupyterNotInstalledPrompt) { + await this.notificationHelper.showJupyterNotInstalledPrompt( + JupyterNotInstalledOrigin.StartPageOpenSampleNotebook, + ); + } + } else { + sendTelemetryEvent(Telemetry.StartPageOpenSampleNotebook); + this.setTelemetryFlags(); - this.openSampleNotebook().ignoreErrors(); + this.openSampleNotebook().ignoreErrors(); + } break; case StartPageMessages.OpenFileBrowser: { sendTelemetryEvent(Telemetry.StartPageOpenFileBrowser); diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 6614f8017284..6f83eaab4087 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -441,7 +441,7 @@ export namespace StartPage { ); export const interactiveWindowDesc = localize( 'StartPage.interactiveWindowDesc', - '- You can create cells on a Python file by typing "#%%"
- Use "
Shift + Enter
" to run a cell, the output will be shown in the interactive window', + '- You can create cells on a Python file by typing "#%%". Make sure you have the Jupyter extension installed.
- Use "
Shift + Enter
" to run a cell, the output will be shown in the interactive window', ); export const releaseNotes = localize( diff --git a/src/client/jupyter/types.ts b/src/client/jupyter/types.ts index dec66cdcb729..fdd28ea3df7b 100644 --- a/src/client/jupyter/types.ts +++ b/src/client/jupyter/types.ts @@ -46,12 +46,10 @@ 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', + StartPageOpenBlankNotebook = 'startpage_open_blank_notebook', + StartPageOpenSampleNotebook = 'startpage_open_sample_notebook', + StartPageOpenInteractiveWindow = 'startpage_open_interactive_window', } export const IJupyterNotInstalledNotificationHelper = Symbol('IJupyterNotInstalledNotificationHelper'); diff --git a/src/startPage-ui/startPage/startPage.tsx b/src/startPage-ui/startPage/startPage.tsx index 2381fdb48d27..34002550a50f 100644 --- a/src/startPage-ui/startPage/startPage.tsx +++ b/src/startPage-ui/startPage/startPage.tsx @@ -214,7 +214,7 @@ export class StartPage extends React.Component implements IMess dangerouslySetInnerHTML={{ __html: getLocString( 'StartPage.interactiveWindowDesc', - '- You can create cells on a Python file by typing "#%%"
- Use "
Shift + Enter
" to run a cell, the output will be shown in the interactive window', + '- You can create cells on a Python file by typing "#%%". Make sure you have the Jupyter extension installed.
- Use "
Shift + Enter
" to run a cell, the output will be shown in the interactive window', ), }} /> diff --git a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts index 6323e30aa09c..616793e04c8b 100644 --- a/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts +++ b/src/test/jupyter/jupyterNotInstalledNotificationHelper.unit.test.ts @@ -110,7 +110,7 @@ suite('Jupyter not installed notification helper', () => { ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageOpenBlankNotebook); sinon.assert.calledOnce(createGlobalPersistentStateStub); sinon.assert.calledOnce(showInformationMessageStub); @@ -142,7 +142,7 @@ suite('Jupyter not installed notification helper', () => { ({ createGlobalPersistentState: createGlobalPersistentStateStub } as unknown) as IPersistentStateFactory, {} as IJupyterExtensionDependencyManager, ); - await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageCreateBlankNotebook); + await notificationHelper.showJupyterNotInstalledPrompt(JupyterNotInstalledOrigin.StartPageOpenBlankNotebook); const result = notificationHelper.shouldShowJupypterExtensionNotInstalledPrompt(); diff --git a/src/test/startPage/startPage.unit.test.ts b/src/test/startPage/startPage.unit.test.ts index 85799f349e82..0f66880f8cff 100644 --- a/src/test/startPage/startPage.unit.test.ts +++ b/src/test/startPage/startPage.unit.test.ts @@ -4,6 +4,7 @@ 'use strict'; import * as assert from 'assert'; +import * as sinon from 'sinon'; import * as typemoq from 'typemoq'; import { ExtensionContext } from 'vscode'; import { @@ -11,15 +12,27 @@ import { IApplicationShell, ICommandManager, IDocumentManager, + IJupyterExtensionDependencyManager, IWebviewPanelProvider, IWorkspaceService, } from '../../client/common/application/types'; import { PythonSettings } from '../../client/common/configSettings'; import { IFileSystem } from '../../client/common/platform/types'; import { StartPage } from '../../client/common/startPage/startPage'; -import { ICodeCssGenerator, IStartPage, IThemeFinder } from '../../client/common/startPage/types'; -import { IConfigurationService, IExtensionContext } from '../../client/common/types'; +import { ICodeCssGenerator, IStartPage, IThemeFinder, StartPageMessages } from '../../client/common/startPage/types'; +import { + IConfigurationService, + IExtensionContext, + IOutputChannel, + IPersistentState, + IPersistentStateFactory, +} from '../../client/common/types'; +import { IJupyterNotInstalledNotificationHelper, JupyterNotInstalledOrigin } from '../../client/jupyter/types'; import { MockAutoSelectionService } from '../mocks/autoSelector'; +import * as Telemetry from '../../client/telemetry'; +import { EventName } from '../../client/telemetry/constants'; +import { JupyterNotInstalledNotificationHelper } from '../../client/jupyter/jupyterNotInstalledNotificationHelper'; +import { Jupyter } from '../../client/common/utils/localize'; suite('StartPage tests', () => { let startPage: IStartPage; @@ -34,7 +47,10 @@ suite('StartPage tests', () => { let appShell: typemoq.IMock; let context: typemoq.IMock; let appEnvironment: typemoq.IMock; + let depsManager: typemoq.IMock; + let outputChannel: typemoq.IMock; let memento: typemoq.IMock; + let notificationHelper: IJupyterNotInstalledNotificationHelper; const dummySettings = new PythonSettings(undefined, new MockAutoSelectionService()); function setupVersions(savedVersion: string, actualVersion: string) { @@ -65,8 +81,23 @@ suite('StartPage tests', () => { appShell = typemoq.Mock.ofType(); context = typemoq.Mock.ofType(); appEnvironment = typemoq.Mock.ofType(); + depsManager = typemoq.Mock.ofType(); + outputChannel = typemoq.Mock.ofType(); memento = typemoq.Mock.ofType(); + // Notification helper object + const stateFactory = typemoq.Mock.ofType(); + const state = typemoq.Mock.ofType>(); + + stateFactory + .setup((s) => s.createGlobalPersistentState(typemoq.It.isAny(), typemoq.It.isAny(), typemoq.It.isAny())) + .returns(() => state.object); + notificationHelper = new JupyterNotInstalledNotificationHelper( + appShell.object, + stateFactory.object, + depsManager.object, + ); + context.setup((c) => c.globalState).returns(() => memento.object); configuration.setup((cs) => cs.getSettings(undefined)).returns(() => dummySettings); @@ -82,9 +113,16 @@ suite('StartPage tests', () => { appShell.object, context.object, appEnvironment.object, + notificationHelper, + depsManager.object, + outputChannel.object, ); }); + teardown(() => { + sinon.restore(); + }); + test('Check extension version', async () => { let savedVersion: string; let actualVersion: string; @@ -116,4 +154,126 @@ suite('StartPage tests', () => { assert.equal(test3, true, 'The actual version is newer, start page should open.'); reset(); }); + + suite('"Jupyter is not installed" prompt tests', () => { + type StartPageMessageForTests = IStartPage & { + onMessage(message: string, payload: unknown): Promise; + }; + + let startPageWithMessageHandler: StartPageMessageForTests; + let telemetryEvents: { eventName: string; properties: Record }[] = []; + let sendTelemetryEventStub: sinon.SinonStub; + + setup(() => { + sendTelemetryEventStub = sinon + .stub(Telemetry, 'sendTelemetryEvent') + .callsFake((eventName: string, _, properties: Record) => { + const telemetry = { eventName, properties }; + telemetryEvents.push(telemetry); + }); + + startPageWithMessageHandler = (startPage as unknown) as StartPageMessageForTests; + }); + + teardown(() => { + telemetryEvents = []; + Telemetry._resetSharedProperties(); + }); + + const notebookActions = [ + { + testcase: 'a blank notebook', + message: StartPageMessages.OpenBlankNotebook, + entrypoint: JupyterNotInstalledOrigin.StartPageOpenBlankNotebook, + }, + { + testcase: 'a sample notebook', + message: StartPageMessages.OpenSampleNotebook, + entrypoint: JupyterNotInstalledOrigin.StartPageOpenSampleNotebook, + }, + { + testcase: 'the interactive window', + message: StartPageMessages.OpenInteractiveWindow, + entrypoint: JupyterNotInstalledOrigin.StartPageOpenInteractiveWindow, + }, + ]; + + notebookActions.forEach(({ testcase, message, entrypoint }) => { + suite(`When opening ${testcase}`, () => { + test('Should display "Jupyter is not installed" prompt if the Jupyter extension is not installed and the prompt should not be shown', async () => { + depsManager.setup((dm) => dm.isJupyterExtensionInstalled).returns(() => false); + const shouldShowPromptStub = sinon.stub( + notificationHelper, + 'shouldShowJupypterExtensionNotInstalledPrompt', + ); + shouldShowPromptStub.returns(true); + + await startPageWithMessageHandler.onMessage(message, {}); + + sinon.assert.called(sendTelemetryEventStub); + sinon.assert.calledOnce(shouldShowPromptStub); + // 2 events: one when the prompt is displayed, one with the prompt selection (in this case, nothing). + assert.strictEqual(telemetryEvents.length, 2); + assert.deepStrictEqual(telemetryEvents[0], { + eventName: EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, + properties: { entrypoint }, + }); + }); + + test('Should not display "Jupyter is not installed" prompt if the Jupyter extension is not installed and the prompt should not be shown', async () => { + depsManager.setup((dm) => dm.isJupyterExtensionInstalled).returns(() => false); + const shouldShowPromptStub = sinon.stub( + notificationHelper, + 'shouldShowJupypterExtensionNotInstalledPrompt', + ); + shouldShowPromptStub.returns(false); + + await startPageWithMessageHandler.onMessage(StartPageMessages.OpenBlankNotebook, {}); + + sinon.assert.notCalled(sendTelemetryEventStub); + sinon.assert.calledOnce(shouldShowPromptStub); + assert.strictEqual(telemetryEvents.length, 0); + }); + + test('Should not display "Jupyter is not installed" prompt if the Jupyter extension is installed', async () => { + depsManager.setup((dm) => dm.isJupyterExtensionInstalled).returns(() => true); + const shouldShowPromptStub = sinon.stub( + notificationHelper, + 'shouldShowJupypterExtensionNotInstalledPrompt', + ); + shouldShowPromptStub.returns(false); + + await startPageWithMessageHandler.onMessage(StartPageMessages.OpenBlankNotebook, {}); + + sinon.assert.called(sendTelemetryEventStub); + sinon.assert.calledOnce(shouldShowPromptStub); + // There is a telemetry event sent when performing the action. + assert.strictEqual(telemetryEvents.length, 1); + assert.notDeepStrictEqual(telemetryEvents[0], { + eventName: EventName.JUPYTER_NOT_INSTALLED_NOTIFICATION_DISPLAYED, + properties: { entrypoint }, + }); + }); + + test('Should write something in the Python output channel if the Jupyter extension is not installed', async () => { + let output = ''; + outputChannel + .setup((oc) => oc.appendLine(typemoq.It.isAnyString())) + .callback((line: string) => { + output += line; + }) + .verifiable(typemoq.Times.once()); + depsManager.setup((dm) => dm.isJupyterExtensionInstalled).returns(() => false); + + await startPageWithMessageHandler.onMessage(StartPageMessages.OpenBlankNotebook, {}); + + outputChannel.verify( + (oc) => oc.appendLine(Jupyter.jupyterExtensionNotInstalled()), + typemoq.Times.once(), + ); + assert.strictEqual(output, Jupyter.jupyterExtensionNotInstalled()); + }); + }); + }); + }); }); diff --git a/src/test/startPage/startPageIocContainer.ts b/src/test/startPage/startPageIocContainer.ts index e6438af254e0..090a17021c57 100644 --- a/src/test/startPage/startPageIocContainer.ts +++ b/src/test/startPage/startPageIocContainer.ts @@ -33,6 +33,7 @@ import { IApplicationShell, ICommandManager, IDocumentManager, + IJupyterExtensionDependencyManager, IWebviewPanelOptions, IWebviewPanelProvider, IWorkspaceService, @@ -47,6 +48,7 @@ import { ExperimentService } from '../../client/common/experiments/service'; import { InstallationChannelManager } from '../../client/common/installer/channelManager'; import { IInstallationChannelManager } from '../../client/common/installer/types'; import { HttpClient } from '../../client/common/net/httpClient'; +import { PersistentStateFactory } from '../../client/common/persistentState'; import { IS_WINDOWS } from '../../client/common/platform/constants'; import { FileSystem } from '../../client/common/platform/fileSystem'; import { PathUtils } from '../../client/common/platform/pathUtils'; @@ -65,6 +67,7 @@ import { IExtensions, IHttpClient, IPathUtils, + IPersistentStateFactory, IPythonSettings, IsWindows, Resource, @@ -73,6 +76,9 @@ import { sleep } from '../../client/common/utils/async'; import { noop } from '../../client/common/utils/misc'; import { EnvironmentActivationServiceCache } from '../../client/interpreter/activation/service'; +import { JupyterExtensionDependencyManager } from '../../client/jupyter/jupyterExtensionDependencyManager'; +import { JupyterNotInstalledNotificationHelper } from '../../client/jupyter/jupyterNotInstalledNotificationHelper'; +import { IJupyterNotInstalledNotificationHelper } from '../../client/jupyter/types'; import { CacheableLocatorPromiseCache } from '../../client/pythonEnvironments/discovery/locators/services/cacheableLocatorService'; import { MockAutoSelectionService } from '../mocks/autoSelector'; @@ -255,6 +261,17 @@ export class StartPageIocContainer extends UnitTestIocContainer { this.serviceManager.add(IInstallationChannelManager, InstallationChannelManager); + // "Jupyter is not installed" prompt. + this.serviceManager.addSingleton(IPersistentStateFactory, PersistentStateFactory); + this.serviceManager.addSingleton( + IJupyterExtensionDependencyManager, + JupyterExtensionDependencyManager, + ); + this.serviceManager.addSingleton( + IJupyterNotInstalledNotificationHelper, + JupyterNotInstalledNotificationHelper, + ); + const mockMemento = TypeMoq.Mock.ofType(); const mockExtensionContext = TypeMoq.Mock.ofType(); mockExtensionContext.setup((m) => m.globalState).returns(() => mockMemento.object);