On 6/3/2019 2:11 PM, Barret Rhoden wrote:

Hi -

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

quoted

quoted

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