Don’t include files from package dependencies in the syntactic test index

ahoppen · ahoppen · commit abf31a86ed58 · 2024-04-23T22:17:44.000-07:00
`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
diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift b/Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift
@@ -240,7 +240,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)
@@ -254,7 +254,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)
@@ -388,8 +388,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
@@ -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
@@ -0,0 +1,96 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+/// 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 {
+    struct CannotFindToolError: Error {}
+    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 CannotFindToolError()
+    }
+    let process = TSCBasic.Process(
+      arguments: [toolUrl.path] + arguments,
+      workingDirectory: try AbsolutePath(validating: workingDirectory.path)
+    )
+    try process.launch()
+    try await process.waitUntilExit()
+  }
+
+  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", ["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
@@ -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
@@ -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, [])
+  }
 }
