Skip to content

Commit

Permalink
Add warning to setup about latest tested version (#3895)
Browse files Browse the repository at this point in the history
  • Loading branch information
julieg18 authored May 18, 2023
1 parent 8590ae8 commit 30935d8
Show file tree
Hide file tree
Showing 20 changed files with 387 additions and 128 deletions.
47 changes: 46 additions & 1 deletion extension/src/cli/dvc/version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import {
isVersionCompatible,
extractSemver,
ParsedSemver,
CliCompatible
CliCompatible,
isAboveLatestTestedVersion
} from './version'
import { MIN_CLI_VERSION, LATEST_TESTED_CLI_VERSION } from './contract'

Expand Down Expand Up @@ -161,3 +162,47 @@ describe('isVersionCompatible', () => {
expect(isCompatible).toStrictEqual(CliCompatible.NO_CANNOT_VERIFY)
})
})

describe('isAboveLatestTestedVersion', () => {
it('should return undefined if version is undefined', () => {
const result = isAboveLatestTestedVersion(undefined)

expect(result).toStrictEqual(undefined)
})

it('should return true for a version with a minor higher as the latest tested minor and any patch', () => {
const {
major: latestTestedMajor,
minor: latestTestedMinor,
patch: latestTestedPatch
} = extractSemver(LATEST_TESTED_CLI_VERSION) as ParsedSemver

expect(0).toBeLessThan(latestTestedPatch)

let result = isAboveLatestTestedVersion(
[latestTestedMajor, latestTestedMinor + 1, 0].join('.')
)

expect(result).toStrictEqual(true)

result = isAboveLatestTestedVersion(
[latestTestedMajor, latestTestedMinor + 1, latestTestedPatch + 1000].join(
'.'
)
)

expect(result).toStrictEqual(true)

result = isAboveLatestTestedVersion(
[latestTestedMajor, latestTestedMinor + 1, latestTestedPatch].join('.')
)

expect(result).toStrictEqual(true)
})

it('should return false if version is below the latest tested version', () => {
const result = isAboveLatestTestedVersion(MIN_CLI_VERSION)

expect(result).toStrictEqual(false)
})
})
22 changes: 21 additions & 1 deletion extension/src/cli/dvc/version.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { MAX_CLI_VERSION, MIN_CLI_VERSION } from './contract'
import {
LATEST_TESTED_CLI_VERSION,
MAX_CLI_VERSION,
MIN_CLI_VERSION
} from './contract'

