Skip to content

Commit

Permalink
Update DVC Setup Version Details (#3850)
Browse files Browse the repository at this point in the history
* remove information about max version in setup details
* adds information about latest tested version setup details
* removes latest tested version toast
  • Loading branch information
julieg18 authored May 10, 2023
1 parent 2342997 commit 3db8260
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 88 deletions.
15 changes: 1 addition & 14 deletions extension/src/cli/dvc/discovery.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { LATEST_TESTED_CLI_VERSION } from './contract'
import { CliCompatible, isVersionCompatible } from './version'
import { IExtensionSetup } from '../../interfaces'
import { Toast } from '../../vscode/toast'
Expand Down Expand Up @@ -37,12 +36,6 @@ const warnVersionIncompatible = (setup: IExtensionSetup): 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.`
)
}

const warnUserCLIInaccessible = async (
setup: IExtensionSetup
): Promise<void> => {
Expand Down Expand Up @@ -79,9 +72,6 @@ const warnUser = (
return
case CliCompatible.NO_NOT_FOUND:
void warnUserCLIInaccessible(setup)
return
case CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED:
return warnAheadOfLatestTested()
}
}

Expand All @@ -96,10 +86,7 @@ const isCliCompatible = (cliCompatible: CliCompatible): boolean | undefined => {
return
}

return [
CliCompatible.YES,
CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
].includes(cliCompatible)
return cliCompatible === CliCompatible.YES
}

const getVersionDetails = async (
Expand Down
26 changes: 10 additions & 16 deletions extension/src/cli/dvc/version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,40 +91,34 @@ describe('isVersionCompatible', () => {
expect(isCompatible).toStrictEqual(CliCompatible.YES)
})

it('should return not found if the version provided is undefined', () => {
const isCompatible = isVersionCompatible(undefined)

expect(isCompatible).toStrictEqual(CliCompatible.NO_NOT_FOUND)
})

it('should return minor version ahead of tested for a version with a minor higher as the latest tested minor and any patch', () => {
it('should be compatible for a version with a minor higher as the latest tested minor and any patch', () => {
expect(0).toBeLessThan(latestTestedPatch)

let isCompatible = isVersionCompatible(
[latestTestedMajor, latestTestedMinor + 1, 0].join('.')
)

expect(isCompatible).toStrictEqual(
CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
)
expect(isCompatible).toStrictEqual(CliCompatible.YES)

isCompatible = isVersionCompatible(
[latestTestedMajor, latestTestedMinor + 1, latestTestedPatch + 1000].join(
'.'
)
)

expect(isCompatible).toStrictEqual(
CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
)
expect(isCompatible).toStrictEqual(CliCompatible.YES)

isCompatible = isVersionCompatible(
[latestTestedMajor, latestTestedMinor + 1, latestTestedPatch].join('.')
)

expect(isCompatible).toStrictEqual(
CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
)
expect(isCompatible).toStrictEqual(CliCompatible.YES)
})

it('should return not found if the version provided is undefined', () => {
const isCompatible = isVersionCompatible(undefined)

expect(isCompatible).toStrictEqual(CliCompatible.NO_NOT_FOUND)
})

it('should return behind incompatible if the provided version is a patch version before the minimum expected version', () => {
Expand Down
24 changes: 2 additions & 22 deletions extension/src/cli/dvc/version.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import {
MAX_CLI_VERSION,
LATEST_TESTED_CLI_VERSION,
MIN_CLI_VERSION
} from './contract'
import { MAX_CLI_VERSION, MIN_CLI_VERSION } from './contract'

export enum CliCompatible {
NO_CANNOT_VERIFY = 'no-cannot-verify',
NO_INCOMPATIBLE = 'no-incompatible',
NO_NOT_FOUND = 'no-not-found',
YES_MINOR_VERSION_AHEAD_OF_TESTED = 'yes-minor-version-ahead-of-tested',
YES = 'yes'
}

Expand All @@ -23,21 +18,6 @@ export const extractSemver = (stdout: string): ParsedSemver | undefined => {
return { major: Number(major), minor: Number(minor), patch: Number(patch) }
}

const cliIsCompatible = (
currentMajor: number,
currentMinor: number
): CliCompatible => {
const { major: latestTestedMajor, minor: latestTestedMinor } = extractSemver(
LATEST_TESTED_CLI_VERSION
) as ParsedSemver

if (currentMajor === latestTestedMajor && currentMinor > latestTestedMinor) {
return CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
}

return CliCompatible.YES
}

const checkCLIVersion = (currentSemVer: {
major: number
minor: number
Expand All @@ -64,7 +44,7 @@ const checkCLIVersion = (currentSemVer: {
return CliCompatible.NO_INCOMPATIBLE
}

return cliIsCompatible(currentMajor, currentMinor)
return CliCompatible.YES
}

export const isVersionCompatible = (
Expand Down
26 changes: 0 additions & 26 deletions extension/src/setup/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { Response } from '../vscode/response'
import { VscodePython } from '../extensions/python'
import { executeProcess } from '../process/execution'
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'

Expand Down Expand Up @@ -494,31 +493,6 @@ describe('run', () => {
expect(mockedInitialize).not.toHaveBeenCalled()
})

it('should send a specific message and initialize if the Python extension is being used, the CLI is not available in the virtual environment and the global CLI is a minor version ahead of the expected version', async () => {
const { major, minor, patch } = extractSemver(
LATEST_TESTED_CLI_VERSION
) as ParsedSemver
mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
mockedHasRoots.mockReturnValueOnce(true)
mockedShouldWarnUserIfCLIUnavailable
.mockReturnValueOnce(true)
.mockReturnValueOnce(true)
mockedIsPythonExtensionUsed.mockResolvedValueOnce(true)
mockedGetCliVersion
.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce([major, minor + 1, patch].join('.'))

await run(setup)
await flushPromises()
expect(mockedWarnWithOptions).toHaveBeenCalledTimes(1)
expect(mockedWarnWithOptions).toHaveBeenCalledWith(
`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.`
)
expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)
expect(mockedResetMembers).not.toHaveBeenCalled()
expect(mockedInitialize).toHaveBeenCalledTimes(1)
})

it('should send a specific message to the user if the Python extension is not being used and the CLI is not available', async () => {
mockedGetFirstWorkspaceFolder.mockReturnValueOnce(mockedCwd)
mockedShouldWarnUserIfCLIUnavailable.mockReturnValueOnce(true)
Expand Down
15 changes: 9 additions & 6 deletions webview/src/setup/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
MessageFromWebviewType,
MessageToWebviewType
} from 'dvc/src/webview/contract'
import { MAX_CLI_VERSION, MIN_CLI_VERSION } from 'dvc/src/cli/dvc/contract'
import {
LATEST_TESTED_CLI_VERSION,
MIN_CLI_VERSION
} from 'dvc/src/cli/dvc/contract'
import '@testing-library/jest-dom/extend-expect'
import React from 'react'
import { SetupSection, SetupData } from 'dvc/src/setup/webview/contract'
Expand Down Expand Up @@ -492,7 +495,7 @@ describe('App', () => {
expect(studioDetails).toHaveAttribute('open')
})

it('should show the user the version if dvc is installed', () => {
it('should show the user the version, min version, and latested tested version if dvc is installed', () => {
renderApp({
canGitInitialize: false,
cliCompatible: true,
Expand All @@ -512,10 +515,10 @@ describe('App', () => {
})

const envDetails = screen.getByTestId('dvc-env-details')
const command = `1.0.0 (${MIN_CLI_VERSION} <= required < ${MAX_CLI_VERSION}.0.0)`
const firstVersionLine = `1.0.0 (required ${MIN_CLI_VERSION} and above, tested with ${LATEST_TESTED_CLI_VERSION})`

expect(within(envDetails).getByText('Version')).toBeInTheDocument()
expect(within(envDetails).getByText(command)).toBeInTheDocument()
expect(within(envDetails).getByText(firstVersionLine)).toBeInTheDocument()
})

it('should tell the user that version is not found if dvc is not installed', () => {
Expand All @@ -537,10 +540,10 @@ describe('App', () => {
shareLiveToStudio: false
})
const envDetails = screen.getByTestId('dvc-env-details')
const command = `Not found (${MIN_CLI_VERSION} <= required < ${MAX_CLI_VERSION}.0.0)`
const version = `Not found (required ${MIN_CLI_VERSION} and above, tested with ${LATEST_TESTED_CLI_VERSION})`

expect(within(envDetails).getByText('Version')).toBeInTheDocument()
expect(within(envDetails).getByText(command)).toBeInTheDocument()
expect(within(envDetails).getByText(version)).toBeInTheDocument()
})

it('should show the user an example command if dvc is installed', () => {
Expand Down
13 changes: 9 additions & 4 deletions webview/src/setup/components/dvc/DvcEnvDetails.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import React from 'react'
import { DvcCliDetails } from 'dvc/src/setup/webview/contract'
import { MAX_CLI_VERSION, MIN_CLI_VERSION } from 'dvc/src/cli/dvc/contract'
import {
LATEST_TESTED_CLI_VERSION,
MIN_CLI_VERSION
} from 'dvc/src/cli/dvc/contract'
import { DvcEnvInfoRow } from './DvcEnvInfoRow'
import styles from './styles.module.scss'
import { DvcEnvCommandRow } from './DvcEnvCommandRow'
Expand All @@ -14,9 +17,11 @@ export const DvcEnvDetails: React.FC<DvcEnvDetailsProps> = ({
version,
isPythonExtensionUsed
}) => {
const versionText = `${
version || 'Not found'
} (${MIN_CLI_VERSION} <= required < ${MAX_CLI_VERSION}.0.0)`
const versionText = (
<span>{`${
version || 'Not found'
} (required ${MIN_CLI_VERSION} and above, tested with ${LATEST_TESTED_CLI_VERSION})`}</span>
)

return (
<table data-testid="dvc-env-details" className={styles.envDetails}>
Expand Down

0 comments on commit 3db8260

Please sign in to comment.