add flag to disable building with testablity when running tests (#4119)

motivation: allow building tests without testability (testable imports), this can increase build / test cycles when tests do not require the testable imports feature since more is cacheable

changes:
* add a --disable-testable-imports flag to "swift test" with which tests are build without the testablity feature
* add tests

rdar://82448144

Fixtures/Miscellaneous/TestableExe/Tests/TestableExeTests/TestableExeTests.swift

@@ -1,6 +1,6 @@
 import XCTest
-import TestableExe1
-import TestableExe2
+@testable import TestableExe1
+@testable import TestableExe2
 // import TestableExe3
 import class Foundation.Bundle
 
@@ -9,7 +9,7 @@ final class TestableExeTests: XCTestCase {
         // This is an example of a functional test case.
         // Use XCTAssert and related functions to verify your tests produce the correct
         // results.
-        
+
         print(GetGreeting1())
         XCTAssertEqual(GetGreeting1(), "Hello, world")
         print(GetGreeting2())

Sources/Build/BuildPlan.swift

@@ -589,7 +589,7 @@ public final class SwiftTargetBuildDescription {
     public let isTestTarget: Bool
 
     /// True if this is the test discovery target.
-    public let testDiscoveryTarget: Bool
+    public let isTestDiscoveryTarget: Bool
 
     /// True if this module needs to be parsed as a library based on the target type and the configuration
     /// of the source code (for example because it has a single source file whose name isn't "main.swift").
@@ -632,7 +632,7 @@ public final class SwiftTargetBuildDescription {
         buildToolPluginInvocationResults: [BuildToolPluginInvocationResult] = [],
         prebuildCommandResults: [PrebuildCommandResult] = [],
         isTestTarget: Bool? = nil,
-        testDiscoveryTarget: Bool = false,
+        isTestDiscoveryTarget: Bool = false,
         fileSystem: FileSystem
     ) throws {
         assert(target.underlyingTarget is SwiftTarget, "underlying target type mismatch \(target)")
@@ -641,7 +641,7 @@ public final class SwiftTargetBuildDescription {
         self.buildParameters = buildParameters
         // Unless mentioned explicitly, use the target type to determine if this is a test target.
         self.isTestTarget = isTestTarget ?? (target.type == .test)
-        self.testDiscoveryTarget = testDiscoveryTarget
+        self.isTestDiscoveryTarget = isTestDiscoveryTarget
         self.fileSystem = fileSystem
         self.tempsPath = buildParameters.buildPath.appending(component: target.c99name + ".build")
         self.derivedSources = Sources(paths: [], root: tempsPath.appending(component: "DerivedSources"))
@@ -1095,7 +1095,11 @@ public final class SwiftTargetBuildDescription {
 
     /// Testing arguments according to the build configuration.
     private var testingArguments: [String] {
-        if buildParameters.enableTestability {
+        if self.isTestTarget {
+            // 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 buildParameters.enableTestability {
             return ["-enable-testing"]
         } else {
             return []
@@ -1564,7 +1568,7 @@ public class BuildPlan {
                     toolsVersion: toolsVersion,
                     buildParameters: buildParameters,
                     isTestTarget: true,
-                    testDiscoveryTarget: true,
+                    isTestDiscoveryTarget: true,
                     fileSystem: fileSystem
                 )
 

Sources/Build/LLBuildManifestBuilder.swift

@@ -823,7 +823,7 @@ extension LLBuildManifestBuilder {
         for target in plan.targets {
             guard case .swift(let target) = target,
                 target.isTestTarget,
-                target.testDiscoveryTarget else { continue }
+                target.isTestDiscoveryTarget else { continue }
 
             let testDiscoveryTarget = target
 

Sources/Commands/SwiftTestTool.swift

@@ -138,6 +138,10 @@ struct TestToolOptions: ParsableArguments {
     @Option(help: "Test the specified product.")
     var testProduct: String?
 
+    /// Generate LinuxMain entries and exit.
+    @Flag(name: .customLong("testable-imports"), inversion: .prefixedEnableDisable, help: "Enable or disable testable imports. Enabled by default.")
+    var enableTestableImports: Bool = true
+
     /// Returns the test case specifier if overridden in the env.
     private func testCaseSkipOverride() -> TestCaseSpecifier? {
         guard let override = ProcessEnv.vars["_SWIFTPM_SKIP_TESTS_LIST"] else {
@@ -238,7 +242,7 @@ public struct SwiftTestTool: SwiftCommand {
             guard let rootManifest = rootManifests.values.first else {
                 throw StringError("invalid manifests at \(root.packages)")
             }
-            let buildParameters = try swiftTool.buildParametersForTest()
+            let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
             print(codeCovAsJSONPath(buildParameters: buildParameters, packageName: rootManifest.displayName))
 
         case .generateLinuxMain:
@@ -259,7 +263,7 @@ public struct SwiftTestTool: SwiftCommand {
         case .runSerial:
             let toolchain = try swiftTool.getToolchain()
             let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
-            let buildParameters = try swiftTool.buildParametersForTest()
+            let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
 
             // Clean out the code coverage directory that may contain stale
             // profraw files from a previous run of the code coverage tool.
@@ -327,7 +331,7 @@ public struct SwiftTestTool: SwiftCommand {
             let tests = try testSuites
                 .filteredTests(specifier: options.testCaseSpecifier)
                 .skippedTests(specifier: options.testCaseSkip)
-            let buildParameters = try swiftTool.buildParametersForTest()
+            let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
 
             // If there were no matches, emit a warning and exit.
             if tests.isEmpty {
@@ -383,7 +387,7 @@ public struct SwiftTestTool: SwiftCommand {
         // Merge all the profraw files to produce a single profdata file.
         try mergeCodeCovRawDataFiles(swiftTool: swiftTool)
 
-        let buildParameters = try swiftTool.buildParametersForTest()
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
         for product in testProducts {
             // Export the codecov data as JSON.
             let jsonPath = codeCovAsJSONPath(
@@ -399,7 +403,7 @@ public struct SwiftTestTool: SwiftCommand {
         let llvmProf = try swiftTool.getToolchain().getLLVMProf()
 
         // Get the profraw files.
-        let buildParameters = try swiftTool.buildParametersForTest()
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
         let codeCovFiles = try localFileSystem.getDirectoryContents(buildParameters.codeCovPath)
 
         // Construct arguments for invoking the llvm-prof tool.
@@ -423,7 +427,7 @@ public struct SwiftTestTool: SwiftCommand {
     private func exportCodeCovAsJSON(to path: AbsolutePath, testBinary: AbsolutePath, swiftTool: SwiftTool) throws {
         // Export using the llvm-cov tool.
         let llvmCov = try swiftTool.getToolchain().getLLVMCov()
-        let buildParameters = try swiftTool.buildParametersForTest()
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
         let args = [
             llvmCov.pathString,
             "export",
@@ -443,7 +447,8 @@ public struct SwiftTestTool: SwiftCommand {
     ///
     /// - Returns: The paths to the build test products.
     private func buildTestsIfNeeded(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
-        let buildSystem = try swiftTool.createBuildSystem(buildParameters: swiftTool.buildParametersForTest())
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+        let buildSystem = try swiftTool.createBuildSystem(buildParameters: buildParameters)
 
         if options.shouldBuildTests {
             let subset = options.testProduct.map(BuildSubset.product) ?? .allIncludingTests
@@ -1019,8 +1024,17 @@ final class XUnitGenerator {
     }
 }
 
+extension SwiftTool {
+    func buildParametersForTest(options: TestToolOptions) throws -> BuildParameters {
+        try self.buildParametersForTest(
+            enableTestability: options.enableTestableImports
+        )
+    }
+}
+
 private extension Basics.Diagnostic {
     static var noMatchingTests: Self {
         .warning("No matching test cases were run")
     }
 }
+

Sources/Commands/TestingSupport.swift

@@ -134,10 +134,13 @@ enum TestingSupport {
 }
 
 extension SwiftTool {
-    func buildParametersForTest() throws -> BuildParameters {
+    func buildParametersForTest(
+        enableTestability: Bool? = nil
+    ) throws -> BuildParameters {
         var parameters = try self.buildParameters()
-        // for test commands, alway enable building with testability enabled
-        parameters.enableTestability = true
+        // for test commands, we normally enable building with testability
+        // but we let users override this with a flag
+        parameters.enableTestability = enableTestability ?? true
         return parameters
     }
 }

Sources/LLBuildManifest/BuildManifest.swift

@@ -51,7 +51,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = PhonyTool(inputs: inputs, outputs: outputs)
         commands[name] = Command(name: name, tool: tool)
     }
@@ -61,7 +61,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = TestDiscoveryTool(inputs: inputs, outputs: outputs)
         commands[name] = Command(name: name, tool: tool)
     }
@@ -71,7 +71,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = CopyTool(inputs: inputs, outputs: outputs)
         commands[name] = Command(name: name, tool: tool)
     }
@@ -81,7 +81,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = PackageStructureTool(inputs: inputs, outputs: outputs)
         commands[name] = Command(name: name, tool: tool)
     }
@@ -91,7 +91,7 @@ public struct BuildManifest {
         inputs: [Node],
         outputs: [Node]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = ArchiveTool(inputs: inputs, outputs: outputs)
         commands[name] = Command(name: name, tool: tool)
     }
@@ -106,7 +106,7 @@ public struct BuildManifest {
         workingDirectory: String? = nil,
         allowMissingInputs: Bool = false
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = ShellTool(
             description: description,
             inputs: inputs,
@@ -127,7 +127,7 @@ public struct BuildManifest {
         outputs: [Node],
         arguments: [String]
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = SwiftFrontendTool(
                 moduleName: moduleName,
                 description: description,
@@ -146,7 +146,7 @@ public struct BuildManifest {
         arguments: [String],
         dependencies: String? = nil
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = ClangTool(
             description: description,
             inputs: inputs,
@@ -173,7 +173,7 @@ public struct BuildManifest {
         isLibrary: Bool,
         wholeModuleOptimization: Bool
     ) {
-        assert(commands[name] == nil, "aleady had a command named '\(name)'")
+        assert(commands[name] == nil, "already had a command named '\(name)'")
         let tool = SwiftCompilerTool(
             inputs: inputs,
             outputs: outputs,

Sources/SPMBuildCore/BuildParameters.swift

@@ -228,6 +228,12 @@ public struct BuildParameters: Encodable {
         self.isXcodeBuildSystemEnabled = isXcodeBuildSystemEnabled
         self.printManifestGraphviz = printManifestGraphviz
         // 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)
         // decide if to enable the use of test manifests based on platform. this is likely to change in the future
         self.testDiscoveryStrategy = triple.isDarwin() ? .objectiveC : .manifest(generate: forceTestDiscovery)

Tests/CommandsTests/TestToolTests.swift

@@ -8,15 +8,15 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
-import XCTest
-
-import SPMTestSupport
 import Commands
+import SPMTestSupport
+import TSCBasic
+import XCTest
 
 final class TestToolTests: CommandsTestCase {
 
-    private func execute(_ args: [String]) throws -> (stdout: String, stderr: String) {
-        return try SwiftPMProduct.SwiftTest.execute(args)
+    private func execute(_ args: [String], packagePath: AbsolutePath? = nil) throws -> (stdout: String, stderr: String) {
+        return try SwiftPMProduct.SwiftTest.execute(args, packagePath: packagePath)
     }
 
     func testUsage() throws {
@@ -58,4 +58,27 @@ final class TestToolTests: CommandsTestCase {
         }
         #endif
     }
+
+    func testEnableDisableTestability() {
+        fixture(name: "Miscellaneous/TestableExe") { path in
+            // default should run with testability
+            do {
+                let result = try execute(["--vv"], packagePath: path)
+                XCTAssertMatch(result.stdout, .contains("-enable-testing"))
+            }
+
+            // disabled
+            do {
+                _ = try execute(["--disable-testable-imports", "--vv"], packagePath: path)
+            } catch SwiftPMProductError.executionFailure(_, let stdout, _) {
+                XCTAssertMatch(stdout, .contains("was not compiled for testing"))
+            }
+
+            // enabled
+            do {
+                let result = try execute(["--enable-testable-imports", "--vv"], packagePath: path)
+                XCTAssertMatch(result.stdout, .contains("-enable-testing"))
+            }
+        }
+    }
 }