Change of approach: let github compute the diff

Author: ewjoachim

coverage_comment/coverage.py

@@ -286,18 +286,7 @@ def get_diff_coverage_info(
     )
 
 
-def get_added_lines(
-    git: subprocess.Git, base_ref: str
-) -> dict[pathlib.Path, list[int]]:
-    # --unified=0 means we don't get any context lines for chunk, and we
-    # don't merge chunks. This means the headers that describe line number
-    # are always enough to derive what line numbers were added.
-    git.fetch("origin", base_ref, "--depth=1000")
-    diff = git.diff("--unified=0", "FETCH_HEAD...HEAD")
-    return parse_diff_output(diff)
-
-
-def parse_diff_output(diff: str) -> dict[pathlib.Path, list[int]]:
+def get_added_lines(diff: str) -> dict[pathlib.Path, list[int]]:
     current_file: pathlib.Path | None = None
     added_filename_prefix = "+++ b/"
     result: dict[pathlib.Path, list[int]] = {}

coverage_comment/github.py

@@ -254,3 +254,19 @@ def append_to_file(content: str, filepath: pathlib.Path):
 
 def add_job_summary(content: str, github_step_summary: pathlib.Path):
     append_to_file(content=content, filepath=github_step_summary)
+
+
+def get_pr_diff(github: github_client.GitHub, repository: str, pr_number: int) -> str:
+    """
+    Get the diff of a pull request.
+    """
+    return github.repos(repository).pulls(f"{pr_number}.diff").get()
+
+
+def get_branch_diff(
+    github: github_client.GitHub, repository: str, base_branch: str, head_branch: str
+) -> str:
+    """
+    Get the diff of branch.
+    """
+    return github.repos(repository).compare(f"{base_branch}...{head_branch}.diff").get()

coverage_comment/main.py

@@ -132,7 +132,20 @@ def process_pr(
     )
     base_ref = config.GITHUB_BASE_REF or repo_info.default_branch
 
-    added_lines = coverage_module.get_added_lines(git=git, base_ref=base_ref)
+    if config.GITHUB_BRANCH_NAME:
+        diff = github.get_branch_diff(
+            github=gh,
+            repository=config.GITHUB_REPOSITORY,
+            base_branch=base_ref,
+            head_branch=config.GITHUB_BRANCH_NAME,
+        )
+    elif config.GITHUB_PR_NUMBER:
+        diff = github.get_pr_diff(
+            github=gh,
+            repository=config.GITHUB_REPOSITORY,
+            pr_number=config.GITHUB_PR_NUMBER,
+        )
+    added_lines = coverage_module.get_added_lines(diff=diff)
     diff_coverage = coverage_module.get_diff_coverage_info(
         coverage=coverage, added_lines=added_lines
     )

tests/integration/test_coverage.py

