Skip to content

Commit 1c21291

Browse files
simonjbeaumontbkhouri
authored andcommitted
Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage
The current setting for `LLVM_PROFILE_PATH`, used for code coverage, leads to corrupt profile data when tests are run in parallel or when writing "exit tests" with Swift Testing. This also results in the `swift test --enable-code-coverage` command to fail. The `LLVM_PROFILE_PATH` environment variable is used by the runtime to write raw profile files, which are then processed when the test command finishes to produce the coverage results as JSON. The variable supports several pattern variables[^1], including `%Nm`, which is currently set, and is documented to create a pool of files that the runtime will handle synchronisation of. This is fine for parallelism within the process but will not work across different processes. SwiftPM uses multiple invocations of the same binary for parallel testing and users may also fork processes within their tests, which is now a required workflow when using _exit tests_ with Swift Testing, which will fork the process internally. Furthermore, the current setting for this variable uses only `%m` (which implies `N=1`), which makes it even more likely that processes will stomp over each other when writing the raw profile data. We can see a discussion of this happening in practice in #8893. The variable also supports `%p`[^1], which will expand to produce a per-process path for the raw profile, which is probably what we want here, since Swift PM is combining all the profiles in the configured directory. Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage. - Running tests write coverage raw profile data to their own per-process file pool. - Running tests in parallel with code coverage no longer risks corrupting coverage data. - Running exit tests no longer risks corrupting coverage data. - Fixes #8893. --- ```swift // file: ReproTests.swift import Testing import struct Foundation.URL import Darwin import Glibc @suite(.serialized) struct Suite { static func updateLLVMProfilePath() { let key = "LLVM_PROFILE_FILE" let profrawExtension = "profraw" guard let previousValueCString = getenv(key) else { return } let previousValue = String(cString: previousValueCString) let previousPath = URL(filePath: previousValue) guard previousPath.pathExtension == profrawExtension else { return } guard !previousPath.lastPathComponent.contains("%p") else { return } let newPath = previousPath.deletingPathExtension().appendingPathExtension("%p").appendingPathExtension(profrawExtension) let newValue = newPath.path(percentEncoded: false) print("Replacing \(key)=\(previousValue) with \(key)=\(newValue)") setenv(key, newValue, 1) } @test func testA() async { Self.updateLLVMProfilePath() await #expect(processExitsWith: .success) { Subject.a() } } @test func testB() async { Self.updateLLVMProfilePath() await #expect(processExitsWith: .success) { Subject.b() } } } ``` ```swift // file: Subject.swift struct Subject { static func a() { _ = "a" } static func b() { _ = "a" } } ``` Running with just one test results in one per-process profile and 50% coverage, as expected. ```console % swift test --enable-code-coverage --filter Suite.testa ... ◇ Test run started. ↳ Testing Library Version: 6.2 (9ebfc4ebbb2840d) ↳ Target Platform: aarch64-unknown-linux-gnu ◇ Suite Suite started. ◇ Test testa() started. Replacing LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.profraw with LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.%p.profraw ✔ Test testa() passed after 0.018 seconds. ✔ Suite Suite passed after 0.018 seconds. ✔ Test run with 1 test in 1 suite passed after 0.018 seconds. ``` ```console % ls -1 .build/debug/codecov/ default.profdata repro-exit-tests-coverage-corruption.json 'Swift Testing12847901981426048528_0.15828.profraw' 'Swift Testing12847901981426048528_0.profraw' XCTest12847901981426048528_0.profraw % cat .build/debug/codecov/repro-exit-tests-coverage-corruption.json | jq '.data[].files[] | select(.filename == "/pwd/Tests/ReproTests/Subject.swift").summary.functions' { "count": 2, "covered": 1, "percent": 50 } ``` Running the other test also results in one per-process profile and 50% coverage, as expected. ```console % swift test --enable-code-coverage --filter Suite.testb ... ◇ Test run started. ↳ Testing Library Version: 6.2 (9ebfc4ebbb2840d) ↳ Target Platform: aarch64-unknown-linux-gnu ◇ Suite Suite started. ◇ Test testb() started. Replacing LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.profraw with LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.%p.profraw ✔ Test testb() passed after 0.017 seconds. ✔ Suite Suite passed after 0.017 seconds. ✔ Test run with 1 test in 1 suite passed after 0.017 seconds. ``` ```console % ls -1 .build/debug/codecov/ default.profdata repro-exit-tests-coverage-corruption.json 'Swift Testing12847901981426048528_0.15905.profraw' 'Swift Testing12847901981426048528_0.profraw' XCTest12847901981426048528_0.profraw % cat .build/debug/codecov/repro-exit-tests-coverage-corruption.json | jq '.data[].files[] | select(.filename == "/pwd/Tests/ReproTests/Subject.swift").summary.functions' { "count": 2, "covered": 1, "percent": 50 } ``` Running both tests results in two per-process profile and 100% coverage, after merge. ```console % swift test --enable-code-coverage --filter Suite.testa --filter Suite.testb ... ◇ Test run started. ↳ Testing Library Version: 6.2 (9ebfc4ebbb2840d) ↳ Target Platform: aarch64-unknown-linux-gnu ◇ Suite Suite started. ◇ Test testa() started. Replacing LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.profraw with LLVM_PROFILE_FILE=/pwd/.build/aarch64-unknown-linux-gnu/debug/codecov/Swift Testing%m.%p.profraw ✔ Test testa() passed after 0.016 seconds. ◇ Test testb() started. ✔ Test testb() passed after 0.015 seconds. ✔ Suite Suite passed after 0.033 seconds. ✔ Test run with 2 tests in 1 suite passed after 0.033 seconds. ``` ```console % ls -1 .build/debug/codecov/ default.profdata repro-exit-tests-coverage-corruption.json 'Swift Testing12847901981426048528_0.15981.profraw' 'Swift Testing12847901981426048528_0.15988.profraw' 'Swift Testing12847901981426048528_0.profraw' XCTest12847901981426048528_0.profraw % cat .build/debug/codecov/repro-exit-tests-coverage-corruption.json | jq '.data[].files[] | select(.filename == "/pwd/Tests/ReproTests/Subject.swift").summary.functions' { "count": 2, "covered": 2, "percent": 100 } ``` [^1]: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program (cherry picked from commit 5e566d4)
1 parent c506bdc commit 1c21291

