[Code Coverage] Add a tool to check test coverage of a patch #71841
Conversation
|
Moved the code review from Phabricator (https://reviews.llvm.org/D158864) since quite a long we had deprecated Phabricator. |
xgupta
force-pushed
the
git-check-coverage
branch
3 times, most recently
from
ddfe0aa to
724183f
Compare
xgupta
force-pushed
the
git-check-coverage
branch
from
724183f to
d454aa5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
xgupta
force-pushed
the
git-check-coverage
branch
2 times, most recently
from
06e5472 to
acadd0e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
xgupta
force-pushed
the
git-check-coverage
branch
2 times, most recently
from
6ba6bfa to
e3d3e4d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
xgupta
force-pushed
the
git-check-coverage
branch
from
91ca211 to
9a86a6e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
Actually, added a few reviewers who are contributing to LLVM Python code, just to make sure it is following the correct coding practices. |
|
I know we are not doing this right now, but I wonder if we should have tests for python scripts as well so that we can run it with our supported python version and see if it works, because it can be hard to spot in a review. I will make a sweep on the code later today. |
I am very much in favor of starting to add tests for these utilities. I wouldn't block this patch on having tests, but it's definitely a good practice in general. It's on my todo list to get tests up and running for |
There was a problem hiding this comment.
Quite a few nits, lots of other comments on my initial pass.
This looks like it could be pretty useful. Thanks for putting this together! I'm envisioning this as an on demand job that can be run on a PR with a rich representation reported in a comment (something to be added later) could be a pretty good litmus check for a lot of things.
One question: It seems like this doesn't currently cover cases where only source files or only test files are touched. Just looking at test files might be difficult as there's no way to know what to report. But, looking at just source files I think would be doable (would just require running the whole test suite, not sure how expensive this is) and would be really useful for refactorings and probably other cases.
|
Hi @boomanaiden154, Can you please review it once again. I think this is working fine for most basic usages I tried. We can commit an early version and try to improve it incrementally. |
xgupta
force-pushed
the
git-check-coverage
branch
from
8a96e92 to
c7978af
Compare
|
Hi Gentle ping! |
xgupta
force-pushed
the
git-check-coverage
branch
2 times, most recently
from
6d20d40 to
e5ea8e5
Compare
This script create a patch from the HEAD commit, extract modified or added source files, test case files and source code lines, add coverage instrumentation for the affected source files, runs Lit tests, and records which test cases cause each counter to be executed. Then report the number of test cases executing the counter and the number of test cases executing the counter that are also changed in some way by the patch. Thus providing developer the information of inadequately tested source lines.
xgupta
force-pushed
the
git-check-coverage
branch
from
e5ea8e5 to
db85592
Compare
This Python script creates a patch from the HEAD commit, extracts modified or added source files, test case files, and source code lines, adds coverage instrumentation for the affected source files, runs Lit tests, and records which test cases cause each counter to be executed. Then report the number of test cases executing the counter and the number of test cases executing the counter that are also changed in some way by the patch. Thus providing the developer the information of inadequately tested source lines.