Add CoverageTrait for per-test code coverage collection #1430
Conversation
- Add CoverageTrait that collects isolated coverage for each test - Automatically enforces serialized execution (LLVM counters are global) - Resets counters before each test, writes separate .profraw files - Add InstrProfiling.h declaring LLVM profile runtime functions - Add tests demonstrating the functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Code coverage is handled today by whatever tool hosts tests (Xcode or Swift Package Manager.) Can you explain why this is needed? |
|
@grynspan This isn't ready for review, but I'm likely going to close this and go in a different direction. If I decide to do this, I'll make sure I go through the proper channels. |
grynspan
left a comment
grynspan
left a comment
There was a problem hiding this comment.
Thanks for the PR. If you're not ready for a review, please make this a draft PR and we can ignore it until it's further along.
| if let dir = Environment.variable(named: "COVERAGE_OUTPUT_DIR") { | ||
| return dir | ||
| } | ||
| if let cwd = swt_getEarlyCWD() { |
There was a problem hiding this comment.
This function does not do what you want. It is a helper for OpenBSD only.
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2024 Apple Inc. and the Swift project authors |
There was a problem hiding this comment.
Please make sure the copyright year is accurate for new files.
| @@ -0,0 +1,241 @@ | |||
| // | |||
There was a problem hiding this comment.
New source files need to be added to the appropriate CMakeLists.txt file(s).
| /// environment variables. | ||
| private let _coverageEnabled: Bool = { | ||
| // Check for LLVM_PROFILE_FILE environment variable (set by swift test) | ||
| Environment.variable(named: "LLVM_PROFILE_FILE") != nil |
There was a problem hiding this comment.
Please make sure to document any new environment variable usage in EnvironmentVariables.md.
| try await function() | ||
| return | ||
| } | ||
| configuration.isParallelizationEnabled = false |
There was a problem hiding this comment.
This does not cause the test to run serially with respect to other tests, only with respect to any subtests it contains (test functions if it is a suite, test cases if it is a test function.)
| var name = test.name | ||
| if let testCase { | ||
| // Include test case ID for parameterized tests | ||
| name += "-\(testCase.id)" |
There was a problem hiding this comment.
The string form of a test case's ID is not suitable for use as a file name. It may be arbitrarily long (too long to be represented as a filename on some systems) or may contain illegal characters.
| var sanitized = "" | ||
| for char in name { | ||
| switch char { | ||
| case "/", ":", " ", "(", ")", ",", "\\", "<", ">", "\"", "|", "?", "*": |
There was a problem hiding this comment.
Consider hashing the string instead to get a statistically unique filename?
| /// | ||
| /// @note This function is only available when coverage instrumentation is | ||
| /// enabled. Use swt_profilerRuntimeAvailable() to check availability. | ||
| SWT_EXTERN void __llvm_profile_reset_counters(void); |
There was a problem hiding this comment.
What happens if we reference these functions when code is not compiled with coverage enabled?
| #endif | ||
| return dlsym(handle, "__llvm_profile_reset_counters") != NULL; | ||
| #else | ||
| return false; |
There was a problem hiding this comment.
We need Windows support too. And we need to support building for targets that do not have a dynamic loader.
| func profileFunctionsCallable() { | ||
| // Reset counters - should not crash | ||
| __llvm_profile_reset_counters() | ||
| print("Reset counters: OK") |
There was a problem hiding this comment.
Please remove these print() calls.
grynspan
added
enhancement
|
@grynspan This PR is a draft |
|
You replied while I was reviewing. Sorry for the crossed wires! If you do want to pursue this change, perhaps you can join us in a Testing Workgroup meeting and explain your thinking so we can give your work the support it needs (or may need). |
|
All good! Will do. I appreciate it! |
2 participants
No description provided.