Skip to content

Commit d48db30

Browse files
grynspanMaxDesiatov
authored andcommitted
Fix test targets that depend on macros and swift-testing (#7508)
After #7353 landed, I noticed that the build products for test targets were not being emitted correctly. swift-testing and XCTest produce separate build products (with distinct names) but this wasn't happening as intended. It turns out that the changes to split `buildParameters` into `productsBuildParameters` and `toolsBuildParameters` weren't fully propagated to our testing infrastructure. I also noticed `SWIFT_PM_SUPPORTS_SWIFT_TESTING` wasn't being set correctly anymore (same root cause) although we've decided to ignore that flag over in swift-testing anyway (see swiftlang/swift-testing#376.) This regression caused build failures in swift-testing (e.g. [here](https://ci.swift.org/job/pr-swift-testing-macos/663/console)) with the telltale failure signature: > /Users/ec2-user/jenkins/workspace/pr-swift-testing-macos/branch-main/swift-testing/.build/x86_64-apple-macosx/debug/swift-testingPackageTests.xctest/Contents/MacOS/swift-testingPackageTests: /Users/ec2-user/jenkins/workspace/pr-swift-testing-macos/branch-main/swift-testing/.build/x86_64-apple-macosx/debug/swift-testingPackageTests.xctest/Contents/MacOS/swift-testingPackageTests: cannot execute binary file Which indicates that it thinks the filename for the swift-testing build product is the XCTest bundle's executable. This PR plumbs through the two build parameters arguments to everywhere in `swift test` and `swift build` that needs them and resolves the issue. --------- Co-authored-by: Max Desiatov <m_desiatov@apple.com> (cherry picked from commit 5a4c024) # Conflicts: # Sources/Commands/SwiftBuildCommand.swift # Sources/Commands/SwiftTestCommand.swift # Sources/Commands/Utilities/TestingSupport.swift
1 parent 59f1edd commit d48db30

File tree

3 files changed

+94
-52
lines changed

3 files changed

+94
-52
lines changed

Sources/Commands/SwiftBuildCommand.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,21 +180,23 @@ public struct SwiftBuildCommand: AsyncSwiftCommand {
180180
library: library
181181
)
182182
}
183+
var productsBuildParameters = try swiftCommandState.productsBuildParameters
184+
var toolsBuildParameters = try swiftCommandState.toolsBuildParameters
183185
for library in try options.testLibraryOptions.enabledTestingLibraries(swiftCommandState: swiftCommandState) {
184186
updateTestingParameters(of: &productsBuildParameters, library: library)
185187
updateTestingParameters(of: &toolsBuildParameters, library: library)
186188
try build(swiftCommandState, subset: subset, productsBuildParameters: productsBuildParameters, toolsBuildParameters: toolsBuildParameters)
187189
}
188190
} else {
189-
try build(swiftCommandState, subset: subset, productsBuildParameters: productsBuildParameters, toolsBuildParameters: toolsBuildParameters)
191+
try build(swiftCommandState, subset: subset, productsBuildParameters: nil, toolsBuildParameters: nil)
190192
}
191193
}
192194

193195
private func build(
194196
_ swiftCommandState: SwiftCommandState,
195197
subset: BuildSubset,
196-
productsBuildParameters: BuildParameters,
197-
toolsBuildParameters: BuildParameters
198+
productsBuildParameters: BuildParameters?,
199+
toolsBuildParameters: BuildParameters?
198200
) throws {
199201
let buildSystem = try swiftCommandState.createBuildSystem(
200202
explicitProduct: options.product,

Sources/Commands/SwiftTestCommand.swift

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,11 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
236236
throw TestError.xcodeNotInstalled
237237
}
238238

239-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: .xctest)
239+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(options: self.options, library: .xctest)
240240

