"missing line" annotations that span multiple lines?

[nedbat]

The annotations get a bit noisy when an entire multi-line clause of code is missed. There's an annotation on every line. GitHub Action annotations have an endLine attribute, so we could create one annotation for an entire block of lines.

The difficulty is in knowing what block to mark. If lines 10 and 12 are missing, and 11 is blank, it should be one annotation on lines 10-12. But the coverage data will have "missing_lines": [10, 12]. Internally, coverage knows better. For example, coverage report -m would show 10-12, because coverage knows that line 11 wasn't a possibility.

Would you be interested in supporting something like this? We could figure out a good way for coverage to make the better information available.

[ewjoachim]

It would certainly be an excellent idea to take care of this.

Of course, it's not exactly as simple as it seems, but it's not extremely complicated either.

Here are some thought I have on that:

• Coverage already does this by saying both which lines are executed, and which lines are missing. This indirectly also says which lines are "blank" (don't contain code, but may contain comments): the ones that are neither executed nor missing (set(range(len(lines))) - set(executed) - set(missing))
• We used to generate the annotations from our Coverage data object which supposedly holds all the info from the output of coverage json, including missing_lines/executed_lines
• But: at some point this year, we changed this to generating annotations only from the intersection of diff lines and coverage lines, to avoid reporting on lines with missing coverage that the PR didn't touch. So the function started receiving only the DiffCoverage object.
• At that point, the diff-coverage data was extracted from diff-cover but it was slightly painful, and we realized that we could compute it ourselves not too difficultly so we removed diff-cover, though we kept the same format for the DiffCoverage data object.

In the end the annotations are computed here from a DiffCoverage object that doesn't have enough information to do the blocks as you suggest.

I then see 2 ways looking forward:

• Either add more information to the DiffCoverage object when we create them from the Coverage object
• Or pass both the Coverage and the DiffCoverage object to the function that computes the annotations (I'd tend to favor this)

Also, I see a design decision that needs to be taken: we want to annotate all the lines that are both added in the diff and uncovered. Continguous lines should be grouped. We also accept to close some gaps, meaning to include lines that don't contain code. What is the maximum number of blank lines we accept to close a gap ? Should the fact that those blank lines may or may not part of the diff be a factor for whether we accept to put them in the gap or not ?

All the examples below share the same line 1, 2 and 4, only the line 3 changes.

+ line 1 covered
+ line 2 uncovered
+ line 3 covered
+ line 4 uncovered

This should result in 2, 4

+ line 1 covered
+ line 2 uncovered
+ line 3 uncovered
+ line 4 uncovered

This should result in 2-4

+ line 1 covered
+ line 2 uncovered
+ line 3 blank
+ line 4 uncovered

This should result in 2-4. Even if line 3 actually is a hundred of added but blank lines, I believe the group should contain the whole gap (2-104)

+ line 1 covered
+ line 2 uncovered
  line 3 covered
+ line 4 uncovered

This should result in 2, 4

+ line 1 covered
+ line 2 uncovered
  line 3 uncovered
+ line 4 uncovered

I... think it could be either 2-4 or 2,4 ? If it was not just line 3 but actually 100 lines of non-added uncovered lines, we wouldn't want to annotate the gap, we'd want 2 groups, I guess, so maybe we should only accept to close small gaps ? What is "small"? Up to 3 lines would feel okay, I guess? Any opinion?

+ line 1 covered
+ line 2 uncovered
  line 3 blank
+ line 4 uncovered

Probably the same as above?

[ewjoachim]

Ok I'm "nerd sniped" :D

[ewjoachim]

Hey @nedbat :)

Before I merge, would you be able to make a test-run by configuring you action to use @group-annotations instead of @v3 and telling me if annotations behave as you expect ?

Thanks :)

[ewjoachim]

Here's an example of the result. I'm ok even if GitHub annoyingly doesn't show the full scope of the annotation (it displays it as if it was entirely related to the endLine.

I'll merge when tests pass.

[nedbat]

Wow! I turn my back for a few weeks, and it's all done! I thought you would need more information from coverage.py, but I guess not. I will make sure people at work take a look at this. I'm sure it will make them happy! Thanks.

[ewjoachim]

A pleasure!

You may also notice that the comment posted by this action has been updated yesterday, the new format should be a bit richer: