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

From: Barret Rhoden <brho@google.com>
Date: Jun 3, 2019, 18:11 UTC
Message-ID: <b4ce9cbc-71d3-4d7f-1ee2-e4e3fc23b015@google.com>
In-Reply-To: <e18e4391-a574-1f4b-88c7-890ada116f51@gmail.com>

Hi -

On 5/30/19 2:24 PM, Derrick Stolee wrote:

Show 22 quoted lines

>> 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.

Thanks, Barret