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

[Barret Rhoden <brho@google.com>]

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.

Thanks,
Barret