Skip to content

Commit

Permalink
Annotations can span over many added blank lines
Browse files Browse the repository at this point in the history
  • Loading branch information
ewjoachim committed Dec 23, 2023
1 parent d07e077 commit 0b85deb
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 38 deletions.
37 changes: 23 additions & 14 deletions coverage_comment/annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,26 @@ class Annotation:


def compute_contiguous_groups(
violations: list[int], separators: set[int]
values: list[int], separators: set[int], joiners: set[int]
) -> list[tuple[int, int]]:
"""
Given a list of violations and a list of separators, return a list of
ranges (start, included end) describing groups of violations. A group of
violations is considered contiguous if there are no more than
MAX_ANNOTATION_GAP lines between each subsequent pair of violations in the
group, and if none of the lines in the gap are separators.
Given a list of (sorted) values, a list of separators and a list of
joiners, return a list of ranges (start, included end) describing groups of
values.
Groups are created by joining contiguous values together, and in some cases
by merging groups, enclosing a gap of values between them. Gaps that may be
enclosed are small gaps (<= MAX_ANNOTATION_GAP values after removing all
joiners) where no line is a "separator"
"""
contiguous_groups: list[tuple[int, int]] = []
for _, contiguous_group in itertools.groupby(
zip(violations, itertools.count(1)), lambda x: x[1] - x[0]
zip(values, itertools.count(1)), lambda x: x[1] - x[0]
):
grouped_violations = (e[0] for e in contiguous_group)
first = next(grouped_violations)
grouped_values = (e[0] for e in contiguous_group)
first = next(grouped_values)
try:
*_, last = grouped_violations
*_, last = grouped_values
except ValueError:
last = first
contiguous_groups.append((first, last))
Expand All @@ -50,8 +53,10 @@ def reducer(
last_start, last_end = last_group
next_start, next_end = group

gap_is_small = next_start - last_end - 1 <= MAX_ANNOTATION_GAP
gap_contains_separators = set(range(last_end + 1, next_start)) & separators
gap = set(range(last_end + 1, next_start)) - joiners

gap_is_small = len(gap) <= MAX_ANNOTATION_GAP
gap_contains_separators = gap & separators

if gap_is_small and not gap_contains_separators:
acc[-1] = (last_start, next_end)
Expand All @@ -70,7 +75,6 @@ def group_annotations(
for path, diff_file in diff_coverage.files.items():
coverage_file = coverage.files[path]

violations = diff_file.violation_lines
# Lines that are covered or excluded should not be considered for
# filling a gap between violation groups.
# (so, lines that can appear in a gap are lines that are missing, or
Expand All @@ -79,9 +83,14 @@ def group_annotations(
*coverage_file.executed_lines,
*coverage_file.excluded_lines,
}
# Lines that are added should be considered for filling a gap, unless
# they are separators.
joiners = set(diff_file.added_lines) - separators

for start, end in compute_contiguous_groups(
violations=violations, separators=separators
values=diff_file.missing_lines,
separators=separators,
joiners=joiners,
):
yield Annotation(
file=path,
Expand Down
15 changes: 11 additions & 4 deletions coverage_comment/coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import decimal
import json
import pathlib
from collections.abc import Iterable
from collections.abc import Sequence

from coverage_comment import log, subprocess

Expand Down Expand Up @@ -57,7 +57,13 @@ class Coverage:
class FileDiffCoverage:
path: pathlib.Path
percent_covered: decimal.Decimal
violation_lines: list[int]
missing_lines: list[int]
added_lines: list[int]

# for backward compatibility
@property
def violation_lines(self) -> list[int]:
return self.missing_lines


@dataclasses.dataclass
Expand Down Expand Up @@ -253,7 +259,8 @@ def get_diff_coverage_info(
files[path] = FileDiffCoverage(
path=path,
percent_covered=percent_covered,
violation_lines=sorted(missing),
missing_lines=sorted(missing),
added_lines=added_lines_for_file,
)
final_percentage = compute_coverage(
num_covered=total_num_lines - total_num_violations,
Expand Down Expand Up @@ -298,7 +305,7 @@ def parse_diff_output(diff: str) -> dict[pathlib.Path, list[int]]:
return result


def parse_line_number_diff_line(line: str) -> Iterable[int]:
def parse_line_number_diff_line(line: str) -> Sequence[int]:
"""
Parse the "added" part of the line number diff text:
@@ -60,0 +61 @@ def compute_files( -> [61]
Expand Down
9 changes: 6 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ def diff_coverage_obj():
pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage(
path=pathlib.Path("codebase/code.py"),
percent_covered=decimal.Decimal("0.8"),
violation_lines=[7, 9],
missing_lines=[7, 9],
added_lines=[7, 8, 9],
)
},
)
Expand All @@ -262,12 +263,14 @@ def diff_coverage_obj_many_missing_lines():
pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage(
path=pathlib.Path("codebase/code.py"),
percent_covered=decimal.Decimal("0.8"),
violation_lines=[7, 9],
missing_lines=[7, 9],
added_lines=[7, 8, 9],
),
pathlib.Path("codebase/main.py"): coverage_module.FileDiffCoverage(
path=pathlib.Path("codebase/code.py"),
percent_covered=decimal.Decimal("0.8"),
violation_lines=[1, 2, 8, 17],
missing_lines=[1, 2, 8, 17],
added_lines=[1, 2, 3, 4, 5, 6, 7, 8, 17],
),
},
)
Expand Down
44 changes: 27 additions & 17 deletions tests/unit/test_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,40 +8,50 @@


@pytest.mark.parametrize(
"violations, separators, expected",
"values, separators, joiners, expected",
[
# Single line
([1], {1}, [(1, 1)]),
([1], {1}, set(), [(1, 1)]),
# Pair of line
([1, 2], {1, 2}, [(1, 2)]),
([1, 2], {1, 2}, set(), [(1, 2)]),
# Group of lines
([1, 2, 3], {1, 2, 3}, [(1, 3)]),
([1, 2, 3], {1, 2, 3}, set(), [(1, 3)]),
# Pair of lines with a blank line in between
([1, 3], {1, 3}, [(1, 3)]),
([1, 3], {1, 3}, set(), [(1, 3)]),
# Pair of lines with a separator in between
([1, 3], {1, 2, 3}, [(1, 1), (3, 3)]),
([1, 3], {1, 2, 3}, set(), [(1, 1), (3, 3)]),
# 3 groups of lines with separators in between
([1, 3, 5], {1, 2, 3, 4, 5}, [(1, 1), (3, 3), (5, 5)]),
([1, 3, 5], {1, 2, 3, 4, 5}, set(), [(1, 1), (3, 3), (5, 5)]),
# 3 groups of lines with a small gap & no separator in between
([1, 3, 5], {1, 3, 5}, [(1, 5)]),
([1, 3, 5], {1, 3, 5}, set(), [(1, 5)]),
# with a 1-sized gap
([1, 3], {1, 3}, [(1, 3)]),
([1, 3], {1, 3}, set(), [(1, 3)]),
# with a 2-sized gap
([1, 4], {1, 4}, [(1, 4)]),
([1, 4], {1, 4}, set(), [(1, 4)]),
# with a 3-sized gap
([1, 5], {1, 5}, [(1, 5)]),
([1, 5], {1, 5}, set(), [(1, 5)]),
# with a 4-sized gap: that's > MAX_ANNOTATION_GAP so we split
([1, 6], {1, 6}, [(1, 1), (6, 6)]),
([1, 6], {1, 6}, set(), [(1, 1), (6, 6)]),
# with a 5-sized gap but it's all joiners
([1, 7], {1, 7}, {2, 3, 4, 5, 6}, [(1, 7)]),
# same with a separator
([1, 7], {1, 4, 7}, {2, 3, 4, 5, 6}, [(1, 7)]),
# an 8-sized gap with joiners and 2 non-joiners (we merge)
([1, 9], {1, 9}, {2, 3, 5, 7, 8}, [(1, 9)]),
# an 8-sized gap with joiners and 4 non-joiners (we split)
([1, 9], {1, 9}, {2, 3, 7}, [(1, 1), (9, 9)]),
# pair of lines with a gap that is too big, and with a separator in between
([1, 6], {1, 3, 6}, [(1, 1), (6, 6)]),
([1, 6], {1, 3, 6}, set(), [(1, 1), (6, 6)]),
# single line, then group
([1, 2, 3, 5], {1, 2, 3, 5}, [(1, 5)]),
([1, 2, 3, 5], {1, 2, 3, 5}, set(), [(1, 5)]),
# group, then single line
([1, 3, 4, 5], {1, 3, 4, 5}, [(1, 5)]),
([1, 3, 4, 5], {1, 3, 4, 5}, set(), [(1, 5)]),
],
)
def test_compute_contiguous_groups(violations, separators, expected):
result = annotations.compute_contiguous_groups(violations, separators)
def test_compute_contiguous_groups(values, separators, joiners, expected):
result = annotations.compute_contiguous_groups(
values=values, separators=separators, joiners=joiners
)
assert result == expected


Expand Down

0 comments on commit 0b85deb

Please sign in to comment.