Plugin: Stop building with -enable-testing even with release config

Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift

@@ -29,11 +29,27 @@ let package = Package(
                 .target(name: "plugintool")
             ]
         ),
+        .plugin(
+            name: "check-testability",
+            capability: .command(intent: .custom(
+                verb: "check-testability",
+                description: "Check testability of a target"
+            ))
+        ),
         .executableTarget(
             name: "placeholder"
         ),
         .executableTarget(
             name: "plugintool"
         ),
+        .target(
+            name: "InternalModule"
+        ),
+        .testTarget(
+            name: "InternalModuleTests",
+            dependencies: [
+                .target(name: "InternalModule")
+            ]
+        ),
     ]
 )

Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/check-testability/main.swift

@@ -0,0 +1,45 @@
+import Foundation
+import PackagePlugin
+
+@main
+struct CheckTestability: CommandPlugin {
+    // This is a helper for testing target builds to ensure that they are testable.
+    func performCommand(context: PluginContext, arguments: [String]) async throws {
+        // Parse the arguments: <targetName> <config> <shouldTestable>
+        guard arguments.count == 3 else {
+            fatalError("Usage: <targetName> <config> <shouldTestable>")
+        }
+        let rawSubsetName = arguments[0]
+        var subset: PackageManager.BuildSubset
+        switch rawSubsetName {
+        // Special subset names
+        case "all-with-tests":
+            subset = .all(includingTests: true)
+        // By default, treat the subset as a target name
+        default:
+            subset = .target(rawSubsetName)
+        }
+        guard let config = PackageManager.BuildConfiguration(rawValue: arguments[1]) else {
+            fatalError("Invalid configuration: \(arguments[1])")
+        }
+        let shouldTestable = arguments[2] == "true"
+
+        var parameters = PackageManager.BuildParameters()
+        parameters.configuration = config
+        parameters.logging = .verbose
+
+        // Perform the build
+        let result = try packageManager.build(subset, parameters: parameters)
+
+        // Check if the build was successful
+        guard result.succeeded else {
+            fatalError("Build failed: \(result.logText)")
+        }
+
+        // Check if the build log contains "-enable-testing" flag
+        let isTestable = result.logText.contains("-enable-testing")
+        if isTestable != shouldTestable {
+            fatalError("Testability mismatch: expected \(shouldTestable), but got \(isTestable):\n\(result.logText)")
+        }
+    }
+}

Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Sources/InternalModule/InternalModule.swift

@@ -0,0 +1,3 @@
+internal func internalFunction() {
+    print("Internal function")
+}

Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Tests/InternalModuleTests/InternalModuleTests.swift

@@ -0,0 +1 @@
+@testable import InternalModule

Sources/Build/BuildDescription/ProductBuildDescription.swift

@@ -205,7 +205,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
             return []
         case .test:
             // Test products are bundle when using Objective-C, executable when using test entry point.
