-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[6.2] Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage (#8894) #8919
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Closed
simonjbeaumont
wants to merge
1
commit into
swiftlang:release/6.2
from
simonjbeaumont:sb/coverage-profile-path-per-process-6.2
Closed
[6.2] Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage (#8894) #8919
simonjbeaumont
wants to merge
1
commit into
swiftlang:release/6.2
from
simonjbeaumont:sb/coverage-profile-path-per-process-6.2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…wiftlang#8894) ## Motivation 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 swiftlang#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. ## Modifications Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage. ## Result - 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 swiftlang#8893. --- ## Appendix: Demonstrating the merging of per-process profiles ```swift // file: ReproTests.swift import Testing import struct Foundation.URL #if canImport(Darwin) import Darwin #elseif canImport(Glibc) import Glibc #endif @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)
cc @bkhouri |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Backport of #8894]
Motivation
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 theswift 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 variables1, 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 impliesN=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.Modifications
Add %p to LLVM_PROFILE_FILE pattern when running tests with coverage.
Result
.serialized
#8893.Appendix: Demonstrating the merging of per-process profiles
Running with just one test results in one per-process profile and 50% coverage, as expected.
Running the other test also results in one per-process profile and 50% coverage, as expected.
Running both tests results in two per-process profile and 100% coverage, after merge.
https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#running-the-instrumented-program
(cherry picked from commit 5e566d4)
Footnotes