Repository: py-cov-action/python-coverage-comment-action
Pull request #212 (2023-07-23 09:14:52)

Create annotations based on diff-coverage, not full coverage
This is a revamp of https://github.com/py-cov-action/python-coverage-comment-action/pull/188

I believe that the part that failed was:
https://github.com/py-cov-action/python-coverage-comment-action/pull/188/files#diff-5327f4a45499e33cab8758f61e6b738fc200a07b00c58fc231f37a3142461ac7R194-R207

which I removed completely from this PR, as it wasn't needed.

1/ This PR being created by me, we'll get e2e tests
2/ I've remade the commits and remove the things are are much unrelated. We still have a 1st commit that could be somewhat elsewhere, but we were being really inconsistent in types used, and it was visible when switching between cover and diff-cover, so hopefully it's better now


Patch:
diff --git a/coverage_comment/annotations.py b/coverage_comment/annotations.py
index 9b06524..c6c5c1a 100644
--- a/coverage_comment/annotations.py
+++ b/coverage_comment/annotations.py
@@ -4,15 +4,19 @@ from coverage_comment import github
 MISSING_LINES_GROUP_TITLE = "Annotations of lines with missing coverage"
 
 
-def create_pr_annotations(annotation_type: str, coverage: coverage_module.Coverage):
+def create_pr_annotations(
+    annotation_type: str, diff_coverage: coverage_module.DiffCoverage
+):
     github.send_workflow_command(
         command="group", command_value=MISSING_LINES_GROUP_TITLE
     )
 
-    for filename, file_coverage in coverage.files.items():
-        for missing_line in file_coverage.missing_lines:
+    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=filename, line=missing_line
+                annotation_type=annotation_type,
+                file=file_path,
+                line=missing_line,
             )
 
     github.send_workflow_command(command="endgroup", command_value="")
diff --git a/coverage_comment/coverage.py b/coverage_comment/coverage.py
index 25c1384..38c98a6 100644
--- a/coverage_comment/coverage.py
+++ b/coverage_comment/coverage.py
@@ -31,7 +31,7 @@ class CoverageInfo:
 
 @dataclasses.dataclass
 class FileCoverage:
-    path: str
+    path: pathlib.Path
     executed_lines: list[int]
     missing_lines: list[int]
     excluded_lines: list[int]
@@ -42,12 +42,12 @@ class FileCoverage:
 class Coverage:
     meta: CoverageMetadata
     info: CoverageInfo
-    files: dict[str, FileCoverage]
+    files: dict[pathlib.Path, FileCoverage]
 
 
 @dataclasses.dataclass
 class FileDiffCoverage:
-    path: str
+    path: pathlib.Path
     percent_covered: decimal.Decimal
     violation_lines: list[int]
 