@@ -444,3 +444,25 @@ def test_annotations(capsys):
 ::endgroup::"""
     output = capsys.readouterr()
     assert output.err.strip() == expected
+
+
+def test_get_pr_diff(gh, session):
+    session.register("GET", "/repos/foo/bar/pulls/123.diff")(
+        text="diff --git a/foo.py b/foo.py..."
+    )
+
+    result = github.get_pr_diff(github=gh, repository="foo/bar", pr_number=123)
+
+    assert result == "diff --git a/foo.py b/foo.py..."
+
+
+def test_get_branch_diff(gh, session):
+    session.register("GET", "/repos/foo/bar/compare/main...feature.diff")(
+        text="diff --git a/foo.py b/foo.py..."
+    )
+
+    result = github.get_branch_diff(
+        github=gh, repository="foo/bar", base_branch="main", head_branch="feature"
+    )
+
+    assert result == "diff --git a/foo.py b/foo.py..."

tests/integration/test_github.py

@@ -74,8 +74,10 @@ def checker(payload):
         "POST", "/repos/py-cov-action/foobar/issues/2/comments", json=checker
     )(status_code=403)
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     result = main.action(
         config=pull_request_config(
@@ -155,8 +157,10 @@ def checker(payload):
         "POST", "/repos/py-cov-action/foobar/issues/2/comments", json=checker
     )(status_code=403)
 
-    git.register("git fetch origin foo --depth=1000")(stdout=DIFF_STDOUT)
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     result = main.action(
         config=pull_request_config(
@@ -202,8 +206,10 @@ def test_action__pull_request__post_comment(
     # Are there already comments
     session.register("GET", "/repos/py-cov-action/foobar/issues/2/comments")(json=[])
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     comment = None
 
@@ -249,8 +255,11 @@ def test_action__push__non_default_branch(
     session.register("GET", "/repos/py-cov-action/foobar")(
         json={"default_branch": "main", "visibility": "public"}
     )
-    git.register("git fetch origin main --depth=1000")(stdout=DIFF_STDOUT)
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+
+    # What is the diff of the `other` branch
+    session.register("GET", "/repos/py-cov-action/foobar/compare/main...other.diff")(
+        text=DIFF_STDOUT
+    )
 
     payload = json.dumps({"coverage": 30.00})
     # There is an existing badge in this test, allowing to test the coverage evolution
@@ -338,8 +347,10 @@ def test_action__push__non_default_branch__no_pr(
     session.register("GET", "/repos/py-cov-action/foobar")(
         json={"default_branch": "main", "visibility": "public"}
     )
-    git.register("git fetch origin main --depth=1000")(stdout=DIFF_STDOUT)
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the `other` branch
+    session.register("GET", "/repos/py-cov-action/foobar/compare/main...other.diff")(
+        text=DIFF_STDOUT
+    )
 
     payload = json.dumps({"coverage": 30.00})
     # There is an existing badge in this test, allowing to test the coverage evolution
@@ -402,8 +413,10 @@ def test_action__pull_request__force_store_comment(
         "/repos/py-cov-action/foobar/contents/data.json",
     )(text=payload, headers={"content-type": "application/vnd.github.raw+json"})
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     result = main.action(
         config=pull_request_config(FORCE_WORKFLOW_RUN=True, GITHUB_OUTPUT=output_file),
@@ -433,8 +446,10 @@ def test_action__pull_request__post_comment__no_marker(
         "/repos/py-cov-action/foobar/contents/data.json",
     )(status_code=404)
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     result = main.action(
         config=pull_request_config(COMMENT_TEMPLATE="""foo"""),
@@ -458,8 +473,10 @@ def test_action__pull_request__annotations(
         "/repos/py-cov-action/foobar/contents/data.json",
     )(status_code=404)
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     # Who am I
     session.register("GET", "/user")(json={"login": "foo"})
@@ -500,8 +517,10 @@ def test_action__pull_request__post_comment__template_error(
         "/repos/py-cov-action/foobar/contents/data.json",
     )(status_code=404)
 
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT)
+    # What is the diff of the PR
+    session.register("GET", "/repos/py-cov-action/foobar/pulls/2.diff")(
+        text=DIFF_STDOUT
+    )
 
     result = main.action(
         config=pull_request_config(COMMENT_TEMPLATE="""{%"""),

tests/integration/test_main.py

@@ -304,17 +304,6 @@ def test_get_diff_coverage_info(make_coverage_obj, added_lines, update_obj, expe
     assert result == expected
 
 
-def test_get_added_lines(git):
-    diff = (
-        """+++ b/README.md\n@@ -1,2 +1,3 @@\n-# coverage-comment\n+coverage-comment\n"""
-    )
-    git.register("git fetch origin main --depth=1000")()
-    git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=diff)
-    assert coverage.get_added_lines(git=git, base_ref="main") == {
-        pathlib.Path("README.md"): [1, 2, 3]
-    }
-
-
 @pytest.mark.parametrize(
     "line_number_diff_line, expected",
     [
@@ -327,7 +316,7 @@ def test_parse_line_number_diff_line(git, line_number_diff_line, expected):
     assert result == expected
 
 
-def test_parse_diff_output(git):
+def test_get_added_lines(git):
     diff = """diff --git a/action.yml b/action.yml
 deleted file mode 100644
 index 42249d1..0000000
@@ -365,13 +354,13 @@ def test_parse_diff_output(git):
 """
     git.register("git fetch origin main --depth=1000")()
     git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=diff)
-    assert coverage.parse_diff_output(diff=diff) == {
+    assert coverage.get_added_lines(diff=diff) == {
         pathlib.Path("README.md"): [1, 3, 4, 5, 6],
         pathlib.Path("foo.txt"): [1],
     }
 
 
-def test_parse_diff_output__error(git):
+def test_get_added_lines__error(git):
     diff = """
 @@ -0,0 +1,1 @@
 +name: Python Coverage Comment
@@ -381,4 +370,4 @@ def test_parse_diff_output__error(git):
     git.register("git fetch origin main --depth=1000")()
     git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=diff)
     with pytest.raises(ValueError):
-        coverage.parse_diff_output(diff=diff)
+        coverage.get_added_lines(diff=diff)