File tree

3 files changed

+119
-6
lines changed

3 files changed

+119
-6
lines changed

Sources/Commands/Utilities/TestingSupport.swift

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,25 @@ enum TestingSupport {
197197
if buildParameters.testingParameters.enableCodeCoverage {
198198
// Defines the path at which the profraw files will be written on test execution.
199199
//
200-
// `%m` will create a pool of profraw files and append the data from
201-
// each execution in one of the files. This doesn't matter for serial
202-
// execution but is required when the tests are running in parallel as
203-
// SwiftPM repeatedly invokes the test binary with the test case name as
204-
// the filter.
205-
let codecovProfile = buildParameters.buildPath.appending(components: "codecov", "\(library)%m.profraw")
200+
// `%Nm` will create a pool of N profraw files and append the data from each execution
201+
// in one of the files. The runtime takes care of selecting a raw profile from the pool,
202+
// locking it, and updating it before the program exits. If N is not specified, it is
203+
// inferred to be 1.
204+
//
205+
// This is fine for parallel execution within a process, but for parallel tests, SwiftPM
206+
// repeatedly invokes the test binary with the testcase name as the filter and the
207+
// locking cannot be enforced by the runtime across the process boundaries.
208+
//
209+
// It's also possible that tests themselves will fork (e.g. for exit tests provided by
210+
// Swift Testing), which will inherit the environment of the parent process, and so
211+
// write to the same file, leading to profile data corruption.
212+
//
213+
// For these reasons, we unilaterally also add a %p, which will cause uniquely named
214+
// files per process.
215+
//
216+
// These are all merged using `llvm-profdata merge` once the outer test command has
217+
// completed.
218+
let codecovProfile = buildParameters.buildPath.appending(components: "codecov", "\(library)%m.%p.profraw")
206219
env["LLVM_PROFILE_FILE"] = codecovProfile.pathString
207220
}
208221
#if !os(macOS)

Sources/_InternalTestSupport/SkippedTestSupport.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ extension Trait where Self == Testing.ConditionTrait {
5858
#endif
5959
}
6060
}
61+
62+
/// Skip test if compiler is older than 6.2.
63+
public static var requireSwift6_2: Self {
64+
enabled("This test requires Swift 6.2, or newer.") {
65+
#if compiler(>=6.2)
66+
true
67+
#else
68+
false
69+
#endif
70+
}
71+
}
6172
}
6273

