Share module cache between test projects

This improves serial test execution time of eg. `DocumentTestDiscoveryTests` from 36s to 22s because we don’t need to re-build the XCTest module from its interface when using an open source toolchain.

This also uncovered that we weren‘t passing the build setup flags to the prepare command.

rdar://126493151

Author: ahoppen

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -574,6 +574,30 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       "--disable-index-store",
       "--target", target.targetID,
     ]
+    if self.toolsBuildParameters.configuration != self.destinationBuildParameters.configuration {
+      logger.fault(
+        """
+        Preparation is assuming that tools and destination are built using the same configuration, \
+        got tools: \(String(describing: self.toolsBuildParameters.configuration), privacy: .public), \
+        destination: \(String(describing: self.destinationBuildParameters.configuration), privacy: .public)
+        """
+      )
+    }
+    arguments += ["-c", self.destinationBuildParameters.configuration.rawValue]
+    if self.toolsBuildParameters.flags != self.destinationBuildParameters.flags {
+      logger.fault(
+        """
+        Preparation is assuming that tools and destination are built using the same build flags, \
+        got tools: \(String(describing: self.toolsBuildParameters.flags)), \
+        destination: \(String(describing: self.destinationBuildParameters.configuration))
+        """
+      )
+    }
+    arguments += self.destinationBuildParameters.flags.cCompilerFlags.flatMap { ["-Xcc", $0] }
+    arguments += self.destinationBuildParameters.flags.cxxCompilerFlags.flatMap { ["-Xcxx", $0] }
+    arguments += self.destinationBuildParameters.flags.swiftCompilerFlags.flatMap { ["-Xswiftc", $0] }
+    arguments += self.destinationBuildParameters.flags.linkerFlags.flatMap { ["-Xlinker", $0] }
+    arguments += self.destinationBuildParameters.flags.xcbuildFlags?.flatMap { ["-Xxcbuild", $0] } ?? []
     if await swiftBuildSupportsPrepareForIndexing {
       arguments.append("--experimental-prepare-for-indexing")
     }

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -61,6 +61,11 @@ public struct IndexedSingleSwiftFileTestProject {
       "-index-store-path", indexURL.path,
       "-typecheck",
     ]
+    if let globalModuleCache {
+      compilerArguments += [
+        "-module-cache-path", globalModuleCache.path,
+      ]
+    }
     if !indexSystemModules {
       compilerArguments.append("-index-ignore-system-modules")
     }

Sources/SKTestSupport/SwiftPMTestProject.swift

