Recheck for global CLI becoming available (#2928)

mattseddon · web-flow · commit 475574e7b057 · 2022-12-14T12:43:03.000+11:00
* remove recheck from call to setup

* test and refactor
diff --git a/extension/src/cli/dvc/discovery.ts b/extension/src/cli/dvc/discovery.ts
@@ -17,6 +17,8 @@ import {
   isPythonExtensionInstalled,
   selectPythonInterpreter
 } from '../../extensions/python'
+import { getFirstWorkspaceFolder } from '../../vscode/workspaceFolders'
+import { delay } from '../../util/time'
 
 const getToastOptions = (isPythonExtensionInstalled: boolean): Response[] => {
   return isPythonExtensionInstalled
@@ -116,7 +118,9 @@ type CanRunCli = {
   isCompatible: boolean | undefined
 }
 
-const isCliCompatible = (cliCompatible: CliCompatible): boolean | undefined => {
+export const isCliCompatible = (
+  cliCompatible: CliCompatible
+): boolean | undefined => {
   if (cliCompatible === CliCompatible.NO_NOT_FOUND) {
     return
   }
@@ -159,17 +163,15 @@ const processVersionDetails = (
 
 const tryGlobalFallbackVersion = async (
   extension: IExtension,
-  cwd: string,
-  recheck: boolean
+  cwd: string
 ): Promise<CanRunCli> => {
   const tryGlobal = await getVersionDetails(extension, cwd, true)
   const { cliCompatible, isAvailable, isCompatible, version } = tryGlobal
 
-  if (!recheck && extension.hasRoots() && !isCompatible) {
+  if (extension.hasRoots() && !isCompatible) {
     warnUserCLIInaccessibleAnywhere(extension, version)
   }
   if (
-    !recheck &&
     extension.hasRoots() &&
     cliCompatible === CliCompatible.YES_MINOR_VERSION_AHEAD_OF_TESTED
   ) {
@@ -185,8 +187,7 @@ const tryGlobalFallbackVersion = async (
 
 const extensionCanAutoRunCli = async (
   extension: IExtension,
-  cwd: string,
-  recheck: boolean
+  cwd: string
 ): Promise<CanRunCli> => {
   const {
     cliCompatible: pythonCliCompatible,
@@ -196,7 +197,7 @@ const extensionCanAutoRunCli = async (
   } = await getVersionDetails(extension, cwd)
 
   if (pythonCliCompatible === CliCompatible.NO_NOT_FOUND) {
-    return tryGlobalFallbackVersion(extension, cwd, recheck)
+    return tryGlobalFallbackVersion(extension, cwd)
   }
   return processVersionDetails(
     extension,
@@ -209,11 +210,10 @@ const extensionCanAutoRunCli = async (
 
 export const extensionCanRunCli = async (
   extension: IExtension,
-  cwd: string,
-  recheck: boolean
+  cwd: string
 ): Promise<CanRunCli> => {
   if (await extension.isPythonExtensionUsed()) {
-    return extensionCanAutoRunCli(extension, cwd, recheck)
+    return extensionCanAutoRunCli(extension, cwd)
   }
 
   const { cliCompatible, isAvailable, isCompatible, version } =
@@ -227,3 +227,27 @@ export const extensionCanRunCli = async (
     isCompatible
   )
 }
+
+export const recheckGlobal = async (
+  extension: IExtension,
+  setup: () => Promise<void[] | undefined>,
+  recheckInterval: number
+): Promise<void> => {
+  await delay(recheckInterval)
+  const roots = extension.getRoots()
+  const cwd = roots.length > 0 ? roots[0] : getFirstWorkspaceFolder()
+
+  if (!cwd || extension.getAvailable()) {
+    return
+  }
+
+  const version = await extension.getCliVersion(cwd, true)
+  const cliCompatible = isVersionCompatible(version)
+  const isCompatible = isCliCompatible(cliCompatible)
+
+  if (!isCompatible) {
+    return recheckGlobal(extension, setup, recheckInterval)
+  }
+
+  setup()
+}
diff --git a/extension/src/extension.ts b/extension/src/extension.ts
@@ -21,7 +21,7 @@ import { IExtension } from './interfaces'
 import { registerRepositoryCommands } from './repository/commands/register'
 import { ResourceLocator } from './resourceLocator'
 import { definedAndNonEmpty } from './util/array'
-import { setup, setupWorkspace } from './setup'
+import { setup, setupWithGlobalRecheck, setupWorkspace } from './setup'
 import { Status } from './status'
 import { reRegisterVsCodeCommands } from './vscode/commands'
 import { InternalCommands } from './commands/internal'
@@ -204,14 +204,14 @@ export class Extension extends Disposable implements IExtension {
       new RepositoriesTree(this.internalCommands, this.repositories)
     )
 
-    setup(this)
-      .then(async () =>
+    setupWithGlobalRecheck(this)
+      .then(async () => {
         sendTelemetryEvent(
           EventName.EXTENSION_LOAD,
           await this.getEventProperties(),
           { duration: stopWatch.getElapsedTime() }
         )
-      )
+      })
       .catch(async error =>
         sendTelemetryEventAndThrow(
           EventName.EXTENSION_LOAD,
diff --git a/extension/src/interfaces.ts b/extension/src/interfaces.ts
@@ -13,6 +13,7 @@ export interface IExtension {
   resetMembers: () => void
 
   setAvailable: (available: boolean) => void
+  getAvailable: () => boolean
   setCliCompatible: (compatible: boolean | undefined) => void
   setRoots: () => Promise<void>
   unsetPythonBinPath: () => void
diff --git a/extension/src/setup.test.ts b/extension/src/setup.test.ts
@@ -1,6 +1,6 @@
 import { resolve } from 'path'
 import { extensions, Extension, commands } from 'vscode'
-import { setup, setupWorkspace } from './setup'
+import { setup, setupWithGlobalRecheck, setupWorkspace } from './setup'
 import { flushPromises } from './test/util/jest'
 import {
   getConfigValue,
@@ -24,6 +24,7 @@ import {
   MIN_CLI_VERSION
 } from './cli/dvc/constants'
 import { extractSemver, ParsedSemver } from './cli/dvc/version'
+import { delay } from './util/time'
 
 jest.mock('vscode')
 jest.mock('./vscode/config')
@@ -32,7 +33,6 @@ jest.mock('./vscode/quickPick')
 jest.mock('./vscode/toast')
 jest.mock('./vscode/workspaceFolders')
 jest.mock('./processExecution')
-jest.mock('./setupUtil')
 
 const mockedExtensions = jest.mocked(extensions)
 const mockedCommands = jest.mocked(commands)
@@ -63,6 +63,7 @@ const mockedVscodePython = {
 }
 
 const mockedCwd = __dirname
+const mockedGetAvailable = jest.fn()
 const mockedGetCliVersion = jest.fn()
 const mockedGetFirstWorkspaceFolder = jest.mocked(getFirstWorkspaceFolder)
 const mockedHasRoots = jest.fn()
@@ -259,6 +260,7 @@ describe('setupWorkspace', () => {
 
 describe('setup', () => {
   const extension = {
+    getAvailable: mockedGetAvailable,
     getCliVersion: mockedGetCliVersion,
     getRoots: mockedGetRoots,
     hasRoots: mockedHasRoots,
@@ -618,3 +620,102 @@ describe('setup', () => {
     expect(mockedInitialize).not.toHaveBeenCalled()
   })
 })
+
+describe('setupWithGlobalRecheck', () => {
+  const extension = {
+    getAvailable: mockedGetAvailable,
+    getCliVersion: mockedGetCliVersion,
+    getRoots: mockedGetRoots,
+    hasRoots: mockedHasRoots,
+    initialize: mockedInitialize,
+    isPythonExtensionUsed: mockedIsPythonExtensionUsed,
+    resetMembers: mockedResetMembers,
+    setAvailable: mockedSetAvailable,
+    setCliCompatible: mockedSetCliCompatible,
+    setRoots: mockedSetRoots,
+    setupWorkspace: mockedSetupWorkspace,
+    unsetPythonBinPath: mockedUnsetPythonBinPath
+  }
+
+  it('should periodically check to see if the CLI is available globally if the extension fails to initialize', async () => {
+    mockedGetFirstWorkspaceFolder.mockReturnValue(mockedCwd)
+
+    const mockedRecheckInterval = 0
+
+    mockedGetAvailable
+      .mockReturnValueOnce(false)
+      .mockReturnValueOnce(false)
+      .mockReturnValueOnce(false)
+
+    mockedGetCliVersion
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(LATEST_TESTED_CLI_VERSION)
+      .mockResolvedValueOnce(LATEST_TESTED_CLI_VERSION)
+
+    await setupWithGlobalRecheck(extension, mockedRecheckInterval)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(1)
+
+    await delay(mockedRecheckInterval)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(2)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(2)
+
+    await delay(mockedRecheckInterval)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(4)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(3)
+
+    await delay(mockedRecheckInterval)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(4)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(3)
+  })
+
+  it('should not periodically check to see if the CLI is available globally if the extension initializes', async () => {
+    mockedGetFirstWorkspaceFolder.mockReturnValue(mockedCwd)
+
+    mockedGetAvailable.mockReturnValueOnce(true)
+
+    mockedGetCliVersion.mockResolvedValueOnce(LATEST_TESTED_CLI_VERSION)
+
+    await setupWithGlobalRecheck(extension, 0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(1)
+
+    await delay(0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(1)
+
+    await delay(0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(1)
+  })
+
+  it('should stop checking if the extension is available globally if it initializes', async () => {
+    mockedGetFirstWorkspaceFolder.mockReturnValue(mockedCwd)
+
+    mockedGetAvailable.mockReturnValueOnce(false).mockReturnValueOnce(true)
+    mockedGetCliVersion.mockResolvedValueOnce(undefined)
+
+    await setupWithGlobalRecheck(extension, 0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(1)
+
+    await delay(0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(2)
+
+    await delay(0)
+
+    expect(mockedGetCliVersion).toHaveBeenCalledTimes(1)
+    expect(mockedGetAvailable).toHaveBeenCalledTimes(2)
+  })
+})
diff --git a/extension/src/setup.ts b/extension/src/setup.ts
@@ -9,8 +9,7 @@ import { pickFile } from './vscode/resourcePicker'
 import { getFirstWorkspaceFolder } from './vscode/workspaceFolders'
 import { getSelectTitle, Title } from './vscode/title'
 import { isPythonExtensionInstalled } from './extensions/python'
-import { extensionCanRunCli } from './cli/dvc/discovery'
-import { willRecheck } from './setupUtil'
+import { extensionCanRunCli, recheckGlobal } from './cli/dvc/discovery'
 
 const setConfigPath = async (
   option: ConfigKey,
@@ -156,13 +155,11 @@ export const setupWorkspace = async (): Promise<boolean> => {
 
 export const checkAvailable = async (
   extension: IExtension,
-  dvcRootOrFirstFolder: string,
-  recheck = false
+  dvcRootOrFirstFolder: string
 ) => {
   const { isAvailable, isCompatible } = await extensionCanRunCli(
     extension,
-    dvcRootOrFirstFolder,
-    recheck
+    dvcRootOrFirstFolder
   )
 
   extension.setCliCompatible(isCompatible)
@@ -173,11 +170,6 @@ export const checkAvailable = async (
   }
 
   extension.resetMembers()
-
-  if (!isAvailable) {
-    extension.setAvailable(false)
-    willRecheck(extension, dvcRootOrFirstFolder)
-  }
 }
 
 export const setup = async (extension: IExtension) => {
@@ -193,3 +185,14 @@ export const setup = async (extension: IExtension) => {
 
   return checkAvailable(extension, dvcRootOrFirstFolder)
 }
+
+export const setupWithGlobalRecheck = async (
+  extension: IExtension,
+  recheckInterval = 5000
+): Promise<void> => {
+  await setup(extension)
+
+  if (!extension.getAvailable()) {
+    recheckGlobal(extension, () => setup(extension), recheckInterval)
+  }
+}
diff --git a/extension/src/setupUtil.ts b/extension/src/setupUtil.ts
diff --git a/extension/src/test/suite/extension.test.ts b/extension/src/test/suite/extension.test.ts
@@ -55,7 +55,6 @@ suite('Extension Test Suite', () => {
     ])
   })
 
-  // eslint-disable-next-line sonarjs/cognitive-complexity
   describe('dvc.setupWorkspace', () => {
     it('should set dvc.dvcPath to the default when dvc is installed in a virtual environment', async () => {
       stub(Python, 'isPythonExtensionInstalled').returns(true)
