From 1abfb65a51eb0ce2c7b3b945bd9b2b3fbb1d5c86 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 27 Jun 2024 11:49:00 -0400 Subject: [PATCH] Prepend module name to TestItem IDs It is possible to have two identically named suites in two different test targets. These were being erroniously rolled up in to the same parent TestItem. Disambiguate these TestItems by prepending the module name. This has the added benefit of making the TestItem IDs a fully qualified name that can be passed to `swift test`. The module name is pulled from the compiler arguments for the target. If no module name can be found we fall back to the `targetID` for the `ConfiguredTarget`. --- Sources/SKCore/BuildSystemManager.swift | 32 ++ Sources/SourceKitLSP/TestDiscovery.swift | 27 +- Sources/SwiftExtensions/Array+Safe.swift | 19 + Sources/SwiftExtensions/CMakeLists.txt | 1 + .../DocumentTestDiscoveryTests.swift | 261 +++---------- .../WorkspaceTestDiscoveryTests.swift | 356 ++++++++---------- 6 files changed, 291 insertions(+), 405 deletions(-) create mode 100644 Sources/SwiftExtensions/Array+Safe.swift diff --git a/Sources/SKCore/BuildSystemManager.swift b/Sources/SKCore/BuildSystemManager.swift index 3f6fc7815..92a824fad 100644 --- a/Sources/SKCore/BuildSystemManager.swift +++ b/Sources/SKCore/BuildSystemManager.swift @@ -14,6 +14,7 @@ import BuildServerProtocol import Dispatch import LSPLogging import LanguageServerProtocol +import SwiftExtensions import struct TSCBasic.AbsolutePath @@ -154,6 +155,37 @@ extension BuildSystemManager { .first } + /// Returns the target's module name as parsed from the `ConfiguredTarget`'s compiler arguments. + public func moduleName(for document: DocumentURI, in target: ConfiguredTarget) async -> String? { + guard let language = await self.defaultLanguage(for: document), + let buildSettings = await buildSettings(for: document, in: target, language: language) + else { + return nil + } + + switch language { + case .swift: + // Module name is specified in the form -module-name MyLibrary + guard let moduleNameFlagIndex = buildSettings.compilerArguments.firstIndex(of: "-module-name") else { + return nil + } + return buildSettings.compilerArguments[safe: moduleNameFlagIndex + 1] + case .objective_c: + // Specified in the form -fmodule-name=MyLibrary + guard + let moduleNameArgument = buildSettings.compilerArguments.first(where: { + $0.starts(with: "-fmodule-name=") + }), + let moduleName = moduleNameArgument.split(separator: "=").last + else { + return nil + } + return String(moduleName) + default: + return nil + } + } + /// Returns the build settings for `document` from `buildSystem`. /// /// Implementation detail of `buildSettings(for:language:)`. diff --git a/Sources/SourceKitLSP/TestDiscovery.swift b/Sources/SourceKitLSP/TestDiscovery.swift index dcab15fcf..5cda9c633 100644 --- a/Sources/SourceKitLSP/TestDiscovery.swift +++ b/Sources/SourceKitLSP/TestDiscovery.swift @@ -260,7 +260,7 @@ extension SourceKitLSPServer { func workspaceTests(_ req: WorkspaceTestsRequest) async throws -> [TestItem] { return await self.workspaces - .concurrentMap { await self.tests(in: $0) } + .concurrentMap { await self.tests(in: $0).prefixTestsWithModuleName(workspace: $0) } .flatMap { $0 } .sorted { $0.testItem.location < $1.testItem.location } .mergingTestsInExtensions() @@ -272,6 +272,7 @@ extension SourceKitLSPServer { languageService: LanguageService ) async throws -> [TestItem] { return try await documentTestsWithoutMergingExtensions(req, workspace: workspace, languageService: languageService) + .prefixTestsWithModuleName(workspace: workspace) .mergingTestsInExtensions() } @@ -476,6 +477,30 @@ fileprivate extension Array { } return result } + + func prefixTestsWithModuleName(workspace: Workspace) async -> Self { + return await self.asyncMap({ + return AnnotatedTestItem( + testItem: await $0.testItem.prefixIDWithModuleName(workspace: workspace), + isExtension: $0.isExtension + ) + }) + } +} + +extension TestItem { + fileprivate func prefixIDWithModuleName(workspace: Workspace) async -> TestItem { + guard let configuredTarget = await workspace.buildSystemManager.canonicalConfiguredTarget(for: self.location.uri), + let moduleName = await workspace.buildSystemManager.moduleName(for: self.location.uri, in: configuredTarget) + else { + return self + } + + var newTest = self + newTest.id = "\(moduleName).\(newTest.id)" + newTest.children = await newTest.children.asyncMap({ await $0.prefixIDWithModuleName(workspace: workspace) }) + return newTest + } } extension SwiftLanguageService { diff --git a/Sources/SwiftExtensions/Array+Safe.swift b/Sources/SwiftExtensions/Array+Safe.swift new file mode 100644 index 000000000..09435e86f --- /dev/null +++ b/Sources/SwiftExtensions/Array+Safe.swift @@ -0,0 +1,19 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +extension Array { + /// Returns the element at the specified index if it is within the Array's + /// bounds, otherwise `nil`. + public subscript(safe index: Index) -> Element? { + return index >= 0 && index < count ? self[index] : nil + } +} diff --git a/Sources/SwiftExtensions/CMakeLists.txt b/Sources/SwiftExtensions/CMakeLists.txt index 7b024d4d1..f459a0b16 100644 --- a/Sources/SwiftExtensions/CMakeLists.txt +++ b/Sources/SwiftExtensions/CMakeLists.txt @@ -1,5 +1,6 @@ add_library(SwiftExtensions STATIC + Array+Safe.swift AsyncQueue.swift AsyncUtils.swift Collection+Only.swift diff --git a/Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift b/Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift index 21a0fe2bd..37603c9b1 100644 --- a/Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift +++ b/Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift @@ -53,23 +53,16 @@ final class DocumentTestDiscoveryTests: XCTestCase { tests, [ TestItem( - id: "MyTests", + id: "MyLibraryTests.MyTests", label: "MyTests", - disabled: false, - style: TestStyle.xcTest, location: Location(uri: uri, range: positions["1️⃣"]..