Simpler test activation/deactivation in tests (#1214)

Favour using the new `activateExtensionForSuite` and
`activateExtensionForTest` methods, which automatically deactivate the
extension when the suite or test completes.

These methods take optional setup/teardown methods that run on
suite/test setup/teardown. If the setup method returns a promise, this
is run automatically on teardown. This is useful for setup methods that
modify settings, as they can now return the promise directly from
`updateSettings` and any modified settings will be reverted
automatically.

This also prints any logs captured to the output channel if a suite/test
setup/teardown method fails with an error, which was being silently lost
before.

Author: plemarquand

.vscode/launch.json

@@ -33,7 +33,8 @@
 				"${workspaceFolder}/dist/**/*.js"
 			],
 			"env": {
-				"VSCODE_DEBUG": "1"
+				"VSCODE_DEBUG": "1",
+				"VSCODE_TEST": "1"
 			},
 			"preLaunchTask": "compile-tests"
 		},

src/DiagnosticsManager.ts

@@ -59,6 +59,7 @@ export class DiagnosticsManager implements vscode.Disposable {
     private diagnosticCollection: vscode.DiagnosticCollection =
         vscode.languages.createDiagnosticCollection("swift");
     private allDiagnostics: Map<string, vscode.Diagnostic[]> = new Map();
+    private disposed = false;
 
     constructor(context: WorkspaceContext) {
         this.onDidChangeConfigurationDisposible = vscode.workspace.onDidChangeConfiguration(e => {
@@ -114,6 +115,10 @@ export class DiagnosticsManager implements vscode.Disposable {
         sourcePredicate: SourcePredicate,
         newDiagnostics: vscode.Diagnostic[]
     ): void {
+        if (this.disposed) {
+            return;
+        }
+
         const isFromSourceKit = !sourcePredicate(DiagnosticsManager.swiftc);
         // Is a descrepency between SourceKit-LSP and older versions
         // of Swift as to whether the first letter is capitalized or not,
@@ -230,6 +235,7 @@ export class DiagnosticsManager implements vscode.Disposable {
     }
 
     dispose() {
+        this.disposed = true;
         this.diagnosticCollection.dispose();
         this.onDidStartTaskDisposible.dispose();
         this.onDidChangeConfigurationDisposible.dispose();

src/WorkspaceContext.ts

@@ -171,6 +171,14 @@ export class WorkspaceContext implements vscode.Disposable {
         this.lastFocusUri = vscode.window.activeTextEditor?.document.uri;
     }
 
+    async stop() {
+        try {
+            await this.languageClientManager.stop();
+        } catch {
+            // ignore
+        }
+    }
+
     dispose() {
         this.folders.forEach(f => f.dispose());
         this.folders.length = 0;

src/extension.ts

@@ -47,7 +47,7 @@ export interface Api {
     workspaceContext?: WorkspaceContext;
     outputChannel: SwiftOutputChannel;
     activate(): Promise<Api>;
-    deactivate(): void;
+    deactivate(): Promise<void>;
 }
 
 /**
@@ -114,7 +114,10 @@ export async function activate(context: vscode.ExtensionContext): Promise<Api> {
                 workspaceContext: undefined,
                 outputChannel,
                 activate: () => activate(context),
-                deactivate: () => deactivate(context),
+                deactivate: async () => {
+                    await workspaceContext.stop();
+                    await deactivate(context);
+                },
             };
         }
 
@@ -257,7 +260,10 @@ export async function activate(context: vscode.ExtensionContext): Promise<Api> {
             workspaceContext,
             outputChannel,
             activate: () => activate(context),
-            deactivate: () => deactivate(context),
+            deactivate: async () => {
+                await workspaceContext.stop();
+                await deactivate(context);
+            },
         };
     } catch (error) {
         const errorMessage = getErrorDescription(error);
@@ -269,7 +275,6 @@ export async function activate(context: vscode.ExtensionContext): Promise<Api> {
 }
 
 async function deactivate(context: vscode.ExtensionContext): Promise<void> {
-    console.debug("Deactivating Swift for Visual Studio Code...");
     contextKeys.isActivated = false;
     context.subscriptions.forEach(subscription => subscription.dispose());
     context.subscriptions.length = 0;

src/sourcekit-lsp/LanguageClientManager.ts

@@ -245,14 +245,21 @@ export class LanguageClientManager {
         this.cancellationToken = new vscode.CancellationTokenSource();
     }
 
+    // The language client stops asnyhronously, so we need to wait for it to stop
+    // instead of doing it in dispose, which must be synchronous.
+    async stop() {
+        if (this.languageClient && this.languageClient.state === langclient.State.Running) {
+            await this.languageClient.dispose();
+        }
+    }
+
     dispose() {
         this.cancellationToken?.cancel();
         this.cancellationToken?.dispose();
         this.legacyInlayHints?.dispose();
         this.peekDocuments?.dispose();
         this.getReferenceDocument?.dispose();
         this.subscriptions.forEach(item => item.dispose());
-        this.languageClient?.stop();
         this.namedOutputChannels.forEach(channel => channel.dispose());
     }
 

test/integration-tests/BackgroundCompilation.test.ts

@@ -14,30 +14,24 @@
 
 import * as assert from "assert";
 import * as vscode from "vscode";
-import { beforeEach, afterEach } from "mocha";
 import { WorkspaceContext } from "../../src/WorkspaceContext";
 import { testAssetUri } from "../fixtures";
 import { waitForNoRunningTasks } from "../utilities";
 import { Workbench } from "../../src/utilities/commands";
-import { activateExtension, deactivateExtension, updateSettings } from "./utilities/testutilities";
+import { activateExtensionForTest, updateSettings } from "./utilities/testutilities";
 
 suite("BackgroundCompilation Test Suite", () => {
     let workspaceContext: WorkspaceContext;
-    let settingsTeardown: () => Promise<void>;
 
-    beforeEach(async function () {
-        workspaceContext = await activateExtension(this.currentTest);
-
-        assert.notEqual(workspaceContext.folders.length, 0);
-        await waitForNoRunningTasks();
-        settingsTeardown = await updateSettings({
-            "swift.backgroundCompilation": true,
-        });
-    });
-
-    afterEach(async () => {
-        await settingsTeardown();
-        await deactivateExtension();
+    activateExtensionForTest({
+        async setup(ctx) {
+            workspaceContext = ctx;
+            assert.notEqual(workspaceContext.folders.length, 0);
+            await waitForNoRunningTasks();
+            return await updateSettings({
+                "swift.backgroundCompilation": true,
+            });
+        },
     });
 
     suiteTeardown(async () => {

test/integration-tests/DiagnosticsManager.test.ts

@@ -23,11 +23,7 @@ import { DiagnosticsManager } from "../../src/DiagnosticsManager";
 import { FolderContext } from "../../src/FolderContext";
 import { Version } from "../../src/utilities/version";
 import { Workbench } from "../../src/utilities/commands";
-import {
-    activateExtension,
-    deactivateExtension,
-    folderInRootWorkspace,
-} from "./utilities/testutilities";
+import { activateExtensionForSuite, folderInRootWorkspace } from "./utilities/testutilities";
 
 const waitForDiagnostics = (uris: vscode.Uri[], allowEmpty: boolean = true) =>
     new Promise<void>(res =>
@@ -95,26 +91,26 @@ suite("DiagnosticsManager Test Suite", async function () {
     let cppUri: vscode.Uri;
     let cppHeaderUri: vscode.Uri;
 
-    suiteSetup(async function () {
-        workspaceContext = await activateExtension(this.currentTest);
-        toolchain = workspaceContext.toolchain;
-        workspaceFolder = testAssetWorkspaceFolder("diagnostics");
-        cWorkspaceFolder = testAssetWorkspaceFolder("diagnosticsC");
-        cppWorkspaceFolder = testAssetWorkspaceFolder("diagnosticsCpp");
-        folderContext = await folderInRootWorkspace("diagnostics", workspaceContext);
-        cFolderContext = await folderInRootWorkspace("diagnosticsC", workspaceContext);
-        cppFolderContext = await folderInRootWorkspace("diagnosticsCpp", workspaceContext);
-        mainUri = vscode.Uri.file(`${workspaceFolder.uri.path}/Sources/main.swift`);
-        funcUri = vscode.Uri.file(`${workspaceFolder.uri.path}/Sources/func.swift`);
-        cUri = vscode.Uri.file(`${cWorkspaceFolder.uri.path}/Sources/MyPoint/MyPoint.c`);
-        cppUri = vscode.Uri.file(`${cppWorkspaceFolder.uri.path}/Sources/MyPoint/MyPoint.cpp`);
-        cppHeaderUri = vscode.Uri.file(
-            `${cppWorkspaceFolder.uri.path}/Sources/MyPoint/include/MyPoint.h`
-        );
-    });
-
-    suiteTeardown(async () => {
-        await deactivateExtension();
+    activateExtensionForSuite({
+        async setup(ctx) {
+            this.timeout(60000);
+
+            workspaceContext = ctx;
+            toolchain = workspaceContext.toolchain;
+            workspaceFolder = testAssetWorkspaceFolder("diagnostics");
+            cWorkspaceFolder = testAssetWorkspaceFolder("diagnosticsC");
+            cppWorkspaceFolder = testAssetWorkspaceFolder("diagnosticsCpp");
+            folderContext = await folderInRootWorkspace("diagnostics", workspaceContext);
+            cFolderContext = await folderInRootWorkspace("diagnosticsC", workspaceContext);
+            cppFolderContext = await folderInRootWorkspace("diagnosticsCpp", workspaceContext);
+            mainUri = vscode.Uri.file(`${workspaceFolder.uri.path}/Sources/main.swift`);
+            funcUri = vscode.Uri.file(`${workspaceFolder.uri.path}/Sources/func.swift`);
+            cUri = vscode.Uri.file(`${cWorkspaceFolder.uri.path}/Sources/MyPoint/MyPoint.c`);
+            cppUri = vscode.Uri.file(`${cppWorkspaceFolder.uri.path}/Sources/MyPoint/MyPoint.cpp`);
+            cppHeaderUri = vscode.Uri.file(
+                `${cppWorkspaceFolder.uri.path}/Sources/MyPoint/include/MyPoint.h`
+            );
+        },
     });
 
     suite("Parse diagnostics", async () => {

test/integration-tests/ExtensionActivation.test.ts

@@ -15,7 +15,13 @@
 import * as vscode from "vscode";
 import * as assert from "assert";
 import { afterEach } from "mocha";
-import { activateExtension, deactivateExtension } from "./utilities/testutilities";
+import {
+    activateExtension,
+    activateExtensionForSuite,
+    activateExtensionForTest,
+    deactivateExtension,
+} from "./utilities/testutilities";
+import { WorkspaceContext } from "../../src/WorkspaceContext";
 
 suite("Extension Activation/Deactivation Tests", () => {
     suite("Extension Activation", () => {
@@ -53,4 +59,42 @@ suite("Extension Activation/Deactivation Tests", () => {
         assert(ext);
         assert.equal(workspaceContext.subscriptions.length, 0);
     });
+
+    suite("Extension Activation per suite", () => {
+        let workspaceContext: WorkspaceContext | undefined;
+        let capturedWorkspaceContext: WorkspaceContext | undefined;
+        activateExtensionForSuite({
+            async setup(ctx) {
+                workspaceContext = ctx;
+            },
+        });
+
+        test("Assert workspace context is created", () => {
+            assert.ok(workspaceContext);
+            capturedWorkspaceContext = workspaceContext;
+        });
+
+        test("Assert workspace context is not recreated", () => {
+            assert.strictEqual(workspaceContext, capturedWorkspaceContext);
+        });
+    });
+
+    suite("Extension activation per test", () => {
+        let workspaceContext: WorkspaceContext | undefined;
+        let capturedWorkspaceContext: WorkspaceContext | undefined;
+        activateExtensionForTest({
+            async setup(ctx) {
+                workspaceContext = ctx;
+            },
+        });
+
+        test("Assert workspace context is created", () => {
+            assert.ok(workspaceContext);
+            capturedWorkspaceContext = workspaceContext;
+        });
+
+        test("Assert workspace context is recreated per test", () => {
+            assert.notStrictEqual(workspaceContext, capturedWorkspaceContext);
+        });
+    });
 });

test/integration-tests/WorkspaceContext.test.ts

@@ -19,18 +19,16 @@ import { FolderOperation, WorkspaceContext } from "../../src/WorkspaceContext";
 import { createBuildAllTask } from "../../src/tasks/SwiftTaskProvider";
 import { Version } from "../../src/utilities/version";
 import { SwiftExecution } from "../../src/tasks/SwiftExecution";
-import { activateExtension, deactivateExtension } from "./utilities/testutilities";
+import { activateExtensionForSuite } from "./utilities/testutilities";
 
 suite("WorkspaceContext Test Suite", () => {
     let workspaceContext: WorkspaceContext;
     const packageFolder: vscode.Uri = testAssetUri("defaultPackage");
 
-    suiteSetup(async function () {
-        workspaceContext = await activateExtension();
-    });
-
-    suiteTeardown(async () => {
-        await deactivateExtension();
+    activateExtensionForSuite({
+        async setup(ctx) {
+            workspaceContext = ctx;
+        },
     });
 
     suite("Folder Events", () => {

test/integration-tests/commands/build.test.ts

@@ -25,42 +25,40 @@ import { makeDebugConfigurations } from "../../../src/debugger/launch";
 import { Workbench } from "../../../src/utilities/commands";
 import { continueSession, waitForDebugAdapterCommand } from "../../utilities/debug";
 import {
-    activateExtension,
-    deactivateExtension,
+    activateExtensionForSuite,
     folderInRootWorkspace,
     updateSettings,
 } from "../utilities/testutilities";
 
 suite("Build Commands", function () {
     let folderContext: FolderContext;
     let workspaceContext: WorkspaceContext;
-    let settingsTeardown: () => Promise<void>;
     let buildPath: string;
     const uri = testAssetUri("defaultPackage/Sources/PackageExe/main.swift");
     const breakpoints = [
         new vscode.SourceBreakpoint(new vscode.Location(uri, new vscode.Position(2, 0))),
     ];
 
-    suiteSetup(async function () {
-        workspaceContext = await activateExtension();
-        await waitForNoRunningTasks();
-        folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext);
-        buildPath = path.join(folderContext.folder.fsPath, ".build");
-        await workspaceContext.focusFolder(folderContext);
-        await vscode.window.showTextDocument(uri);
-        settingsTeardown = await updateSettings({
-            "swift.autoGenerateLaunchConfigurations": true,
-        });
-        await makeDebugConfigurations(folderContext, undefined, true);
+    activateExtensionForSuite({
+        async setup(ctx) {
+            workspaceContext = ctx;
+            await waitForNoRunningTasks();
+            folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext);
+            buildPath = path.join(folderContext.folder.fsPath, ".build");
+            await workspaceContext.focusFolder(folderContext);
+            await vscode.window.showTextDocument(uri);
+            const settingsTeardown = await updateSettings({
+                "swift.autoGenerateLaunchConfigurations": true,
+            });
+            await makeDebugConfigurations(folderContext, undefined, true);
+            return settingsTeardown;
+        },
+        async teardown() {
+            await vscode.commands.executeCommand(Workbench.ACTION_CLOSEALLEDITORS);
+        },
     });
 
-    suiteTeardown(async () => {
-        await settingsTeardown();
-        await vscode.commands.executeCommand(Workbench.ACTION_CLOSEALLEDITORS);
-        await deactivateExtension();
-    });
-
-    teardown(() => {
+    teardown(async () => {
         // Remove the build directory after each test case
         if (fs.existsSync(buildPath)) {
             fs.rmSync(buildPath, { recursive: true, force: true });