From bacac399e2369b129fe2c987fb1ed8e0d52bf7fc Mon Sep 17 00:00:00 2001 From: Joe Meissler Date: Sun, 20 Apr 2025 19:41:50 -0700 Subject: [PATCH 1/5] use three dot notation to calculate diff --- coverage_comment/coverage.py | 2 +- tests/integration/conftest.py | 101 +++++++++++++++++++++++++++++ tests/integration/test_coverage.py | 32 +++++++++ tests/integration/test_main.py | 18 ++--- tests/unit/test_coverage.py | 6 +- 5 files changed, 146 insertions(+), 13 deletions(-) create mode 100644 tests/integration/conftest.py create mode 100644 tests/integration/test_coverage.py diff --git a/coverage_comment/coverage.py b/coverage_comment/coverage.py index 591e9de9..2cb4699a 100644 --- a/coverage_comment/coverage.py +++ b/coverage_comment/coverage.py @@ -293,7 +293,7 @@ def get_added_lines( # 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", "--", ".") + diff = git.diff("--unified=0", "FETCH_HEAD...HEAD") return parse_diff_output(diff) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py new file mode 100644 index 00000000..d6aace95 --- /dev/null +++ b/tests/integration/conftest.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +import os +import pathlib +import subprocess +import uuid + +import pytest + + +@pytest.fixture +def in_integration_env(integration_env, integration_dir): + curdir = os.getcwd() + os.chdir(integration_dir) + yield integration_dir + os.chdir(curdir) + + +@pytest.fixture +def integration_dir(tmp_path: pathlib.Path): + test_dir = tmp_path / "integration_test" + test_dir.mkdir() + return test_dir + + +@pytest.fixture +def file_path(integration_dir): + return integration_dir / "foo.py" + + +@pytest.fixture +def write_file(file_path): + def _(*variables): + content = "import os" + for i, var in enumerate(variables): + content += f"""\nif os.environ.get("{var}"):\n {i}\n""" + file_path.write_text(content, encoding="utf8") + + return _ + + +@pytest.fixture +def run_coverage(file_path, integration_dir): + def _(*variables): + subprocess.check_call( + ["coverage", "run", "--parallel", file_path.name], + cwd=integration_dir, + env=os.environ | dict.fromkeys(variables, "1"), + ) + + return _ + + +@pytest.fixture +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, request): + 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") + commit() + + add_branch_mark = request.node.get_closest_marker("add_branches") + for additional_branch in add_branch_mark.args if add_branch_mark else []: + subprocess.check_call( + ["git", "switch", "-c", additional_branch], + cwd=integration_dir, + ) + + subprocess.check_call( + ["git", "switch", "-c", "branch"], + cwd=integration_dir, + ) + + write_file("A", "B", "C", "D") + commit() + + run_coverage("A", "C") + subprocess.check_call(["git", "fetch", "origin"], cwd=integration_dir) diff --git a/tests/integration/test_coverage.py b/tests/integration/test_coverage.py new file mode 100644 index 00000000..8722bd7e --- /dev/null +++ b/tests/integration/test_coverage.py @@ -0,0 +1,32 @@ +from __future__ import annotations + +import subprocess + +from coverage_comment import coverage +from coverage_comment import subprocess as subprocess_module + + +def test_get_added_lines( + commit, file_path, integration_dir, in_integration_env, write_file +): + """ + Lines added in the base_ref should not appear as added in HEAD + """ + git = subprocess_module.Git() + relative_file_path = file_path.relative_to(integration_dir) + + def _check_added_lines(): + added_lines = coverage.get_added_lines(git, "main") + + assert added_lines == { + relative_file_path: list(range(7, 13)) # Line numbers start at 1 + } + + _check_added_lines() + subprocess.check_call(["git", "switch", "main"], cwd=integration_dir) + write_file("E", "F") + commit() + subprocess.check_call(["git", "push", "origin", "main"], cwd=integration_dir) + subprocess.check_call(["git", "switch", "branch"], cwd=integration_dir) + + _check_added_lines() diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index f925e436..fd3d58cb 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -172,7 +172,7 @@ def checker(payload): )(status_code=403) git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) result = main.action( config=pull_request_config( @@ -253,7 +253,7 @@ def checker(payload): )(status_code=403) git.register("git fetch origin foo --depth=1000")(stdout=DIFF_STDOUT) - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) result = main.action( config=pull_request_config( @@ -300,7 +300,7 @@ def test_action__pull_request__post_comment( 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 -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) comment = None @@ -347,7 +347,7 @@ def test_action__push__non_default_branch( 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 -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) payload = json.dumps({"coverage": 30.00}) # There is an existing badge in this test, allowing to test the coverage evolution @@ -436,7 +436,7 @@ def test_action__push__non_default_branch__no_pr( 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 -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) payload = json.dumps({"coverage": 30.00}) # There is an existing badge in this test, allowing to test the coverage evolution @@ -500,7 +500,7 @@ def test_action__pull_request__force_store_comment( )(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 -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) result = main.action( config=pull_request_config(FORCE_WORKFLOW_RUN=True, GITHUB_OUTPUT=output_file), @@ -531,7 +531,7 @@ def test_action__pull_request__post_comment__no_marker( )(status_code=404) git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) result = main.action( config=pull_request_config(COMMENT_TEMPLATE="""foo"""), @@ -556,7 +556,7 @@ def test_action__pull_request__annotations( )(status_code=404) git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) # Who am I session.register("GET", "/user")(json={"login": "foo"}) @@ -598,7 +598,7 @@ def test_action__pull_request__post_comment__template_error( )(status_code=404) git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=DIFF_STDOUT) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=DIFF_STDOUT) result = main.action( config=pull_request_config(COMMENT_TEMPLATE="""{%"""), diff --git a/tests/unit/test_coverage.py b/tests/unit/test_coverage.py index 621219da..bed0fe31 100644 --- a/tests/unit/test_coverage.py +++ b/tests/unit/test_coverage.py @@ -309,7 +309,7 @@ def test_get_added_lines(git): """+++ 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 -- .")(stdout=diff) + 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] } @@ -364,7 +364,7 @@ def test_parse_diff_output(git): rename to coverage_comment/annotations2.py """ git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=diff) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=diff) assert coverage.parse_diff_output(diff=diff) == { pathlib.Path("README.md"): [1, 3, 4, 5, 6], pathlib.Path("foo.txt"): [1], @@ -379,6 +379,6 @@ def test_parse_diff_output__error(git): index 1f1d9a4..e69de29 100644 """ git.register("git fetch origin main --depth=1000")() - git.register("git diff --unified=0 FETCH_HEAD -- .")(stdout=diff) + git.register("git diff --unified=0 FETCH_HEAD...HEAD")(stdout=diff) with pytest.raises(ValueError): coverage.parse_diff_output(diff=diff) From cdd42287b5140343f98e668ef5ae49cedc359445 Mon Sep 17 00:00:00 2001 From: Joe Meissler Date: Mon, 26 May 2025 09:04:14 -0700 Subject: [PATCH 2/5] remove copied fixtures --- tests/integration/test_main.py | 97 ---------------------------------- 1 file changed, 97 deletions(-) diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index fd3d58cb..5729233b 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -1,59 +1,12 @@ from __future__ import annotations import json -import os import pathlib -import subprocess -import uuid import pytest from coverage_comment import main - -@pytest.fixture -def in_integration_env(integration_env, integration_dir): - curdir = os.getcwd() - os.chdir(integration_dir) - yield integration_dir - os.chdir(curdir) - - -@pytest.fixture -def integration_dir(tmp_path: pathlib.Path): - test_dir = tmp_path / "integration_test" - test_dir.mkdir() - return test_dir - - -@pytest.fixture -def file_path(integration_dir): - return integration_dir / "foo.py" - - -@pytest.fixture -def write_file(file_path): - def _(*variables): - content = "import os" - for i, var in enumerate(variables): - content += f"""\nif os.environ.get("{var}"):\n {i}\n""" - file_path.write_text(content, encoding="utf8") - - return _ - - -@pytest.fixture -def run_coverage(file_path, integration_dir): - def _(*variables): - subprocess.check_call( - ["coverage", "run", "--parallel", file_path.name], - cwd=integration_dir, - env=os.environ | dict.fromkeys(variables, "1"), - ) - - return _ - - DIFF_STDOUT = """diff --git a/foo.py b/foo.py index 6c08c94..b65c612 100644 --- a/foo.py @@ -68,56 +21,6 @@ def _(*variables): """ -@pytest.fixture -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, request): - 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") - commit() - - add_branch_mark = request.node.get_closest_marker("add_branches") - for additional_branch in add_branch_mark.args if add_branch_mark else []: - subprocess.check_call( - ["git", "switch", "-c", additional_branch], - cwd=integration_dir, - ) - - subprocess.check_call( - ["git", "switch", "-c", "branch"], - cwd=integration_dir, - ) - - write_file("A", "B", "C", "D") - commit() - - run_coverage("A", "C") - subprocess.check_call(["git", "fetch", "origin"], cwd=integration_dir) - - def test_action__invalid_event_name(session, push_config, in_integration_env, get_logs): session.register("GET", "/repos/py-cov-action/foobar")( json={"default_branch": "main", "visibility": "public"} From 66208c71ea0814a095cb77c5b7d00a70eff4f12f Mon Sep 17 00:00:00 2001 From: Joe Meissler Date: Mon, 26 May 2025 09:14:55 -0700 Subject: [PATCH 3/5] simplify test --- tests/integration/test_coverage.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_coverage.py b/tests/integration/test_coverage.py index 8722bd7e..5243f7b7 100644 --- a/tests/integration/test_coverage.py +++ b/tests/integration/test_coverage.py @@ -15,18 +15,16 @@ def test_get_added_lines( git = subprocess_module.Git() relative_file_path = file_path.relative_to(integration_dir) - def _check_added_lines(): - added_lines = coverage.get_added_lines(git, "main") + assert coverage.get_added_lines(git, "main") == { + relative_file_path: list(range(7, 13)) # Line numbers start at 1 + } - assert added_lines == { - relative_file_path: list(range(7, 13)) # Line numbers start at 1 - } - - _check_added_lines() subprocess.check_call(["git", "switch", "main"], cwd=integration_dir) write_file("E", "F") commit() subprocess.check_call(["git", "push", "origin", "main"], cwd=integration_dir) subprocess.check_call(["git", "switch", "branch"], cwd=integration_dir) - _check_added_lines() + assert coverage.get_added_lines(git, "main") == { + relative_file_path: list(range(7, 13)) # Line numbers start at 1 + } From 3efb5dda08b043729f241a0848436945c7e2df2c Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 13 Jul 2025 13:14:30 +0200 Subject: [PATCH 4/5] Change of approach: let github compute the diff --- coverage_comment/coverage.py | 13 +--------- coverage_comment/github.py | 24 +++++++++++++++++ coverage_comment/main.py | 15 ++++++++++- tests/integration/test_coverage.py | 30 ---------------------- tests/integration/test_github.py | 26 +++++++++++++++++++ tests/integration/test_main.py | 41 +++++++++++++++++------------- tests/unit/test_coverage.py | 19 +++----------- 7 files changed, 92 insertions(+), 76 deletions(-) delete mode 100644 tests/integration/test_coverage.py diff --git a/coverage_comment/coverage.py b/coverage_comment/coverage.py index 2cb4699a..cd47b95e 100644 --- a/coverage_comment/coverage.py +++ b/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]] = {} diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 9a06931e..56124983 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -254,3 +254,27 @@ 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(pr_number) + .get(headers={"Accept": "application/vnd.github.v3.diff"}) + ) + + +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}") + .get(headers={"Accept": "application/vnd.github.v3.diff"}) + ) diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 20f46468..5b679f8a 100644 --- a/coverage_comment/main.py +++ b/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 ) diff --git a/tests/integration/test_coverage.py b/tests/integration/test_coverage.py deleted file mode 100644 index 5243f7b7..00000000 --- a/tests/integration/test_coverage.py +++ /dev/null @@ -1,30 +0,0 @@ -from __future__ import annotations - -import subprocess - -from coverage_comment import coverage -from coverage_comment import subprocess as subprocess_module - - -def test_get_added_lines( - commit, file_path, integration_dir, in_integration_env, write_file -): - """ - Lines added in the base_ref should not appear as added in HEAD - """ - git = subprocess_module.Git() - relative_file_path = file_path.relative_to(integration_dir) - - assert coverage.get_added_lines(git, "main") == { - relative_file_path: list(range(7, 13)) # Line numbers start at 1 - } - - subprocess.check_call(["git", "switch", "main"], cwd=integration_dir) - write_file("E", "F") - commit() - subprocess.check_call(["git", "push", "origin", "main"], cwd=integration_dir) - subprocess.check_call(["git", "switch", "branch"], cwd=integration_dir) - - assert coverage.get_added_lines(git, "main") == { - relative_file_path: list(range(7, 13)) # Line numbers start at 1 - } diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index a5b99ec1..39fb3c48 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -444,3 +444,29 @@ 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", + headers={"Accept": "application/vnd.github.v3.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", + headers={"Accept": "application/vnd.github.v3.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..." diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index 5729233b..e98c949e 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -74,8 +74,8 @@ 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")(text=DIFF_STDOUT) result = main.action( config=pull_request_config( @@ -155,8 +155,8 @@ 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")(text=DIFF_STDOUT) result = main.action( config=pull_request_config( @@ -202,8 +202,8 @@ 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")(text=DIFF_STDOUT) comment = None @@ -249,8 +249,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")( + 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 +341,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")( + 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 +407,8 @@ 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")(text=DIFF_STDOUT) result = main.action( config=pull_request_config(FORCE_WORKFLOW_RUN=True, GITHUB_OUTPUT=output_file), @@ -433,8 +438,8 @@ 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")(text=DIFF_STDOUT) result = main.action( config=pull_request_config(COMMENT_TEMPLATE="""foo"""), @@ -458,8 +463,8 @@ 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")(text=DIFF_STDOUT) # Who am I session.register("GET", "/user")(json={"login": "foo"}) @@ -500,8 +505,8 @@ 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")(text=DIFF_STDOUT) result = main.action( config=pull_request_config(COMMENT_TEMPLATE="""{%"""), diff --git a/tests/unit/test_coverage.py b/tests/unit/test_coverage.py index bed0fe31..c37ed50d 100644 --- a/tests/unit/test_coverage.py +++ b/tests/unit/test_coverage.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) From d07a12685938af264409b30da6b7c9c30ec566fc Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 13 Jul 2025 13:14:42 +0200 Subject: [PATCH 5/5] GitHub client: return text by default --- coverage_comment/github_client.py | 16 +++++++--------- tests/unit/test_github_client.py | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/coverage_comment/github_client.py b/coverage_comment/github_client.py index 92a4d9e6..306edc72 100644 --- a/coverage_comment/github_client.py +++ b/coverage_comment/github_client.py @@ -83,10 +83,7 @@ def _http( **header_kwargs, **requests_kwargs, ) - if bytes: - contents = response.content - else: - contents = response_contents(response) + contents = response_contents(response=response, bytes=bytes) try: response.raise_for_status() @@ -103,14 +100,15 @@ def _http( def response_contents( response: httpx.Response, + bytes: bool, ) -> JsonObject | str | bytes: + if bytes: + return response.content + if response.headers.get("content-type", "").startswith("application/json"): return response.json(object_hook=JsonObject) - if response.headers.get("content-type", "").startswith( - "application/vnd.github.raw+json" - ): - return response.text - return response.content + + return response.text class JsonObject(dict): diff --git a/tests/unit/test_github_client.py b/tests/unit/test_github_client.py index 3f07b271..6de9c38d 100644 --- a/tests/unit/test_github_client.py +++ b/tests/unit/test_github_client.py @@ -79,4 +79,4 @@ def test_github_client__get_error_non_json(session, gh): with pytest.raises(github_client.ApiError) as exc_info: gh.repos.get() - assert str(exc_info.value) == "b'{foobar'" + assert str(exc_info.value) == "{foobar"