fixup

Author: tomerd

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/TestingSupport.swift

@@ -139,6 +139,7 @@ extension SwiftTool {
     ) throws -> BuildParameters {
         var parameters = try self.buildParameters()
         // 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/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

@@ -69,9 +69,8 @@ final class TestToolTests: CommandsTestCase {
 
             // disabled
             do {
-                let result = try execute(["--disable-testable-imports", "--vv"], packagePath: path)
+                _ = try execute(["--disable-testable-imports", "--vv"], packagePath: path)
             } catch SwiftPMProductError.executionFailure(_, let stdout, _) {
-                XCTAssertNoMatch(stdout, .contains("-enable-testing"))
                 XCTAssertMatch(stdout, .contains("was not compiled for testing"))
             }