export enum CliCompatible {
NO_CANNOT_VERIFY = 'no-cannot-verify',
Expand Down Expand Up @@ -67,3 +71,19 @@ export const isVersionCompatible = (

return checkCLIVersion(currentSemVer)
}

export const isAboveLatestTestedVersion = (version: string | undefined) => {
if (!version) {
return undefined
}

const { major: currentMajor, minor: currentMinor } = extractSemver(
version
) as ParsedSemver

const { major: latestTestedMajor, minor: latestTestedMinor } = extractSemver(
LATEST_TESTED_CLI_VERSION
) as ParsedSemver

return currentMajor === latestTestedMajor && currentMinor > latestTestedMinor
}
2 changes: 2 additions & 0 deletions extension/src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import { getValidInput } from '../vscode/inputBox'
import { Title } from '../vscode/title'
import { getDVCAppDir } from '../util/appdirs'
import { getOptions } from '../cli/dvc/options'
import { isAboveLatestTestedVersion } from '../cli/dvc/version'

export class Setup
extends BaseRepository<TSetupData>
Expand Down Expand Up @@ -414,6 +415,7 @@ export class Setup
cliCompatible: this.getCliCompatible(),
dvcCliDetails,
hasData,
isAboveLatestTestedVersion: isAboveLatestTestedVersion(this.cliVersion),
isPythonExtensionUsed,
isStudioConnected: this.studioIsConnected,
needsGitCommit,
Expand Down
1 change: 1 addition & 0 deletions extension/src/setup/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type SetupData = {
remoteList: RemoteList
sectionCollapsed: typeof DEFAULT_SECTION_COLLAPSED | undefined
shareLiveToStudio: boolean
isAboveLatestTestedVersion: boolean | undefined
}

export enum SetupSection {
Expand Down
4 changes: 3 additions & 1 deletion extension/src/setup/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ export class WebviewMessages {
pythonBinPath,
remoteList,
sectionCollapsed,
shareLiveToStudio
shareLiveToStudio,
isAboveLatestTestedVersion
}: SetupData) {
void this.getWebview()?.show({
canGitInitialize,
cliCompatible,
dvcCliDetails,
hasData,
isAboveLatestTestedVersion,
isPythonExtensionUsed,
isStudioConnected,
needsGitCommit,
Expand Down
4 changes: 4 additions & 0 deletions extension/src/test/suite/setup/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ suite('Setup Test Suite', () => {
cliCompatible: undefined,
dvcCliDetails: { command: 'dvc', version: undefined },
hasData: false,
isAboveLatestTestedVersion: undefined,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down Expand Up @@ -282,6 +283,7 @@ suite('Setup Test Suite', () => {
cliCompatible: true,
dvcCliDetails: { command: 'dvc', version: MIN_CLI_VERSION },
hasData: false,
isAboveLatestTestedVersion: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down Expand Up @@ -333,6 +335,7 @@ suite('Setup Test Suite', () => {
version: MIN_CLI_VERSION
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: false,
Expand Down Expand Up @@ -384,6 +387,7 @@ suite('Setup Test Suite', () => {
version: MIN_CLI_VERSION
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: true,
Expand Down
3 changes: 2 additions & 1 deletion webview/icons/codicons.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ export const codicons = [
'sort-precedence',
'star-empty',
'star-full',
'trash'
'trash',
'warning'
]
4 changes: 2 additions & 2 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import {
dragEnter,
dragLeave
} from '../../test/dragDrop'
import { SectionDescription } from '../../shared/components/sectionContainer/SectionContainer'
import { SectionDescriptionMainText } from '../../shared/components/sectionContainer/SectionDescription'
import { DragEnterDirection } from '../../shared/components/dragDrop/util'
import { clearSelection, createWindowTextSelection } from '../../test/selection'
import * as EventCurrentTargetDistances from '../../shared/components/dragDrop/currentTarget'
Expand Down Expand Up @@ -648,7 +648,7 @@ describe('App', () => {
const summaryElement = await screen.findByText('Custom')
createWindowTextSelection(
// eslint-disable-next-line testing-library/no-node-access
SectionDescription['custom-plots'].props.children,
SectionDescriptionMainText['custom-plots'].props.children,
2
)
fireEvent.click(summaryElement, {
Expand Down
65 changes: 57 additions & 8 deletions webview/src/setup/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const DEFAULT_DATA = {
version: '1.0.0'
},
hasData: false,
isAboveLatestTestedVersion: false,
isPythonExtensionUsed: false,
isStudioConnected: false,
needsGitCommit: false,
Expand Down Expand Up @@ -394,7 +395,9 @@ describe('App', () => {
projectInitialized: false
})

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[0]
const iconWrapper = within(
screen.getByTestId('dvc-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.ERROR)
Expand All @@ -405,13 +408,47 @@ describe('App', () => {
renderApp({ remoteList: { mockRoot: undefined } })
expect(screen.queryByText('DVC is not setup')).not.toBeInTheDocument()

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[0]
const iconWrapper = within(
screen.getByTestId('dvc-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.PASSED)
).toBeInTheDocument()
})

it('should add a warning icon and message if version is above the latest tested version', () => {
renderApp({
isAboveLatestTestedVersion: true
})

const iconWrapper = within(
screen.getByTestId('dvc-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.WARNING)
).toBeInTheDocument()

fireEvent.mouseEnter(iconWrapper)

expect(
screen.getByText(
'The located version has not been tested against the extension. If you are experiencing unexpected behaviour, first try to update the extension. If there are no updates available, please downgrade DVC to the same minor version as displayed and reload the window.'
)
).toBeInTheDocument()
})
})

describe('Experiments', () => {
it('should show a screen saying that dvc is not setup if the project is not initialized', () => {
renderApp({
projectInitialized: false
})

expect(screen.getByText('DVC is not setup')).toBeInTheDocument()
})

it('should open the dvc section when clicking the Setup DVC button on the dvc is not setup screen', () => {
renderApp({
projectInitialized: false,
Expand Down Expand Up @@ -489,7 +526,9 @@ describe('App', () => {
it('should show an error icon if experiments are not setup', () => {
renderApp()

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[1]
const iconWrapper = within(
screen.getByTestId('experiments-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.ERROR)
Expand All @@ -501,7 +540,9 @@ describe('App', () => {
cliCompatible: false
})

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[1]
const iconWrapper = within(
screen.getByTestId('experiments-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.ERROR)
Expand All @@ -513,7 +554,9 @@ describe('App', () => {
hasData: true
})

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[1]
const iconWrapper = within(
screen.getByTestId('experiments-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.PASSED)
Expand Down Expand Up @@ -571,7 +614,9 @@ describe('App', () => {
cliCompatible: false
})

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[3]
const iconWrapper = within(
screen.getByTestId('studio-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.ERROR)
Expand All @@ -581,7 +626,9 @@ describe('App', () => {
it('should show an info icon if dvc is compatible but studio is not connected', () => {
renderApp()

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[3]
const iconWrapper = within(
screen.getByTestId('studio-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.INFO)
Expand Down Expand Up @@ -622,7 +669,9 @@ describe('App', () => {
isStudioConnected: true
})

const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[3]
const iconWrapper = within(
screen.getByTestId('studio-section-details')
).getByTestId('info-tooltip-toggle')

expect(
within(iconWrapper).getByTestId(TooltipIconType.PASSED)
Expand Down
Loading

0 comments on commit 30935d8

Please sign in to comment.