241241
// Remove test output from prior runs and validate priors.
242-
if self.options.enableExperimentalTestOutput && buildParameters.triple.supportsTestSummary {
243-
_ = try? localFileSystem.removeFileTree(buildParameters.testOutputPath)
242+
if self.options.enableExperimentalTestOutput && productsBuildParameters.triple.supportsTestSummary {
243+
_ = try? localFileSystem.removeFileTree(productsBuildParameters.testOutputPath)
244244
}
245245

246246
let testProducts = try buildTestsIfNeeded(swiftCommandState: swiftCommandState, library: .xctest)
@@ -249,7 +249,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
249249
try await runTestProducts(
250250
testProducts,
251251
additionalArguments: xctestArgs,
252-
buildParameters: buildParameters,
252+
productsBuildParameters: productsBuildParameters,
253253
swiftCommandState: swiftCommandState,
254254
library: .xctest
255255
)
@@ -276,7 +276,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
276276
// Clean out the code coverage directory that may contain stale
277277
// profraw files from a previous run of the code coverage tool.
278278
if self.options.enableCodeCoverage {
279-
try swiftCommandState.fileSystem.removeFileTree(buildParameters.codeCovPath)
279+
try swiftCommandState.fileSystem.removeFileTree(productsBuildParameters.codeCovPath)
280280
}
281281

282282
// Run the tests using the parallel runner.
@@ -286,7 +286,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
286286
toolchain: toolchain,
287287
numJobs: options.numberOfWorkers ?? ProcessInfo.processInfo.activeProcessorCount,
288288
buildOptions: globalOptions.build,
289-
buildParameters: buildParameters,
289+
productsBuildParameters: productsBuildParameters,
290290
shouldOutputSuccess: swiftCommandState.logLevel <= .info,
291291
observabilityScope: swiftCommandState.observabilityScope
292292
)
@@ -305,7 +305,7 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
305305
}
306306

307307
if self.options.enableExperimentalTestOutput, !runner.ranSuccessfully {
308-
try Self.handleTestOutput(buildParameters: buildParameters, packagePath: testProducts[0].packagePath)
308+
try Self.handleTestOutput(productsBuildParameters: productsBuildParameters, packagePath: testProducts[0].packagePath)
309309
}
310310
}
311311
}
@@ -366,13 +366,13 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
366366
// MARK: - swift-testing
367367

368368
private func swiftTestingRun(_ swiftCommandState: SwiftCommandState) async throws {
369-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: .swiftTesting)
369+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(options: self.options, library: .swiftTesting)
370370
let testProducts = try buildTestsIfNeeded(swiftCommandState: swiftCommandState, library: .swiftTesting)
371371
let additionalArguments = Array(CommandLine.arguments.dropFirst())
372372
try await runTestProducts(
373373
testProducts,
374374
additionalArguments: additionalArguments,
375-
buildParameters: buildParameters,
375+
productsBuildParameters: productsBuildParameters,
376376
swiftCommandState: swiftCommandState,
377377
library: .swiftTesting
378378
)
@@ -408,20 +408,20 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
408408
private func runTestProducts(
409409
_ testProducts: [BuiltTestProduct],
410410
additionalArguments: [String],
411-
buildParameters: BuildParameters,
411+
productsBuildParameters: BuildParameters,
412412
swiftCommandState: SwiftCommandState,
413413
library: BuildParameters.Testing.Library
414414
) async throws {
415415
// Clean out the code coverage directory that may contain stale
416416
// profraw files from a previous run of the code coverage tool.
417417
if self.options.enableCodeCoverage {
418-
try swiftCommandState.fileSystem.removeFileTree(buildParameters.codeCovPath)
418+
try swiftCommandState.fileSystem.removeFileTree(productsBuildParameters.codeCovPath)
419419
}
420420

421421
let toolchain = try swiftCommandState.getTargetToolchain()
422422
let testEnv = try TestingSupport.constructTestEnvironment(
423423
toolchain: toolchain,
424-
buildParameters: buildParameters,
424+
destinationBuildParameters: productsBuildParameters,
425425
sanitizers: globalOptions.build.sanitizers,
426426
library: library
427427
)
@@ -451,17 +451,17 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
451451
}
452452

453453
if self.options.enableExperimentalTestOutput, !ranSuccessfully {
454-
try Self.handleTestOutput(buildParameters: buildParameters, packagePath: testProducts[0].packagePath)
454+
try Self.handleTestOutput(productsBuildParameters: productsBuildParameters, packagePath: testProducts[0].packagePath)
455455
}
456456
}
457457

