Skip to content

Conversation

derrickstolee
Copy link

@derrickstolee derrickstolee commented Sep 12, 2018

We have coverage targets in our Makefile for using gcov to display line coverage based on our test suite. The way I like to do it is to run:

make coverage-test
make coverage-report

This leaves the repo in a state where every X.c file that was covered has an X.c.gcov file containing the coverage counts for every line, and "#####" at every uncovered line.

There have been a few bugs in recent patches what would have been caught if the test suite covered those blocks (including a few of mine). I want to work towards a "sensible" amount of coverage on new topics. In my opinion, this means that any logic should be covered, but the 'die()' blocks in error cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code is not covered. To help, I created the 'contrib/coverage-diff.sh' script. After creating the coverage statistics at a version (say, 'topic') you can then run

contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the test suite. For example, I ran this against the 'next' branch (e82ca0) versus 'master' (f84b9b) and got the following output:

builtin/commit.c
76f2f5c1e3 builtin/commit.c 1657) write_commit_graph_reachable(get_object_directory(), 0, 0);

builtin/fsck.c
66ec0390e7 builtin/fsck.c 862) midx_argv[2] = "--object-dir";
66ec0390e7 builtin/fsck.c 863) midx_argv[3] = alt->path;
66ec0390e7 builtin/fsck.c 864) if (run_command(&midx_verify))
66ec0390e7 builtin/fsck.c 865) errors_found |= ERROR_COMMIT_GRAPH;

fsck.c
fb8952077d  214) die_errno("Could not read '%s'", path);

midx.c
56ee7ff156  949) return 0;
cc6af73c02  990) midx_report(_("failed to load pack-index for packfile %s"),
cc6af73c02  991)     e.p->pack_name);
cc6af73c02  992) break;

Commits introducing uncovered code:
Derrick Stolee      56ee7ff15: multi-pack-index: add 'verify' verb
Derrick Stolee      66ec0390e: fsck: verify multi-pack-index
Derrick Stolee      cc6af73c0: multi-pack-index: verify object offsets
Junio C Hamano      76f2f5c1e: Merge branch 'ab/commit-graph-progress' into next
René Scharfe      fb8952077: fsck: use strbuf_getline() to read skiplist file

Thanks,
-Stolee

CHANGES IN V3: I took Junio's perl script verbatim, which speeds up the performance greatly. Some of
the other sed commands needed some massaging, but also added extra cleanup. Thanks for the help!

CHANGES IN V4: I reduced the blame output using -s which decreases the width. I include a summary of the commit authors at the end to help people see the lines they wrote. This version is also copied into a build definition in the public Git project on Azure Pipelines [1]. I'll use this build definition to generate the coverage report after each "What's Cooking" email.

[1] https://git.visualstudio.com/git/_build?definitionId=5

Cc: peff@peff.net

@derrickstolee
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 12, 2018

Submitted as pull.40.git.gitgitgadget@gmail.com

@derrickstolee
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 13, 2018

Submitted as pull.40.v2.git.gitgitgadget@gmail.com

@derrickstolee
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 21, 2018

Submitted as pull.40.v3.git.gitgitgadget@gmail.com

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

    make coverage-test
    make coverage-report

This leaves the repo in a state where every X.c file that was covered has
an X.c.gcov file containing the coverage counts for every line, and "#####"
at every uncovered line.

There have been a few bugs in recent patches what would have been caught
if the test suite covered those blocks (including a few of mine). I want
to work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks covering
very unlikely (or near-impossible) situations may not warrant coverage.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

    contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the
test suite. The output uses 'git blame -s' format so you can find the commits
responsible and view the line numbers for quick access to the context, but
trims leading tabs in the file contents to reduce output width.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 8, 2018

Submitted as pull.40.v4.git.gitgitgadget@gmail.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant