From 9b2753149aac6b5cf165d185221e75dbd0fa09c0 Mon Sep 17 00:00:00 2001 From: Michael Rawdon Date: Thu, 21 Aug 2025 14:35:58 -0700 Subject: [PATCH] Add a facility for tests to opt in to loading the downloadable Metal toolchain This will greatly reduce the number of tests which fail when there is a problem with the downloadable Metal toolchain. This is off by default, since very few tests need this. Two tests currently opt in. --- Sources/SWBTestSupport/CoreBasedTests.swift | 6 +- Sources/SWBTestSupport/CoreTestSupport.swift | 66 +++++++++++-------- .../BuildCommandTests.swift | 2 +- .../BuildOperationTests.swift | 2 +- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/Sources/SWBTestSupport/CoreBasedTests.swift b/Sources/SWBTestSupport/CoreBasedTests.swift index 1ac926fd..3a175a85 100644 --- a/Sources/SWBTestSupport/CoreBasedTests.swift +++ b/Sources/SWBTestSupport/CoreBasedTests.swift @@ -23,10 +23,11 @@ package protocol CoreBasedTests { } extension CoreBasedTests { - package static func makeCore(skipLoadingPluginsNamed: Set = [], registerExtraPlugins: @PluginExtensionSystemActor (PluginManager) -> Void = { _ in }, simulatedInferiorProductsPath: Path? = nil, environment: [String: String] = [:], _ delegate: TestingCoreDelegate? = nil, sourceLocation: SourceLocation = #_sourceLocation) async throws -> Core { + /// This will create a customized `Core` object using the specified parameters, providing a test with detailed control over the contents of the `Core` it uses. + package static func makeCore(skipLoadingPluginsNamed: Set = [], registerExtraPlugins: @PluginExtensionSystemActor (PluginManager) -> Void = { _ in }, simulatedInferiorProductsPath: Path? = nil, environment: [String: String] = [:], _ delegate: TestingCoreDelegate? = nil, configurationDelegate: TestingCoreConfigurationDelegate? = nil, sourceLocation: SourceLocation = #_sourceLocation) async throws -> Core { let core: Result do { - let theCore = try await Core.createInitializedTestingCore(skipLoadingPluginsNamed: skipLoadingPluginsNamed, registerExtraPlugins: registerExtraPlugins, simulatedInferiorProductsPath: simulatedInferiorProductsPath, environment: environment, delegate: delegate) + let theCore = try await Core.createInitializedTestingCore(skipLoadingPluginsNamed: skipLoadingPluginsNamed, registerExtraPlugins: registerExtraPlugins, simulatedInferiorProductsPath: simulatedInferiorProductsPath, environment: environment, delegate: delegate, configurationDelegate: configurationDelegate) core = .success(theCore) } catch { core = .failure(error) @@ -48,6 +49,7 @@ extension CoreBasedTests { return try Self._getCore(core: core, sourceLocation: sourceLocation) } + /// This private method caches the `Core` object created in `GetCore(sourceLocation:)`, and returns the same object if queried again. private static func _getCore(core: Result, sourceLocation: SourceLocation = #_sourceLocation) throws -> Core { do { return try core.get() diff --git a/Sources/SWBTestSupport/CoreTestSupport.swift b/Sources/SWBTestSupport/CoreTestSupport.swift index 5ce4bedf..a424a3d6 100644 --- a/Sources/SWBTestSupport/CoreTestSupport.swift +++ b/Sources/SWBTestSupport/CoreTestSupport.swift @@ -47,7 +47,7 @@ extension Core { /// Get an initialized Core suitable for testing. /// /// This function requires there to be no errors during loading the core. - package static func createInitializedTestingCore(skipLoadingPluginsNamed: Set, registerExtraPlugins: @PluginExtensionSystemActor (PluginManager) -> Void, simulatedInferiorProductsPath: Path? = nil, environment: [String:String] = [:], delegate: TestingCoreDelegate? = nil) async throws -> Core { + package static func createInitializedTestingCore(skipLoadingPluginsNamed: Set, registerExtraPlugins: @PluginExtensionSystemActor (PluginManager) -> Void, simulatedInferiorProductsPath: Path? = nil, environment: [String:String] = [:], delegate: TestingCoreDelegate? = nil, configurationDelegate: TestingCoreConfigurationDelegate? = nil) async throws -> Core { // When this code is being loaded directly via unit tests, find the running Xcode path. // // This is a "well known" launch parameter set in Xcode's schemes. @@ -74,33 +74,35 @@ extension Core { // // If the given environment already contains `EXTERNAL_TOOLCHAINS_DIR` and `TOOLCHAINS`, we're assuming that we do not have to obtain any toolchain information. var environment = environment - if (try? ProcessInfo.processInfo.hostOperatingSystem()) == .macOS, !(environment.contains("EXTERNAL_TOOLCHAINS_DIR") && environment.contains("TOOLCHAINS")) { - let activeDeveloperPath: Path - if let developerPath { - activeDeveloperPath = developerPath.path - } else { - activeDeveloperPath = try await Xcode.getActiveDeveloperDirectoryPath() - } - let defaultToolchainPath = activeDeveloperPath.join("Toolchains/XcodeDefault.xctoolchain") - - if !localFS.exists(defaultToolchainPath.join("usr/metal/current")) { - struct MetalToolchainInfo: Decodable { - let buildVersion: String - let status: String - let toolchainIdentifier: String - let toolchainSearchPath: String + if configurationDelegate?.loadMetalToolchain == true { + if (try? ProcessInfo.processInfo.hostOperatingSystem()) == .macOS, !(environment.contains("EXTERNAL_TOOLCHAINS_DIR") && environment.contains("TOOLCHAINS")) { + let activeDeveloperPath: Path + if let developerPath { + activeDeveloperPath = developerPath.path + } else { + activeDeveloperPath = try await Xcode.getActiveDeveloperDirectoryPath() } - - let result = try await Process.getOutput(url: URL(fileURLWithPath: activeDeveloperPath.join("usr/bin/xcodebuild").str), arguments: ["-showComponent", "metalToolchain", "-json"], environment: ["DEVELOPER_DIR": activeDeveloperPath.str]) - if result.exitStatus != .exit(0) { - throw StubError.error("xcodebuild failed: \(String(data: result.stdout, encoding: .utf8) ?? "")\n\(String(data: result.stderr, encoding: .utf8) ?? "")") + let defaultToolchainPath = activeDeveloperPath.join("Toolchains/XcodeDefault.xctoolchain") + + if !localFS.exists(defaultToolchainPath.join("usr/metal/current")) { + struct MetalToolchainInfo: Decodable { + let buildVersion: String + let status: String + let toolchainIdentifier: String + let toolchainSearchPath: String + } + + let result = try await Process.getOutput(url: URL(fileURLWithPath: activeDeveloperPath.join("usr/bin/xcodebuild").str), arguments: ["-showComponent", "metalToolchain", "-json"], environment: ["DEVELOPER_DIR": activeDeveloperPath.str]) + if result.exitStatus != .exit(0) { + throw StubError.error("xcodebuild failed: \(String(data: result.stdout, encoding: .utf8) ?? "")\n\(String(data: result.stderr, encoding: .utf8) ?? "")") + } + + let metalToolchainInfo = try JSONDecoder().decode(MetalToolchainInfo.self, from: result.stdout) + environment.addContents(of: [ + "TOOLCHAINS": "\(metalToolchainInfo.toolchainIdentifier) $(inherited)", + "EXTERNAL_TOOLCHAINS_DIR": metalToolchainInfo.toolchainSearchPath, + ]) } - - let metalToolchainInfo = try JSONDecoder().decode(MetalToolchainInfo.self, from: result.stdout) - environment.addContents(of: [ - "TOOLCHAINS": "\(metalToolchainInfo.toolchainIdentifier) $(inherited)", - "EXTERNAL_TOOLCHAINS_DIR": metalToolchainInfo.toolchainSearchPath, - ]) } } @@ -246,3 +248,15 @@ package final class TestingCoreDelegate: CoreDelegate, Sendable { return _diagnosticsEngine.diagnostics.pathMessageTuples(.warning) } } + +/// Individual classes may pass an instance of this protocol to `CoreBasedTests.makeCore()` to configure which special elements of the testing core they need. `Core.createInitializedTestingCore()` (above) will configure the core based on what's passed here. +/// +/// This allows tests which don't care about those elements to not fail because of errors trying to load them. +package struct TestingCoreConfigurationDelegate: Sendable { + /// Only tests which are exercising Metal should need to load the Metal toolchain, so only those tests will fail if loading the toolchain fails. + package let loadMetalToolchain: Bool + + package init(loadMetalToolchain: Bool = false) { + self.loadMetalToolchain = loadMetalToolchain + } +} diff --git a/Tests/SWBBuildSystemTests/BuildCommandTests.swift b/Tests/SWBBuildSystemTests/BuildCommandTests.swift index c2aa61af..5651d63a 100644 --- a/Tests/SWBBuildSystemTests/BuildCommandTests.swift +++ b/Tests/SWBBuildSystemTests/BuildCommandTests.swift @@ -325,7 +325,7 @@ fileprivate struct BuildCommandTests: CoreBasedTests { @Test(.requireSDKs(.macOS), .requireXcode16()) func singleFileCompileMetal() async throws { - let core = try await getCore() + let core = try await Self.makeCore(configurationDelegate: TestingCoreConfigurationDelegate(loadMetalToolchain: true)) try await withTemporaryDirectory { tmpDirPath async throws -> Void in let testWorkspace = try await TestWorkspace( "Test", diff --git a/Tests/SWBBuildSystemTests/BuildOperationTests.swift b/Tests/SWBBuildSystemTests/BuildOperationTests.swift index 8bac3097..3798b5bd 100644 --- a/Tests/SWBBuildSystemTests/BuildOperationTests.swift +++ b/Tests/SWBBuildSystemTests/BuildOperationTests.swift @@ -5794,7 +5794,7 @@ That command depends on command in Target 'agg2' (project \'aProject\'): script @Test(.requireSDKs(.macOS)) func incrementalMetalLinkWithCodeSign() async throws { - let core = try await getCore() + let core = try await Self.makeCore(configurationDelegate: TestingCoreConfigurationDelegate(loadMetalToolchain: true)) try await withTemporaryDirectory { tmpDirPath async throws -> Void in let testWorkspace = try await TestWorkspace( "Test",