-            switch self.buildParameters.testingParameters.testProductStyle {
+            switch self.buildParameters.testProductStyle {
             case .loadableBundle:
                 args += ["-Xlinker", "-bundle"]
             case .entryPointExecutable:

Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift

@@ -955,7 +955,7 @@ public final class SwiftModuleBuildDescription {
             // test targets must be built with -enable-testing
             // since its required for test discovery (the non objective-c reflection kind)
             return ["-enable-testing"]
-        } else if self.buildParameters.testingParameters.enableTestability {
+        } else if self.buildParameters.enableTestability {
             return ["-enable-testing"]
         } else {
             return []

Sources/Build/BuildPlan/BuildPlan+Test.swift

@@ -43,7 +43,7 @@ extension BuildPlan {
     ) throws -> [(product: ResolvedProduct, discoveryTargetBuildDescription: SwiftModuleBuildDescription?, entryPointTargetBuildDescription: SwiftModuleBuildDescription)] {
         var explicitlyEnabledDiscovery = false
         var explicitlySpecifiedPath: AbsolutePath?
-        if case let .entryPointExecutable(caseExplicitlyEnabledDiscovery, caseExplicitlySpecifiedPath) = destinationBuildParameters.testingParameters.testProductStyle {
+        if case let .entryPointExecutable(caseExplicitlyEnabledDiscovery, caseExplicitlySpecifiedPath) = destinationBuildParameters.testProductStyle {
             explicitlyEnabledDiscovery = caseExplicitlyEnabledDiscovery
             explicitlySpecifiedPath = caseExplicitlySpecifiedPath
         }

Sources/Build/LLBuildCommands.swift

@@ -103,7 +103,7 @@ final class TestDiscoveryCommand: CustomLLBuildCommand, TestBuildCommand {
     private func execute(fileSystem: Basics.FileSystem, tool: TestDiscoveryTool) throws {
         let outputs = tool.outputs.compactMap { try? AbsolutePath(validating: $0.name) }
 
-        if case .loadableBundle = context.productsBuildParameters.testingParameters.testProductStyle {
+        if case .loadableBundle = context.productsBuildParameters.testProductStyle {
             // When building an XCTest bundle, test discovery is handled by the
             // test harness process (i.e. this is the Darwin path.)
             for file in outputs {
@@ -222,7 +222,7 @@ final class TestEntryPointCommand: CustomLLBuildCommand, TestBuildCommand {
             testObservabilitySetup = ""
         }
 
-        let isXCTMainAvailable: String = switch buildParameters.testingParameters.testProductStyle {
+        let isXCTMainAvailable: String = switch buildParameters.testProductStyle {
         case .entryPointExecutable:
             "canImport(XCTest)"
         case .loadableBundle:

Sources/Commands/Utilities/PluginDelegate.swift

@@ -147,6 +147,10 @@ final class PluginDelegate: PluginInvocationDelegate {
         switch subset {
         case .all(let includingTests):
             buildSubset = includingTests ? .allIncludingTests : .allExcludingTests
+            if includingTests {
+                // Enable testability if we're building tests explicitly.
+                buildParameters.testingParameters.explicitlyEnabledTestability = true
+            }
         case .product(let name):
             buildSubset = .product(name)
             explicitProduct = name
@@ -227,7 +231,7 @@ final class PluginDelegate: PluginInvocationDelegate {
         // which ones they are until we've built them and can examine the binaries.
         let toolchain = try swiftCommandState.getHostToolchain()
         var toolsBuildParameters = try swiftCommandState.toolsBuildParameters
-        toolsBuildParameters.testingParameters.enableTestability = true
+        toolsBuildParameters.testingParameters.explicitlyEnabledTestability = true
         toolsBuildParameters.testingParameters.enableCodeCoverage = parameters.enableCodeCoverage
         let buildSystem = try await swiftCommandState.createBuildSystem(
             traitConfiguration: .init(),

Sources/Commands/Utilities/TestingSupport.swift

@@ -250,27 +250,10 @@ extension SwiftCommandState {
         experimentalTestOutput: Bool
     ) -> BuildParameters {
         var parameters = parameters
-
-        var explicitlyEnabledDiscovery = false
-        var explicitlySpecifiedPath: AbsolutePath?
-        if case let .entryPointExecutable(
-            explicitlyEnabledDiscoveryValue,
-            explicitlySpecifiedPathValue
-        ) = parameters.testingParameters.testProductStyle {
-            explicitlyEnabledDiscovery = explicitlyEnabledDiscoveryValue
-            explicitlySpecifiedPath = explicitlySpecifiedPathValue
-        }
-        parameters.testingParameters = .init(
-            configuration: parameters.configuration,
-            targetTriple: parameters.triple,
-            forceTestDiscovery: explicitlyEnabledDiscovery,
-            testEntryPointPath: explicitlySpecifiedPath
-        )
-
         parameters.testingParameters.enableCodeCoverage = enableCodeCoverage
         // for test commands, we normally enable building with testability
         // but we let users override this with a flag
-        parameters.testingParameters.enableTestability = enableTestability ?? true
+        parameters.testingParameters.explicitlyEnabledTestability = enableTestability ?? true
         parameters.shouldSkipBuilding = shouldSkipBuilding
         parameters.testingParameters.experimentalTestOutput = experimentalTestOutput
         return parameters

Sources/CoreCommands/BuildSystemSupport.swift

@@ -36,7 +36,7 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
         observabilityScope: ObservabilityScope?
     ) async throws -> any BuildSystem {
         _ = try await swiftCommandState.getRootPackageInformation()
-        let testEntryPointPath = productsBuildParameters?.testingParameters.testProductStyle.explicitlySpecifiedEntryPointPath
+        let testEntryPointPath = productsBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
         return try BuildOperation(
             productsBuildParameters: try productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
             toolsBuildParameters: try toolsBuildParameters ?? self.swiftCommandState.toolsBuildParameters,

Sources/CoreCommands/SwiftCommandState.swift

@@ -826,8 +826,6 @@ public final class SwiftCommandState {
                 isVerbose: self.logLevel <= .info
             ),
             testingParameters: .init(
-                configuration: options.build.configuration ?? self.preferredBuildConfiguration,
-                targetTriple: triple,
                 forceTestDiscovery: options.build.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery
                 testEntryPointPath: options.build.testEntryPointPath
             )

Sources/SPMBuildCore/BuildParameters/BuildParameters+Testing.swift

@@ -77,18 +77,20 @@ extension BuildParameters {
         /// Whether to enable code coverage.
         public var enableCodeCoverage: Bool
 
-        /// Whether building for testability is enabled.
-        public var enableTestability: Bool
+        /// Whether building for testability is explicitly enabled or disabled.
+        package var explicitlyEnabledTestability: Bool?
 
         /// Whether or not to enable the experimental test output mode.
         public var experimentalTestOutput: Bool
 
-        /// The style of test product to produce.
-        public var testProductStyle: TestProductStyle
+        /// Whether to force test discovery.
+        fileprivate var explicitlyEnabledDiscovery: Bool
+
+        /// The path to the test entry point file, if one was specified explicitly
+        /// via `--experimental-test-entry-point-path <file>`.
+        fileprivate var explicitlySpecifiedPath: AbsolutePath?
 
         public init(
-            configuration: BuildConfiguration,
-            targetTriple: Triple,
             enableCodeCoverage: Bool = false,
             enableTestability: Bool? = nil,
             experimentalTestOutput: Bool = false,
@@ -97,18 +99,29 @@ extension BuildParameters {
         ) {
             self.enableCodeCoverage = enableCodeCoverage
             self.experimentalTestOutput = experimentalTestOutput
-            // decide on testability based on debug/release config
-            // the goals of this being based on the build configuration is
-            // that `swift build` followed by a `swift test` will need to do minimal rebuilding
-            // given that the default configuration for `swift build` is debug
-            // and that `swift test` normally requires building with testable enabled.
-            // when building and testing in release mode, one can use the '--disable-testable-imports' flag
-            // to disable testability in `swift test`, but that requires that the tests do not use the testable imports feature
-            self.enableTestability =  enableTestability ?? (.debug == configuration)
-            self.testProductStyle = targetTriple.isDarwin() ? .loadableBundle : .entryPointExecutable(
-                explicitlyEnabledDiscovery: forceTestDiscovery,
-                explicitlySpecifiedPath: testEntryPointPath
-            )
+            self.explicitlyEnabledTestability = enableTestability
+            self.explicitlyEnabledDiscovery = forceTestDiscovery
+            self.explicitlySpecifiedPath = testEntryPointPath
         }
     }
+
+    /// Whether building for testability is enabled.
+    public var enableTestability: Bool {
+        // decide on testability based on debug/release config
+        // the goals of this being based on the build configuration is
+        // that `swift build` followed by a `swift test` will need to do minimal rebuilding
+        // given that the default configuration for `swift build` is debug
+        // and that `swift test` normally requires building with testable enabled.
+        // when building and testing in release mode, one can use the '--disable-testable-imports' flag
+        // to disable testability in `swift test`, but that requires that the tests do not use the testable imports feature
+        self.testingParameters.explicitlyEnabledTestability ?? (self.configuration == .debug)
+    }
+
+    /// The style of test product to produce.
+    public var testProductStyle: TestProductStyle {
+        return triple.isDarwin() ? .loadableBundle : .entryPointExecutable(
+            explicitlyEnabledDiscovery: testingParameters.explicitlyEnabledDiscovery,
+            explicitlySpecifiedPath: testingParameters.explicitlySpecifiedPath
+        )
+    }
 }

Sources/SPMBuildCore/BuildParameters/BuildParameters.swift

@@ -163,7 +163,7 @@ public struct BuildParameters: Encodable {
         driverParameters: Driver = .init(),
         linkingParameters: Linking = .init(),
         outputParameters: Output = .init(),
-        testingParameters: Testing? = nil
+        testingParameters: Testing = .init()
     ) throws {
         let triple = try triple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompilerPath)
         self.debuggingParameters = debuggingParameters ?? .init(
@@ -218,7 +218,7 @@ public struct BuildParameters: Encodable {
         self.driverParameters = driverParameters
         self.linkingParameters = linkingParameters
         self.outputParameters = outputParameters
-        self.testingParameters = testingParameters ?? .init(configuration: configuration, targetTriple: triple)
+        self.testingParameters = testingParameters
     }
 
     /// The path to the build directory (inside the data directory).

Sources/XCBuildSupport/XcodeBuildSystem.swift

@@ -371,7 +371,7 @@ extension PIFBuilderParameters {
         self.init(
             triple: buildParameters.triple,
             isPackageAccessModifierSupported: buildParameters.driverParameters.isPackageAccessModifierSupported,
-            enableTestability: buildParameters.testingParameters.enableTestability,
+            enableTestability: buildParameters.enableTestability,
             shouldCreateDylibForDynamicProducts: buildParameters.shouldCreateDylibForDynamicProducts,
             toolchainLibDir: (try? buildParameters.toolchain.toolchainLibDir) ?? .root,
             pkgConfigDirectories: buildParameters.pkgConfigDirectories,

Tests/CommandsTests/PackageCommandTests.swift

@@ -2448,6 +2448,35 @@ final class PackageCommandTests: CommandsTestCase {
         }
     }
 
+    func testCommandPluginBuildTestability() async throws {
+        // Plugin arguments: check-testability <targetName> <config> <shouldTestable>
+
+        // Overall configuration: debug, plugin build request: debug -> without testability
+        try await fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in
+            try await XCTAssertAsyncNoThrow(try await SwiftPM.Package.execute(["-c", "debug", "check-testability", "InternalModule", "debug", "true"], packagePath: fixturePath))
+        }
+
+        // Overall configuration: debug, plugin build request: release -> without testability
+        try await fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in
+            try await XCTAssertAsyncNoThrow(try await SwiftPM.Package.execute(["-c", "debug", "check-testability", "InternalModule", "release", "false"], packagePath: fixturePath))
+        }
+
+        // Overall configuration: release, plugin build request: debug -> with testability
+        try await fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in
+            try await XCTAssertAsyncNoThrow(try await SwiftPM.Package.execute(["-c", "release", "check-testability", "InternalModule", "debug", "true"], packagePath: fixturePath))
+        }
+
+        // Overall configuration: release, plugin build request: release -> with testability
+        try await fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in
+            try await XCTAssertAsyncNoThrow(try await SwiftPM.Package.execute(["-c", "release", "check-testability", "InternalModule", "release", "false"], packagePath: fixturePath))
+        }
+
+        // Overall configuration: release, plugin build request: release including tests -> with testability
+        try await fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in
+            try await XCTAssertAsyncNoThrow(try await SwiftPM.Package.execute(["-c", "release", "check-testability", "all-with-tests", "release", "true"], packagePath: fixturePath))
+        }
+    }
+
     // Test logging of builds initiated by a command plugin
     func testCommandPluginBuildLogs() async throws {
         // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require).

Tests/CommandsTests/TestCommandTests.swift

@@ -85,6 +85,15 @@ final class TestCommandTests: CommandsTestCase {
         }
     }
 
+    func testWithReleaseConfiguration() async throws {
+        try await fixture(name: "Miscellaneous/TestableExe") { fixturePath in
+            do {
+                let result = try await execute(["-c", "release", "--vv"], packagePath: fixturePath)
+                XCTAssertMatch(result.stderr, .contains("-enable-testing"))
+            }
+        }
+    }
+
     func testSwiftTestParallel() async throws {
         try await fixture(name: "Miscellaneous/ParallelTestsPkg") { fixturePath in
             // First try normal serial testing.

Tests/SPMBuildCoreTests/BuildParametersTests.swift

@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+@testable import SPMBuildCore
+import Basics
+import struct PackageModel.BuildEnvironment
+import _InternalTestSupport
+import XCTest
+
+final class BuildParametersTests: XCTestCase {
+    func testConfigurationDependentProperties() throws {
+        // Ensure that properties that depend on the "configuration" property are
+        // correctly updated after modifying the configuration.
+        var parameters = mockBuildParameters(
+            destination: .host,
+            environment: BuildEnvironment(platform: .linux, configuration: .debug)
+        )
+        XCTAssertEqual(parameters.enableTestability, true)
+        parameters.configuration = .release
+        XCTAssertEqual(parameters.enableTestability, false)
+    }
+}