From 2bfa7c6bad13c41c6330ce71ed36d01b04616ee9 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 15 Aug 2022 11:57:58 +1000 Subject: [PATCH 1/4] add select interpreter option to setup workspace toast --- extension/src/extensions/python.ts | 6 +++++- extension/src/setup.test.ts | 25 ++++++++++++++++++++++++- extension/src/setup.ts | 21 ++++++++++++++------- extension/src/vscode/response.ts | 1 + 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/extension/src/extensions/python.ts b/extension/src/extensions/python.ts index 7c72c56afb..7c310345d1 100644 --- a/extension/src/extensions/python.ts +++ b/extension/src/extensions/python.ts @@ -1,4 +1,4 @@ -import { Event, Uri } from 'vscode' +import { commands, Event, Uri } from 'vscode' import { executeProcess } from '../processExecution' import { getExtensionAPI, isInstalled } from '../vscode/extensions' @@ -54,3 +54,7 @@ export const getOnDidChangePythonExecutionDetails = async () => { } export const isPythonExtensionInstalled = () => isInstalled(PYTHON_EXTENSION_ID) + +export const selectPythonInterpreter = () => { + commands.executeCommand('python.setInterpreter') +} diff --git a/extension/src/setup.test.ts b/extension/src/setup.test.ts index edfc6fb86f..d1eaa5d474 100644 --- a/extension/src/setup.test.ts +++ b/extension/src/setup.test.ts @@ -1,5 +1,5 @@ import { resolve } from 'path' -import { extensions, Extension } from 'vscode' +import { extensions, Extension, commands } from 'vscode' import { setup, setupWorkspace } from './setup' import { flushPromises } from './test/util/jest' import { @@ -25,6 +25,9 @@ jest.mock('./vscode/toast') jest.mock('./vscode/workspaceFolders') const mockedExtensions = jest.mocked(extensions) +const mockedCommands = jest.mocked(commands) +const mockedExecuteCommand = jest.fn() +mockedCommands.executeCommand = mockedExecuteCommand const mockedCanRunCli = jest.fn() const mockedHasRoots = jest.fn() @@ -277,6 +280,25 @@ describe('setup', () => { expect(mockedGetConfigValue).toBeCalledTimes(1) expect(mockedWarnWithOptions).toBeCalledTimes(1) expect(mockedSetupWorkspace).toBeCalledTimes(1) + expect(mockedExecuteCommand).not.toBeCalled() + expect(mockedSetUserConfigValue).not.toBeCalled() + expect(mockedResetMembers).toBeCalledTimes(1) + expect(mockedInitialize).not.toBeCalled() + }) + + it('should try to select the python interpreter if the workspace contains a DVC project, the cli cannot be found and the user selects select the python interpreter', async () => { + mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd) + mockedHasRoots.mockReturnValueOnce(true) + mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) + mockedWarnWithOptions.mockResolvedValueOnce(Response.SELECT_INTERPRETER) + + await setup(extension) + await flushPromises() + expect(mockedSetRoots).toBeCalledTimes(1) + expect(mockedGetConfigValue).toBeCalledTimes(1) + expect(mockedWarnWithOptions).toBeCalledTimes(1) + expect(mockedSetupWorkspace).toBeCalledTimes(0) + expect(mockedExecuteCommand).toBeCalledTimes(1) expect(mockedSetUserConfigValue).not.toBeCalled() expect(mockedResetMembers).toBeCalledTimes(1) expect(mockedInitialize).not.toBeCalled() @@ -294,6 +316,7 @@ describe('setup', () => { expect(mockedGetConfigValue).toBeCalledTimes(1) expect(mockedWarnWithOptions).toBeCalledTimes(1) expect(mockedSetupWorkspace).not.toBeCalled() + expect(mockedExecuteCommand).not.toBeCalled() expect(mockedSetUserConfigValue).toBeCalledTimes(1) expect(mockedResetMembers).toBeCalledTimes(1) expect(mockedInitialize).not.toBeCalled() diff --git a/extension/src/setup.ts b/extension/src/setup.ts index e6c998476e..47845798bf 100644 --- a/extension/src/setup.ts +++ b/extension/src/setup.ts @@ -15,7 +15,10 @@ import { getFirstWorkspaceFolder } from './vscode/workspaceFolders' import { Response } from './vscode/response' import { getSelectTitle, Title } from './vscode/title' import { Toast } from './vscode/toast' -import { isPythonExtensionInstalled } from './extensions/python' +import { + isPythonExtensionInstalled, + selectPythonInterpreter +} from './extensions/python' const setConfigPath = async ( option: ConfigKey, @@ -166,15 +169,19 @@ const warnUserCLIInaccessible = async ( return } const response = await Toast.warnWithOptions( - 'An error was thrown when trying to access the CLI.', + 'An error was thrown when trying to access the CLI. Please ensure the correct interpreter is set for auto Python environment activation.', Response.SETUP_WORKSPACE, + Response.SELECT_INTERPRETER, Response.NEVER ) - if (response === Response.SETUP_WORKSPACE) { - extension.setupWorkspace() - } - if (response === Response.NEVER) { - setUserConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE, true) + + switch (response) { + case Response.SELECT_INTERPRETER: + return selectPythonInterpreter() + case Response.SETUP_WORKSPACE: + return extension.setupWorkspace() + case Response.NEVER: + return setUserConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE, true) } } diff --git a/extension/src/vscode/response.ts b/extension/src/vscode/response.ts index 2769d77327..37b2b9da1d 100644 --- a/extension/src/vscode/response.ts +++ b/extension/src/vscode/response.ts @@ -7,6 +7,7 @@ export enum Response { NEVER = "Don't Show Again", NO = 'No', SELECT_MOST_RECENT = 'Select Most Recent', + SELECT_INTERPRETER = 'Select Python Interpreter', SETUP_WORKSPACE = 'Setup The Workspace', SHOW = 'Show', TURN_OFF = 'Turn Off', From d59296cd04f9536752f5c15bf101b323ff500606 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 15 Aug 2022 12:47:09 +1000 Subject: [PATCH 2/4] self review --- extension/src/setup.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/setup.test.ts b/extension/src/setup.test.ts index d1eaa5d474..ad9cf87ed9 100644 --- a/extension/src/setup.test.ts +++ b/extension/src/setup.test.ts @@ -286,7 +286,7 @@ describe('setup', () => { expect(mockedInitialize).not.toBeCalled() }) - it('should try to select the python interpreter if the workspace contains a DVC project, the cli cannot be found and the user selects select the python interpreter', async () => { + it('should try to select the python interpreter if the workspace contains a DVC project, the cli cannot be found and the user decides to select the python interpreter', async () => { mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd) mockedHasRoots.mockReturnValueOnce(true) mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) From 0772e3d6710cacf97e912036fcf4da4f74809199 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 15 Aug 2022 12:54:24 +1000 Subject: [PATCH 3/4] add note into install dvc markdown --- extension/resources/walkthrough/install-dvc.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extension/resources/walkthrough/install-dvc.md b/extension/resources/walkthrough/install-dvc.md index 1a2caff988..f881c531ce 100644 --- a/extension/resources/walkthrough/install-dvc.md +++ b/extension/resources/walkthrough/install-dvc.md @@ -22,3 +22,6 @@ right environment:

DVC Setup Wizard

+ +> **Note**: The correct Python interpreter must be set for the current workspace +> when relying on the Python extension for auto environment activation. From 5efc302b67a6a08d16dda0c6c79e2e447419c069 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 15 Aug 2022 15:11:54 +1000 Subject: [PATCH 4/4] only send environment based text and options when ms python is installed --- extension/src/setup.test.ts | 48 ++++++++++++++++++++++++++++++++++++- extension/src/setup.ts | 32 +++++++++++++++++++++---- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/extension/src/setup.test.ts b/extension/src/setup.test.ts index ad9cf87ed9..987b1351a1 100644 --- a/extension/src/setup.test.ts +++ b/extension/src/setup.test.ts @@ -1,4 +1,4 @@ -import { resolve } from 'path' +import { join, resolve } from 'path' import { extensions, Extension, commands } from 'vscode' import { setup, setupWorkspace } from './setup' import { flushPromises } from './test/util/jest' @@ -16,6 +16,8 @@ import { import { getFirstWorkspaceFolder } from './vscode/workspaceFolders' import { Toast } from './vscode/toast' import { Response } from './vscode/response' +import { VscodePython } from './extensions/python' +import { executeProcess } from './processExecution' jest.mock('vscode') jest.mock('./vscode/config') @@ -23,12 +25,35 @@ jest.mock('./vscode/resourcePicker') jest.mock('./vscode/quickPick') jest.mock('./vscode/toast') jest.mock('./vscode/workspaceFolders') +jest.mock('./processExecution') const mockedExtensions = jest.mocked(extensions) const mockedCommands = jest.mocked(commands) const mockedExecuteCommand = jest.fn() mockedCommands.executeCommand = mockedExecuteCommand +const mockedExecuteProcess = jest.mocked(executeProcess) + +const mockedGetExtension = jest.fn() +mockedExtensions.getExtension = mockedGetExtension + +const mockedReady = jest.fn() + +const mockedSettings = { + getExecutionDetails: () => ({ + execCommand: [join('some', 'bin', 'path')] + }) +} + +const mockedVscodePythonAPI = { + ready: mockedReady, + settings: mockedSettings +} as unknown as VscodePython + +const mockedVscodePython = { + activate: () => Promise.resolve(mockedVscodePythonAPI) +} + const mockedCanRunCli = jest.fn() const mockedHasRoots = jest.fn() const mockedGetFirstWorkspaceFolder = jest.mocked(getFirstWorkspaceFolder) @@ -259,8 +284,14 @@ describe('setup', () => { mockedHasRoots.mockReturnValueOnce(true) mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) mockedWarnWithOptions.mockResolvedValueOnce(undefined) + mockedExecuteProcess.mockImplementation(({ executable }) => + Promise.resolve(executable) + ) + mockedReady.mockResolvedValue(true) + mockedGetExtension.mockReturnValue(mockedVscodePython) await setup(extension) + await flushPromises() expect(mockedSetRoots).toBeCalledTimes(1) expect(mockedGetConfigValue).toBeCalledTimes(1) expect(mockedWarnWithOptions).toBeCalledTimes(1) @@ -273,6 +304,11 @@ describe('setup', () => { mockedHasRoots.mockReturnValueOnce(true) mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) mockedWarnWithOptions.mockResolvedValueOnce(Response.SETUP_WORKSPACE) + mockedExecuteProcess.mockImplementation(({ executable }) => + Promise.resolve(executable) + ) + mockedReady.mockResolvedValue(true) + mockedGetExtension.mockReturnValue(mockedVscodePython) await setup(extension) await flushPromises() @@ -291,6 +327,11 @@ describe('setup', () => { mockedHasRoots.mockReturnValueOnce(true) mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) mockedWarnWithOptions.mockResolvedValueOnce(Response.SELECT_INTERPRETER) + mockedExecuteProcess.mockImplementation(({ executable }) => + Promise.resolve(executable) + ) + mockedReady.mockResolvedValue(true) + mockedGetExtension.mockReturnValue(mockedVscodePython) await setup(extension) await flushPromises() @@ -309,6 +350,11 @@ describe('setup', () => { mockedHasRoots.mockReturnValueOnce(true) mockedCanRunCli.mockRejectedValueOnce(new Error('command not found: dvc')) mockedWarnWithOptions.mockResolvedValueOnce(Response.NEVER) + mockedExecuteProcess.mockImplementation(({ executable }) => + Promise.resolve(executable) + ) + mockedReady.mockResolvedValue(true) + mockedGetExtension.mockReturnValue(mockedVscodePython) await setup(extension) await flushPromises() diff --git a/extension/src/setup.ts b/extension/src/setup.ts index 47845798bf..638f837915 100644 --- a/extension/src/setup.ts +++ b/extension/src/setup.ts @@ -16,6 +16,7 @@ import { Response } from './vscode/response' import { getSelectTitle, Title } from './vscode/title' import { Toast } from './vscode/toast' import { + getPythonBinPath, isPythonExtensionInstalled, selectPythonInterpreter } from './extensions/python' @@ -162,17 +163,40 @@ export const setupWorkspace = async (): Promise => { return pickCliPath() } +const getToastText = async ( + isPythonExtensionInstalled: boolean +): Promise => { + const text = 'An error was thrown when trying to access the CLI.' + if (!isPythonExtensionInstalled) { + return text + } + const binPath = await getPythonBinPath() + + return ( + text + + ` For auto Python environment activation ensure the correct interpreter is set. Active Python interpreter: ${binPath}. ` + ) +} + +const getToastOptions = (isPythonExtensionInstalled: boolean): Response[] => { + return isPythonExtensionInstalled + ? [Response.SETUP_WORKSPACE, Response.SELECT_INTERPRETER, Response.NEVER] + : [Response.SETUP_WORKSPACE, Response.NEVER] +} + const warnUserCLIInaccessible = async ( extension: IExtension ): Promise => { if (getConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE)) { return } + + const isMsPythonInstalled = isPythonExtensionInstalled() + const warningText = await getToastText(isMsPythonInstalled) + const response = await Toast.warnWithOptions( - 'An error was thrown when trying to access the CLI. Please ensure the correct interpreter is set for auto Python environment activation.', - Response.SETUP_WORKSPACE, - Response.SELECT_INTERPRETER, - Response.NEVER + warningText, + ...getToastOptions(isMsPythonInstalled) ) switch (response) {