Merge pull request #343 from py-cov-action/template-fixes-round-2

Author: ewjoachim

coverage_comment/template.py

@@ -108,14 +108,6 @@ class FileInfo:
     diff: coverage_module.FileDiffCoverage | None
     previous: coverage_module.FileCoverage | None
 
-    @property
-    def new_missing_lines(self) -> list[int]:
-        missing_lines = set(self.coverage.missing_lines)
-        if self.previous:
-            missing_lines -= set(self.previous.missing_lines)
-
-        return sorted(missing_lines)
-
 
 def get_comment_markdown(
     *,
@@ -209,7 +201,7 @@ def select_files(
             diff=diff_coverage_file,
             previous=previous_coverage_file,
         )
-        has_diff = bool(diff_coverage_file)
+        has_diff = bool(diff_coverage_file and diff_coverage_file.added_statements)
         has_evolution_from_previous = (
             previous_coverage_file.info != coverage_file.info
             if previous_coverage_file
@@ -220,12 +212,31 @@ def select_files(
             files.append(file_info)
 
     count_files = len(files)
-    files = sorted(files, key=lambda x: len(x.new_missing_lines), reverse=True)
+    files = sorted(files, key=sort_order, reverse=True)
     if max_files is not None:
         files = files[:max_files]
     return sorted(files, key=lambda x: x.path), count_files
 
 
+def sort_order(file_info: FileInfo) -> tuple[int, int, int]:
+    """
+    Sort order for files:
+    1. Files with the most new missing lines
+    2. Files with the most added lines (from the diff)
+    3. Files with the most new executed lines (including not in the diff)
+    """
+    new_missing_lines = len(file_info.coverage.missing_lines)
+    if file_info.previous:
+        new_missing_lines -= len(file_info.previous.missing_lines)
+
+    added_statements = len(file_info.diff.added_statements) if file_info.diff else 0
+    new_covered_lines = len(file_info.coverage.executed_lines)
+    if file_info.previous:
+        new_covered_lines -= len(file_info.previous.executed_lines)
+
+    return abs(new_missing_lines), added_statements, abs(new_covered_lines)
+
+
 def get_readme_markdown(
     is_public: bool,
     readme_url: str,

coverage_comment/template_files/comment.md.j2

@@ -244,8 +244,7 @@ _This PR does not seem to contain any modification to coverable code._
 
 {%- if max_files and count_files > max_files %}
 
-> [!NOTE]
-> The report is truncated to {{ max_files }} files out of {{ count_files }}. To see the full report, please visit the workflow summary page.
+_The report is truncated to {{ max_files }} files out of {{ count_files }}. To see the full report, please visit the workflow summary page._
 
 {% endif %}
 

tests/conftest.py

@@ -291,10 +291,13 @@ def _(code: str, has_branches: bool = True) -> coverage_module.Coverage:
         )
         line_number = 0
         # (we start at 0 because the first line will be empty for readabilty)
-        for line in code.splitlines()[1:]:
+        for line in code.splitlines():
             line = line.strip()
+            if not line:
+                continue
             if line.startswith("# file: "):
                 current_file = pathlib.Path(line.split("# file: ")[1])
+                line_number = 0
                 continue
             assert current_file, (line, current_file, code)
             line_number += 1
@@ -383,8 +386,10 @@ def _(code: str) -> tuple[coverage_module.Coverage, coverage_module.DiffCoverage
         current_file = None
         # (we start at 0 because the first line will be empty for readabilty)
         line_number = 0
-        for line in code.splitlines()[1:]:
+        for line in code.splitlines():
             line = line.strip()
+            if not line:
+                continue
             if line.startswith("# file: "):
                 new_code += line + "\n"
                 current_file = pathlib.Path(line.split("# file: ")[1])

tests/unit/test_diff_grouper.py

@@ -24,8 +24,7 @@ def test_group_annotations_more_files(
     )
 
     assert list(result) == [
-        groups.Group(file=pathlib.Path("codebase/code.py"), line_start=6, line_end=8),
-        groups.Group(
-            file=pathlib.Path("codebase/other.py"), line_start=17, line_end=17
-        ),
+        groups.Group(file=pathlib.Path("codebase/code.py"), line_start=5, line_end=8),
+        groups.Group(file=pathlib.Path("codebase/other.py"), line_start=1, line_end=1),
+        groups.Group(file=pathlib.Path("codebase/other.py"), line_start=3, line_end=5),
     ]

tests/unit/test_template.py

@@ -639,6 +639,30 @@ def test_get_marker(marker_id, result):
             3,
             id="truncated_and_ordered",
         ),
+        pytest.param(
+            """
+            # file: a.py
+            1 covered
+            # file: c.py
+            1 covered
+            # file: b.py
+            1 covered
+            """,
+            """
+            # file: a.py
+            1
+            2 covered
+            # file: c.py
+            + 1 covered
+            # file: b.py
+            1 covered
+            1 covered
+            """,
+            2,
+            ["b.py", "c.py"],
+            2,
+            id="truncated_and_ordered_sort_order_advanced",
+        ),
         pytest.param(
             """
             # file: a.py
@@ -703,6 +727,90 @@ def test_select_files__no_previous(
     assert total == 1
 
 
+@pytest.mark.parametrize(
+    "previous_code, current_code_and_diff, expected_new_missing, expected_added, expected_new_covered",
+    [
+        pytest.param(
+            """
+            # file: a.py
+            1 covered
+            2 missing
+            """,
+            """
+            # file: a.py
+            + 1
+            2 covered
+            + 3 missing
+            + 4 missing
+            + 5 covered
+            """,
+            1,
+            3,
+            1,
+            id="added_code",
+        ),
+        pytest.param(
+            """
+            # file: a.py
+            1 covered
+            2 missing
+            3 covered
+            4 missing
+            """,
+            """
+            # file: a.py
+            + 1 missing
+            """,
+            1,
+            1,
+            2,
+            id="removed_code",
+        ),
+    ],
+)
+def test_sort_order(
+    make_coverage_and_diff,
+    make_coverage,
+    previous_code,
+    current_code_and_diff,
+    expected_new_missing,
+    expected_added,
+    expected_new_covered,
+):
+    previous_cov = make_coverage(previous_code)
+    cov, diff_cov = make_coverage_and_diff(current_code_and_diff)
+    path = pathlib.Path("a.py")
+    file_info = template.FileInfo(
+        path=path,
+        coverage=cov.files[path],
+        diff=diff_cov.files[path],
+        previous=previous_cov.files[path],
+    )
+    new_missing, added, new_covered = template.sort_order(file_info=file_info)
+    assert new_missing == expected_new_missing
+    assert added == expected_added
+    assert new_covered == expected_new_covered
+
+
+def test_sort_order__none(make_coverage):
+    cov = make_coverage(
+        """
+        # file: a.py
+        1 covered
+        """
+    )
+    file_info = template.FileInfo(
+        path=pathlib.Path("a.py"),
+        coverage=cov.files[pathlib.Path("a.py")],
+        diff=None,
+        previous=None,
+    )
+    new_missing, added, new_covered = template.sort_order(file_info=file_info)
+    assert new_missing == 0
+    assert added == 0
+    assert new_covered == 1
+
+
 def test_get_readme_markdown():
     result = template.get_readme_markdown(
         is_public=True,