6374
extension Trait where Self == Testing.Bug {

Tests/IntegrationTests/SwiftPMTests.swift

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,4 +201,93 @@ private struct SwiftPMTests {
201201
}
202202
}
203203
}
204+
205+
@Test(.requireSwift6_2)
206+
func testCodeCoverageMergedAcrossSubprocesses() async throws {
207+
try await withTemporaryDirectory { tmpDir in
208+
let packagePath = tmpDir.appending(component: "test-package-coverage")
209+
try localFileSystem.createDirectory(packagePath)
210+
try await executeSwiftPackage(packagePath, extraArgs: ["init", "--type", "empty"])
211+
try await executeSwiftPackage(packagePath, extraArgs: ["add-target", "--type", "test", "ReproTests"])
212+
try localFileSystem.writeFileContents(
213+
AbsolutePath(validating: "Tests/ReproTests/Subject.swift", relativeTo: packagePath),
214+
string: """
215+
struct Subject {
216+
static func a() { _ = "a" }
217+
static func b() { _ = "b" }
218+
}
219+
"""
220+
)
221+
try localFileSystem.writeFileContents(
222+
AbsolutePath(validating: "Tests/ReproTests/ReproTests.swift", relativeTo: packagePath),
223+
string: """
224+
import Testing
225+
import class Foundation.ProcessInfo
226+
@Suite struct Suite {
227+
@Test func testProfilePathCanary() throws {
228+
let pattern = try #require(ProcessInfo.processInfo.environment["LLVM_PROFILE_FILE"])
229+
#expect(pattern.hasSuffix(".%p.profraw"))
230+
}
231+
@Test func testA() async { await #expect(processExitsWith: .success) { Subject.a() } }
232+
@Test func testB() async { await #expect(processExitsWith: .success) { Subject.b() } }
233+
}
234+
"""
235+
)
236+
let expectedCoveragePath = try await executeSwiftTest(packagePath, extraArgs: ["--show-coverage-path"]).stdout.trimmingCharacters(in: .whitespacesAndNewlines)
237+
try await executeSwiftTest(packagePath, extraArgs: ["--enable-code-coverage", "--disable-xctest"])
238+
let coveragePath = try AbsolutePath(validating: expectedCoveragePath)
239+
240+
// Check the coverage path exists.
241+
#expect(localFileSystem.exists(coveragePath))
242+
243+
// This resulting coverage file should be merged JSON, with a schema that valiades against this subset.
244+
struct Coverage: Codable {
245+
var data: [Entry]
246+
struct Entry: Codable {
247+
var files: [File]
248+
struct File: Codable {
249+
var filename: String
250+
var summary: Summary
251+
struct Summary: Codable {
252+
var functions: Functions
253+
struct Functions: Codable {
254+
var count, covered: Int
255+
var percent: Double
256+
}
257+
}
258+
}
259+
}
260+
}
261+
let coverageJSON = try localFileSystem.readFileContents(coveragePath)
262+
let coverage = try JSONDecoder().decode(Coverage.self, from: Data(coverageJSON.contents))
263+
264+
// Check for 100% coverage for Subject.swift, which should happen because the per-PID files got merged.
265+
let subjectCoverage = coverage.data.first?.files.first(where: { $0.filename.hasSuffix("Subject.swift") })
266+
#expect(subjectCoverage?.summary.functions.count == 2)
267+
#expect(subjectCoverage?.summary.functions.covered == 2)
268+
#expect(subjectCoverage?.summary.functions.percent == 100)
269+
270+
// Check the directory with the coverage path contains the profraw files.
271+
let coverageDirectory = coveragePath.parentDirectory
272+
let coverageDirectoryContents = try localFileSystem.getDirectoryContents(coverageDirectory)
273+
274+
// SwiftPM uses an LLVM_PROFILE_FILE that ends with ".%p.profraw", which we validated in the test above.
275+
// Let's first check all the files have the expected extension.
276+
let profrawFiles = coverageDirectoryContents.filter { $0.hasSuffix(".profraw") }
277+
278+
// Then check that %p expanded as we expected: to something that plausibly looks like a PID.
279+
for profrawFile in profrawFiles {
280+
let shouldBePID = try #require(profrawFile.split(separator: ".").dropLast().last)
281+
#expect(Int(shouldBePID) != nil)
282+
}
283+
284+
// Group the files by binary identifier (have a different prefix, before the per-PID suffix).
285+
let groups = Dictionary(grouping: profrawFiles) { path in path.split(separator: ".").dropLast(2) }.values
286+
287+
// Check each group has 3 files: one per PID (the above suite has 2 exit tests => 2 forks => 3 PIDs total).
288+
for binarySpecificProfrawFiles in groups {
289+
#expect(binarySpecificProfrawFiles.count == 3)
290+
}
291+
}
292+
}
204293
}

0 commit comments

Comments
 (0)