Merge pull request #211 from py-cov-action/comment-improvements

README.md

@@ -322,6 +322,15 @@ jobs:
     # Same with orange. Below is red.
     MINIMUM_ORANGE: 70
 
+    # Maximum number of files to display in the comment. If there are more
+    # files than this number, they will only appear in the workflow summary.
+    # The selected files are the ones with the most new uncovered lines. The
+    # closer this number gets to 35, the higher the risk that it reaches
+    # GitHub's maximum comment size limit of 65536 characters. If you want
+    # more files, you may need to use a custom comment template (see below).
+    # (Feel free to open an issue.)
+    MAX_FILES_IN_COMMENT: 25
+
     # If true, will run `coverage combine` before reading the `.coverage` file.
     MERGE_COVERAGE_FILES: false
 
@@ -583,6 +592,14 @@ to use the svg badge directly, and not the `shields.io` URL.
 
 ## Upgrading from v2 to v3
 
-- When upgrading, we change the location and format where the coverage
-  data is kept. Pull request that have not been re-based may be displaying
-  slightly wrong information.
+When upgrading, we change the location and format where the coverage
+data is kept. Pull request that have not been re-based may be displaying
+slightly wrong information.
+
+## New comment format starting with 3.19
+
+Starting with 3.19, the format for the Pull Request changed to a table
+with badges. We've been iterating a lot on the new format.
+It's perfectly ok if you preferred the old format. In that case, see
+#335 for instructions on how to emulate the old format using
+`COMMENT_TEMPLATE`.

action.yml

@@ -70,6 +70,17 @@ inputs:
       the badge will be orange. Otherwise it will be red.
     default: 70
     required: false
+  MAX_FILES_IN_COMMENT:
+    description: >
+      Maximum number of files to display in the comment. If there are more
+      files than this number, they will only appear in the workflow summary.
+      The selected files are the ones with the most new uncovered lines. The
+      closer this number gets to 35, the higher the risk that it reaches
+      GitHub's maximum comment size limit of 65536 characters. If you want
+      more files, you may need to use a custom comment template.
+      (Feel free to open an issue.)
+    default: 25
+    required: false
   MERGE_COVERAGE_FILES:
     description: >
       If true, will run `coverage combine` before reading the `.coverage` file.

coverage_comment/badge.py

