Add unit tests, handle a few error flows

src/LanguageServerManager.spec.ts

@@ -6,11 +6,15 @@ import { DefinitionRepository } from './DefinitionRepository';
 import { DeclarationProvider } from './DeclarationProvider';
 import type { ExtensionContext } from 'vscode';
 import * as path from 'path';
-import { standardizePath as s } from 'brighterscript';
+import { Deferred, standardizePath as s } from 'brighterscript';
 import * as fsExtra from 'fs-extra';
 import URI from 'vscode-uri';
 import { languageServerInfoCommand } from './commands/LanguageServerInfoCommand';
-
+import type { StateChangeEvent } from 'vscode-languageclient/node';
+import {
+    LanguageClient,
+    State
+} from 'vscode-languageclient/node';
 const Module = require('module');
 const sinon = createSandbox();
 
@@ -37,24 +41,71 @@ describe('LanguageServerManager', () => {
             new DeclarationProvider()
         );
         languageServerManager['context'] = {
+            asAbsolutePath: vscode.context.asAbsolutePath,
             subscriptions: [],
-            asAbsolutePath: () => { },
             globalState: {
-                get: () => {
-
-                },
-                update: () => {
-
-                }
+                get: () => { },
+                update: () => { }
             }
         } as unknown as ExtensionContext;
     });
 
+    function stubConstructClient(processor?: (LanguageClient) => void) {
+        sinon.stub(languageServerManager as any, 'constructLanguageClient').callsFake(() => {
+            const client = {
+                start: () => { },
+                onDidChangeState: (cb) => {
+                },
+                onReady: () => Promise.resolve(),
+                onNotification: () => { }
+            };
+            processor?.(client);
+            return client;
+        });
+    }
+
     afterEach(() => {
         sinon.restore();
         fsExtra.removeSync(tempDir);
     });
 
