Skip to content

Commit

Permalink
fixup
Browse files Browse the repository at this point in the history
  • Loading branch information
tomerd committed Nov 17, 2020
1 parent aca14f4 commit c16c62c
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 33 deletions.
22 changes: 7 additions & 15 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ extension BuildParameters {
if configuration == .debug {
addIndexStoreArguments = true
} else if target.type == .test {
// 👀 not sure
// Test discovery requires an index store for the test target to discover the tests
addIndexStoreArguments = true
} else {
Expand Down Expand Up @@ -636,7 +635,7 @@ public final class SwiftTargetBuildDescription {
args += buildParameters.indexStoreArguments(for: target)
args += buildParameters.toolchain.extraSwiftCFlags
args += optimizationArguments
args += testingArguments(for: target)
args += testingArguments
args += ["-g"]
args += ["-j\(buildParameters.jobs)"]
args += activeCompilationConditions
Expand Down Expand Up @@ -758,7 +757,7 @@ public final class SwiftTargetBuildDescription {
result += buildParameters.targetTripleArgs(for: target)
result += ["-swift-version", swiftVersion.rawValue]
result += optimizationArguments
result += testingArguments(for: target)
result += testingArguments
result += ["-g"]
result += ["-j\(buildParameters.jobs)"]
result += activeCompilationConditions
Expand Down Expand Up @@ -807,7 +806,7 @@ public final class SwiftTargetBuildDescription {
result += buildParameters.indexStoreArguments(for: target)
result += buildParameters.toolchain.extraSwiftCFlags
result += optimizationArguments
result += testingArguments(for: target)
result += testingArguments
result += ["-g"]
result += ["-j\(buildParameters.jobs)"]
result += activeCompilationConditions
Expand Down Expand Up @@ -962,18 +961,11 @@ public final class SwiftTargetBuildDescription {
}

/// Testing arguments according to the build configuration.
private func testingArguments(for target: ResolvedTarget) -> [String] {
//return ["-enable-testing"]
switch buildParameters.configuration {
case .debug:
private var testingArguments: [String] {
if buildParameters.enableTestability {
return ["-enable-testing"]
case .release:
// 👀 not sure
if target.type == .test {
return ["-enable-testing"]
} else {
return []
}
} else {
return []
}
}

Expand Down
25 changes: 17 additions & 8 deletions Sources/Commands/SwiftTestTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public struct SwiftTestTool: SwiftCommand {
let workspace = try swiftTool.getActiveWorkspace()
let root = try swiftTool.getWorkspaceRoot()
let rootManifest = workspace.loadRootManifests(packages: root.packages, diagnostics: swiftTool.diagnostics)[0]
let buildParameters = try swiftTool.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()
print(codeCovAsJSONPath(buildParameters: buildParameters, packageName: rootManifest.name))

case .generateLinuxMain:
Expand All @@ -233,7 +233,7 @@ public struct SwiftTestTool: SwiftCommand {
case .runSerial:
let toolchain = try swiftTool.getToolchain()
let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
let buildParameters = try swiftTool.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()

// Clean out the code coverage directory that may contain stale
// profraw files from a previous run of the code coverage tool.
Expand Down Expand Up @@ -298,7 +298,7 @@ public struct SwiftTestTool: SwiftCommand {
let tests = testSuites
.filteredTests(specifier: options.testCaseSpecifier)
.skippedTests(specifier: options.testCaseSkip)
let buildParameters = try swiftTool.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()

// If there were no matches, emit a warning and exit.
if tests.isEmpty {
Expand Down Expand Up @@ -340,7 +340,7 @@ public struct SwiftTestTool: SwiftCommand {
// Merge all the profraw files to produce a single profdata file.
try mergeCodeCovRawDataFiles(swiftTool: swiftTool)

let buildParameters = try swiftTool.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()
for product in testProducts {
// Export the codecov data as JSON.
let jsonPath = codeCovAsJSONPath(
Expand All @@ -356,7 +356,7 @@ public struct SwiftTestTool: SwiftCommand {
let llvmProf = try swiftTool.getToolchain().getLLVMProf()

// Get the profraw files.
let buildParameters = try swiftTool.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()
let codeCovFiles = try localFileSystem.getDirectoryContents(buildParameters.codeCovPath)

// Construct arguments for invoking the llvm-prof tool.
Expand All @@ -380,7 +380,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.buildParameters()
let buildParameters = try swiftTool.buildParametersForTest()
let args = [
llvmCov.pathString,
"export",
Expand Down Expand Up @@ -467,7 +467,7 @@ public struct SwiftTestTool: SwiftCommand {
#if os(macOS)
let data: String = try withTemporaryFile { tempFile in
let args = [xctestHelperPath(swiftTool: swiftTool).pathString, path.pathString, tempFile.path.pathString]
var env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParameters())
var env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParametersForTest())
// Add the sdk platform path if we have it. If this is not present, we
// might always end up failing.
if let sdkPlatformFrameworksPath = Destination.sdkPlatformFrameworkPaths() {
Expand All @@ -479,7 +479,7 @@ public struct SwiftTestTool: SwiftCommand {
return try localFileSystem.readFileContents(tempFile.path).validDescription ?? ""
}
#else
let env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParameters())
let env = try constructTestEnvironment(toolchain: try swiftTool.getToolchain(), options: swiftOptions, buildParameters: swiftTool.buildParametersForTest())
let args = [path.description, "--dump-tests-json"]
let data = try Process.checkNonZeroExit(arguments: args, environment: env)
#endif
Expand Down Expand Up @@ -1100,3 +1100,12 @@ private extension Diagnostic.Message {
.warning("No matching test cases were run")
}
}

private extension SwiftTool {
func buildParametersForTest() throws -> BuildParameters {
var parameters = try self.buildParameters()
// for test commands, alway enable building with testability enabled
parameters.enableTestability = true
return parameters
}
}
10 changes: 10 additions & 0 deletions Sources/SPMBuildCore/BuildParameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ public struct BuildParameters: Encodable {

/// Extra arguments to pass when using xcbuild.
public var xcbuildFlags: [String]

// Whether the building for testability is enabled.
public var enableTestability: Bool

public init(
dataPath: AbsolutePath,
Expand Down Expand Up @@ -174,6 +177,13 @@ public struct BuildParameters: Encodable {
self.useExplicitModuleBuild = useExplicitModuleBuild
self.isXcodeBuildSystemEnabled = isXcodeBuildSystemEnabled
self.printManifestGraphviz = printManifestGraphviz
// decide on testability based on config
switch self.configuration {
case .debug:
self.enableTestability = true
case .release:
self.enableTestability = false
}
}

/// The path to the build directory (inside the data directory).
Expand Down
13 changes: 6 additions & 7 deletions Sources/XCBuildSupport/PIFBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,18 @@ import PackageGraph
/// The parameters required by `PIFBuilder`.
public struct PIFBuilderParameters {

// 👀 not sure if this should go also, how is it used?
/// Whether or not test discovery is enabled.
public let enableTestDiscovery: Bool
/// Whether or not build for testability is enabled.
public let enableTestability: Bool

/// Whether to create dylibs for dynamic library products.
public let shouldCreateDylibForDynamicProducts: Bool

/// Creates a `PIFBuilderParameters` instance.
/// - Parameters:
/// - enableTestDiscovery: Whether or not test discovery is enabled.
/// - enableTestability: Whether or not build for testability is enabled.
/// - shouldCreateDylibForDynamicProducts: Whether to create dylibs for dynamic library products.
public init(enableTestDiscovery: Bool, shouldCreateDylibForDynamicProducts: Bool) {
self.enableTestDiscovery = enableTestDiscovery
public init(enableTestability: Bool, shouldCreateDylibForDynamicProducts: Bool) {
self.enableTestability = enableTestability
self.shouldCreateDylibForDynamicProducts = shouldCreateDylibForDynamicProducts
}
}
Expand Down Expand Up @@ -324,7 +323,7 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder {
releaseSettings[.GCC_OPTIMIZATION_LEVEL] = "s"
releaseSettings[.SWIFT_OPTIMIZATION_LEVEL] = "-Owholemodule"

if parameters.enableTestDiscovery {
if parameters.enableTestability {
releaseSettings[.ENABLE_TESTABILITY] = "YES"
}

Expand Down
3 changes: 1 addition & 2 deletions Sources/XCBuildSupport/XcodeBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ extension BuildConfiguration {
extension PIFBuilderParameters {
public init(_ buildParameters: BuildParameters) {
self.init(
// 👀 not sure
enableTestDiscovery: true,
enableTestability: buildParameters.enableTestability,
shouldCreateDylibForDynamicProducts: buildParameters.shouldCreateDylibForDynamicProducts
)
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/XCBuildSupportTests/PIFBuilderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,7 @@ extension PIFBuilderParameters {
shouldCreateDylibForDynamicProducts: Bool = false
) -> Self {
PIFBuilderParameters(
enableTestDiscovery: false,
enableTestability: false,
shouldCreateDylibForDynamicProducts: shouldCreateDylibForDynamicProducts
)
}
Expand Down

0 comments on commit c16c62c

Please sign in to comment.