458-
private static func handleTestOutput(buildParameters: BuildParameters, packagePath: AbsolutePath) throws {
459-
guard localFileSystem.exists(buildParameters.testOutputPath) else {
458+
private static func handleTestOutput(productsBuildParameters: BuildParameters, packagePath: AbsolutePath) throws {
459+
guard localFileSystem.exists(productsBuildParameters.testOutputPath) else {
460460
print("No existing test output found.")
461461
return
462462
}
463463

464-
let lines = try String(contentsOfFile: buildParameters.testOutputPath.pathString).components(separatedBy: "\n")
464+
let lines = try String(contentsOfFile: productsBuildParameters.testOutputPath.pathString).components(separatedBy: "\n")
465465
let events = try lines.map { try JSONDecoder().decode(TestEventRecord.self, from: $0) }
466466

467467
let caseEvents = events.compactMap { $0.caseEvent }
@@ -505,10 +505,10 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
505505
// Merge all the profraw files to produce a single profdata file.
506506
try mergeCodeCovRawDataFiles(swiftCommandState: swiftCommandState, library: library)
507507

508-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
508+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
509509
for product in testProducts {
510510
// Export the codecov data as JSON.
511-
let jsonPath = buildParameters.codeCovAsJSONPath(packageName: rootManifest.displayName)
511+
let jsonPath = productsBuildParameters.codeCovAsJSONPath(packageName: rootManifest.displayName)
512512
try exportCodeCovAsJSON(to: jsonPath, testBinary: product.binaryPath, swiftCommandState: swiftCommandState, library: library)
513513
}
514514
}
@@ -519,18 +519,18 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
519519
let llvmProf = try swiftCommandState.getTargetToolchain().getLLVMProf()
520520

521521
// Get the profraw files.
522-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
523-
let codeCovFiles = try swiftCommandState.fileSystem.getDirectoryContents(buildParameters.codeCovPath)
522+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
523+
let codeCovFiles = try swiftCommandState.fileSystem.getDirectoryContents(productsBuildParameters.codeCovPath)
524524

525525
// Construct arguments for invoking the llvm-prof tool.
526526
var args = [llvmProf.pathString, "merge", "-sparse"]
527527
for file in codeCovFiles {
528-
let filePath = buildParameters.codeCovPath.appending(component: file)
528+
let filePath = productsBuildParameters.codeCovPath.appending(component: file)
529529
if filePath.extension == "profraw" {
530530
args.append(filePath.pathString)
531531
}
532532
}
533-
args += ["-o", buildParameters.codeCovDataFile.pathString]
533+
args += ["-o", productsBuildParameters.codeCovDataFile.pathString]
534534

535535
try TSCBasic.Process.checkNonZeroExit(arguments: args)
536536
}
@@ -544,11 +544,11 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
544544
) throws {
545545
// Export using the llvm-cov tool.
546546
let llvmCov = try swiftCommandState.getTargetToolchain().getLLVMCov()
547-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
547+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
548548
let args = [
549549
llvmCov.pathString,
550550
"export",
551-
"-instr-profile=\(buildParameters.codeCovDataFile)",
551+
"-instr-profile=\(productsBuildParameters.codeCovDataFile)",
552552
testBinary.pathString
553553
]
554554
let result = try TSCBasic.Process.popen(arguments: args)
@@ -567,10 +567,11 @@ public struct SwiftTestCommand: AsyncSwiftCommand {
567567
swiftCommandState: SwiftCommandState,
568568
library: BuildParameters.Testing.Library
569569
) throws -> [BuiltTestProduct] {
570-
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
570+
let (productsBuildParameters, toolsBuildParameters) = try swiftCommandState.buildParametersForTest(options: self.options, library: library)
571571
return try Commands.buildTestsIfNeeded(
572572
swiftCommandState: swiftCommandState,
573-
buildParameters: buildParameters,
573+
productsBuildParameters: productsBuildParameters,
574+
toolsBuildParameters: toolsBuildParameters,
574575
testProduct: self.options.sharedOptions.testProduct
575576
)
576577
}
@@ -620,8 +621,8 @@ extension SwiftTestCommand {
620621
guard let rootManifest = rootManifests.values.first else {
621622
throw StringError("invalid manifests at \(root.packages)")
622623
}
623-
let buildParameters = try swiftCommandState.buildParametersForTest(enableCodeCoverage: true, library: .xctest)
624-
print(buildParameters.codeCovAsJSONPath(packageName: rootManifest.displayName))
624+
let (productsBuildParameters, _) = try swiftCommandState.buildParametersForTest(enableCodeCoverage: true, library: .xctest)
625+
print(productsBuildParameters.codeCovAsJSONPath(packageName: rootManifest.displayName))
625626
}
626627
}
627628

