On Thu, Aug 15, 2019 at 10:01:04PM -0400, Derrick Stolee wrote:
> Here is today's test coverage report.
Are the scripts you use to generate these available somewhere?
I think it's useful to look at uncovered code, but I often struggle to
figure out whether the parts attached to my name are relevant. In
particular, I think two changes to the report format might help:
1. Include names alongside commit ids when listing uncovered lines. I
know that will end up with some overly-long lines, but it makes it
easy to grep for one's name to find relevant sections of the file
(as opposed to finding your name at the bottom and
cross-referencing with actual content lines).
Seeing that an uncovered line is a BUG(), for example, makes it
easy to know that it's not really an interesting uncovered case in
the code.
2. Include more context. Just taking a random example from this email:
> builtin/rebase.c
> e191cc8b 129) strbuf_addstr(&buf, strategy_opts);
We know what the uncovered line was trying to do, but more interesting
is likely the conditional that causes it to be uncovered. In this case
the surrounding code is:
if (opts->ignore_whitespace) {
struct strbuf buf = STRBUF_INIT;
if (strategy_opts)
strbuf_addstr(&buf, strategy_opts);
strbuf_addstr(&buf, " --ignore-space-change");
free(strategy_opts);
strategy_opts = strbuf_detach(&buf, NULL);
}
even the usual 3 lines of diff context would make it a lot quicker to
understand what's going on (it only kicks in when multiple strategy
options are used).
(As an aside, this code leaks the newly allocated buffer and leaves a
dangling pointer in opts->strategy_opts, but that's all orthogonal to
the uncovered line; I'll send a separate message to the original
author).
Anyway, I wonder if we could adjust the output of the script to make
reading it that way a bit easier.
-Peff
