From 01cd3343752ca011ed49cc2a5e785cafc2236bfa Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Fri, 22 Dec 2023 01:29:47 +0100 Subject: [PATCH 1/5] Group annotations for (almost) contiguous lines --- coverage_comment/annotations.py | 98 ++++++++++++++++++++++++++------ coverage_comment/github.py | 33 +++++++---- coverage_comment/main.py | 15 ++++- tests/integration/test_github.py | 38 ++++++------- tests/integration/test_main.py | 2 +- tests/unit/test_annotations.py | 72 +++++++++++++++-------- 6 files changed, 182 insertions(+), 76 deletions(-) diff --git a/coverage_comment/annotations.py b/coverage_comment/annotations.py index 310573d7..ef469f93 100644 --- a/coverage_comment/annotations.py +++ b/coverage_comment/annotations.py @@ -1,24 +1,90 @@ from __future__ import annotations +import dataclasses +import functools +import itertools +import pathlib +from collections.abc import Iterable + from coverage_comment import coverage as coverage_module -from coverage_comment import github -MISSING_LINES_GROUP_TITLE = "Annotations of lines with missing coverage" +MAX_ANNOTATION_GAP = 3 -def create_pr_annotations( - annotation_type: str, diff_coverage: coverage_module.DiffCoverage -): - github.send_workflow_command( - command="group", command_value=MISSING_LINES_GROUP_TITLE - ) +@dataclasses.dataclass(frozen=True) +class Annotation: + file: pathlib.Path + line_start: int + line_end: int - for file_path, file_diff_coverage in diff_coverage.files.items(): - for missing_line in file_diff_coverage.violation_lines: - github.create_missing_coverage_annotation( - annotation_type=annotation_type, - file=file_path, - line=missing_line, - ) - github.send_workflow_command(command="endgroup", command_value="") +def compute_contiguous_groups( + violations: list[int], separators: 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. + """ + contiguous_groups: list[tuple[int, int]] = [] + for _, contiguous_group in itertools.groupby( + zip(violations, itertools.count(1)), lambda x: x[1] - x[0] + ): + grouped_violations = (e[0] for e in contiguous_group) + first = next(grouped_violations) + try: + *_, last = grouped_violations + except ValueError: + last = first + contiguous_groups.append((first, last)) + + def reducer( + acc: list[tuple[int, int]], group: tuple[int, int] + ) -> list[tuple[int, int]]: + if not acc: + return [group] + + last_group = acc[-1] + 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 + + if gap_is_small and not gap_contains_separators: + acc[-1] = (last_start, next_end) + return acc + + acc.append(group) + return acc + + return functools.reduce(reducer, contiguous_groups, []) + + +def group_annotations( + coverage: coverage_module.Coverage, + diff_coverage: coverage_module.DiffCoverage, +) -> Iterable[Annotation]: + 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 + # lines that do not contain code: blank lines or lines containing comments) + separators = { + *coverage_file.executed_lines, + *coverage_file.excluded_lines, + } + + for start, end in compute_contiguous_groups( + violations=violations, separators=separators + ): + yield Annotation( + file=path, + line_start=start, + line_end=end, + ) diff --git a/coverage_comment/github.py b/coverage_comment/github.py index f41c0d45..20131aed 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -10,7 +10,8 @@ from coverage_comment import github_client, log GITHUB_ACTIONS_LOGIN = "github-actions[bot]" -MISSING_COVERAGE_MESSAGE = "This line has no coverage" +MISSING_COVERAGE_MESSAGE = "Missing coverage" +MISSING_LINES_GROUP_TITLE = "Annotations of lines with missing coverage" class CannotDeterminePR(Exception): @@ -199,17 +200,27 @@ def send_workflow_command(command: str, command_value: str, **kwargs: str) -> No ) -def create_missing_coverage_annotation( - annotation_type: str, file: pathlib.Path, line: int +def create_missing_coverage_annotations( + annotation_type: str, annotations: list[tuple[pathlib.Path, int, int]] ): - send_workflow_command( - command=annotation_type, - command_value=MISSING_COVERAGE_MESSAGE, - # This will produce \ paths when running on windows. - # GHA doc is unclear whether this is right or not. - file=str(file), - line=str(line), - ) + """ + Create annotations for lines with missing coverage. + + annotation_type: The type of annotation to create. Can be either "error" or "warning". + annotations: A list of tuples of the form (file, line_start, line_end) + """ + send_workflow_command(command="group", command_value=MISSING_LINES_GROUP_TITLE) + for file, line_start, line_end in annotations: + send_workflow_command( + command=annotation_type, + command_value=MISSING_COVERAGE_MESSAGE, + # This will produce \ paths when running on windows. + # GHA doc is unclear whether this is right or not. + file=str(file), + line=str(line_start), + endLine=str(line_end), + ) + send_workflow_command(command="endgroup", command_value="") def append_to_file(content: str, filepath: pathlib.Path): diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 022df8ae..1857c44c 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -9,7 +9,9 @@ from coverage_comment import activity as activity_module from coverage_comment import ( - annotations, + annotations as annotations_module, +) +from coverage_comment import ( comment_file, communication, files, @@ -201,8 +203,15 @@ def process_pr( pr_number = None if pr_number is not None and config.ANNOTATE_MISSING_LINES: - annotations.create_pr_annotations( - annotation_type=config.ANNOTATION_TYPE, diff_coverage=diff_coverage + annotations = annotations_module.group_annotations( + coverage=coverage, diff_coverage=diff_coverage + ) + github.create_missing_coverage_annotations( + annotation_type=config.ANNOTATION_TYPE, + annotations=[ + (annotation.file, annotation.line_start, annotation.line_end) + for annotation in annotations + ], ) try: diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 75b00de3..429eb68c 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -325,27 +325,6 @@ def test_send_workflow_command(capsys): assert output.err.strip() == "::foo file=main.py,line=1,title=someTitle::bar" -def test_create_missing_coverage_annotation(capsys): - github.create_missing_coverage_annotation( - annotation_type="warning", file=pathlib.Path("test.py"), line=42 - ) - output = capsys.readouterr() - assert ( - output.err.strip() - == "::warning file=test.py,line=42::This line has no coverage" - ) - - -def test_create_missing_coverage_annotation__annotation_type(capsys): - github.create_missing_coverage_annotation( - annotation_type="error", file=pathlib.Path("test.py"), line=42 - ) - output = capsys.readouterr() - assert ( - output.err.strip() == "::error file=test.py,line=42::This line has no coverage" - ) - - def test_add_job_summary(summary_file): github.add_job_summary( content="[job summary part 1]\n", github_step_summary=summary_file @@ -356,3 +335,20 @@ def test_add_job_summary(summary_file): content="[job summary part 2]", github_step_summary=summary_file ) assert summary_file.read_text() == "[job summary part 1]\n[job summary part 2]" + + +def test_annotations(capsys): + github.create_missing_coverage_annotations( + annotation_type="warning", + annotations=[ + (pathlib.Path("codebase/code.py"), 1, 3), + (pathlib.Path("codebase/main.py"), 5, 5), + ], + ) + + expected = """::group::Annotations of lines with missing coverage +::warning file=codebase/code.py,line=1,endLine=3::Missing coverage +::warning file=codebase/main.py,line=5,endLine=5::Missing coverage +::endgroup::""" + output = capsys.readouterr() + assert output.err.strip() == expected diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index baea4def..5a076888 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -568,7 +568,7 @@ def test_action__pull_request__annotations( git=git, ) expected = """::group::Annotations of lines with missing coverage -::warning file=foo.py,line=12::This line has no coverage +::warning file=foo.py,line=12,endLine=12::Missing coverage ::endgroup::""" output = capsys.readouterr() diff --git a/tests/unit/test_annotations.py b/tests/unit/test_annotations.py index ebe12776..05f3c737 100644 --- a/tests/unit/test_annotations.py +++ b/tests/unit/test_annotations.py @@ -1,33 +1,57 @@ from __future__ import annotations -from coverage_comment import annotations +import pathlib +import pytest + +from coverage_comment import annotations -def test_annotations(diff_coverage_obj, capsys): - annotations.create_pr_annotations( - annotation_type="warning", diff_coverage=diff_coverage_obj - ) - expected = """::group::Annotations of lines with missing coverage -::warning file=codebase/code.py,line=7::This line has no coverage -::warning file=codebase/code.py,line=9::This line has no coverage -::endgroup::""" - output = capsys.readouterr() - assert output.err.strip() == expected +@pytest.mark.parametrize( + "violations, separators, expected", + [ + # Single line + ([1], {1}, [(1, 1)]), + # Pair of line + ([1, 2], {1, 2}, [(1, 2)]), + # Group of lines + ([1, 2, 3], {1, 2, 3}, [(1, 3)]), + # Pair of lines with a blank line in between + ([1, 3], {1, 3}, [(1, 3)]), + # Pair of lines with a separator in between + ([1, 3], {1, 2, 3}, [(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)]), + # 3 groups of lines with a small gap & no separator in between + ([1, 3, 5], {1, 3, 5}, [(1, 5)]), + # with a 1-sized gap + ([1, 3], {1, 3}, [(1, 3)]), + # with a 2-sized gap + ([1, 4], {1, 4}, [(1, 4)]), + # with a 3-sized gap + ([1, 5], {1, 5}, [(1, 5)]), + # with a 4-sized gap: that's > MAX_ANNOTATION_GAP so we split + ([1, 6], {1, 6}, [(1, 1), (6, 6)]), + # 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)]), + # single line, then group + ([1, 2, 3, 5], {1, 2, 3, 5}, [(1, 5)]), + # group, then single line + ([1, 3, 4, 5], {1, 3, 4, 5}, [(1, 5)]), + ], +) +def test_compute_contiguous_groups(violations, separators, expected): + result = annotations.compute_contiguous_groups(violations, separators) + assert result == expected -def test_annotations_several_files(diff_coverage_obj_many_missing_lines, capsys): - annotations.create_pr_annotations( - annotation_type="notice", diff_coverage=diff_coverage_obj_many_missing_lines +def test_group_annotations(coverage_obj, diff_coverage_obj): + result = annotations.group_annotations( + coverage=coverage_obj, diff_coverage=diff_coverage_obj ) - expected = """::group::Annotations of lines with missing coverage -::notice file=codebase/code.py,line=7::This line has no coverage -::notice file=codebase/code.py,line=9::This line has no coverage -::notice file=codebase/main.py,line=1::This line has no coverage -::notice file=codebase/main.py,line=2::This line has no coverage -::notice file=codebase/main.py,line=8::This line has no coverage -::notice file=codebase/main.py,line=17::This line has no coverage -::endgroup::""" - output = capsys.readouterr() - assert output.err.strip() == expected + assert list(result) == [ + annotations.Annotation( + file=pathlib.Path("codebase/code.py"), line_start=7, line_end=9 + ) + ] From 36c4eda3274252490892f7d3f062a07bfef634b6 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Fri, 22 Dec 2023 01:30:19 +0100 Subject: [PATCH 2/5] (small unrelated fixes) --- coverage_comment/settings.py | 2 +- tests/integration/test_github.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/coverage_comment/settings.py b/coverage_comment/settings.py index c3e297fd..acc09c5e 100644 --- a/coverage_comment/settings.py +++ b/coverage_comment/settings.py @@ -130,7 +130,7 @@ def GITHUB_PR_NUMBER(self) -> int | None: @property def GITHUB_BRANCH_NAME(self) -> str | None: - # "refs/head/my_branch_name" + # "refs/heads/my_branch_name" if "heads" in self.GITHUB_REF: return self.GITHUB_REF.split("/", 2)[2] return None diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 429eb68c..6fb5515f 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -63,8 +63,8 @@ def test_download_artifact(gh, session, zip_bytes): github=gh, repository="foo/bar", artifact_name="foo", - run_id="123", - filename="foo.txt", + run_id=123, + filename=pathlib.Path("foo.txt"), ) assert result == "bar" @@ -83,8 +83,8 @@ def test_download_artifact__no_artifact(gh, session): github=gh, repository="foo/bar", artifact_name="foo", - run_id="123", - filename="foo.txt", + run_id=123, + filename=pathlib.Path("foo.txt"), ) @@ -104,8 +104,8 @@ def test_download_artifact__no_file(gh, session, zip_bytes): github=gh, repository="foo/bar", artifact_name="foo", - run_id="123", - filename="bar.txt", + run_id=123, + filename=pathlib.Path("bar.txt"), ) @@ -303,6 +303,10 @@ def test_set_output(output_file): assert output_file.read_text() == "foo=true\n" +def test_set_output__empty(): + assert github.set_output(github_output=None, foo=True) is None + + def test_get_workflow_command(): output = github.get_workflow_command( command="foo", command_value="bar", file="main.py", line="1", title="someTitle" From 7bcfa3a8b0d41df8b59fa6e4ca33636c9ef7b09c Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 24 Dec 2023 00:18:04 +0100 Subject: [PATCH 3/5] Annotations can span over many added blank lines --- coverage_comment/annotations.py | 37 ++++++++++++++++----------- coverage_comment/coverage.py | 15 ++++++++--- tests/conftest.py | 9 ++++--- tests/unit/test_annotations.py | 44 ++++++++++++++++++++------------- 4 files changed, 67 insertions(+), 38 deletions(-) diff --git a/coverage_comment/annotations.py b/coverage_comment/annotations.py index ef469f93..d8f61094 100644 --- a/coverage_comment/annotations.py +++ b/coverage_comment/annotations.py @@ -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)) @@ -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) @@ -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 @@ -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, diff --git a/coverage_comment/coverage.py b/coverage_comment/coverage.py index bd298695..f1976cb7 100644 --- a/coverage_comment/coverage.py +++ b/coverage_comment/coverage.py @@ -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 @@ -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 @@ -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, @@ -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] diff --git a/tests/conftest.py b/tests/conftest.py index 58383250..9399d5bc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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], ) }, ) @@ -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], ), }, ) diff --git a/tests/unit/test_annotations.py b/tests/unit/test_annotations.py index 05f3c737..4b154d43 100644 --- a/tests/unit/test_annotations.py +++ b/tests/unit/test_annotations.py @@ -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 From 13efc7b4950a7da185fa0816ac08bce29e92853e Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 24 Dec 2023 01:15:08 +0100 Subject: [PATCH 4/5] Update tests --- tests/unit/test_coverage.py | 12 ++++++++---- tests/unit/test_template.py | 6 ++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_coverage.py b/tests/unit/test_coverage.py index 1a6b3284..ca45b07d 100644 --- a/tests/unit/test_coverage.py +++ b/tests/unit/test_coverage.py @@ -137,7 +137,8 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.5"), - violation_lines=[3], + missing_lines=[3], + added_lines=[1, 3], ) }, ), @@ -173,7 +174,8 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("1"), - violation_lines=[], + missing_lines=[], + added_lines=[4, 5, 6], ) }, ), @@ -205,12 +207,14 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("1"), - violation_lines=[], + missing_lines=[], + added_lines=[4, 5, 6], ), pathlib.Path("codebase/other.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/other.py"), percent_covered=decimal.Decimal("0.5"), - violation_lines=[13], + missing_lines=[13], + added_lines=[10, 13], ), }, ), diff --git a/tests/unit/test_template.py b/tests/unit/test_template.py index 5f409e8f..b62cdafe 100644 --- a/tests/unit/test_template.py +++ b/tests/unit/test_template.py @@ -138,12 +138,14 @@ def test_template_full(): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.5"), - violation_lines=[5], + missing_lines=[5], + added_lines=[5], ), pathlib.Path("codebase/other.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/other.py"), percent_covered=decimal.Decimal("1"), - violation_lines=[], + missing_lines=[], + added_lines=[4, 5, 6], ), }, ) From ce9f98e545a348a861bd84ac93bae68999600afc Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 24 Dec 2023 12:08:09 +0100 Subject: [PATCH 5/5] GitHub's UI fails to display the lines the annotation refers to, so we add it in the message --- coverage_comment/github.py | 14 ++++++++++---- tests/integration/test_github.py | 4 ++-- tests/integration/test_main.py | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 20131aed..4ca3591a 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -10,8 +10,6 @@ from coverage_comment import github_client, log GITHUB_ACTIONS_LOGIN = "github-actions[bot]" -MISSING_COVERAGE_MESSAGE = "Missing coverage" -MISSING_LINES_GROUP_TITLE = "Annotations of lines with missing coverage" class CannotDeterminePR(Exception): @@ -209,16 +207,24 @@ def create_missing_coverage_annotations( annotation_type: The type of annotation to create. Can be either "error" or "warning". annotations: A list of tuples of the form (file, line_start, line_end) """ - send_workflow_command(command="group", command_value=MISSING_LINES_GROUP_TITLE) + send_workflow_command( + command="group", command_value="Annotations of lines with missing coverage" + ) for file, line_start, line_end in annotations: + if line_start == line_end: + message = f"Missing coverage on line {line_start}" + else: + message = f"Missing coverage on lines {line_start}-{line_end}" + send_workflow_command( command=annotation_type, - command_value=MISSING_COVERAGE_MESSAGE, + command_value=message, # This will produce \ paths when running on windows. # GHA doc is unclear whether this is right or not. file=str(file), line=str(line_start), endLine=str(line_end), + title="Missing coverage", ) send_workflow_command(command="endgroup", command_value="") diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 6fb5515f..066a9656 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -351,8 +351,8 @@ def test_annotations(capsys): ) expected = """::group::Annotations of lines with missing coverage -::warning file=codebase/code.py,line=1,endLine=3::Missing coverage -::warning file=codebase/main.py,line=5,endLine=5::Missing coverage +::warning file=codebase/code.py,line=1,endLine=3,title=Missing coverage::Missing coverage on lines 1-3 +::warning file=codebase/main.py,line=5,endLine=5,title=Missing coverage::Missing coverage on line 5 ::endgroup::""" output = capsys.readouterr() assert output.err.strip() == expected diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index 5a076888..17995c01 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -568,7 +568,7 @@ def test_action__pull_request__annotations( git=git, ) expected = """::group::Annotations of lines with missing coverage -::warning file=foo.py,line=12,endLine=12::Missing coverage +::warning file=foo.py,line=12,endLine=12,title=Missing coverage::Missing coverage on line 12 ::endgroup::""" output = capsys.readouterr()