New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Granular code coverage with introspection #56102
Conversation
This is an automated comment for commit 13a6f88 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
I'm considering two implementation options:
It looks like the first option is better for actual code coverage. I'm also considering various details of the coverage data:
I'm also considering possible extensions of this instrumentation:
|
Another interesting example: https://pastila.nl/?00006624/d2e0f6a4abad5d74badcd274d84db8a6#5TT5XQ8sXXBV8KyzvVVyoA== |
I'm thinking about where to enable it - either it will be another build type (release+coverage) and another test run, or it will be included in the debug build. |
We can run tests in parallel with coverage using multiple clickhouse-server processes. |
Example of unique lines covered by a test:
|
Tests sorted by unique places in code, not covered by any other tests: |
client, local, and other tools should dump the coverage into a file for subsequent insertion - we can control it with an environment variable. |
Functional tests can finish in several hours in sequential mode - this is passable. |
At this point, is the goal to have a real "test coverage map" in the CH database? For the sake of CI jobs simplicity I'd steer towards separate job (build/run). Maybe use it for master workflow only would be sufficient, but this of course depends on our needs. |
tests/clickhouse-test
Outdated
@@ -1173,6 +1173,22 @@ class TestCase: | |||
description_full += result.reason.value | |||
|
|||
description_full += result.description | |||
|
|||
if BuildFlags.SANITIZE_COVERAGE in args.build_flags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is args
defined here?
after 5 minutes of jumping around the code
Oh well.. it's very obfuscated, where it's global, local, and where it has a completely different meaning than anywhere else in the file. But yes, it's definied globally in if __name__ == "main"
Having a separate build job and check will be reasonable. It will be good if it runs for PRs because we can report if the coverage increased or decreased, the newly covered lines of code, etc. It will require functional integration and, possibly, unit tests. It could be nice to have it in Fuzzer and Stress tests, but for that purpose, we can reuse one of the existing jobs, e.g., replace release to release+coverage. There are different goals for coverage:
But it will require a few more steps:
Note: if we calculate coverage by edges (there could be many unique edges on a single line due to template instantiations), it will be quite low, maybe 30%. We can calculate the coverage by symbols (functions, taking different template instantiations differently).
This will be easy to do.
This will be easy to do. |
Let's do this:
|
This: #58792 |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add a new build option
SANITIZE_COVERAGE
. If it is enabled, the code is instrumented to track the coverage. The collected information is available inside ClickHouse with: (1) a new functioncoverage
that returns an array of unique addresses in the code found after the previous coverage reset; (2)SYSTEM RESET COVERAGE
query that resets the accumulated data. This allows us to compare the coverage of different tests, including differential code coverage. Continuation of #20539.Example: https://pastila.nl/?00013e20/348ab745ec310eeb1994a3cca223db8e#2WCaoENmvHHK539A2WX6jQ==