Skip to content

Commit

Permalink
fixup
Browse files Browse the repository at this point in the history
  • Loading branch information
tomerd committed Feb 11, 2022
1 parent 456a370 commit ad7bf44
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 8 deletions.
14 changes: 9 additions & 5 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down Expand Up @@ -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)")
Expand All @@ -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"))
Expand Down Expand Up @@ -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 []
Expand Down Expand Up @@ -1564,7 +1568,7 @@ public class BuildPlan {
toolsVersion: toolsVersion,
buildParameters: buildParameters,
isTestTarget: true,
testDiscoveryTarget: true,
isTestDiscoveryTarget: true,
fileSystem: fileSystem
)

Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/LLBuildManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions Sources/Commands/TestingSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/SPMBuildCore/BuildParameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions Tests/CommandsTests/TestToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}

Expand Down

0 comments on commit ad7bf44

Please sign in to comment.