Skip to content

Commit b753bce

Browse files
tomerdtomerd
authored
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
1 parent aa566b5 commit b753bce

File tree

8 files changed

+83
-33
lines changed

8 files changed

+83
-33
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import XCTest
2-
import TestableExe1
3-
import TestableExe2
2+
@testable import TestableExe1
3+
@testable import TestableExe2
44
// import TestableExe3
55
import class Foundation.Bundle
66

@@ -9,7 +9,7 @@ final class TestableExeTests: XCTestCase {
99
// This is an example of a functional test case.
1010
// Use XCTAssert and related functions to verify your tests produce the correct
1111
// results.
12-
12+
1313
print(GetGreeting1())
1414
XCTAssertEqual(GetGreeting1(), "Hello, world")
1515
print(GetGreeting2())

‎Sources/Build/BuildPlan.swift

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ public final class SwiftTargetBuildDescription {
589589
public let isTestTarget: Bool
590590

591591
/// True if this is the test discovery target.
592-
public let testDiscoveryTarget: Bool
592+
public let isTestDiscoveryTarget: Bool
593593

594594
/// True if this module needs to be parsed as a library based on the target type and the configuration
595595
/// 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 {
632632
buildToolPluginInvocationResults: [BuildToolPluginInvocationResult] = [],
633633
prebuildCommandResults: [PrebuildCommandResult] = [],
634634
isTestTarget: Bool? = nil,
635-
testDiscoveryTarget: Bool = false,
635+
isTestDiscoveryTarget: Bool = false,
636636
fileSystem: FileSystem
637637
) throws {
638638
assert(target.underlyingTarget is SwiftTarget, "underlying target type mismatch \(target)")
@@ -641,7 +641,7 @@ public final class SwiftTargetBuildDescription {
641641
self.buildParameters = buildParameters
642642
// Unless mentioned explicitly, use the target type to determine if this is a test target.
643643
self.isTestTarget = isTestTarget ?? (target.type == .test)
644-
self.testDiscoveryTarget = testDiscoveryTarget
644+
self.isTestDiscoveryTarget = isTestDiscoveryTarget
645645
self.fileSystem = fileSystem
646646
self.tempsPath = buildParameters.buildPath.appending(component: target.c99name + ".build")
647647
self.derivedSources = Sources(paths: [], root: tempsPath.appending(component: "DerivedSources"))
@@ -1095,7 +1095,11 @@ public final class SwiftTargetBuildDescription {
10951095

10961096
/// Testing arguments according to the build configuration.
10971097
private var testingArguments: [String] {
1098-
if buildParameters.enableTestability {
1098+
if self.isTestTarget {
1099+
// test targets must be built with -enable-testing
1100+
// since its required for test discovery (the non objective-c reflection kind)
1101+
return ["-enable-testing"]
1102+
} else if buildParameters.enableTestability {
10991103
return ["-enable-testing"]
11001104
} else {
11011105
return []
@@ -1564,7 +1568,7 @@ public class BuildPlan {
15641568
toolsVersion: toolsVersion,
15651569
buildParameters: buildParameters,
15661570
isTestTarget: true,
1567-
testDiscoveryTarget: true,
1571+
isTestDiscoveryTarget: true,
15681572
fileSystem: fileSystem
15691573
)
15701574

‎Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,7 @@ extension LLBuildManifestBuilder {
823823
for target in plan.targets {
824824
guard case .swift(let target) = target,
825825
target.isTestTarget,
826-
target.testDiscoveryTarget else { continue }
826+
target.isTestDiscoveryTarget else { continue }
827827

828828
let testDiscoveryTarget = target
829829

‎Sources/Commands/SwiftTestTool.swift

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ struct TestToolOptions: ParsableArguments {
138138
@Option(help: "Test the specified product.")
139139
var testProduct: String?
140140

141+
/// Generate LinuxMain entries and exit.
142+
@Flag(name: .customLong("testable-imports"), inversion: .prefixedEnableDisable, help: "Enable or disable testable imports. Enabled by default.")
143+
var enableTestableImports: Bool = true
144+
141145
/// Returns the test case specifier if overridden in the env.
142146
private func testCaseSkipOverride() -> TestCaseSpecifier? {
143147
guard let override = ProcessEnv.vars["_SWIFTPM_SKIP_TESTS_LIST"] else {
@@ -238,7 +242,7 @@ public struct SwiftTestTool: SwiftCommand {
238242
guard let rootManifest = rootManifests.values.first else {
239243
throw StringError("invalid manifests at \(root.packages)")
240244
}
241-
let buildParameters = try swiftTool.buildParametersForTest()
245+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
242246
print(codeCovAsJSONPath(buildParameters: buildParameters, packageName: rootManifest.displayName))
243247

244248
case .generateLinuxMain:
@@ -259,7 +263,7 @@ public struct SwiftTestTool: SwiftCommand {
259263
case .runSerial:
260264
let toolchain = try swiftTool.getToolchain()
261265
let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
262-
let buildParameters = try swiftTool.buildParametersForTest()
266+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
263267

264268
// Clean out the code coverage directory that may contain stale
265269
// profraw files from a previous run of the code coverage tool.
@@ -327,7 +331,7 @@ public struct SwiftTestTool: SwiftCommand {
327331
let tests = try testSuites
328332
.filteredTests(specifier: options.testCaseSpecifier)
329333
.skippedTests(specifier: options.testCaseSkip)
330-
let buildParameters = try swiftTool.buildParametersForTest()
334+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
331335

332336
// If there were no matches, emit a warning and exit.
333337
if tests.isEmpty {
@@ -383,7 +387,7 @@ public struct SwiftTestTool: SwiftCommand {
383387
// Merge all the profraw files to produce a single profdata file.
384388
try mergeCodeCovRawDataFiles(swiftTool: swiftTool)
385389

386-
let buildParameters = try swiftTool.buildParametersForTest()
390+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
387391
for product in testProducts {
388392
// Export the codecov data as JSON.
389393
let jsonPath = codeCovAsJSONPath(
@@ -399,7 +403,7 @@ public struct SwiftTestTool: SwiftCommand {
399403
let llvmProf = try swiftTool.getToolchain().getLLVMProf()
400404

401405
// Get the profraw files.
402-
let buildParameters = try swiftTool.buildParametersForTest()
406+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
403407
let codeCovFiles = try localFileSystem.getDirectoryContents(buildParameters.codeCovPath)
404408

405409
// Construct arguments for invoking the llvm-prof tool.
@@ -423,7 +427,7 @@ public struct SwiftTestTool: SwiftCommand {
423427
private func exportCodeCovAsJSON(to path: AbsolutePath, testBinary: AbsolutePath, swiftTool: SwiftTool) throws {
424428
// Export using the llvm-cov tool.
425429
let llvmCov = try swiftTool.getToolchain().getLLVMCov()
426-
let buildParameters = try swiftTool.buildParametersForTest()
430+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
427431
let args = [
428432
llvmCov.pathString,
429433
"export",
@@ -443,7 +447,8 @@ public struct SwiftTestTool: SwiftCommand {
443447
///
444448
/// - Returns: The paths to the build test products.
445449
private func buildTestsIfNeeded(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
446-
let buildSystem = try swiftTool.createBuildSystem(buildParameters: swiftTool.buildParametersForTest())
450+
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
451+
let buildSystem = try swiftTool.createBuildSystem(buildParameters: buildParameters)
447452

448453
if options.shouldBuildTests {
449454
let subset = options.testProduct.map(BuildSubset.product) ?? .allIncludingTests
@@ -1019,8 +1024,17 @@ final class XUnitGenerator {
10191024
}
10201025
}
10211026

1027+
extension SwiftTool {
1028+
func buildParametersForTest(options: TestToolOptions) throws -> BuildParameters {
1029+
try self.buildParametersForTest(
1030+
enableTestability: options.enableTestableImports
1031+
)
1032+
}
1033+
}
1034+
10221035
private extension Basics.Diagnostic {
10231036
static var noMatchingTests: Self {
10241037
.warning("No matching test cases were run")
10251038
}
10261039
}
1040+

‎Sources/Commands/TestingSupport.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,13 @@ enum TestingSupport {
134134
}
135135

136136
extension SwiftTool {
137-
func buildParametersForTest() throws -> BuildParameters {
137+
func buildParametersForTest(
138+
enableTestability: Bool? = nil
139+
) throws -> BuildParameters {
138140
var parameters = try self.buildParameters()
139-
// for test commands, alway enable building with testability enabled
140-
parameters.enableTestability = true
141+
// for test commands, we normally enable building with testability
142+
// but we let users override this with a flag
143+
parameters.enableTestability = enableTestability ?? true
141144
return parameters
142145
}
143146
}

‎Sources/LLBuildManifest/BuildManifest.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public struct BuildManifest {
5151
inputs: [Node],
5252
outputs: [Node]
5353
) {
54-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
54+
assert(commands[name] == nil, "already had a command named '\(name)'")
5555
let tool = PhonyTool(inputs: inputs, outputs: outputs)
5656
commands[name] = Command(name: name, tool: tool)
5757
}
@@ -61,7 +61,7 @@ public struct BuildManifest {
6161
inputs: [Node],
6262
outputs: [Node]
6363
) {
64-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
64+
assert(commands[name] == nil, "already had a command named '\(name)'")
6565
let tool = TestDiscoveryTool(inputs: inputs, outputs: outputs)
6666
commands[name] = Command(name: name, tool: tool)
6767
}
@@ -71,7 +71,7 @@ public struct BuildManifest {
7171
inputs: [Node],
7272
outputs: [Node]
7373
) {
74-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
74+
assert(commands[name] == nil, "already had a command named '\(name)'")
7575
let tool = CopyTool(inputs: inputs, outputs: outputs)
7676
commands[name] = Command(name: name, tool: tool)
7777
}
@@ -81,7 +81,7 @@ public struct BuildManifest {
8181
inputs: [Node],
8282
outputs: [Node]
8383
) {
84-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
84+
assert(commands[name] == nil, "already had a command named '\(name)'")
8585
let tool = PackageStructureTool(inputs: inputs, outputs: outputs)
8686
commands[name] = Command(name: name, tool: tool)
8787
}
@@ -91,7 +91,7 @@ public struct BuildManifest {
9191
inputs: [Node],
9292
outputs: [Node]
9393
) {
94-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
94+
assert(commands[name] == nil, "already had a command named '\(name)'")
9595
let tool = ArchiveTool(inputs: inputs, outputs: outputs)
9696
commands[name] = Command(name: name, tool: tool)
9797
}
@@ -106,7 +106,7 @@ public struct BuildManifest {
106106
workingDirectory: String? = nil,
107107
allowMissingInputs: Bool = false
108108
) {
109-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
109+
assert(commands[name] == nil, "already had a command named '\(name)'")
110110
let tool = ShellTool(
111111
description: description,
112112
inputs: inputs,
@@ -127,7 +127,7 @@ public struct BuildManifest {
127127
outputs: [Node],
128128
arguments: [String]
129129
) {
130-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
130+
assert(commands[name] == nil, "already had a command named '\(name)'")
131131
let tool = SwiftFrontendTool(
132132
moduleName: moduleName,
133133
description: description,
@@ -146,7 +146,7 @@ public struct BuildManifest {
146146
arguments: [String],
147147
dependencies: String? = nil
148148
) {
149-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
149+
assert(commands[name] == nil, "already had a command named '\(name)'")
150150
let tool = ClangTool(
151151
description: description,
152152
inputs: inputs,
@@ -173,7 +173,7 @@ public struct BuildManifest {
173173
isLibrary: Bool,
174174
wholeModuleOptimization: Bool
175175
) {
176-
assert(commands[name] == nil, "aleady had a command named '\(name)'")
176+
assert(commands[name] == nil, "already had a command named '\(name)'")
177177
let tool = SwiftCompilerTool(
178178
inputs: inputs,
179179
outputs: outputs,

‎Sources/SPMBuildCore/BuildParameters.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ public struct BuildParameters: Encodable {
228228
self.isXcodeBuildSystemEnabled = isXcodeBuildSystemEnabled
229229
self.printManifestGraphviz = printManifestGraphviz
230230
// decide on testability based on debug/release config
231+
// the goals of this being based on the build configuration is
232+
// that `swift build` followed by a `swift test` will need to do minimal rebuilding
233+
// given that the default configuration for `swift build` is debug
234+
// and that `swift test` normally requires building with testable enabled.
235+
// when building and testing in release mode, one can use the '--disable-testable-imports' flag
236+
// to disable testability in `swift test`, but that requires that the tests do not use the testable imports feature
231237
self.enableTestability = enableTestability ?? (.debug == configuration)
232238
// decide if to enable the use of test manifests based on platform. this is likely to change in the future
233239
self.testDiscoveryStrategy = triple.isDarwin() ? .objectiveC : .manifest(generate: forceTestDiscovery)

‎Tests/CommandsTests/TestToolTests.swift

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@
88
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11-
import XCTest
12-
13-
import SPMTestSupport
1411
import Commands
12+
import SPMTestSupport
13+
import TSCBasic
14+
import XCTest
1515

1616
final class TestToolTests: CommandsTestCase {
1717

18-
private func execute(_ args: [String]) throws -> (stdout: String, stderr: String) {
19-
return try SwiftPMProduct.SwiftTest.execute(args)
18+
private func execute(_ args: [String], packagePath: AbsolutePath? = nil) throws -> (stdout: String, stderr: String) {
19+
return try SwiftPMProduct.SwiftTest.execute(args, packagePath: packagePath)
2020
}
2121

2222
func testUsage() throws {
@@ -58,4 +58,27 @@ final class TestToolTests: CommandsTestCase {
5858
}
5959
#endif
6060
}
61+
62+
func testEnableDisableTestability() {
63+
fixture(name: "Miscellaneous/TestableExe") { path in
64+
// default should run with testability
65+
do {
66+
let result = try execute(["--vv"], packagePath: path)
67+
XCTAssertMatch(result.stdout, .contains("-enable-testing"))
68+
}
69+
70+
// disabled
71+
do {
72+
_ = try execute(["--disable-testable-imports", "--vv"], packagePath: path)
73+
} catch SwiftPMProductError.executionFailure(_, let stdout, _) {
74+
XCTAssertMatch(stdout, .contains("was not compiled for testing"))
75+
}
76+
77+
// enabled
78+
do {
79+
let result = try execute(["--enable-testable-imports", "--vv"], packagePath: path)
80+
XCTAssertMatch(result.stdout, .contains("-enable-testing"))
81+
}
82+
}
83+
}
6184
}

0 commit comments

Comments
 (0)