Re: Git Test Coverage Report (Thursday, May 30th)

From: Barret Rhoden <brho@google.com>
Date: Jun 4, 2019, 16:38 UTC
Message-ID: <9ab619bb-9deb-4e57-a3ad-9e996425b783@google.com>
In-Reply-To: <80a23fb8-5ea8-dba3-ce7d-f6f5d4c02310@gmail.com>

On 6/3/19 2:40 PM, Derrick Stolee wrote:

Show 41 quoted lines

> On 6/3/2019 2:11 PM, Barret Rhoden wrote:
>> Hi -
>>
>> On 5/30/19 2:24 PM, Derrick Stolee wrote:
>>>> 8934ac8c 1190)     ent->ignored == next->ignored &&
>>>> 8934ac8c 1191)     ent->unblamable == next->unblamable) {
>>> These lines are part of this diff:
>>>
>>> --- a/blame.c
>>> +++ b/blame.c
>>> @@ -479,7 +479,9 @@ void blame_coalesce(struct blame_scoreboard *sb)
>>>
>>>           for (ent = sb->ent; ent && (next = ent->next); ent = next) {
>>>                   if (ent->suspect == next->suspect &&
>>> -                   ent->s_lno + ent->num_lines == next->s_lno) {
>>> +                   ent->s_lno + ent->num_lines == next->s_lno &&
>>> +                   ent->ignored == next->ignored &&
>>> +                   ent->unblamable == next->unblamable) {
>>>                           ent->num_lines += next->num_lines;
>>>                           ent->next = next->next;
>>>                           blame_origin_decref(next->suspect);
>>>
>>> The fact that they are uncovered means that the && chain is short-circuited at
>>> "ent->s_lno + ent->num_lines == next->s_lno" before the new conditions can be
>>> checked. So, the block inside is never covered. It includes a call to
>>> blame_origin_decref() and free(), so it would be good to try and exercise this region.
>>
>> What is your setup for determining if a line is uncovered?  Are you running something like gcov for all of the tests in t/?
>>
>> I removed this change, and none of the other blame tests appeared to trigger this code block either, independently of this change.  (I put an assert(0) inside the block).
>>
>> However, two of our blame-ignore tests do get past the first two checks in the if clause, (the suspects are equal and the s_lno chunks are adjacent) and we do check the ignored/unblamable conditions.
>>
>> Specifically, if I undo this change and put an assert(0) in that block, two of our tests hit that code, and one of our tests fails if I don't do the check for ignored/unblamable.
> 
> The tests use gcov while running the tests in t/. Here is the build [1].
> 
> There are some i/o errors happening in the build, which I have not
> full diagnosed. It is entirely possible that you actually are covered,
> but there was an error collecting the coverage statistics. The simplest
> thing to do is to insert a die() statement and re-run the tests.

It looks like no existing tests cover that block in blame_coalesce(), regardless of my commit. That's based on putting die() in there and running make in t/. So at the worst, my patch isn't decreasing coverage. That's a pretty low bar. =)

I'll try to come up with a test, independent of my blame-ignore work, that can get in that block.

Thanks,

Barret