From 85d6b98ebd9be534a399f288607f05ff3f484d25 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 13 Dec 2024 11:12:03 -0500 Subject: [PATCH 1/3] Test suites in nested extensions have incorrect parent in test explorer 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 test target might be `Foo`, and one of the child `testItems` in the document tests response 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, syntheize the intermediate children just like we synthesize the test target. --- src/TestExplorer/TestDiscovery.ts | 50 +++++++++++++++++++ src/TestExplorer/TestExplorer.ts | 24 ++++----- .../testexplorer/TestDiscovery.test.ts | 27 ++++++++++ 3 files changed, 86 insertions(+), 15 deletions(-) diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index c0017897b..3d901a544 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -66,6 +66,56 @@ 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 { + 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 index cd164e453..9190bc4d4 100644 --- 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 index 3946a9f72..9f14091b8 100644 --- a/test/integration-tests/testexplorer/TestDiscovery.test.ts +++ b/test/integration-tests/testexplorer/TestDiscovery.test.ts @@ -18,6 +18,7 @@ import { beforeEach } from "mocha"; import { TestClass, updateTests, + updateTestsForTarget, updateTestsFromClasses, } from "../../../src/TestExplorer/TestDiscovery"; import { reduceTestItemChildren } from "../../../src/TestExplorer/TestUtils"; @@ -158,6 +159,32 @@ 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"); + + 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: "XCTest" }, { id: "runnable" }], + children: [ + { + id: "AppTarget.AppTests/ChildTests", + tags: [{ id: "XCTest" }, { 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()); From dcfe7c445f98b3b0fe2dae6ebc7d4f2c6242b465 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 13 Dec 2024 16:00:36 -0500 Subject: [PATCH 2/3] Test even deeper nesting --- .../testexplorer/TestDiscovery.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/integration-tests/testexplorer/TestDiscovery.test.ts b/test/integration-tests/testexplorer/TestDiscovery.test.ts index 9f14091b8..efb9b73cd 100644 --- a/test/integration-tests/testexplorer/TestDiscovery.test.ts +++ b/test/integration-tests/testexplorer/TestDiscovery.test.ts @@ -160,7 +160,7 @@ suite("TestDiscovery Suite", () => { }); test("handles adding a test to an existing parent when updating with a partial tree", () => { - const child = testItem("AppTarget.AppTests/ChildTests"); + const child = testItem("AppTarget.AppTests/ChildTests/SubChildTests"); updateTestsForTarget(testController, { id: "AppTarget", label: "AppTarget" }, [child]); @@ -176,7 +176,13 @@ suite("TestDiscovery Suite", () => { { id: "AppTarget.AppTests/ChildTests", tags: [{ id: "XCTest" }, { id: "runnable" }], - children: [], + children: [ + { + id: "AppTarget.AppTests/ChildTests/SubChildTests", + tags: [{ id: "XCTest" }, { id: "runnable" }], + children: [], + }, + ], }, ], }, From 2b32d02a9c2830a3c9d1aee26bf436eef53be4ec Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 16 Dec 2024 10:15:31 -0500 Subject: [PATCH 3/3] Only synthesize children for swift-testing tests --- src/TestExplorer/TestDiscovery.ts | 5 +++++ .../testexplorer/TestDiscovery.test.ts | 13 +++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index 3d901a544..a80cf6c9b 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -79,6 +79,11 @@ export function updateTestsForTarget( // 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 diff --git a/test/integration-tests/testexplorer/TestDiscovery.test.ts b/test/integration-tests/testexplorer/TestDiscovery.test.ts index efb9b73cd..b7fae9cf4 100644 --- a/test/integration-tests/testexplorer/TestDiscovery.test.ts +++ b/test/integration-tests/testexplorer/TestDiscovery.test.ts @@ -24,6 +24,7 @@ import { 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; @@ -50,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: [], @@ -160,7 +161,7 @@ suite("TestDiscovery Suite", () => { }); test("handles adding a test to an existing parent when updating with a partial tree", () => { - const child = testItem("AppTarget.AppTests/ChildTests/SubChildTests"); + const child = testItem("AppTarget.AppTests/ChildTests/SubChildTests", "swift-testing"); updateTestsForTarget(testController, { id: "AppTarget", label: "AppTarget" }, [child]); @@ -171,15 +172,15 @@ suite("TestDiscovery Suite", () => { children: [ { id: "AppTarget.AppTests", - tags: [{ id: "XCTest" }, { id: "runnable" }], + tags: [{ id: "swift-testing" }, { id: "runnable" }], children: [ { id: "AppTarget.AppTests/ChildTests", - tags: [{ id: "XCTest" }, { id: "runnable" }], + tags: [{ id: "swift-testing" }, { id: "runnable" }], children: [ { id: "AppTarget.AppTests/ChildTests/SubChildTests", - tags: [{ id: "XCTest" }, { id: "runnable" }], + tags: [{ id: "swift-testing" }, { id: "runnable" }], children: [], }, ],