Tests - Skip Dependency Command Tests (#1879)

* Tests - Activate extension for each test in Dependency Command Tests

Refactor the dependency command tests to be simpler, and also skip them, because they're still flaky

Author: plemarquand

package.json

@@ -553,7 +553,7 @@
           "swift.disableAutoResolve": {
             "type": "boolean",
             "default": false,
-            "markdownDescription": "Disable automatic running of `swift package resolve` whenever the `Package.swift` or `Package.resolve` files are updated. This will also disable searching for command plugins and the initial test discovery process.",
+            "markdownDescription": "Disable automatic running of `swift package resolve` whenever the `Package.swift` or `Package.resolved` files are updated. This will also disable searching for command plugins and the initial test discovery process.",
             "scope": "machine-overridable"
           },
           "swift.diagnosticsCollection": {
@@ -2000,7 +2000,7 @@
     "integration-test": "npm test -- --label integrationTests",
     "unit-test": "npm test -- --label unitTests",
     "coverage": "npm test -- --coverage",
-    "compile-tests": "del-cli ./assets/test/**/.build && npm run compile",
+    "compile-tests": "del-cli ./assets/test/**/.build && del-cli ./assets/test/**/.spm-cache && npm run compile",
     "package": "tsx ./scripts/package.ts",
     "dev-package": "tsx ./scripts/dev_package.ts",
     "preview-package": "tsx ./scripts/preview_package.ts",

src/TestExplorer/TestExplorer.ts

@@ -99,21 +99,42 @@ export class TestExplorer {
     ): Promise<void> {
         const target = await folder.swiftPackage.getTarget(uri.fsPath);
         if (target?.type !== "test") {
+            this.logger.info(
+                `Target ${target} is not a test target, aborting looking for tests within it`,
+                "Test Explorer"
+            );
             return;
         }
 
+        this.logger.info(`Getting tests for ${uri.toString()}`, "Test Explorer");
         try {
             const tests = await this.lspTestDiscovery.getDocumentTests(folder.swiftPackage, uri);
+            this.logger.info(
+                `LSP test discovert found ${tests.length} top level tests`,
+                "Test Explorer"
+            );
             TestDiscovery.updateTestsForTarget(
                 this.controller,
                 { id: target.c99name, label: target.name },
                 tests,
                 uri
             );
+            this.logger.info(
+                `Emitting test item change after LSP test discovery for ${uri.toString()}`,
+                "Test Explorer"
+            );
             this.onTestItemsDidChangeEmitter.fire(this.controller);
-        } catch {
+        } catch (error) {
+            this.logger.error(
+                `Error occurred during LSP test discovery for ${uri.toString()}: ${error}`,
+                "Test Explorer"
+            );
             // Fallback to parsing document symbols for XCTests only
             const tests = parseTestsFromDocumentSymbols(target.name, symbols, uri);
+            this.logger.info(
+                `Parsed ${tests.length} top level tests from document symbols from ${uri.toString()}`,
+                "Test Explorer"
+            );
             this.updateTests(this.controller, tests, uri);
         }
     }

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

@@ -13,28 +13,25 @@
 //===----------------------------------------------------------------------===//
 import { expect } from "chai";
 import * as fs from "fs/promises";
+import { beforeEach } from "mocha";
 import * as path from "path";
 import * as vscode from "vscode";
 
 import { FolderContext } from "@src/FolderContext";
+import { ResolvedDependency } from "@src/SwiftPackage";
 import { WorkspaceContext } from "@src/WorkspaceContext";
 import { Commands } from "@src/commands";
-import { PackageNode, ProjectPanelProvider } from "@src/ui/ProjectPanelProvider";
 
 import { testAssetUri } from "../../fixtures";
 import { tag } from "../../tags";
 import { waitForNoRunningTasks } from "../../utilities/tasks";
-import {
-    activateExtensionForSuite,
-    findWorkspaceFolder,
-    folderInRootWorkspace,
-} from "../utilities/testutilities";
+import { activateExtensionForTest, findWorkspaceFolder } from "../utilities/testutilities";
 
 tag("large").suite("Dependency Commands Test Suite", function () {
     let depsContext: FolderContext;
     let workspaceContext: WorkspaceContext;
 
-    activateExtensionForSuite({
+    activateExtensionForTest({
         async setup(ctx) {
             workspaceContext = ctx;
             depsContext = findWorkspaceFolder("dependencies", workspaceContext)!;
@@ -56,95 +53,40 @@ tag("large").suite("Dependency Commands Test Suite", function () {
         expect(result).to.be.true;
     });
 
-    suite("Swift: Use Local Dependency", function () {
-        let treeProvider: ProjectPanelProvider;
-
+    // Skipping because these tests are currently flakey in CI
+    suite.skip("Swift: Use Local Dependency", function () {
         setup(async () => {
             await waitForNoRunningTasks();
-            treeProvider = new ProjectPanelProvider(workspaceContext);
-        });
-
-        teardown(() => {
-            treeProvider?.dispose();
         });
 
-        async function getDependency() {
-            const headers = await treeProvider.getChildren();
-            const header = headers.find(n => n.name === "Dependencies") as PackageNode;
-            workspaceContext.logger.info(
-                `getDependency: Current headers: ${headers.map(n => n.name)}`
-            );
-            if (!header) {
-                return;
-            }
-
-            const children = await header.getChildren();
-            workspaceContext.logger.info(
-                `getDependencyInState: Current children for "Dependencies" entry: ${children.map(n => n.name).join(", ")}`
-            );
-            return children.find(
-                n => n.name.toLocaleLowerCase() === "swift-markdown"
-            ) as PackageNode;
-        }
-
-        // Wait for the dependency to switch to the expected state.
-        // This doesn't happen immediately after the USE_LOCAL_DEPENDENCY
-        // and RESET_PACKAGE commands because the file watcher on
-        // workspace-state.json needs to trigger.
-        async function getDependencyInState(state: "remote" | "editing") {
-            let depType: string | undefined;
-            for (let i = 0; i < 10; i++) {
-                const dep = await getDependency();
-                if (dep?.type === state) {
-                    return dep;
-                }
-                depType = dep?.type;
-                await new Promise(resolve => setTimeout(resolve, 1000));
+        beforeEach(async function () {
+            // Clean the Package.resolved before every test to ensure we start from a known state
+            try {
+                await fs.rm(path.join(depsContext.folder.fsPath, "Package.resolved"));
+            } catch {
+                // if we haven't done a resolve yet, the file won't exist
             }
 
-            const headers = await treeProvider.getChildren();
-            const headerNames = headers.map(n => n.name);
-            const depChildren = await (
-                headers.find(n => n.name === "Dependencies") as PackageNode
-            )?.getChildren();
-            const childrenNames = depChildren?.map(n => n.name) ?? [];
-
-            const dependenciesFolderContext = await folderInRootWorkspace(
-                "dependencies",
-                workspaceContext
-            );
-            const resolvedPath = path.join(
-                dependenciesFolderContext.folder.fsPath,
-                "Package.resolved"
-            );
-            const packageResolvedContents = await fs.readFile(resolvedPath, "utf8");
-
-            throw Error(
-                `Could not find dependency with state "${state}", instead it was "${depType}". Current headers: ${headerNames.map(h => `"${h}"`).join(", ")}, Current children for "Dependencies" entry: ${childrenNames.map(c => `"${c}"`).join(", ")}\nContents of Package.resolved:\n${packageResolvedContents}`
-            );
-        }
+            // Perform a resolve first to make sure that dependencies are up to date
+            await vscode.commands.executeCommand(Commands.RESOLVE_DEPENDENCIES);
 
-        async function useLocalDependencyTest() {
             workspaceContext.logger.info(
                 "useLocalDependencyTest: Fetching the dependency in the 'remote' state"
             );
 
-            // spm edit with user supplied local version of dependency
-            const item = await getDependencyInState("remote");
+            // Get the dependency in remote state
+            const remoteDep = await getDependencyInState("remote");
             const localDep = testAssetUri("swift-markdown");
 
             workspaceContext.logger.info(
                 "useLocalDependencyTest: Resolving latest dependencies before editing"
             );
 
-            // Perform a resolve first to make sure that dependencies are up to date
-            await vscode.commands.executeCommand(Commands.RESOLVE_DEPENDENCIES);
-
             workspaceContext.logger.info(`Configuring ${localDep.fsPath} to the "editing" state`);
 
             const result = await vscode.commands.executeCommand(
                 Commands.USE_LOCAL_DEPENDENCY,
-                item,
+                createPackageNode(remoteDep),
                 localDep,
                 depsContext
             );
@@ -162,11 +104,75 @@ tag("large").suite("Dependency Commands Test Suite", function () {
             workspaceContext.logger.info(
                 "useLocalDependencyTest: Use local dependency was verified to be in 'editing' state"
             );
+        });
+
+        /**
+         * Get the swift-markdown dependency from the package dependencies
+         */
+        async function getSwiftMarkdownDependency(): Promise<ResolvedDependency | undefined> {
+            // Reload workspace state to get latest dependency information
+            await depsContext.reloadWorkspaceState();
+
+            const dependencies = await depsContext.swiftPackage.rootDependencies;
+            const swiftMarkdownDep = dependencies.find(
+                dep => dep.identity.toLowerCase() === "swift-markdown"
+            );
+
+            workspaceContext.logger.info(
+                `getSwiftMarkdownDependency: Found dependency with type "${swiftMarkdownDep?.type}"`
+            );
+
+            return swiftMarkdownDep;
         }
 
-        test("Swift: Reset Package Dependencies", async function () {
-            await useLocalDependencyTest();
+        /**
+         * Create a PackageNode from a ResolvedDependency for use with commands
+         */
+        function createPackageNode(dependency: ResolvedDependency): any {
+            return {
+                __isPackageNode: true,
+                name: dependency.identity,
+                location: dependency.location,
+                type: dependency.type,
+                path: dependency.path ?? "",
+                dependency: dependency,
+            };
+        }
+
+        /**
+         * Wait for the dependency to switch to the expected state.
+         * This doesn't happen immediately after the USE_LOCAL_DEPENDENCY
+         * and RESET_PACKAGE commands because the file watcher on
+         * workspace-state.json needs to trigger.
+         */
+        async function getDependencyInState(
+            state: "remote" | "editing"
+        ): Promise<ResolvedDependency> {
+            let currentDep: ResolvedDependency | undefined;
+
+            for (let i = 0; i < 10; i++) {
+                currentDep = await getSwiftMarkdownDependency();
 
+                workspaceContext.logger.info(
+                    `getDependencyInState: Current state of dependency is "${currentDep?.type}", waiting for "${state}"`
+                );
+
+                if (currentDep?.type === state) {
+                    return currentDep;
+                }
+
+                await new Promise(resolve => setTimeout(resolve, 1000));
+            }
+
+            const dependencies = await depsContext.swiftPackage.rootDependencies;
+            const dependencyNames = dependencies.map(dep => dep.identity);
+
+            throw Error(
+                `Could not find swift-markdown dependency with state "${state}", instead it was "${currentDep?.type}". Available dependencies: ${dependencyNames.join(", ")}`
+            );
+        }
+
+        test("Swift: Reset Package Dependencies", async function () {
             workspaceContext.logger.info("Resetting package dependency to remote version");
 
             // spm reset
@@ -183,13 +189,12 @@ tag("large").suite("Dependency Commands Test Suite", function () {
         });
 
         test("Swift: Unedit To Original Version", async function () {
-            await useLocalDependencyTest();
-
             workspaceContext.logger.info("Unediting package dependency to original version");
 
+            const editingDep = await getDependencyInState("editing");
             const result = await vscode.commands.executeCommand(
                 Commands.UNEDIT_DEPENDENCY,
-                await getDependencyInState("editing"),
+                createPackageNode(editingDep),
                 depsContext
             );
             expect(result).to.be.true;

test/integration-tests/editor/CommentCompletion.test.ts

@@ -15,6 +15,7 @@ import * as assert from "assert";
 import * as vscode from "vscode";
 
 import { CommentCompletionProviders } from "@src/editor/CommentCompletion";
+import { Workbench } from "@src/utilities/commands";
 
 suite("CommentCompletion Test Suite", () => {
     let provider: CommentCompletionProviders;
@@ -23,7 +24,10 @@ suite("CommentCompletion Test Suite", () => {
         provider = new CommentCompletionProviders();
     });
 
-    teardown(() => provider.dispose());
+    teardown(async () => {
+        provider.dispose();
+        await vscode.commands.executeCommand(Workbench.ACTION_CLOSEALLEDITORS);
+    });
 
     suite("Function Comment Completion", () => {
         test("Completion on line that isn't a comment", async () => {