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.
Thanks,
-Stolee
[1] https://dev.azure.com/git/git/_build/results?buildId=615