@@ -146,8 +146,8 @@ def extract_info(data) -> Coverage:
             show_contexts=data["meta"]["show_contexts"],
         ),
         files={
-            path: FileCoverage(
-                path=path,
+            pathlib.Path(path): FileCoverage(
+                path=pathlib.Path(path),
                 excluded_lines=file_data["excluded_lines"],
                 executed_lines=file_data["executed_lines"],
                 missing_lines=file_data["missing_lines"],
@@ -235,8 +235,8 @@ def extract_diff_info(data) -> DiffCoverage:
         ),
         num_changed_lines=data["num_changed_lines"],
         files={
-            path: FileDiffCoverage(
-                path=path,
+            pathlib.Path(path): FileDiffCoverage(
+                path=pathlib.Path(path),
                 percent_covered=decimal.Decimal(str(file_data["percent_covered"]))
                 / decimal.Decimal("100"),
                 violation_lines=file_data["violation_lines"],
diff --git a/coverage_comment/github.py b/coverage_comment/github.py
index a4d62eb..5653a18 100644
--- a/coverage_comment/github.py
+++ b/coverage_comment/github.py
@@ -192,11 +192,15 @@ def send_workflow_command(command: str, command_value: str, **kwargs: str) -> No
     )
 
 
-def create_missing_coverage_annotation(annotation_type: str, file: str, line: int):
+def create_missing_coverage_annotation(
+    annotation_type: str, file: pathlib.Path, line: int
+):
     send_workflow_command(
         command=annotation_type,
         command_value=MISSING_COVERAGE_MESSAGE,
-        file=file,
+        # This will produce \ paths when running on windows.
+        # GHA doc is unclear whether this is right or not.
+        file=str(file),
         line=str(line),
     )
 
diff --git a/coverage_comment/main.py b/coverage_comment/main.py
index 74e5a79..beeb107 100644
--- a/coverage_comment/main.py
+++ b/coverage_comment/main.py
@@ -73,15 +73,19 @@ def action(
 
     if event_name in {"pull_request", "push"}:
         coverage = coverage_module.get_coverage_info(merge=config.MERGE_COVERAGE_FILES)
+
         if event_name == "pull_request":
+            diff_coverage = coverage_module.get_diff_coverage_info(
+                base_ref=config.GITHUB_BASE_REF
+            )
             if config.ANNOTATE_MISSING_LINES:
                 annotations.create_pr_annotations(
-                    annotation_type=config.ANNOTATION_TYPE, coverage=coverage
+                    annotation_type=config.ANNOTATION_TYPE, diff_coverage=diff_coverage
                 )
-
             return generate_comment(
                 config=config,
                 coverage=coverage,
+                diff_coverage=diff_coverage,
                 github_session=github_session,
             )
         else:
@@ -105,15 +109,13 @@ def action(
 def generate_comment(
     config: settings.Config,
     coverage: coverage_module.Coverage,
+    diff_coverage: coverage_module.DiffCoverage,
     github_session: httpx.Client,
 ) -> int:
     log.info("Generating comment for PR")
 
     gh = github_client.GitHub(session=github_session)
 
-    diff_coverage = coverage_module.get_diff_coverage_info(
-        base_ref=config.GITHUB_BASE_REF
-    )
     previous_coverage_data_file = storage.get_datafile_contents(
         github=gh,
         repository=config.GITHUB_REPOSITORY,


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 378b671..4eee0c1 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -153,8 +153,8 @@ def coverage_obj():
             missing_branches=1,
         ),
         files={
-            "codebase/code.py": coverage_module.FileCoverage(
-                path="codebase/code.py",
+            pathlib.Path("codebase/code.py"): coverage_module.FileCoverage(
+                path=pathlib.Path("codebase/code.py"),
                 executed_lines=[1, 2, 5, 6, 9],
                 missing_lines=[7, 9],
                 excluded_lines=[],
@@ -195,8 +195,8 @@ def coverage_obj_no_branch():
             missing_branches=None,
         ),
         files={
-            "codebase/code.py": coverage_module.FileCoverage(
-                path="codebase/code.py",
+            pathlib.Path("codebase/code.py"): coverage_module.FileCoverage(
+                path=pathlib.Path("codebase/code.py"),
                 executed_lines=[1, 2, 5, 6, 9],
                 missing_lines=[7],
                 excluded_lines=[],
@@ -217,77 +217,40 @@ def coverage_obj_no_branch():
 
 
 @pytest.fixture
-def coverage_obj_many_missing_lines():
-    return coverage_module.Coverage(
-        meta=coverage_module.CoverageMetadata(
-            version="1.2.3",
-            timestamp=datetime.datetime(2000, 1, 1),
-            branch_coverage=True,
-            show_contexts=False,
-        ),
-        info=coverage_module.CoverageInfo(
-            covered_lines=7,
-            num_statements=10,
-            percent_covered=decimal.Decimal("0.8"),
-            missing_lines=12,
-            excluded_lines=0,
-            num_branches=2,
-            num_partial_branches=1,
-            covered_branches=1,
-            missing_branches=1,
-        ),
+def diff_coverage_obj():
+    return coverage_module.DiffCoverage(
+        total_num_lines=5,
+        total_num_violations=1,
+        total_percent_covered=decimal.Decimal("0.8"),
+        num_changed_lines=39,
         files={
-            "codebase/main.py": coverage_module.FileCoverage(
-                path="codebase/main.py",
-                executed_lines=[1, 2, 5, 6, 9],
-                missing_lines=[3, 7, 13, 21, 123],
-                excluded_lines=[],
-                info=coverage_module.CoverageInfo(
-                    covered_lines=5,
-                    num_statements=10,
-                    percent_covered=decimal.Decimal("0.5"),
-                    missing_lines=5,
-                    excluded_lines=0,
-                    num_branches=2,
-                    num_partial_branches=1,
-                    covered_branches=1,
-                    missing_branches=1,
-                ),
-            ),
-            "codebase/caller.py": coverage_module.FileCoverage(
-                path="codebase/caller.py",
-                executed_lines=[1, 2, 5],
-                missing_lines=[13, 21, 212],
-                excluded_lines=[],
-                info=coverage_module.CoverageInfo(
-                    covered_lines=3,
-                    num_statements=6,
-                    percent_covered=decimal.Decimal("0.5"),
-                    missing_lines=3,
-                    excluded_lines=0,
-                    num_branches=2,
-                    num_partial_branches=1,
-                    covered_branches=1,
-                    missing_branches=1,
-                ),
-            ),
+            pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage(
+                path=pathlib.Path("codebase/code.py"),
+                percent_covered=decimal.Decimal("0.8"),
+                violation_lines=[7, 9],
+            )
         },
     )
 
 
 @pytest.fixture
-def diff_coverage_obj():
+def diff_coverage_obj_many_missing_lines():
     return coverage_module.DiffCoverage(
         total_num_lines=5,
         total_num_violations=1,
         total_percent_covered=decimal.Decimal("0.8"),
         num_changed_lines=39,
         files={
-            "codebase/code.py": coverage_module.FileDiffCoverage(
-                path="codebase/code.py",
+            pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage(
+                path=pathlib.Path("codebase/code.py"),
                 percent_covered=decimal.Decimal("0.8"),
                 violation_lines=[7, 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],
+            ),
         },
     )
 
diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py
index d0ec1a9..f43c20e 100644
--- a/tests/integration/test_github.py
+++ b/tests/integration/test_github.py
@@ -1,3 +1,5 @@
+import pathlib
+
 import pytest
 
 from coverage_comment import github
@@ -315,7 +317,7 @@ def test_send_workflow_command(capsys):
 
 def test_create_missing_coverage_annotation(capsys):
     github.create_missing_coverage_annotation(
-        annotation_type="warning", file="test.py", line=42
+        annotation_type="warning", file=pathlib.Path("test.py"), line=42
     )
     output = capsys.readouterr()
     assert (
@@ -326,7 +328,7 @@ def test_create_missing_coverage_annotation(capsys):
 
 def test_create_missing_coverage_annotation__annotation_type(capsys):
     github.create_missing_coverage_annotation(
-        annotation_type="error", file="test.py", line=42
+        annotation_type="error", file=pathlib.Path("test.py"), line=42
     )
     output = capsys.readouterr()
     assert (
diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py
index 6469076..8481801 100644
--- a/tests/integration/test_main.py
+++ b/tests/integration/test_main.py
@@ -3,6 +3,7 @@ import json
 import os
 import pathlib
 import subprocess
+import uuid
 
 import pytest
 
@@ -18,8 +19,10 @@ def in_integration_env(integration_env, integration_dir):
 
 
 @pytest.fixture
-def integration_dir(tmpdir_factory):
-    return tmpdir_factory.mktemp("integration_test")
+def integration_dir(tmp_path: pathlib.Path):
+    test_dir = tmp_path / "integration_test"
+    test_dir.mkdir()
+    return test_dir
 
 
 @pytest.fixture
@@ -42,7 +45,7 @@ def write_file(file_path):
 def run_coverage(file_path, integration_dir):
     def _(*variables):
         subprocess.check_call(
-            ["coverage", "run", "--parallel", file_path.basename],
+            ["coverage", "run", "--parallel", file_path.name],
             cwd=integration_dir,
             env=os.environ | dict.fromkeys(variables, "1"),
         )
@@ -51,33 +54,44 @@ def run_coverage(file_path, integration_dir):
 
 
 @pytest.fixture
-def integration_env(integration_dir, write_file, run_coverage):
+def commit(integration_dir):
+    def _():
+        subprocess.check_call(
+            ["git", "add", "."],
+            cwd=integration_dir,
+        )
+        subprocess.check_call(
+            ["git", "commit", "-m", str(uuid.uuid4())],
+            cwd=integration_dir,
+            env={
+                "GIT_AUTHOR_NAME": "foo",
+                "GIT_AUTHOR_EMAIL": "foo",
+                "GIT_COMMITTER_NAME": "foo",
+                "GIT_COMMITTER_EMAIL": "foo",
+                "GIT_CONFIG_GLOBAL": "/dev/null",
+                "GIT_CONFIG_SYSTEM": "/dev/null",
+            },
+        )
+
+    return _
+
+
+@pytest.fixture
+def integration_env(integration_dir, write_file, run_coverage, commit):
     subprocess.check_call(["git", "init", "-b", "main"], cwd=integration_dir)
     # diff coverage reads the "origin/{...}" branch so we simulate an origin remote
     subprocess.check_call(["git", "remote", "add", "origin", "."], cwd=integration_dir)
     write_file("A", "B")
 
-    subprocess.check_call(
-        ["git", "add", "."],
-        cwd=integration_dir,
-    )
-    subprocess.check_call(
-        ["git", "commit", "-m", "commit"],
-        cwd=integration_dir,
-        env={
-            "GIT_AUTHOR_NAME": "foo",
-            "GIT_AUTHOR_EMAIL": "foo",
-            "GIT_COMMITTER_NAME": "foo",
-            "GIT_COMMITTER_EMAIL": "foo",
-            "GIT_CONFIG_GLOBAL": "/dev/null",
-            "GIT_CONFIG_SYSTEM": "/dev/null",
-        },
-    )
+    commit()
     subprocess.check_call(
         ["git", "switch", "-c", "branch"],
         cwd=integration_dir,
     )
-    write_file("A", "B", "C")
+
+    write_file("A", "B", "C", "D")
+    commit()
+
     run_coverage("A", "C")
     subprocess.check_call(["git", "fetch", "origin"], cwd=integration_dir)
 
@@ -128,10 +142,10 @@ def test_action__pull_request__store_comment(
     assert comment == comment_file
     assert comment == summary_file.read_text()
     assert "No coverage data of the default branch was found for comparison" in comment
-    assert "The coverage rate is `85.71%`" in comment
-    assert "`100%` of new lines are covered." in comment
+    assert "The coverage rate is `77.77%`" in comment
+    assert "`75%` of new lines are covered." in comment
     assert (
-        "### foo.py\n`100%` of new lines are covered (`85.71%` of the complete file)"
+        "### foo.py\n`75%` of new lines are covered (`77.77%` of the complete file)"
         in comment
     )
     assert (
@@ -188,7 +202,7 @@ def test_action__pull_request__post_comment(
     assert result == 0
 
     assert not pathlib.Path("python-coverage-comment-action.txt").exists()
-    assert "The coverage rate went from `30%` to `85.71%` :arrow_up:" in comment
+    assert "The coverage rate went from `30%` to `77.77%` :arrow_up:" in comment
     assert comment == summary_file.read_text()
 
     expected_output = "COMMENT_FILE_WRITTEN=false\n"
@@ -267,7 +281,7 @@ def test_action__pull_request__annotations(
         git=None,
     )
     expected = """::group::Annotations of lines with missing coverage
-::warning file=foo.py,line=6::This line has no coverage
+::warning file=foo.py,line=12::This line has no coverage
 ::endgroup::"""
     output = capsys.readouterr()
 
@@ -320,7 +334,7 @@ def test_action__push__default_branch(
     )
     session.register(
         "GET",
-        "https://img.shields.io/static/v1?label=Coverage&message=85%25&color=orange",
+        "https://img.shields.io/static/v1?label=Coverage&message=77%25&color=orange",
     )(text="<this is a svg badge>")
 
     git.register("git branch --show-current")(stdout="foo")
@@ -383,7 +397,7 @@ def test_action__push__default_branch__private(
     )
     session.register(
         "GET",
-        "https://img.shields.io/static/v1?label=Coverage&message=85%25&color=orange",
+        "https://img.shields.io/static/v1?label=Coverage&message=77%25&color=orange",
     )(text="<this is a svg badge>")
 
     git.register("git branch --show-current")(stdout="foo")
diff --git a/tests/unit/test_annotations.py b/tests/unit/test_annotations.py
index d5d6434..5dfcde7 100644
--- a/tests/unit/test_annotations.py
+++ b/tests/unit/test_annotations.py
@@ -1,8 +1,10 @@
 from coverage_comment import annotations
 
 
-def test_annotations(coverage_obj, capsys):
-    annotations.create_pr_annotations(annotation_type="warning", coverage=coverage_obj)
+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
@@ -12,20 +14,18 @@ def test_annotations(coverage_obj, capsys):
     assert output.err.strip() == expected
 
 
-def test_annotations_several_files(coverage_obj_many_missing_lines, capsys):
+def test_annotations_several_files(diff_coverage_obj_many_missing_lines, capsys):
     annotations.create_pr_annotations(
-        annotation_type="notice", coverage=coverage_obj_many_missing_lines
+        annotation_type="notice", diff_coverage=diff_coverage_obj_many_missing_lines
     )
 
     expected = """::group::Annotations of lines with missing coverage
-::notice file=codebase/main.py,line=3::This line has no coverage
-::notice file=codebase/main.py,line=7::This line has no coverage
-::notice file=codebase/main.py,line=13::This line has no coverage
-::notice file=codebase/main.py,line=21::This line has no coverage
-::notice file=codebase/main.py,line=123::This line has no coverage
-::notice file=codebase/caller.py,line=13::This line has no coverage
-::notice file=codebase/caller.py,line=21::This line has no coverage
-::notice file=codebase/caller.py,line=212::This line has no 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