Ensure LS client stops when installing Flow CLI & add tests (#478)

extension/src/dependency-installer/dependency-installer.ts

@@ -1,18 +1,24 @@
 import { window } from 'vscode'
 import { InstallFlowCLI } from './installers/flow-cli-installer'
-import { Installer, InstallError } from './installer'
+import { Installer, InstallerConstructor, InstallerContext, InstallError } from './installer'
 import { promptUserErrorMessage } from '../ui/prompts'
 import { StateCache } from '../utils/state-cache'
+import { LanguageServerAPI } from '../server/language-server'
 
-const INSTALLERS: Array<new (refreshDependencies: () => Promise<void>) => Installer> = [
+const INSTALLERS: InstallerConstructor[] = [
   InstallFlowCLI
 ]
 
 export class DependencyInstaller {
   registeredInstallers: Installer[] = []
   missingDependencies: StateCache<Installer[]>
+  #installerContext: InstallerContext
 
-  constructor () {
+  constructor (languageServer: LanguageServerAPI) {
+    this.#installerContext = {
+      refreshDependencies: this.checkDependencies.bind(this),
+      langaugeServerApi: languageServer
+    }
     this.#registerInstallers()
 
     // Create state cache for missing dependencies
@@ -51,13 +57,13 @@ export class DependencyInstaller {
 
   #registerInstallers (): void {
     // Recursively register installers and their dependencies in the correct order
-    (function registerInstallers (this: DependencyInstaller, installers: Array<new (refreshDependencies: () => Promise<void>) => Installer>) {
+    (function registerInstallers (this: DependencyInstaller, installers: InstallerConstructor[]) {
       installers.forEach((_installer) => {
         // Check if installer is already registered
         if (this.registeredInstallers.find(x => x instanceof _installer) != null) { return }
 
         // Register installer and dependencies
-        const installer = new _installer(this.checkDependencies.bind(this))
+        const installer = new _installer(this.#installerContext)
         registerInstallers.bind(this)(installer.dependencies)
         this.registeredInstallers.push(installer)
       })

extension/src/dependency-installer/installer.ts

@@ -1,15 +1,23 @@
 /* Abstract Installer class */
 import { window } from 'vscode'
 import { envVars } from '../utils/shell/env-vars'
+import { LanguageServerAPI } from '../server/language-server'
 
 // InstallError is thrown if install fails
 export class InstallError extends Error {}
 
+export interface InstallerContext {
+  refreshDependencies: () => Promise<void>
+  langaugeServerApi: LanguageServerAPI
+}
+
+export type InstallerConstructor = new (context: InstallerContext) => Installer
+
 export abstract class Installer {
-  dependencies: Array<new (refreshDependencies: () => Promise<void>) => Installer>
+  dependencies: InstallerConstructor[]
   #installerName: string
 
-  constructor (name: string, dependencies: Array<new (refreshDependencies: () => Promise<void>) => Installer>) {
+  constructor (name: string, dependencies: InstallerConstructor[]) {
     this.dependencies = dependencies
     this.#installerName = name
   }

extension/src/dependency-installer/installers/flow-cli-installer.ts

@@ -2,10 +2,9 @@
 import { window } from 'vscode'
 import { execVscodeTerminal, tryExecPowerShell, tryExecUnixDefault } from '../../utils/shell/exec'
 import { promptUserInfoMessage, promptUserErrorMessage } from '../../ui/prompts'
-import { Installer } from '../installer'
+import { Installer, InstallerConstructor, InstallerContext } from '../installer'
 import * as semver from 'semver'
 import fetch from 'node-fetch'
-import { ext } from '../../main'
 import { HomebrewInstaller } from './homebrew-installer'
 import { flowVersion } from '../../utils/flow-version'
 
@@ -25,21 +24,23 @@ const BASH_INSTALL_FLOW_CLI = (githubToken?: string): string =>
 const VERSION_INFO_URL = 'https://raw.githubusercontent.com/onflow/flow-cli/master/version.txt'
 export class InstallFlowCLI extends Installer {
   #githubToken: string | undefined
+  #context: InstallerContext
 
-  constructor (private readonly refreshDependencies: () => Promise<void>) {
+  constructor (context: InstallerContext) {
     // Homebrew is a dependency for macos and linux
-    const dependencies: Array<new (refreshDependencies: () => Promise<void>) => Installer> = []
+    const dependencies: InstallerConstructor[] = []
     if (process.platform === 'darwin') {
       dependencies.push(HomebrewInstaller)
     }
 
     super('Flow CLI', dependencies)
     this.#githubToken = process.env.GITHUB_TOKEN
+    this.#context = context
   }
 
   async install (): Promise<void> {
-    const isActive = ext?.languageServer.isActive ?? false
-    if (isActive) await ext?.languageServer.deactivate()
+    const isActive = this.#context.langaugeServerApi.isActive ?? false
+    if (isActive) await this.#context.langaugeServerApi.deactivate()
     const OS_TYPE = process.platform
 
     try {
@@ -57,7 +58,7 @@ export class InstallFlowCLI extends Installer {
     } catch {
       void window.showErrorMessage('Failed to install Flow CLI')
     }
-    if (isActive) await ext?.languageServer.activate()
+    if (isActive) await this.#context.langaugeServerApi.activate()
   }
 
   async #install_macos (): Promise<void> {
@@ -89,7 +90,7 @@ export class InstallFlowCLI extends Installer {
         'Install latest Flow CLI',
         async () => {
           await this.runInstall()
-          await this.refreshDependencies()
+          await this.#context.refreshDependencies()
         }
       )
     }
@@ -108,7 +109,7 @@ export class InstallFlowCLI extends Installer {
         'Install latest Flow CLI',
         async () => {
           await this.runInstall()
-          await this.refreshDependencies()
+          await this.#context.refreshDependencies()
         }
       )
       return false

extension/src/extension.ts

@@ -31,18 +31,25 @@ export class Extension {
     // Register JSON schema provider
     if (ctx != null) JSONSchemaProvider.register(ctx, flowVersion)
 
-    // Initialize Language Server
+    // Initialize Flow Config
     const flowConfig = new FlowConfig(settings)
+    void flowConfig.activate()
+
+    // Initialize Language Server
     this.languageServer = new LanguageServerAPI(settings, flowConfig)
-    void flowConfig.activate().then(() => {
-      void this.languageServer.activate()
-    })
 
     // Check for any missing dependencies
     // The language server will start if all dependencies are installed
     // Otherwise, the language server will not start and will start after
     // the user installs the missing dependencies
-    this.#dependencyInstaller = new DependencyInstaller()
+    this.#dependencyInstaller = new DependencyInstaller(this.languageServer)
+    this.#dependencyInstaller.missingDependencies.subscribe((missing) => {
+      if (missing.length === 0) {
+        void this.languageServer.activate()
+      } else {
+        void this.languageServer.deactivate()
+      }
+    })
 
     // Initialize ExtensionCommands
     this.#commands = new CommandController(this.#dependencyInstaller)

extension/src/server/language-server.ts

@@ -27,6 +27,7 @@ export class LanguageServerAPI {
   }
 
   async activate (): Promise<void> {
+    if (this.isActive) return
     await this.deactivate()
 
     this.#isActive = true
@@ -38,6 +39,7 @@ export class LanguageServerAPI {
     this.#isActive = false
     this.#configPathSubscription?.unsubscribe()
     this.#configModifiedSubscription?.unsubscribe()
+    await this.stopClient()
   }
 
   get isActive (): boolean {

extension/test/integration/0 - dependencies.test.ts

@@ -1,15 +1,65 @@
 import * as assert from 'assert'
 import { DependencyInstaller } from '../../src/dependency-installer/dependency-installer'
 import { MaxTimeout } from '../globals'
+import { InstallFlowCLI } from '../../src/dependency-installer/installers/flow-cli-installer'
+import { stub } from 'sinon'
 
 // Note: Dependency installation must run before other integration tests
 suite('Dependency Installer', () => {
   test('Install Missing Dependencies', async () => {
-    const dependencyManager = new DependencyInstaller()
+    const mockLangaugeServerApi = {
+      activate: stub(),
+      deactivate: stub(),
+      isActive: true
+    }
+    const dependencyManager = new DependencyInstaller(mockLangaugeServerApi as any)
     await assert.doesNotReject(async () => { await dependencyManager.installMissing() })
 
     // Check that all dependencies are installed
     await dependencyManager.checkDependencies()
     assert.deepStrictEqual(await dependencyManager.missingDependencies.getValue(), [])
   }).timeout(MaxTimeout)
+
+  test('Flow CLI installer restarts langauge server if active', async () => {
+    const mockLangaugeServerApi = {
+      activate: stub().callsFake(async () => {
+        mockLangaugeServerApi.isActive = true
+      }),
+      deactivate: stub().callsFake(async () => {
+        mockLangaugeServerApi.isActive = false
+      }),
+      isActive: true
+    }
+    const mockInstallerContext = {
+      refreshDependencies: async () => {},
+      langaugeServerApi: mockLangaugeServerApi as any
+    }
+    const flowCliInstaller = new InstallFlowCLI(mockInstallerContext)
+
+    await assert.doesNotReject(async () => { await flowCliInstaller.install() })
+    assert(mockLangaugeServerApi.deactivate.calledOnce)
+    assert(mockLangaugeServerApi.activate.calledOnce)
+    assert(mockLangaugeServerApi.deactivate.calledBefore(mockLangaugeServerApi.activate))
+  }).timeout(MaxTimeout)
+
+  test('Flow CLI installer does not restart langauge server if inactive', async () => {
+    const mockLangaugeServerApi = {
+      activate: stub().callsFake(async () => {
+        mockLangaugeServerApi.isActive = true
+      }),
+      deactivate: stub().callsFake(async () => {
+        mockLangaugeServerApi.isActive = false
+      }),
+      isActive: false
+    }
+    const mockInstallerContext = {
+      refreshDependencies: async () => {},
+      langaugeServerApi: mockLangaugeServerApi as any
+    }
+    const flowCliInstaller = new InstallFlowCLI(mockInstallerContext)
+
+    await assert.doesNotReject(async () => { await flowCliInstaller.install() })
+    assert(mockLangaugeServerApi.activate.notCalled)
+    assert(mockLangaugeServerApi.deactivate.notCalled)
+  }).timeout(MaxTimeout)
 })

extension/test/integration/1 - language-server.test.ts

@@ -5,21 +5,25 @@ import { LanguageServerAPI } from '../../src/server/language-server'
 import { FlowConfig } from '../../src/server/flow-config'
 import { Settings } from '../../src/settings/settings'
 import { MaxTimeout } from '../globals'
-import { of } from 'rxjs'
+import { Subject } from 'rxjs'
 import { State } from 'vscode-languageclient'
 
 suite('Language Server & Emulator Integration', () => {
   let LS: LanguageServerAPI
   let settings: Settings
   let mockConfig: FlowConfig
+  let fileModified$: Subject<void>
+  let pathChanged$: Subject<string>
 
   before(async function () {
     this.timeout(MaxTimeout)
     // Initialize language server
     settings = getMockSettings()
+    fileModified$ = new Subject<void>()
+    pathChanged$ = new Subject<string>()
     mockConfig = {
-      fileModified$: of(),
-      pathChanged$: of(),
+      fileModified$,
+      pathChanged$,
       configPath: null
     } as any
 
@@ -37,4 +41,17 @@ suite('Language Server & Emulator Integration', () => {
     assert.notStrictEqual(LS.client, undefined)
     assert.equal(LS.client?.state, State.Running)
   })
+
+  test('Deactivate Language Server Client', async () => {
+    const client = LS.client
+    await LS.deactivate()
+
+    // Check that client remains stopped even if config changes
+    fileModified$.next()
+    pathChanged$.next('foo')
+
+    assert.equal(client?.state, State.Stopped)
+    assert.equal(LS.client, null)
+    assert.equal(LS.clientState$.getValue(), State.Stopped)
+  })
 })