From 1d9b67b0b962f7a9dd233579184c8ab79acfd3c0 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Tue, 23 Apr 2024 22:17:44 -0700 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20include=20files=20from=20packag?= =?UTF-8?q?e=20dependencies=20in=20the=20syntactic=20test=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SwiftPMBuildSystem.testFiles()` returned all source files in the package, including files of package dependencies. This caused us to index those files for tests in the syntactic test index, which we should not. Make `SwiftPMBuildSystem.testFiles` only return files from the root package. Also add test infrastructure to be able to test cross-package functionality. rdar://126965614 --- .../SwiftPMBuildSystem.swift | 13 ++- .../SKTestSupport/MultiFileTestProject.swift | 17 ++- .../SwiftPMDependencyProject.swift | 108 ++++++++++++++++++ .../SKTestSupport/SwiftPMTestProject.swift | 17 ++- .../WorkspaceTestDiscoveryTests.swift | 41 +++++++ 5 files changed, 185 insertions(+), 11 deletions(-) create mode 100644 Sources/SKTestSupport/SwiftPMDependencyProject.swift diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift index e669f5e68..2e056a92c 100644 --- a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift +++ b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift @@ -263,7 +263,7 @@ extension SwiftPMBuildSystem { self.fileToTarget = [AbsolutePath: SwiftBuildTarget]( modulesGraph.allTargets.flatMap { target in return target.sources.paths.compactMap { - guard let buildTarget = buildDescription.getBuildTarget(for: target) else { + guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else { return nil } return (key: $0, value: buildTarget) @@ -277,7 +277,7 @@ extension SwiftPMBuildSystem { self.sourceDirToTarget = [AbsolutePath: SwiftBuildTarget]( modulesGraph.allTargets.compactMap { (target) -> (AbsolutePath, SwiftBuildTarget)? in - guard let buildTarget = buildDescription.getBuildTarget(for: target) else { + guard let buildTarget = buildDescription.getBuildTarget(for: target, in: modulesGraph) else { return nil } return (key: target.sources.root, value: buildTarget) @@ -439,8 +439,13 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem { } public func testFiles() -> [DocumentURI] { - // We should only include source files from test targets (https://github.com/apple/sourcekit-lsp/issues/1174). - return fileToTarget.map { DocumentURI($0.key.asURL) } + return fileToTarget.compactMap { (path, target) -> DocumentURI? in + guard target.isPartOfRootPackage else { + // Don't consider files from package dependencies as possible test files. + return nil + } + return DocumentURI(path.asURL) + } } public func addTestFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async { diff --git a/Sources/SKTestSupport/MultiFileTestProject.swift b/Sources/SKTestSupport/MultiFileTestProject.swift index 1484c4a64..cd9b8f000 100644 --- a/Sources/SKTestSupport/MultiFileTestProject.swift +++ b/Sources/SKTestSupport/MultiFileTestProject.swift @@ -31,6 +31,15 @@ public struct RelativeFileLocation: Hashable, ExpressibleByStringLiteral { let components = value.components(separatedBy: "/") self.init(directories: components.dropLast(), components.last!) } + + public func url(relativeTo: URL) -> URL { + var url = relativeTo + for directory in directories { + url = url.appendingPathComponent(directory) + } + url = url.appendingPathComponent(fileName) + return url + } } /// A test project that writes multiple files to disk and opens a `TestSourceKitLSPClient` client with a workspace @@ -69,7 +78,7 @@ public class MultiFileTestProject { /// File contents can also contain `$TEST_DIR`, which gets replaced by the temporary directory. public init( files: [RelativeFileLocation: String], - workspaces: (URL) -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, + workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, usePullDiagnostics: Bool = true, testName: String = #function ) async throws { @@ -79,11 +88,7 @@ public class MultiFileTestProject { var fileData: [String: FileData] = [:] for (fileLocation, markedText) in files { let markedText = markedText.replacingOccurrences(of: "$TEST_DIR", with: scratchDirectory.path) - var fileURL = scratchDirectory - for directory in fileLocation.directories { - fileURL = fileURL.appendingPathComponent(directory) - } - fileURL = fileURL.appendingPathComponent(fileLocation.fileName) + let fileURL = fileLocation.url(relativeTo: scratchDirectory) try FileManager.default.createDirectory( at: fileURL.deletingLastPathComponent(), withIntermediateDirectories: true diff --git a/Sources/SKTestSupport/SwiftPMDependencyProject.swift b/Sources/SKTestSupport/SwiftPMDependencyProject.swift new file mode 100644 index 000000000..ba97a512d --- /dev/null +++ b/Sources/SKTestSupport/SwiftPMDependencyProject.swift @@ -0,0 +1,108 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 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 +// +//===----------------------------------------------------------------------===// + +import Foundation +import ISDBTibs +import XCTest + +import struct TSCBasic.AbsolutePath +import class TSCBasic.Process +import enum TSCBasic.ProcessEnv +import struct TSCBasic.ProcessResult + +/// A SwiftPM package that gets written to disk and for which a Git repository is initialized with a commit tagged +/// `1.0.0`. This repository can then be used as a dependency for another package, usually a `SwiftPMTestProject`. +public class SwiftPMDependencyProject { + /// The directory in which the repository lives. + public let packageDirectory: URL + + private func runCommand(_ toolName: String, _ arguments: [String], workingDirectory: URL) async throws { + enum Error: Swift.Error { + case cannotFindTool(toolName: String) + case processedTerminatedWithNonZeroExitCode(ProcessResult) + } + guard let toolUrl = findTool(name: toolName) else { + if ProcessEnv.block["SWIFTCI_USE_LOCAL_DEPS"] == nil { + // Never skip the test in CI, similar to what SkipUnless does. + throw XCTSkip("\(toolName) cannot be found") + } + throw Error.cannotFindTool(toolName: toolName) + } + print([toolUrl.path] + arguments) + let process = TSCBasic.Process( + arguments: [toolUrl.path] + arguments, + workingDirectory: try AbsolutePath(validating: workingDirectory.path) + ) + try process.launch() + let processResult = try await process.waitUntilExit() + guard processResult.exitStatus == .terminated(code: 0) else { + throw Error.processedTerminatedWithNonZeroExitCode(processResult) + } + } + + public static let defaultPackageManifest: String = """ + // swift-tools-version: 5.7 + + import PackageDescription + + let package = Package( + name: "MyDependency", + products: [.library(name: "MyDependency", targets: ["MyDependency"])], + targets: [.target(name: "MyDependency")] + ) + """ + + public init( + files: [RelativeFileLocation: String], + manifest: String = defaultPackageManifest, + testName: String = #function + ) async throws { + packageDirectory = try testScratchDir(testName: testName).appendingPathComponent("MyDependency") + + var files = files + files["Package.swift"] = manifest + + for (fileLocation, contents) in files { + let fileURL = fileLocation.url(relativeTo: packageDirectory) + try FileManager.default.createDirectory( + at: fileURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try contents.write(to: fileURL, atomically: true, encoding: .utf8) + } + + try await runCommand("git", ["init"], workingDirectory: packageDirectory) + try await runCommand( + "git", + ["add"] + files.keys.map { $0.url(relativeTo: packageDirectory).path }, + workingDirectory: packageDirectory + ) + try await runCommand( + "git", + ["-c", "user.name=Dummy", "-c", "user.email=noreply@swift.org", "commit", "-m", "Initial commit"], + workingDirectory: packageDirectory + ) + try await runCommand("git", ["tag", "1.0.0"], workingDirectory: packageDirectory) + } + + deinit { + if cleanScratchDirectories { + try? FileManager.default.removeItem(at: packageDirectory) + } + } + + /// Function that makes sure the project stays alive until this is called. Otherwise, the `SwiftPMDependencyProject` + /// might get deinitialized, which deletes the package on disk. + public func keepAlive() { + withExtendedLifetime(self) { _ in } + } +} diff --git a/Sources/SKTestSupport/SwiftPMTestProject.swift b/Sources/SKTestSupport/SwiftPMTestProject.swift index 958a9d1b2..fa0a9dea7 100644 --- a/Sources/SKTestSupport/SwiftPMTestProject.swift +++ b/Sources/SKTestSupport/SwiftPMTestProject.swift @@ -38,7 +38,7 @@ public class SwiftPMTestProject: MultiFileTestProject { public init( files: [RelativeFileLocation: String], manifest: String = SwiftPMTestProject.defaultPackageManifest, - workspaces: (URL) -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, + workspaces: (URL) async throws -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] }, build: Bool = false, allowBuildFailure: Bool = false, usePullDiagnostics: Bool = true, @@ -59,6 +59,7 @@ public class SwiftPMTestProject: MultiFileTestProject { filesByPath[RelativeFileLocation(directories: directories, fileLocation.fileName)] = contents } filesByPath["Package.swift"] = manifest + try await super.init( files: filesByPath, workspaces: workspaces, @@ -96,4 +97,18 @@ public class SwiftPMTestProject: MultiFileTestProject { environment["SWIFTPM_ENABLE_CLANG_INDEX_STORE"] = "1" try await Process.checkNonZeroExit(arguments: arguments, environmentBlock: environment) } + + /// Resolve package dependencies for the package at `path`. + public static func resolvePackageDependencies(at path: URL) async throws { + guard let swift = await ToolchainRegistry.forTesting.default?.swift?.asURL else { + throw Error.swiftNotFound + } + let arguments = [ + swift.path, + "package", + "resolve", + "--package-path", path.path, + ] + try await Process.checkNonZeroExit(arguments: arguments) + } } diff --git a/Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift b/Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift index 5763dbc21..74742e3c2 100644 --- a/Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift +++ b/Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift @@ -807,4 +807,45 @@ final class WorkspaceTestDiscoveryTests: XCTestCase { let testsAfterFileReAdded = try await project.testClient.send(WorkspaceTestsRequest()) XCTAssertEqual(testsAfterFileReAdded, expectedTests) } + + func testDontIncludeTestsFromDependentPackageInSyntacticIndex() async throws { + let dependencyProject = try await SwiftPMDependencyProject(files: [ + "Sources/MyDependency/MyDependency.swift": """ + class MySuperclass {} + class LooksALittleLikeTests: MySuperclass { + func testSomething() {} + } + """ + ]) + defer { dependencyProject.keepAlive() } + + let project = try await SwiftPMTestProject( + files: [ + "Test.swift": "" + ], + manifest: """ + // swift-tools-version: 5.7 + + import PackageDescription + + let package = Package( + name: "MyLibrary", + dependencies: [.package(url: "\(dependencyProject.packageDirectory)", from: "1.0.0")], + targets: [ + .target( + name: "MyLibrary", + dependencies: [.product(name: "MyDependency", package: "MyDependency")] + ) + ] + ) + """, + workspaces: { scratchDirectory in + try await SwiftPMTestProject.resolvePackageDependencies(at: scratchDirectory) + return [WorkspaceFolder(uri: DocumentURI(scratchDirectory))] + } + ) + + let tests = try await project.testClient.send(WorkspaceTestsRequest()) + XCTAssertEqual(tests, []) + } }