+    describe('lsp crash tracking', () => {
+        it('shows popup after a stop without a subsequent start/restart/running', async () => {
+            let changeState: (event: StateChangeEvent) => void;
+            //disable starting so we can manually test
+            sinon.stub(languageServerManager, 'syncVersionAndTryRun').callsFake(() => Promise.resolve());
+
+            await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']);
+
+            languageServerManager['lspRunTracker'].debounceDelay = 100;
+
+            let registerOnDidChangeStateDeferred = new Deferred();
+            stubConstructClient((client) => {
+                client.onDidChangeState = (cb) => {
+                    changeState = cb as unknown as any;
+                    registerOnDidChangeStateDeferred.resolve();
+                };
+            });
+
+            void languageServerManager['enableLanguageServer']();
+
+            await registerOnDidChangeStateDeferred.promise;
+            let showErrorMessageDeferred = new Deferred();
+            sinon.stub(vscode.window, 'showErrorMessage').callsFake(() => {
+                showErrorMessageDeferred.resolve();
+            });
+
+            //call the callback with the stopped state
+            changeState({
+                oldState: State.Stopped,
+                newState: State.Stopped
+            });
+
+            // the test will fail if the error message not shown
+            await showErrorMessageDeferred.promise;
+        });
+    });
+
     describe('updateStatusbar', () => {
         it('does not crash when undefined', () => {
             delete languageServerManager['statusbarItem'];
@@ -94,6 +145,7 @@ describe('LanguageServerManager', () => {
 
     describe('enableLanguageServer', () => {
         it('properly handles runtime exception', async () => {
+            stubConstructClient();
             languageServerManager['client'] = {} as any;
             sinon.stub(languageServerManager as any, 'ready').callsFake(() => {
                 throw new Error('failed for test');

src/LanguageServerManager.ts

@@ -44,9 +44,10 @@ class LspRunTracker {
     }
 
     public setState(state: State) {
-        clearTimeout(this.timeoutHandle);
         //if language server is running, clear any timers
-        if (state === State.Stopped) {
+        if (state === State.Starting || state === State.Running) {
+            clearTimeout(this.timeoutHandle);
+        } else {
             this.timeoutHandle = setTimeout(() => {
                 clearTimeout(this.timeoutHandle);
                 this.emitter.emit('stopped');
@@ -146,9 +147,64 @@ export class LanguageServerManager {
 
     private clientDispose: Disposable;
 
+    /**
+     * Create a new LanguageClient instance
+     * @returns
+     */
+    private constructLanguageClient() {
+
+        // The server is implemented in node
+        let serverModule = this.context.asAbsolutePath(
+            path.join('dist', 'LanguageServerRunner.js')
+        );
+
+        //give the runner the specific version of bsc to run
+        const args = [
+            this.selectedBscInfo.path,
+            (this.context.extensionMode === vscode.ExtensionMode.Development).toString()
+        ];
+        // If the extension is launched in debug mode then the debug server options are used
+        // Otherwise the run options are used
+        let serverOptions: ServerOptions = {
+            run: {
+                module: serverModule,
+                transport: TransportKind.ipc,
+                args: args
+            },
+            debug: {
+                module: serverModule,
+                transport: TransportKind.ipc,
+                args: args,
+                // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging
+                options: { execArgv: ['--nolazy', '--inspect=6009'] }
+            }
+        };
+
+        // Options to control the language client
+        let clientOptions: LanguageClientOptions = {
+            // Register the server for various types of documents
+            documentSelector: [
+                { scheme: 'file', language: 'brightscript' },
+                { scheme: 'file', language: 'brighterscript' },
+                { scheme: 'file', language: 'xml' }
+            ],
+            synchronize: {
+                // Notify the server about file changes to every filetype it cares about
+                fileEvents: workspace.createFileSystemWatcher('**/*')
+            }
+        };
+
+        // Create the language client and start the client.
+        return new LanguageClient(
+            'brighterScriptLanguageServer',
+            LANGUAGE_SERVER_NAME,
+            serverOptions,
+            clientOptions
+        );
+    }
+
     private async enableLanguageServer() {
         try {
-
             //if we already have a language server, nothing more needs to be done
             if (this.client) {
                 return await this.ready();
@@ -167,54 +223,7 @@ export class LanguageServerManager {
             //disable the simple providers (the language server will handle all of these)
             this.disableSimpleProviders();
 
-            // The server is implemented in node
-            let serverModule = this.context.asAbsolutePath(
-                path.join('dist', 'LanguageServerRunner.js')
-            );
-
-            //give the runner the specific version of bsc to run
-            const args = [
-                this.selectedBscInfo.path,
-                (this.context.extensionMode === vscode.ExtensionMode.Development).toString()
-            ];
-            // If the extension is launched in debug mode then the debug server options are used
-            // Otherwise the run options are used
-            let serverOptions: ServerOptions = {
-                run: {
-                    module: serverModule,
-                    transport: TransportKind.ipc,
-                    args: args
-                },
-                debug: {
-                    module: serverModule,
-                    transport: TransportKind.ipc,
-                    args: args,
-                    // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging
-                    options: { execArgv: ['--nolazy', '--inspect=6009'] }
-                }
-            };
-
-            // Options to control the language client
-            let clientOptions: LanguageClientOptions = {
-                // Register the server for various types of documents
-                documentSelector: [
-                    { scheme: 'file', language: 'brightscript' },
-                    { scheme: 'file', language: 'brighterscript' },
-                    { scheme: 'file', language: 'xml' }
-                ],
-                synchronize: {
-                    // Notify the server about file changes to every filetype it cares about
-                    fileEvents: workspace.createFileSystemWatcher('**/*')
-                }
-            };
-
-            // Create the language client and start the client.
-            this.client = new LanguageClient(
-                'brighterScriptLanguageServer',
-                LANGUAGE_SERVER_NAME,
-                serverOptions,
-                clientOptions
-            );
+            this.client = this.constructLanguageClient();
 
             this.client.onDidChangeState((event: StateChangeEvent) => {
                 console.log(new Date().toLocaleTimeString(), 'onDidChangeState', State[event.newState]);
@@ -223,6 +232,7 @@ export class LanguageServerManager {
 
             // Start the client. This will also launch the server
             this.clientDispose = this.client.start();
+
             await this.client.onReady();
 
             this.client.onNotification('critical-failure', (message) => {
@@ -231,13 +241,16 @@ export class LanguageServerManager {
             this.registerBusyStatusHandler();
             this.deferred.resolve(true);
         } catch (e) {
-            console.error(e);
-            void this.client?.stop?.();
+            //stop the client by any means necessary
+            try {
+                void this.client?.stop?.();
+            } catch { }
             delete this.client;
 
             this.refreshDeferred();
 
-            this.deferred.reject(e);
+            this.deferred?.reject(e);
+            throw e;
         }
         return this.ready();
     }

src/managers/WebviewViewProviderManager.spec.ts

@@ -19,18 +19,7 @@ describe('WebviewViewProviderManager', () => {
 
     before(() => {
         context = {
-            ...vscode.context,
-            extensionPath: '',
-            subscriptions: [],
-            asAbsolutePath: () => { },
-            globalState: {
-                get: () => {
-
-                },
-                update: () => {
-
-                }
-            }
+            ...vscode.context
         };
 
         config.host = '86.75.30.9';

src/mockVscode.spec.ts

@@ -1,5 +1,6 @@
 import { EventEmitter } from 'eventemitter3';
 import type { Command, Range, TreeDataProvider, TreeItemCollapsibleState, Uri, WorkspaceFolder, ConfigurationScope, ExtensionContext, WorkspaceConfiguration, OutputChannel, QuickPickItem } from 'vscode';
+import * as path from 'path';
 
 //copied from vscode to help with unit tests
 enum QuickPickItemKind {
@@ -14,6 +15,7 @@ afterEach(() => {
 });
 
 export let vscode = {
+    version: '1.89.1',
     env: {
         //disable all telemetry reporting during unit tests
         telemetryConfiguration: {
@@ -22,6 +24,11 @@ export let vscode = {
             isCrashEnabled: false
         }
     },
+    ExtensionMode: {
+        Production: 1,
+        Development: 2,
+        Test: 3
+    },
     CompletionItem: class { },
     CodeLens: class { },
     CodeAction: class { },
@@ -72,10 +79,12 @@ export let vscode = {
 
         }
     },
+    CodeActionKind: {
+    },
     context: {
         subscriptions: [],
-        asAbsolutePath: () => {
-            return '';
+        asAbsolutePath: (arg) => {
+            return path.resolve(arg);
         },
         extensionUri: undefined as Uri,
         extensionPath: '',