Test suites in nested extensions have incorrect parent in test explor…

…er (#1275)
swiftlang · Dec 16, 2024 · 5cc4236 · 5cc4236
1 parent 1cbbb45
commit 5cc4236
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 17 deletions.
diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts
@@ -66,6 +66,61 @@ export function updateTestsFromClasses(
     updateTests(testController, targets);
 }
 
+export function updateTestsForTarget(
+    testController: vscode.TestController,
+    testTarget: { id: string; label: string },
+    testItems: TestClass[],
+    filterFile?: vscode.Uri
+) {
+    // Because swift-testing suites can be defined through nested extensions the tests
+    // provided might not be directly parented to the test target. For instance, the
+    // target might be `Foo`, and one of the child `testItems` might be `Foo.Bar/Baz`.
+    // If we simply attach the `testItems` to the root test target then the intermediate
+    // suite `Bar` will be dropped. To avoid this, we syntheize the intermediate children
+    // just like we synthesize the test target.
+    function synthesizeChildren(testItem: TestClass): TestClass {
+        // Only Swift Testing tests can be nested in a way that requires synthesis.
+        if (testItem.style === "XCTest") {
+            return testItem;
+        }
+
+        const item = { ...testItem };
+        // To determine if any root level test items are missing a parent we check how many
+        // components there are in the ID. If there are more than one (the test target) then
+        // we synthesize all the intermediary test items.
+        const idComponents = testItem.id.split(/\.|\//);
+        idComponents.pop(); // Remove the last component to get the parent ID components
+        if (idComponents.length > 1) {
+            let newId = idComponents.slice(0, 2).join(".");
+            const remainingIdComponents = idComponents.slice(2);
+            if (remainingIdComponents.length) {
+                newId += "/" + remainingIdComponents.join("/");
+            }
+            return synthesizeChildren({
+                id: newId,
+                label: idComponents[idComponents.length - 1],
+                children: [item],
+                location: undefined,
+                disabled: false,
+                style: item.style,
+                tags: item.tags,
+            });
+        }
+        return item;
+    }
+
+    const testTargetClass: TestClass = {
+        id: testTarget.id,
+        label: testTarget.label,
+        children: testItems.map(synthesizeChildren),
+        location: undefined,
+        disabled: false,
+        style: "test-target",
+        tags: [],
+    };
+    updateTests(testController, [testTargetClass], filterFile);
+}
+
 /**
  * Update Test Controller TestItems based off array of TestTargets
  * @param testController Test controller

diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts
@@ -210,23 +210,17 @@ export class TestExplorer {
                 if (target && target.type === "test") {
                     testExplorer.lspTestDiscovery
                         .getDocumentTests(folder.swiftPackage, uri)
-                        .then(
-                            tests =>
-                                [
-                                    {
-                                        id: target.c99name,
-                                        label: target.name,
-                                        children: tests,
-                                        location: undefined,
-                                        disabled: false,
-                                        style: "test-target",
-                                        tags: [],
-                                    },
-                                ] as TestDiscovery.TestClass[]
+                        .then(tests =>
+                            TestDiscovery.updateTestsForTarget(
+                                testExplorer.controller,
+                                { id: target.c99name, label: target.name },
+                                tests,
+                                uri
+                            )
                         )
                         // Fallback to parsing document symbols for XCTests only
-                        .catch(() => parseTestsFromDocumentSymbols(target.name, symbols, uri))
-                        .then(tests => {
+                        .catch(() => {
+                            const tests = parseTestsFromDocumentSymbols(target.name, symbols, uri);
                             testExplorer.updateTests(testExplorer.controller, tests, uri);
                         });
                 }

diff --git a/test/integration-tests/testexplorer/TestDiscovery.test.ts b/test/integration-tests/testexplorer/TestDiscovery.test.ts
@@ -18,11 +18,13 @@ import { beforeEach } from "mocha";
 import {
     TestClass,
     updateTests,
+    updateTestsForTarget,
     updateTestsFromClasses,
 } from "../../../src/TestExplorer/TestDiscovery";
 import { reduceTestItemChildren } from "../../../src/TestExplorer/TestUtils";
 import { SwiftPackage, Target, TargetType } from "../../../src/SwiftPackage";
 import { SwiftToolchain } from "../../../src/toolchain/toolchain";
+import { TestStyle } from "../../../src/sourcekit-lsp/extensions";
 
 suite("TestDiscovery Suite", () => {
     let testController: vscode.TestController;
@@ -49,12 +51,12 @@ suite("TestDiscovery Suite", () => {
         );
     }
 
-    function testItem(id: string): TestClass {
+    function testItem(id: string, style: TestStyle = "XCTest"): TestClass {
         return {
             id,
             label: id,
             disabled: false,
-            style: "XCTest",
+            style,
             location: undefined,
             tags: [],
             children: [],
@@ -158,6 +160,38 @@ suite("TestDiscovery Suite", () => {
         assert.deepStrictEqual(testController.items.get("foo")?.label, "New Label");
     });
 
+    test("handles adding a test to an existing parent when updating with a partial tree", () => {
+        const child = testItem("AppTarget.AppTests/ChildTests/SubChildTests", "swift-testing");
+
+        updateTestsForTarget(testController, { id: "AppTarget", label: "AppTarget" }, [child]);
+
+        assert.deepStrictEqual(testControllerChildren(testController.items), [
+            {
+                id: "AppTarget",
+                tags: [{ id: "test-target" }, { id: "runnable" }],
+                children: [
+                    {
+                        id: "AppTarget.AppTests",
+                        tags: [{ id: "swift-testing" }, { id: "runnable" }],
+                        children: [
+                            {
+                                id: "AppTarget.AppTests/ChildTests",
+                                tags: [{ id: "swift-testing" }, { id: "runnable" }],
+                                children: [
+                                    {
+                                        id: "AppTarget.AppTests/ChildTests/SubChildTests",
+                                        tags: [{ id: "swift-testing" }, { id: "runnable" }],
+                                        children: [],
+                                    },
+                                ],
+                            },
+                        ],
+                    },
+                ],
+            },
+        ]);
+    });
+
     test("updates tests from classes within a swift package", async () => {
         const file = vscode.Uri.file("file:///some/file.swift");
         const swiftPackage = await SwiftPackage.create(file, await SwiftToolchain.create());