@@ -632,7 +633,7 @@ extension SwiftTestCommand {
632633

633634
func run(_ swiftCommandState: SwiftCommandState) throws {
634635
try SwiftTestCommand.handleTestOutput(
635-
buildParameters: try swiftCommandState.productsBuildParameters,
636+
productsBuildParameters: try swiftCommandState.productsBuildParameters,
636637
packagePath: localFileSystem.currentWorkingDirectory ?? .root // by definition runs in the current working directory
637638
)
638639
}
@@ -660,12 +661,16 @@ extension SwiftTestCommand {
660661
// MARK: - XCTest
661662

662663
private func xctestRun(_ swiftCommandState: SwiftCommandState) throws {
663-
let buildParameters = try swiftCommandState.buildParametersForTest(
664+
let (productsBuildParameters, toolsBuildParameters) = try swiftCommandState.buildParametersForTest(
664665
enableCodeCoverage: false,
665666
shouldSkipBuilding: sharedOptions.shouldSkipBuilding,
666667
library: .xctest
667668
)
668-
let testProducts = try buildTestsIfNeeded(swiftCommandState: swiftCommandState, buildParameters: buildParameters)
669+
let testProducts = try buildTestsIfNeeded(
670+
swiftCommandState: swiftCommandState,
671+
productsBuildParameters: productsBuildParameters,
672+
toolsBuildParameters: toolsBuildParameters
673+
)
669674
let testSuites = try TestingSupport.getTestSuites(
670675
in: testProducts,
671676
swiftCommandState: swiftCommandState,
@@ -684,20 +689,21 @@ extension SwiftTestCommand {
684689
// MARK: - swift-testing
685690

686691
private func swiftTestingRun(_ swiftCommandState: SwiftCommandState) throws {
687-
let buildParameters = try swiftCommandState.buildParametersForTest(
692+
let (productsBuildParameters, toolsBuildParameters) = try swiftCommandState.buildParametersForTest(
688693
enableCodeCoverage: false,
689694
shouldSkipBuilding: sharedOptions.shouldSkipBuilding,
690695
library: .swiftTesting
691696
)
692697
let testProducts = try buildTestsIfNeeded(
693698
swiftCommandState: swiftCommandState,
694-
buildParameters: buildParameters
699+
productsBuildParameters: productsBuildParameters,
700+
toolsBuildParameters: toolsBuildParameters
695701
)
696702

697703
let toolchain = try swiftCommandState.getTargetToolchain()
698704
let testEnv = try TestingSupport.constructTestEnvironment(
699705
toolchain: toolchain,
700-
buildParameters: buildParameters,
706+
destinationBuildParameters: productsBuildParameters,
701707
sanitizers: globalOptions.build.sanitizers,
702708
library: .swiftTesting
703709
)
@@ -737,11 +743,13 @@ extension SwiftTestCommand {
737743

738744
private func buildTestsIfNeeded(
739745
swiftCommandState: SwiftCommandState,
740-
buildParameters: BuildParameters
746+
productsBuildParameters: BuildParameters,
747+
toolsBuildParameters: BuildParameters
741748
) throws -> [BuiltTestProduct] {
742749
return try Commands.buildTestsIfNeeded(
743750
swiftCommandState: swiftCommandState,
744-
buildParameters: buildParameters,
751+
productsBuildParameters: productsBuildParameters,
752+
toolsBuildParameters: toolsBuildParameters,
745753
testProduct: self.sharedOptions.testProduct
746754
)
747755
}
@@ -934,7 +942,7 @@ final class ParallelTestRunner {
934942
private let toolchain: UserToolchain
935943

936944
private let buildOptions: BuildOptions
937-
private let buildParameters: BuildParameters
945+
private let productsBuildParameters: BuildParameters
938946

939947
/// Number of tests to execute in parallel.
940948
private let numJobs: Int
@@ -951,7 +959,7 @@ final class ParallelTestRunner {
951959
toolchain: UserToolchain,
952960
numJobs: Int,
953961
buildOptions: BuildOptions,
954-
buildParameters: BuildParameters,
962+
productsBuildParameters: BuildParameters,
955963
shouldOutputSuccess: Bool,
956964
observabilityScope: ObservabilityScope
957965
) {
@@ -978,7 +986,7 @@ final class ParallelTestRunner {
978986
}
979987

980988
self.buildOptions = buildOptions
981-
self.buildParameters = buildParameters
989+
self.productsBuildParameters = productsBuildParameters
982990

983991
assert(numJobs > 0, "num jobs should be > 0")
984992
}
@@ -1008,7 +1016,7 @@ final class ParallelTestRunner {
10081016

10091017
let testEnv = try TestingSupport.constructTestEnvironment(
10101018
toolchain: self.toolchain,
1011-
buildParameters: self.buildParameters,
1019+
destinationBuildParameters: self.productsBuildParameters,
10121020
sanitizers: self.buildOptions.sanitizers,
10131021
library: .xctest // swift-testing does not use ParallelTestRunner
10141022
)
@@ -1076,7 +1084,7 @@ final class ParallelTestRunner {
10761084

10771085
// Print test results.
10781086
for test in processedTests.get() {
1079-
if (!test.success || shouldOutputSuccess) && !buildParameters.testingParameters.experimentalTestOutput {
1087+
if (!test.success || shouldOutputSuccess) && !productsBuildParameters.testingParameters.experimentalTestOutput {
10801088
// command's result output goes on stdout
10811089
// ie "swift test" should output to stdout
10821090
print(test.output)
@@ -1295,7 +1303,7 @@ extension SwiftCommandState {
12951303
func buildParametersForTest(
12961304
options: TestCommandOptions,
12971305
library: BuildParameters.Testing.Library
1298-
) throws -> BuildParameters {
1306+
) throws -> (productsBuildParameters: BuildParameters, toolsBuildParameters: BuildParameters) {
12991307
var result = try self.buildParametersForTest(
13001308
enableCodeCoverage: options.enableCodeCoverage,
13011309
enableTestability: options.enableTestableImports,
@@ -1304,7 +1312,8 @@ extension SwiftCommandState {
13041312
library: library
13051313
)
13061314
if try options.testLibraryOptions.enableSwiftTestingLibrarySupport(swiftCommandState: self) {
1307-
result.flags.swiftCompilerFlags += ["-DSWIFT_PM_SUPPORTS_SWIFT_TESTING"]
1315+
result.productsBuildParameters.flags.swiftCompilerFlags += ["-DSWIFT_PM_SUPPORTS_SWIFT_TESTING"]
1316+
result.toolsBuildParameters.flags.swiftCompilerFlags += ["-DSWIFT_PM_SUPPORTS_SWIFT_TESTING"]
13081317
}
13091318
return result
13101319
}
@@ -1364,10 +1373,14 @@ private extension Basics.Diagnostic {
13641373
/// - Returns: The paths to the build test products.
13651374
private func buildTestsIfNeeded(
13661375
swiftCommandState: SwiftCommandState,
1367-
buildParameters: BuildParameters,
1376+
productsBuildParameters: BuildParameters,
1377+
toolsBuildParameters: BuildParameters,
13681378
testProduct: String?
13691379
) throws -> [BuiltTestProduct] {
1370-
let buildSystem = try swiftCommandState.createBuildSystem(productsBuildParameters: buildParameters)
1380+
let buildSystem = try swiftCommandState.createBuildSystem(
1381+
productsBuildParameters: productsBuildParameters,
1382+
toolsBuildParameters: toolsBuildParameters
1383+
)
13711384

13721385
let subset = testProduct.map(BuildSubset.product) ?? .allIncludingTests
13731386
try buildSystem.build(subset: subset)

0 commit comments

Comments
 (0)