@@ -209,14 +209,19 @@ public class SwiftPMTestProject: MultiFileTestProject {
     guard let swift = await ToolchainRegistry.forTesting.default?.swift?.asURL else {
       throw Error.swiftNotFound
     }
-    let arguments = [
+    var arguments = [
       swift.path,
       "build",
       "--package-path", path.path,
       "--build-tests",
       "-Xswiftc", "-index-ignore-system-modules",
       "-Xcc", "-index-ignore-system-symbols",
     ]
+    if let globalModuleCache {
+      arguments += [
+        "-Xswiftc", "-module-cache-path", "-Xswiftc", globalModuleCache.path,
+      ]
+    }
     var environment = ProcessEnv.block
     // FIXME: SwiftPM does not index-while-building on non-Darwin platforms for C-family files (rdar://117744039).
     // Force-enable index-while-building with the environment variable.

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -44,12 +44,6 @@ public final class TestSourceKitLSPClient: MessageHandler {
   /// `nonisolated(unsafe)` is fine because `nextRequestID` is atomic.
   private nonisolated(unsafe) var nextRequestID = AtomicUInt32(initialValue: 0)
 
-  /// If the server is not using the global module cache, the path of the local
-  /// module cache.
-  ///
-  /// This module cache will be deleted when the test server is destroyed.
-  private let moduleCache: URL?
-
   /// The server that handles the requests.
   public let server: SourceKitLSPServer
 
@@ -102,7 +96,6 @@ public final class TestSourceKitLSPClient: MessageHandler {
   ///     needed.
   public init(
     serverOptions: SourceKitLSPServer.Options = .testDefault,
-    useGlobalModuleCache: Bool = true,
     initialize: Bool = true,
     initializationOptions: LSPAny? = nil,
     capabilities: ClientCapabilities = ClientCapabilities(),
@@ -112,14 +105,9 @@ public final class TestSourceKitLSPClient: MessageHandler {
     preInitialization: ((TestSourceKitLSPClient) -> Void)? = nil,
     cleanUp: @Sendable @escaping () -> Void = {}
   ) async throws {
-    if !useGlobalModuleCache {
-      moduleCache = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
-    } else {
-      moduleCache = nil
-    }
     var serverOptions = serverOptions
-    if let moduleCache {
-      serverOptions.buildSetup.flags.swiftCompilerFlags += ["-module-cache-path", moduleCache.path]
+    if let globalModuleCache {
+      serverOptions.buildSetup.flags.swiftCompilerFlags += ["-module-cache-path", globalModuleCache.path]
     }
     if enableBackgroundIndexing {
       serverOptions.experimentalFeatures.append(.backgroundIndexing)
@@ -191,9 +179,6 @@ public final class TestSourceKitLSPClient: MessageHandler {
     sema.wait()
     self.send(ExitNotification())
 
-    if let moduleCache {
-      try? FileManager.default.removeItem(at: moduleCache)
-    }
     cleanUp()
   }
 

Sources/SKTestSupport/Utils.swift

@@ -112,3 +112,15 @@ fileprivate extension URL {
     #endif
   }
 }
+
+var globalModuleCache: URL? = {
+  if let customModuleCache = ProcessInfo.processInfo.environment["SOURCEKIT_LSP_TEST_MODULE_CACHE"] {
+    if customModuleCache.isEmpty {
+      return nil
+    }
+    return URL(fileURLWithPath: customModuleCache)
+  }
+  return FileManager.default.temporaryDirectory.realpath
+    .appendingPathComponent("sourcekit-lsp-test-scratch")
+    .appendingPathComponent("shared-module-cache")
+}()

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -928,4 +928,53 @@ final class BackgroundIndexingTests: XCTestCase {
       satisfying: { $0.message.contains("Preparing") }
     )
   }
+
+  func testUseBuildFlagsDuringPreparation() async throws {
+    var serverOptions = SourceKitLSPServer.Options.testDefault
+    serverOptions.buildSetup.flags.swiftCompilerFlags += ["-D", "MY_FLAG"]
+    let project = try await SwiftPMTestProject(
+      files: [
+        "Lib/Lib.swift": """
+        #if MY_FLAG
+        public func foo() -> Int { 1 }
+        #endif
+        """,
+        "Client/Client.swift": """
+        import Lib
+
+        func test() -> String {
+          return foo()
+        }
+        """,
+      ],
+      manifest: """
+        let package = Package(
+          name: "MyLibrary",
+          targets: [
+            .target(name: "Lib"),
+            .target(name: "Client", dependencies: ["Lib"]),
+          ]
+        )
+        """,
+      serverOptions: serverOptions,
+      enableBackgroundIndexing: true
+    )
+
+    // Check that we get an error about the return type of `foo` (`Int`) not being convertible to the return type of
+    // `test` (`String`), which indicates that `Lib` had `foo` and was thus compiled with `-D MY_FLAG`
+    let (uri, _) = try project.openDocument("Client.swift")
+    let diagnostics = try await project.testClient.send(
+      DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri))
+    )
+    guard case .full(let diagnostics) = diagnostics else {
+      XCTFail("Expected full diagnostics report")
+      return
+    }
+    XCTAssert(
+      diagnostics.items.contains(where: {
+        $0.message == "Cannot convert return expression of type 'Int' to return type 'String'"
+      }),
+      "Did not get expected diagnostic: \(diagnostics)"
+    )
+  }
 }

Tests/SourceKitLSPTests/SwiftInterfaceTests.swift

@@ -22,12 +22,7 @@ import XCTest
 
 final class SwiftInterfaceTests: XCTestCase {
   func testSystemModuleInterface() async throws {
-    // This is the only test that references modules from the SDK (Foundation).
-    // `testSystemModuleInterface` has been flaky for a long while and a
-    // hypothesis is that it was failing because of a malformed global module
-    // cache that might still be present from previous CI runs. If we use a
-    // local module cache, we define away that source of bugs.
-    let testClient = try await TestSourceKitLSPClient(useGlobalModuleCache: false)
+    let testClient = try await TestSourceKitLSPClient()
     let url = URL(fileURLWithPath: "/\(UUID())/a.swift")
     let uri = DocumentURI(url)
 

Utilities/build-script-helper.py

@@ -7,6 +7,7 @@
 import shutil
 import subprocess
 import sys
+import tempfile
 from typing import Dict, List
 
 
@@ -228,15 +229,17 @@ def run_tests(swift_exec: str, args: argparse.Namespace) -> None:
         '--test-product', 'SourceKitLSPPackageTests'
     ] + swiftpm_args
 
-    # Try running tests in parallel. If that fails, run tests in serial to get capture more readable output.
-    try:
-        check_call(cmd + ['--parallel'], additional_env=additional_env, verbose=args.verbose)
-    except:
-        print('--- Running tests in parallel failed. Re-running tests serially to capture more actionable output.')
-        sys.stdout.flush()
-        check_call(cmd, additional_env=additional_env, verbose=args.verbose)
-        # Return with non-zero exit code even if serial test execution succeeds.
-        raise SystemExit(1)
+    with tempfile.TemporaryDirectory() as test_module_cache:
+        additional_env['SOURCEKIT_LSP_TEST_MODULE_CACHE'] = f"{test_module_cache}/module-cache"
+        # Try running tests in parallel. If that fails, run tests in serial to get capture more readable output.
+        try:
+            check_call(cmd + ['--parallel'], additional_env=additional_env, verbose=args.verbose)
+        except:
+            print('--- Running tests in parallel failed. Re-running tests serially to capture more actionable output.')
+            sys.stdout.flush()
+            check_call(cmd, additional_env=additional_env, verbose=args.verbose)
+            # Return with non-zero exit code even if serial test execution succeeds.
+            raise SystemExit(1)
 
 
 def install_binary(exe: str, source_dir: str, install_dir: str, verbose: bool) -> None: