From 6c950c1a7145d9ac32e80e0aa7fd129da99cb45a Mon Sep 17 00:00:00 2001 From: julieg18 Date: Fri, 28 Apr 2023 16:44:30 -0500 Subject: [PATCH 1/9] wip --- extension/src/cli/dvc/discovery.ts | 76 ++++++++++++------------------ extension/src/status.ts | 28 +++-------- 2 files changed, 37 insertions(+), 67 deletions(-) diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts index e7ad9aa01a..ef02e1892b 100644 --- a/extension/src/cli/dvc/discovery.ts +++ b/extension/src/cli/dvc/discovery.ts @@ -1,8 +1,4 @@ -import { - LATEST_TESTED_CLI_VERSION, - MAX_CLI_VERSION, - MIN_CLI_VERSION -} from './contract' +import { LATEST_TESTED_CLI_VERSION } from './contract' import { CliCompatible, isVersionCompatible } from './version' import { IExtensionSetup } from '../../interfaces' import { Toast } from '../../vscode/toast' @@ -12,22 +8,33 @@ import { getConfigValue, setUserConfigValue } from '../../vscode/config' -import { getPythonBinPath } from '../../extensions/python' import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders' import { delay } from '../../util/time' import { SetupSection } from '../../setup/webview/contract' - +// TBD Can we simplify further? export const warnUnableToVerifyVersion = () => Toast.warnWithOptions( 'The extension cannot initialize as we were unable to verify the DVC CLI version.' ) +const warnWithSetupAction = async ( + setup: IExtensionSetup, + warningText: string +): Promise => { + const response = await Toast.warnWithOptions(warningText, Response.SHOW_SETUP) + + if (response === Response.SHOW_SETUP) { + return setup.showSetup(SetupSection.DVC) + } +} + export const warnVersionIncompatible = ( - version: string, + setup: IExtensionSetup, update: 'CLI' | 'extension' ): void => { - void Toast.warnWithOptions( - `The extension cannot initialize because you are using version ${version} of the DVC CLI. The expected version is ${MIN_CLI_VERSION} <= DVC < ${MAX_CLI_VERSION}. Please upgrade to the most recent version of the ${update} and reload this window.` + void warnWithSetupAction( + setup, + `The extension cannot initialize because you are using the wrong version of the ${update}` ) } @@ -59,38 +66,21 @@ const warnUserCLIInaccessible = async ( } } -const warnUserCLIInaccessibleAnywhere = async ( - setup: IExtensionSetup, - globalDvcVersion: string | undefined -): Promise => { - const binPath = await getPythonBinPath() - - return warnUserCLIInaccessible( - setup, - `The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. ${ - globalDvcVersion ? globalDvcVersion + ' is' : 'The CLI is also not' - } installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${ - binPath || 'unset' - }.` - ) -} - const warnUser = ( setup: IExtensionSetup, - cliCompatible: CliCompatible, - version: string | undefined + cliCompatible: CliCompatible ): void => { if (!setup.shouldWarnUserIfCLIUnavailable()) { return } switch (cliCompatible) { case CliCompatible.NO_BEHIND_MIN_VERSION: - return warnVersionIncompatible(version as string, 'CLI') + return warnVersionIncompatible(setup, 'CLI') case CliCompatible.NO_CANNOT_VERIFY: void warnUnableToVerifyVersion() return case CliCompatible.NO_MAJOR_VERSION_AHEAD: - return warnVersionIncompatible(version as string, 'extension') + return warnVersionIncompatible(setup, 'extension') case CliCompatible.NO_NOT_FOUND: void warnUserCLIInaccessible( setup, @@ -139,11 +129,10 @@ const getVersionDetails = async ( const processVersionDetails = ( setup: IExtensionSetup, cliCompatible: CliCompatible, - version: string | undefined, isAvailable: boolean, isCompatible: boolean | undefined ): CanRunCli => { - warnUser(setup, cliCompatible, version) + warnUser(setup, cliCompatible) return { isAvailable, isCompatible @@ -155,10 +144,13 @@ const tryGlobalFallbackVersion = async ( cwd: string ): Promise => { const tryGlobal = await getVersionDetails(setup, cwd, true) - const { cliCompatible, isAvailable, isCompatible, version } = tryGlobal + const { cliCompatible, isAvailable, isCompatible } = tryGlobal if (setup.shouldWarnUserIfCLIUnavailable() && !isCompatible) { - void warnUserCLIInaccessibleAnywhere(setup, version) + void warnUserCLIInaccessible( + setup, + 'The extension is unable to initialize as the DVC CLI was not located.' + ) } if ( setup.shouldWarnUserIfCLIUnavailable() && @@ -181,8 +173,7 @@ const extensionCanAutoRunCli = async ( const { cliCompatible: pythonCliCompatible, isAvailable: pythonVersionIsAvailable, - isCompatible: pythonVersionIsCompatible, - version: pythonVersion + isCompatible: pythonVersionIsCompatible } = await getVersionDetails(setup, cwd) if (pythonCliCompatible === CliCompatible.NO_NOT_FOUND) { @@ -191,7 +182,6 @@ const extensionCanAutoRunCli = async ( return processVersionDetails( setup, pythonCliCompatible, - pythonVersion, pythonVersionIsAvailable, pythonVersionIsCompatible ) @@ -205,16 +195,12 @@ export const extensionCanRunCli = async ( return extensionCanAutoRunCli(setup, cwd) } - const { cliCompatible, isAvailable, isCompatible, version } = - await getVersionDetails(setup, cwd) - - return processVersionDetails( + const { cliCompatible, isAvailable, isCompatible } = await getVersionDetails( setup, - cliCompatible, - version, - isAvailable, - isCompatible + cwd ) + + return processVersionDetails(setup, cliCompatible, isAvailable, isCompatible) } const checkVersion = async ( diff --git a/extension/src/status.ts b/extension/src/status.ts index 5647bbed3c..fe3c30ae68 100644 --- a/extension/src/status.ts +++ b/extension/src/status.ts @@ -58,9 +58,9 @@ export class Status extends Disposable { } private setState(isWorking: boolean) { - const { indicator, tooltip } = this.getEnvDetails() + const indicator = this.getEnvIndicator() this.statusBarItem.text = this.getText(isWorking, indicator) - this.statusBarItem.tooltip = tooltip + this.statusBarItem.tooltip = this.getEnvIndicator() this.statusBarItem.color = this.getColor() @@ -91,39 +91,23 @@ export class Status extends Disposable { return } - if (this.available) { - return { - command: RegisteredCommands.EXTENSION_SETUP_WORKSPACE, - title: Title.SETUP_WORKSPACE - } - } - return { command: RegisteredCommands.SETUP_SHOW, title: Title.SHOW_SETUP } } - private getEnvDetails() { + private getEnvIndicator() { const dvcPath = this.config.getCliPath() const pythonBinPath = this.config.getPythonBinPath() if (dvcPath || !pythonBinPath) { - return { - indicator: '(Global)', - tooltip: `Locate DVC at: ${dvcPath || 'dvc'}` - } + return '(Global)' } if (this.config.isPythonExtensionUsed()) { - return { - indicator: '(Auto)', - tooltip: `Locate DVC in the Python environment selected by the Python extension: ${pythonBinPath}` - } + return '(Auto)' } - return { - indicator: '(Manual)', - tooltip: `Locate DVC in this Python environment: ${pythonBinPath}` - } + return '(Manual)' } } From 8ff17c4e6d3b819644b963f1e88a54d12cd56805 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 1 May 2023 18:44:49 -0500 Subject: [PATCH 2/9] Simplify logic and add tests --- extension/src/cli/dvc/discovery.ts | 38 +++++++------------------ extension/src/setup/collect.ts | 1 - extension/src/setup/runner.test.ts | 16 ++++------- extension/src/test/suite/status.test.ts | 21 ++++---------- 4 files changed, 22 insertions(+), 54 deletions(-) diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts index ef02e1892b..df2f3716bc 100644 --- a/extension/src/cli/dvc/discovery.ts +++ b/extension/src/cli/dvc/discovery.ts @@ -11,11 +11,6 @@ import { import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders' import { delay } from '../../util/time' import { SetupSection } from '../../setup/webview/contract' -// TBD Can we simplify further? -export const warnUnableToVerifyVersion = () => - Toast.warnWithOptions( - 'The extension cannot initialize as we were unable to verify the DVC CLI version.' - ) const warnWithSetupAction = async ( setup: IExtensionSetup, @@ -28,6 +23,11 @@ const warnWithSetupAction = async ( } } +export const warnUnableToVerifyVersion = () => + Toast.warnWithOptions( + 'The extension cannot initialize as we were unable to verify the DVC CLI version.' + ) + export const warnVersionIncompatible = ( setup: IExtensionSetup, update: 'CLI' | 'extension' @@ -45,22 +45,21 @@ export const warnAheadOfLatestTested = (): void => { } const warnUserCLIInaccessible = async ( - setup: IExtensionSetup, - warningText: string + setup: IExtensionSetup ): Promise => { if (getConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE)) { return } const response = await Toast.warnWithOptions( - warningText, + 'An error was thrown when trying to access the CLI.', Response.SHOW_SETUP, Response.NEVER ) switch (response) { case Response.SHOW_SETUP: - return setup.showSetup(SetupSection.EXPERIMENTS) + return setup.showSetup(SetupSection.DVC) case Response.NEVER: return setUserConfigValue(ConfigKey.DO_NOT_SHOW_CLI_UNAVAILABLE, true) } @@ -82,10 +81,7 @@ const warnUser = ( case CliCompatible.NO_MAJOR_VERSION_AHEAD: return warnVersionIncompatible(setup, 'extension') case CliCompatible.NO_NOT_FOUND: - void warnUserCLIInaccessible( - setup, - 'An error was thrown when trying to access the CLI.' - ) + void warnUserCLIInaccessible(setup) return case CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED: return warnAheadOfLatestTested() @@ -146,24 +142,11 @@ const tryGlobalFallbackVersion = async ( const tryGlobal = await getVersionDetails(setup, cwd, true) const { cliCompatible, isAvailable, isCompatible } = tryGlobal - if (setup.shouldWarnUserIfCLIUnavailable() && !isCompatible) { - void warnUserCLIInaccessible( - setup, - 'The extension is unable to initialize as the DVC CLI was not located.' - ) - } - if ( - setup.shouldWarnUserIfCLIUnavailable() && - cliCompatible === CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED - ) { - warnAheadOfLatestTested() - } - if (isCompatible) { setup.unsetPythonBinPath() } - return { isAvailable, isCompatible } + return processVersionDetails(setup, cliCompatible, isAvailable, isCompatible) } const extensionCanAutoRunCli = async ( @@ -179,6 +162,7 @@ const extensionCanAutoRunCli = async ( if (pythonCliCompatible === CliCompatible.NO_NOT_FOUND) { return tryGlobalFallbackVersion(setup, cwd) } + return processVersionDetails( setup, pythonCliCompatible, diff --git a/extension/src/setup/collect.ts b/extension/src/setup/collect.ts index 75ad20c157..88bb843031 100644 --- a/extension/src/setup/collect.ts +++ b/extension/src/setup/collect.ts @@ -13,6 +13,5 @@ export const collectSectionCollapsed = ( acc[section as SetupSection] = true } } - return acc } diff --git a/extension/src/setup/runner.test.ts b/extension/src/setup/runner.test.ts index fe190e1f55..184fedd95e 100644 --- a/extension/src/setup/runner.test.ts +++ b/extension/src/setup/runner.test.ts @@ -19,11 +19,7 @@ import { Toast } from '../vscode/toast' import { Response } from '../vscode/response' import { VscodePython } from '../extensions/python' import { executeProcess } from '../process/execution' -import { - LATEST_TESTED_CLI_VERSION, - MAX_CLI_VERSION, - MIN_CLI_VERSION -} from '../cli/dvc/contract' +import { LATEST_TESTED_CLI_VERSION, MIN_CLI_VERSION } from '../cli/dvc/contract' import { extractSemver, ParsedSemver } from '../cli/dvc/version' import { delay } from '../util/time' import { Title } from '../vscode/title' @@ -472,9 +468,8 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - `The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. ${belowMinVersion} is installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${mockedPythonPath}.`, - Response.SHOW_SETUP, - Response.NEVER + 'The extension cannot initialize because you are using the wrong version of the CLI', + Response.SHOW_SETUP ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(2) expect(mockedResetMembers).toHaveBeenCalledTimes(1) @@ -539,7 +534,8 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - `The extension cannot initialize because you are using version ${MajorAhead} of the DVC CLI. The expected version is ${MIN_CLI_VERSION} <= DVC < ${MAX_CLI_VERSION}. Please upgrade to the most recent version of the extension and reload this window.` + 'The extension cannot initialize because you are using the wrong version of the extension', + 'Setup' ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(1) expect(mockedResetMembers).toHaveBeenCalledTimes(1) @@ -564,7 +560,7 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - `The extension is unable to initialize. The CLI was not located using the interpreter provided by the Python extension. The CLI is also not installed globally. For auto Python environment activation, ensure the correct interpreter is set. Active Python interpreter: ${mockedPythonPath}.`, + 'An error was thrown when trying to access the CLI.', Response.SHOW_SETUP, Response.NEVER ) diff --git a/extension/src/test/suite/status.test.ts b/extension/src/test/suite/status.test.ts index 230aee4f5a..a2c5271e6d 100644 --- a/extension/src/test/suite/status.test.ts +++ b/extension/src/test/suite/status.test.ts @@ -34,9 +34,9 @@ suite('Status Test Suite', () => { const loadingText = '$(loading~spin) DVC (Global)' const waitingText = '$(circle-large-outline) DVC (Global)' - const setupWorkspaceCommand = { - command: RegisteredCommands.EXTENSION_SETUP_WORKSPACE, - title: Title.SETUP_WORKSPACE + const setupShowCommand = { + command: RegisteredCommands.SETUP_SHOW, + title: Title.SHOW_SETUP } it('should show the correct status of the cli', async () => { @@ -88,7 +88,7 @@ suite('Status Test Suite', () => { await status.setAvailability(true) expect(mockStatusBarItem.text).to.equal(waitingText) - expect(mockStatusBarItem.command).to.deep.equal(setupWorkspaceCommand) + expect(mockStatusBarItem.command).to.deep.equal(setupShowCommand) processStarted.fire(firstFinishedCommand) @@ -118,7 +118,7 @@ suite('Status Test Suite', () => { }) expect(mockStatusBarItem.text).to.equal(waitingText) - expect(mockStatusBarItem.command).to.deep.equal(setupWorkspaceCommand) + expect(mockStatusBarItem.command).to.deep.equal(setupShowCommand) await status.setAvailability(false) @@ -224,7 +224,6 @@ suite('Status Test Suite', () => { await status.setAvailability(true) expect(mockStatusBarItem.text).to.equal(waitingText) - expect(mockStatusBarItem.tooltip).to.equal('Locate DVC at: dvc') const mockPythonPath = resolve('a', 'virtual', 'environment') @@ -235,9 +234,6 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Auto)' ) - expect(mockStatusBarItem.tooltip).to.equal( - `Locate DVC in the Python environment selected by the Python extension: ${mockPythonPath}` - ) setupMocks(false, undefined, mockPythonPath) @@ -246,9 +242,6 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Manual)' ) - expect(mockStatusBarItem.tooltip).to.equal( - `Locate DVC in this Python environment: ${mockPythonPath}` - ) const mockDvcPath = resolve('path', 'to', 'dvc') @@ -259,9 +252,6 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Global)' ) - expect(mockStatusBarItem.tooltip).to.equal( - `Locate DVC at: ${mockDvcPath}` - ) setupMocks(false, 'dvc', mockPythonPath) @@ -270,7 +260,6 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Global)' ) - expect(mockStatusBarItem.tooltip).to.equal('Locate DVC at: dvc') }) }) }) From e9e9e779227ed18dab5c3cbde7b204c86bb1bc9e Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 1 May 2023 19:13:19 -0500 Subject: [PATCH 3/9] Fix up typos --- extension/src/cli/dvc/discovery.ts | 10 ++++++---- extension/src/setup/collect.ts | 1 + extension/src/setup/runner.test.ts | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts index df2f3716bc..f59723c720 100644 --- a/extension/src/cli/dvc/discovery.ts +++ b/extension/src/cli/dvc/discovery.ts @@ -23,10 +23,12 @@ const warnWithSetupAction = async ( } } -export const warnUnableToVerifyVersion = () => - Toast.warnWithOptions( +export const warnUnableToVerifyVersion = (setup: IExtensionSetup) => { + void warnWithSetupAction( + setup, 'The extension cannot initialize as we were unable to verify the DVC CLI version.' ) +} export const warnVersionIncompatible = ( setup: IExtensionSetup, @@ -34,7 +36,7 @@ export const warnVersionIncompatible = ( ): void => { void warnWithSetupAction( setup, - `The extension cannot initialize because you are using the wrong version of the ${update}` + `The extension cannot initialize because you are using the wrong version of the ${update}.` ) } @@ -76,7 +78,7 @@ const warnUser = ( case CliCompatible.NO_BEHIND_MIN_VERSION: return warnVersionIncompatible(setup, 'CLI') case CliCompatible.NO_CANNOT_VERIFY: - void warnUnableToVerifyVersion() + void warnUnableToVerifyVersion(setup) return case CliCompatible.NO_MAJOR_VERSION_AHEAD: return warnVersionIncompatible(setup, 'extension') diff --git a/extension/src/setup/collect.ts b/extension/src/setup/collect.ts index 88bb843031..75ad20c157 100644 --- a/extension/src/setup/collect.ts +++ b/extension/src/setup/collect.ts @@ -13,5 +13,6 @@ export const collectSectionCollapsed = ( acc[section as SetupSection] = true } } + return acc } diff --git a/extension/src/setup/runner.test.ts b/extension/src/setup/runner.test.ts index 184fedd95e..bfb6cb37e1 100644 --- a/extension/src/setup/runner.test.ts +++ b/extension/src/setup/runner.test.ts @@ -468,7 +468,7 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - 'The extension cannot initialize because you are using the wrong version of the CLI', + 'The extension cannot initialize because you are using the wrong version of the CLI.', Response.SHOW_SETUP ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(2) @@ -534,7 +534,7 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - 'The extension cannot initialize because you are using the wrong version of the extension', + 'The extension cannot initialize because you are using the wrong version of the extension.', 'Setup' ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(1) From 7b177c44d603869564d48f368bedcfc48d078a01 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 1 May 2023 19:35:36 -0500 Subject: [PATCH 4/9] Fix typo --- extension/src/status.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/extension/src/status.ts b/extension/src/status.ts index fe3c30ae68..909ef4d48e 100644 --- a/extension/src/status.ts +++ b/extension/src/status.ts @@ -60,7 +60,6 @@ export class Status extends Disposable { private setState(isWorking: boolean) { const indicator = this.getEnvIndicator() this.statusBarItem.text = this.getText(isWorking, indicator) - this.statusBarItem.tooltip = this.getEnvIndicator() this.statusBarItem.color = this.getColor() From 4b1f4a7dddd50323fdab7c79764bd6915860db7f Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 2 May 2023 11:13:19 -0500 Subject: [PATCH 5/9] Add tooltip for now --- extension/src/status.ts | 20 +++++++++++++++----- extension/src/test/suite/status.test.ts | 16 ++++++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/extension/src/status.ts b/extension/src/status.ts index 909ef4d48e..ea90d45e9f 100644 --- a/extension/src/status.ts +++ b/extension/src/status.ts @@ -58,8 +58,9 @@ export class Status extends Disposable { } private setState(isWorking: boolean) { - const indicator = this.getEnvIndicator() + const { indicator, tooltip } = this.getEnvDetails() this.statusBarItem.text = this.getText(isWorking, indicator) + this.statusBarItem.tooltip = tooltip this.statusBarItem.color = this.getColor() @@ -96,17 +97,26 @@ export class Status extends Disposable { } } - private getEnvIndicator() { + private getEnvDetails() { const dvcPath = this.config.getCliPath() const pythonBinPath = this.config.getPythonBinPath() if (dvcPath || !pythonBinPath) { - return '(Global)' + return { + indicator: '(Global)', + tooltip: `Locate DVC at: ${dvcPath || 'dvc'}` + } } if (this.config.isPythonExtensionUsed()) { - return '(Auto)' + return { + indicator: '(Auto)', + tooltip: `Locate DVC in the Python environment selected by the Python extension: ${pythonBinPath}` + } } - return '(Manual)' + return { + indicator: '(Manual)', + tooltip: `Locate DVC in this Python environment: ${pythonBinPath}` + } } } diff --git a/extension/src/test/suite/status.test.ts b/extension/src/test/suite/status.test.ts index a2c5271e6d..fb9d2f2638 100644 --- a/extension/src/test/suite/status.test.ts +++ b/extension/src/test/suite/status.test.ts @@ -123,10 +123,7 @@ suite('Status Test Suite', () => { await status.setAvailability(false) expect(mockStatusBarItem.text).to.equal(disabledText) - expect(mockStatusBarItem.command).to.deep.equal({ - command: RegisteredCommands.SETUP_SHOW, - title: Title.SHOW_SETUP - }) + expect(mockStatusBarItem.command).to.deep.equal(setupShowCommand) }) it('should floor the number of workers at 0', async () => { @@ -224,6 +221,7 @@ suite('Status Test Suite', () => { await status.setAvailability(true) expect(mockStatusBarItem.text).to.equal(waitingText) + expect(mockStatusBarItem.tooltip).to.equal('Locate DVC at: dvc') const mockPythonPath = resolve('a', 'virtual', 'environment') @@ -234,6 +232,9 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Auto)' ) + expect(mockStatusBarItem.tooltip).to.equal( + `Locate DVC in the Python environment selected by the Python extension: ${mockPythonPath}` + ) setupMocks(false, undefined, mockPythonPath) @@ -242,6 +243,9 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Manual)' ) + expect(mockStatusBarItem.tooltip).to.equal( + `Locate DVC in this Python environment: ${mockPythonPath}` + ) const mockDvcPath = resolve('path', 'to', 'dvc') @@ -252,6 +256,9 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Global)' ) + expect(mockStatusBarItem.tooltip).to.equal( + `Locate DVC at: ${mockDvcPath}` + ) setupMocks(false, 'dvc', mockPythonPath) @@ -260,6 +267,7 @@ suite('Status Test Suite', () => { expect(mockStatusBarItem.text).to.equal( '$(circle-large-outline) DVC (Global)' ) + expect(mockStatusBarItem.tooltip).to.equal('Locate DVC at: dvc') }) }) }) From 857aa4cec98cf65eb0dbb4303f09f88215a46c41 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 2 May 2023 11:14:12 -0500 Subject: [PATCH 6/9] Increase code coverage --- extension/src/cli/dvc/discovery.ts | 6 +++--- extension/src/setup/runner.test.ts | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts index 112e919513..980a739f74 100644 --- a/extension/src/cli/dvc/discovery.ts +++ b/extension/src/cli/dvc/discovery.ts @@ -23,14 +23,14 @@ const warnWithSetupAction = async ( } } -export const warnUnableToVerifyVersion = (setup: IExtensionSetup) => { +const warnUnableToVerifyVersion = (setup: IExtensionSetup) => { void warnWithSetupAction( setup, 'The extension cannot initialize as we were unable to verify the DVC CLI version.' ) } -export const warnVersionIncompatible = ( +const warnVersionIncompatible = ( setup: IExtensionSetup, update: 'CLI' | 'extension' ): void => { @@ -40,7 +40,7 @@ export const warnVersionIncompatible = ( ) } -export const warnAheadOfLatestTested = (): void => { +const warnAheadOfLatestTested = (): void => { void Toast.warnWithOptions( `The located DVC CLI is at least a minor version ahead of the latest version the extension was tested with (${LATEST_TESTED_CLI_VERSION}). This could lead to unexpected behaviour. Please upgrade to the most recent version of the extension and reload this window.` ) diff --git a/extension/src/setup/runner.test.ts b/extension/src/setup/runner.test.ts index 1fd56e3ac6..256b3dc07c 100644 --- a/extension/src/setup/runner.test.ts +++ b/extension/src/setup/runner.test.ts @@ -451,6 +451,24 @@ describe('run', () => { expect(mockedInitialize).toHaveBeenCalledTimes(1) }) + it('should send a specific message to the user if the extension is unable to verify the version', async () => { + const unverifyableVersion = 'not a valid version' + mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd) + mockedShouldWarnUserIfCLIUnavailable.mockReturnValueOnce(true) + mockedGetCliVersion.mockResolvedValueOnce(unverifyableVersion) + + await run(setup) + await flushPromises() + expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) + expect(mockedWarnWithOptions).toHaveBeenCalledWith( + 'The extension cannot initialize as we were unable to verify the DVC CLI version.', + Response.SHOW_SETUP + ) + expect(mockedGetCliVersion).toHaveBeenCalledTimes(1) + expect(mockedResetMembers).toHaveBeenCalledTimes(1) + expect(mockedInitialize).not.toHaveBeenCalled() + }) + it('should send a specific message to the user if the Python extension is being used, the CLI is not available in the virtual environment and the global CLI is not compatible', async () => { const belowMinVersion = '2.0.0' mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd) From 4350616f8cbf4573d8896f657dc53794fe05d6d1 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Tue, 2 May 2023 11:23:00 -0500 Subject: [PATCH 7/9] Use SETUP_SHOW_DVC in extension status button --- extension/src/status.ts | 2 +- extension/src/test/suite/status.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/src/status.ts b/extension/src/status.ts index ea90d45e9f..dbbc79bf32 100644 --- a/extension/src/status.ts +++ b/extension/src/status.ts @@ -92,7 +92,7 @@ export class Status extends Disposable { } return { - command: RegisteredCommands.SETUP_SHOW, + command: RegisteredCommands.SETUP_SHOW_DVC, title: Title.SHOW_SETUP } } diff --git a/extension/src/test/suite/status.test.ts b/extension/src/test/suite/status.test.ts index fb9d2f2638..33251f1945 100644 --- a/extension/src/test/suite/status.test.ts +++ b/extension/src/test/suite/status.test.ts @@ -35,7 +35,7 @@ suite('Status Test Suite', () => { const waitingText = '$(circle-large-outline) DVC (Global)' const setupShowCommand = { - command: RegisteredCommands.SETUP_SHOW, + command: RegisteredCommands.SETUP_SHOW_DVC, title: Title.SHOW_SETUP } From 42d1c1138a161162de29288f720b10f1606d55e9 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Wed, 3 May 2023 08:49:03 -0500 Subject: [PATCH 8/9] Update SHOW_SETUP command --- extension/src/status.ts | 7 +++++++ extension/src/test/suite/status.test.ts | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/extension/src/status.ts b/extension/src/status.ts index dbbc79bf32..5a17e3aab6 100644 --- a/extension/src/status.ts +++ b/extension/src/status.ts @@ -91,6 +91,13 @@ export class Status extends Disposable { return } + if (this.available) { + return { + command: RegisteredCommands.SETUP_SHOW, + title: Title.SHOW_SETUP + } + } + return { command: RegisteredCommands.SETUP_SHOW_DVC, title: Title.SHOW_SETUP diff --git a/extension/src/test/suite/status.test.ts b/extension/src/test/suite/status.test.ts index 33251f1945..7179db44da 100644 --- a/extension/src/test/suite/status.test.ts +++ b/extension/src/test/suite/status.test.ts @@ -35,7 +35,7 @@ suite('Status Test Suite', () => { const waitingText = '$(circle-large-outline) DVC (Global)' const setupShowCommand = { - command: RegisteredCommands.SETUP_SHOW_DVC, + command: RegisteredCommands.SETUP_SHOW, title: Title.SHOW_SETUP } @@ -123,7 +123,10 @@ suite('Status Test Suite', () => { await status.setAvailability(false) expect(mockStatusBarItem.text).to.equal(disabledText) - expect(mockStatusBarItem.command).to.deep.equal(setupShowCommand) + expect(mockStatusBarItem.command).to.deep.equal({ + command: RegisteredCommands.SETUP_SHOW_DVC, + title: Title.SHOW_SETUP + }) }) it('should floor the number of workers at 0', async () => { From b1ec9921053ce5d5cf39662575b037f5f0346be8 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Wed, 3 May 2023 09:13:24 -0500 Subject: [PATCH 9/9] Simplify incompatible version toasts --- extension/src/cli/dvc/discovery.ts | 13 ++++--------- extension/src/cli/dvc/version.test.ts | 16 ++++++++-------- extension/src/cli/dvc/version.ts | 16 ++++++---------- extension/src/setup/runner.test.ts | 4 ++-- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts index 980a739f74..016c3c52bc 100644 --- a/extension/src/cli/dvc/discovery.ts +++ b/extension/src/cli/dvc/discovery.ts @@ -30,13 +30,10 @@ const warnUnableToVerifyVersion = (setup: IExtensionSetup) => { ) } -const warnVersionIncompatible = ( - setup: IExtensionSetup, - update: 'CLI' | 'extension' -): void => { +const warnVersionIncompatible = (setup: IExtensionSetup): void => { void warnWithSetupAction( setup, - `The extension cannot initialize because you are using the wrong version of the ${update}.` + 'The extension cannot initialize because the DVC CLI version is incompatible.' ) } @@ -75,13 +72,11 @@ const warnUser = ( return } switch (cliCompatible) { - case CliCompatible.NO_BEHIND_MIN_VERSION: - return warnVersionIncompatible(setup, 'CLI') + case CliCompatible.NO_INCOMPATIBLE: + return warnVersionIncompatible(setup) case CliCompatible.NO_CANNOT_VERIFY: void warnUnableToVerifyVersion(setup) return - case CliCompatible.NO_MAJOR_VERSION_AHEAD: - return warnVersionIncompatible(setup, 'extension') case CliCompatible.NO_NOT_FOUND: void warnUserCLIInaccessible(setup) return diff --git a/extension/src/cli/dvc/version.test.ts b/extension/src/cli/dvc/version.test.ts index 7e0c1083dd..6897d3fd93 100644 --- a/extension/src/cli/dvc/version.test.ts +++ b/extension/src/cli/dvc/version.test.ts @@ -127,34 +127,34 @@ describe('isVersionCompatible', () => { ) }) - it('should return behind min version if the provided version is a patch version before the minimum expected version', () => { + it('should return behind incompatible if the provided version is a patch version before the minimum expected version', () => { const isCompatible = isVersionCompatible( [minMajor, minMinor, minPatch - 1].join('.') ) - expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION) + expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE) }) - it('should return behind min version if the provided minor version is before the minimum expected version', () => { + it('should return behind incompatible if the provided minor version is before the minimum expected version', () => { const isCompatible = isVersionCompatible( [minMajor, minMinor - 1, minPatch + 100].join('.') ) - expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION) + expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE) }) - it('should return behind min version if the provided major version is before the minimum expected version', () => { + it('should return behind incompatible if the provided major version is before the minimum expected version', () => { const isCompatible = isVersionCompatible( [minMajor - 1, minMinor + 1000, minPatch + 100].join('.') ) - expect(isCompatible).toStrictEqual(CliCompatible.NO_BEHIND_MIN_VERSION) + expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE) }) - it('should return major ahead if the provided major version is above the expected major version', () => { + it('should return incompatible if the provided major version is above the expected major version', () => { const isCompatible = isVersionCompatible('3.0.0') - expect(isCompatible).toStrictEqual(CliCompatible.NO_MAJOR_VERSION_AHEAD) + expect(isCompatible).toStrictEqual(CliCompatible.NO_INCOMPATIBLE) }) it('should return cannot verify if the provided version is malformed', () => { diff --git a/extension/src/cli/dvc/version.ts b/extension/src/cli/dvc/version.ts index 6b38ae35a0..540ce4ca17 100644 --- a/extension/src/cli/dvc/version.ts +++ b/extension/src/cli/dvc/version.ts @@ -5,9 +5,8 @@ import { } from './contract' export enum CliCompatible { - NO_BEHIND_MIN_VERSION = 'no-behind-min-version', NO_CANNOT_VERIFY = 'no-cannot-verify', - NO_MAJOR_VERSION_AHEAD = 'no-major-version-ahead', + NO_INCOMPATIBLE = 'no-incompatible', NO_NOT_FOUND = 'no-not-found', YES_MINOR_VERSION_AHEAD_OF_TESTED = 'yes-minor-version-ahead-of-tested', YES = 'yes' @@ -49,23 +48,20 @@ const checkCLIVersion = (currentSemVer: { minor: currentMinor, patch: currentPatch } = currentSemVer - - if (currentMajor >= Number(MAX_CLI_VERSION)) { - return CliCompatible.NO_MAJOR_VERSION_AHEAD - } - const { major: minMajor, minor: minMinor, patch: minPatch } = extractSemver(MIN_CLI_VERSION) as ParsedSemver - if ( + const isAheadMaxVersion = currentMajor >= Number(MAX_CLI_VERSION) + const isBehindMinVersion = currentMajor < minMajor || currentMinor < minMinor || (currentMinor === minMinor && currentPatch < Number(minPatch)) - ) { - return CliCompatible.NO_BEHIND_MIN_VERSION + + if (isAheadMaxVersion || isBehindMinVersion) { + return CliCompatible.NO_INCOMPATIBLE } return cliIsCompatible(currentMajor, currentMinor) diff --git a/extension/src/setup/runner.test.ts b/extension/src/setup/runner.test.ts index 256b3dc07c..21c8fcd770 100644 --- a/extension/src/setup/runner.test.ts +++ b/extension/src/setup/runner.test.ts @@ -486,7 +486,7 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - 'The extension cannot initialize because you are using the wrong version of the CLI.', + 'The extension cannot initialize because the DVC CLI version is incompatible.', Response.SHOW_SETUP ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(2) @@ -552,7 +552,7 @@ describe('run', () => { await flushPromises() expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1) expect(mockedWarnWithOptions).toHaveBeenCalledWith( - 'The extension cannot initialize because you are using the wrong version of the extension.', + 'The extension cannot initialize because the DVC CLI version is incompatible.', 'Setup' ) expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)