@@ -24,6 +24,19 @@ def get_badge_color(
         return "red"
 
 
+def get_evolution_badge_color(
+    delta: decimal.Decimal | int,
+    up_is_good: bool = True,
+    neutral_color: str = "lightgrey",
+) -> str:
+    if delta == 0:
+        return neutral_color
+    elif (delta > 0) is up_is_good:
+        return "brightgreen"
+    else:
+        return "red"
+
+
 def compute_badge_endpoint_data(
     line_rate: decimal.Decimal,
     color: str,
@@ -53,6 +66,15 @@ def compute_badge_image(
     ).text
 
 
+def get_static_badge_url(label: str, message: str, color: str) -> str:
+    if not color or not message:
+        raise ValueError("color and message are required")
+    code = "-".join(
+        e.replace("_", "__").replace("-", "--") for e in (label, message, color) if e
+    )
+    return "https://img.shields.io/badge/" + urllib.parse.quote(f"{code}.svg")
+
+
 def get_endpoint_url(endpoint_url: str) -> str:
     return f"https://img.shields.io/endpoint?url={endpoint_url}"
 

coverage_comment/coverage.py

@@ -10,6 +10,9 @@
 from coverage_comment import log, subprocess
 
 
+# The dataclasses in this module are accessible in the template, which is overridable by the user.
+# As a coutesy, we should do our best to keep the existing fields for backward compatibility,
+# and if we really can't and can't add properties, at least bump the major version.
 @dataclasses.dataclass
 class CoverageMetadata:
     version: str
@@ -57,13 +60,17 @@ class Coverage:
 class FileDiffCoverage:
     path: pathlib.Path
     percent_covered: decimal.Decimal
-    missing_lines: list[int]
+    covered_statements: list[int]
+    missing_statements: list[int]
+    added_statements: list[int]
+    # Added lines tracks all the lines that were added in the diff, not just
+    # the statements (so it includes comments, blank lines, etc.)
     added_lines: list[int]
 
     # for backward compatibility
     @property
     def violation_lines(self) -> list[int]:
-        return self.missing_lines
+        return self.missing_statements
 
 
 @dataclasses.dataclass
@@ -188,10 +195,8 @@ def extract_info(data: dict, coverage_path: pathlib.Path) -> Coverage:
                     covered_lines=file_data["summary"]["covered_lines"],
                     num_statements=file_data["summary"]["num_statements"],
                     percent_covered=compute_coverage(
-                        file_data["summary"]["covered_lines"]
-                        + file_data["summary"].get("covered_branches", 0),
-                        file_data["summary"]["num_statements"]
-                        + file_data["summary"].get("num_branches", 0),
+                        file_data["summary"]["covered_lines"],
+                        file_data["summary"]["num_statements"],
                     ),
                     missing_lines=file_data["summary"]["missing_lines"],
                     excluded_lines=file_data["summary"]["excluded_lines"],
@@ -209,10 +214,8 @@ def extract_info(data: dict, coverage_path: pathlib.Path) -> Coverage:
             covered_lines=data["totals"]["covered_lines"],
             num_statements=data["totals"]["num_statements"],
             percent_covered=compute_coverage(
-                data["totals"]["covered_lines"]
-                + data["totals"].get("covered_branches", 0),
-                data["totals"]["num_statements"]
-                + data["totals"].get("num_branches", 0),
+                data["totals"]["covered_lines"],
+                data["totals"]["num_statements"],
             ),
             missing_lines=data["totals"]["missing_lines"],
             excluded_lines=data["totals"]["excluded_lines"],
@@ -245,9 +248,9 @@ def get_diff_coverage_info(
 
         missing = set(file.missing_lines) & set(added_lines_for_file)
         count_missing = len(missing)
-        # Even partially covered lines are considered as covered, no line
-        # appears in both counts
-        count_total = count_executed + count_missing
+
+        added = executed | missing
+        count_total = len(added)
 
         total_num_lines += count_total
         total_num_violations += count_missing
@@ -259,7 +262,9 @@ def get_diff_coverage_info(
         files[path] = FileDiffCoverage(
             path=path,
             percent_covered=percent_covered,
-            missing_lines=sorted(missing),
+            covered_statements=sorted(executed),
+            missing_statements=sorted(missing),
+            added_statements=sorted(added),
             added_lines=added_lines_for_file,
         )
     final_percentage = compute_coverage(

coverage_comment/diff_grouper.py

@@ -0,0 +1,40 @@
+from __future__ import annotations
+
+from collections.abc import Iterable
+
+from coverage_comment import coverage as coverage_module
+from coverage_comment import groups
+
+MAX_ANNOTATION_GAP = 3
+
+
+def get_diff_missing_groups(
+    coverage: coverage_module.Coverage,
+    diff_coverage: coverage_module.DiffCoverage,
+) -> Iterable[groups.Group]:
+    for path, diff_file in diff_coverage.files.items():
+        coverage_file = coverage.files[path]
+
+        # 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,
+        }
+        # 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 groups.compute_contiguous_groups(
+            values=diff_file.missing_statements,
+            separators=separators,
+            joiners=joiners,
+            max_gap=MAX_ANNOTATION_GAP,
+        ):
+            yield groups.Group(
+                file=path,
+                line_start=start,
+                line_end=end,
+            )

coverage_comment/files.py

@@ -108,10 +108,18 @@ def compute_datafile(
     )
 
 
-def parse_datafile(contents) -> decimal.Decimal:
-    return decimal.Decimal(str(json.loads(contents)["coverage"])) / decimal.Decimal(
+def parse_datafile(contents) -> tuple[coverage.Coverage | None, decimal.Decimal]:
+    file_contents = json.loads(contents)
+    coverage_rate = decimal.Decimal(str(file_contents["coverage"])) / decimal.Decimal(
         "100"
     )
+    try:
+        return coverage.extract_info(
+            data=file_contents["raw_data"],
+            coverage_path=pathlib.Path(file_contents["coverage_path"]),
+        ), coverage_rate
+    except KeyError:
+        return None, coverage_rate
 
 
 class ImageURLs(TypedDict):

coverage_comment/annotations.py → coverage_comment/groups.py

@@ -4,22 +4,17 @@
 import functools
 import itertools
 import pathlib
-from collections.abc import Iterable
-
-from coverage_comment import coverage as coverage_module
-
-MAX_ANNOTATION_GAP = 3
 
 
 @dataclasses.dataclass(frozen=True)
-class Annotation:
+class Group:
     file: pathlib.Path
     line_start: int
     line_end: int
 
 
 def compute_contiguous_groups(
-    values: list[int], separators: set[int], joiners: set[int]
+    values: list[int], separators: set[int], joiners: set[int], max_gap: int
 ) -> list[tuple[int, int]]:
     """
     Given a list of (sorted) values, a list of separators and a list of
@@ -28,8 +23,8 @@ def compute_contiguous_groups(
 
     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"
+    enclosed are small gaps (<= max_gap values after removing all joiners)
+    where no line is a "separator"
     """
     contiguous_groups: list[tuple[int, int]] = []
     for _, contiguous_group in itertools.groupby(
@@ -55,7 +50,7 @@ def reducer(
 
         gap = set(range(last_end + 1, next_start)) - joiners
 
-        gap_is_small = len(gap) <= MAX_ANNOTATION_GAP
+        gap_is_small = len(gap) <= max_gap
         gap_contains_separators = gap & separators
 
         if gap_is_small and not gap_contains_separators:
@@ -66,34 +61,3 @@ def reducer(
         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]
-
-        # 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,
-        }
-        # 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(
-            values=diff_file.missing_lines,
-            separators=separators,
-            joiners=joiners,
-        ):
-            yield Annotation(
-                file=path,
-                line_start=start,
-                line_end=end,
-            )

coverage_comment/main.py

@@ -8,12 +8,10 @@
 import httpx
 
 from coverage_comment import activity as activity_module
-from coverage_comment import (
-    annotations as annotations_module,
-)
 from coverage_comment import (
     comment_file,
     communication,
+    diff_grouper,
     files,
     github,
     github_client,
@@ -151,16 +149,33 @@ def process_pr(
             branch=config.FINAL_COVERAGE_DATA_BRANCH,
         )
 
-    previous_coverage = None
+    previous_coverage, previous_coverage_rate = None, None
     if previous_coverage_data_file:
-        previous_coverage = files.parse_datafile(contents=previous_coverage_data_file)
+        previous_coverage, previous_coverage_rate = files.parse_datafile(
+            contents=previous_coverage_data_file
+        )
 
     marker = template.get_marker(marker_id=config.SUBPROJECT_ID)
+
+    files_info, count_files = template.select_files(
+        coverage=coverage,
+        diff_coverage=diff_coverage,
+        previous_coverage=previous_coverage,
+        max_files=config.MAX_FILES_IN_COMMENT,
+    )
     try:
         comment = template.get_comment_markdown(
             coverage=coverage,
             diff_coverage=diff_coverage,
-            previous_coverage_rate=previous_coverage,
+            previous_coverage=previous_coverage,
+            previous_coverage_rate=previous_coverage_rate,
+            files=files_info,
+            count_files=count_files,
+            max_files=config.MAX_FILES_IN_COMMENT,
+            minimum_green=config.MINIMUM_GREEN,
+            minimum_orange=config.MINIMUM_ORANGE,
+            repo_name=config.GITHUB_REPOSITORY,
+            pr_number=config.GITHUB_PR_NUMBER,
             base_template=template.read_template_file("comment.md.j2"),
             custom_template=config.COMMENT_TEMPLATE,
             pr_targets_default_branch=pr_targets_default_branch,
@@ -203,7 +218,7 @@ def process_pr(
             pr_number = None
 
     if pr_number is not None and config.ANNOTATE_MISSING_LINES:
-        annotations = annotations_module.group_annotations(
+        annotations = diff_grouper.get_diff_missing_groups(
             coverage=coverage, diff_coverage=diff_coverage
         )
         github.create_missing_coverage_annotations(

coverage_comment/settings.py

@@ -60,6 +60,7 @@ class Config:
     MERGE_COVERAGE_FILES: bool = False
     ANNOTATE_MISSING_LINES: bool = False
     ANNOTATION_TYPE: str = "warning"
+    MAX_FILES_IN_COMMENT: int = 25
     VERBOSE: bool = False
     # Only for debugging, not exposed in the action:
     FORCE_WORKFLOW_RUN: bool = False