diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 66060712..0830af76 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -28,6 +28,12 @@ class NoArtifact(Exception): pass +class CannotGetDiff(Exception): + """Raised when the diff cannot be fetched from GitHub.""" + + pass + + @dataclasses.dataclass class RepositoryInfo: default_branch: str @@ -289,11 +295,19 @@ def get_pr_diff(github: github_client.GitHub, repository: str, pr_number: int) - """ Get the diff of a pull request. """ - return ( - github.repos(repository) - .pulls(pr_number) - .get(headers={"Accept": "application/vnd.github.v3.diff"}, text=True) - ) + try: + return ( + github.repos(repository) + .pulls(pr_number) + .get(headers={"Accept": "application/vnd.github.v3.diff"}, text=True) + ) + except github_client.ApiError as exc: + if _is_too_large_error(exc): + raise CannotGetDiff( + "The diff for this PR is too large to be retrieved from GitHub's API " + "(maximum 300 files). Diff coverage is not available for this PR." + ) from exc + raise def get_branch_diff( @@ -302,8 +316,31 @@ def get_branch_diff( """ Get the diff of branch. """ - return ( - github.repos(repository) - .compare(f"{base_branch}...{head_branch}") - .get(headers={"Accept": "application/vnd.github.v3.diff"}, text=True) - ) + try: + return ( + github.repos(repository) + .compare(f"{base_branch}...{head_branch}") + .get(headers={"Accept": "application/vnd.github.v3.diff"}, text=True) + ) + except github_client.ApiError as exc: + if _is_too_large_error(exc): + raise CannotGetDiff( + "The diff for this branch is too large to be retrieved from GitHub's API " + "(maximum 300 files). Diff coverage is not available for this branch." + ) from exc + raise + + +def _is_too_large_error(exc: github_client.ApiError) -> bool: + """ + Check if the error is a "too_large" error from GitHub API. + + GitHub returns this error when the diff exceeds the maximum number of files (300). + The error response body is JSON from GitHub's API. + """ + try: + error_data: dict[str, Any] = json.loads(str(exc)) + errors: list[dict[str, Any]] = error_data.get("errors", []) + return any(error.get("code") == "too_large" for error in errors) + except (json.JSONDecodeError, TypeError): + return False diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 094c55af..acca2a10 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -133,21 +133,27 @@ def process_pr( ) base_ref = config.GITHUB_BASE_REF or repo_info.default_branch - 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, - ) - else: # pragma: no cover - raise Exception("Unreachable code") + failure_msg: str | None = None + try: + 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, + ) + else: # pragma: no cover + raise Exception("Unreachable code") + except github.CannotGetDiff as exc: + failure_msg = str(exc) + log.warning(failure_msg, exc_info=True) + diff = "" added_lines = coverage_module.get_added_lines(diff=diff) diff_coverage = coverage_module.get_diff_coverage_info( @@ -219,6 +225,7 @@ def process_pr( pr_targets_default_branch=pr_targets_default_branch, marker=marker, subproject_id=config.SUBPROJECT_ID, + failure_msg=failure_msg, ) # Same as above except `max_files` is None summary_comment = template.get_comment_markdown( @@ -240,6 +247,7 @@ def process_pr( pr_targets_default_branch=pr_targets_default_branch, marker=marker, subproject_id=config.SUBPROJECT_ID, + failure_msg=failure_msg, ) except template.MissingMarker: log.error( diff --git a/coverage_comment/template.py b/coverage_comment/template.py index 7ed6278a..910df0e4 100644 --- a/coverage_comment/template.py +++ b/coverage_comment/template.py @@ -141,6 +141,7 @@ def get_comment_markdown( subproject_id: str | None = None, custom_template: str | None = None, pr_targets_default_branch: bool = True, + failure_msg: str | None = None, ): loader = CommentLoader(base_template=base_template, custom_template=custom_template) env = SandboxedEnvironment(loader=loader) @@ -188,6 +189,7 @@ def get_comment_markdown( subproject_id=subproject_id, marker=marker, pr_targets_default_branch=pr_targets_default_branch, + failure_msg=failure_msg, ) except jinja2.exceptions.TemplateError as exc: raise TemplateError from exc diff --git a/coverage_comment/template_files/comment.md.j2 b/coverage_comment/template_files/comment.md.j2 index 53816f92..f637f34e 100644 --- a/coverage_comment/template_files/comment.md.j2 +++ b/coverage_comment/template_files/comment.md.j2 @@ -19,12 +19,25 @@ {#- Coverage diff badge -#} {#- space #} {# space -#} {%- block diff_coverage_badge -%} +{%- if failure_msg -%} + + +{%- else -%} {%- set text = (diff_coverage.total_percent_covered | pct) ~ " of the statement lines added by this PR are covered" -%} +{%- endif -%} {%- endblock diff_coverage_badge -%} {%- endblock coverage_badges -%} +{%- block diff_coverage_failure_message -%} +{%- if failure_msg %} + +> [!WARNING] +> {{ failure_msg }} + +{% endif -%} +{%- endblock diff_coverage_failure_message -%} {%- macro statements_badge(path, statements_count, previous_statements_count) -%} {% if previous_statements_count is not none -%} diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 1f6c9092..9e37cd5b 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -4,7 +4,7 @@ import pytest -from coverage_comment import github +from coverage_comment import github, github_client @pytest.mark.parametrize( @@ -491,3 +491,99 @@ def test_get_branch_diff(gh, session): ) assert result == "diff --git a/foo.py b/foo.py..." + + +def test_get_pr_diff__too_large(gh, session): + error_response = { + "message": "Sorry, the diff exceeded the maximum number of files (300).", + "errors": [{"resource": "PullRequest", "field": "diff", "code": "too_large"}], + "documentation_url": "https://docs.github.com/rest/pulls/pulls#list-pull-requests-files", + "status": "406", + } + session.register( + "GET", + "/repos/foo/bar/pulls/123", + headers={"Accept": "application/vnd.github.v3.diff"}, + )(json=error_response, status_code=406) + + with pytest.raises(github.CannotGetDiff) as exc_info: + github.get_pr_diff(github=gh, repository="foo/bar", pr_number=123) + + assert "too large" in str(exc_info.value) + assert "maximum 300 files" in str(exc_info.value) + + +def test_get_branch_diff__too_large(gh, session): + error_response = { + "message": "Sorry, the diff exceeded the maximum number of files (300).", + "errors": [{"resource": "PullRequest", "field": "diff", "code": "too_large"}], + "documentation_url": "https://docs.github.com/rest/pulls/pulls#list-pull-requests-files", + "status": "406", + } + session.register( + "GET", + "/repos/foo/bar/compare/main...feature", + headers={"Accept": "application/vnd.github.v3.diff"}, + )(json=error_response, status_code=406) + + with pytest.raises(github.CannotGetDiff) as exc_info: + github.get_branch_diff( + github=gh, repository="foo/bar", base_branch="main", head_branch="feature" + ) + + assert "too large" in str(exc_info.value) + assert "maximum 300 files" in str(exc_info.value) + + +def test_get_pr_diff__other_error(gh, session): + error_response = {"message": "Some other error", "errors": []} + session.register( + "GET", + "/repos/foo/bar/pulls/123", + headers={"Accept": "application/vnd.github.v3.diff"}, + )(json=error_response, status_code=500) + + with pytest.raises(github_client.ApiError): + github.get_pr_diff(github=gh, repository="foo/bar", pr_number=123) + + +def test_get_branch_diff__other_error(gh, session): + error_response = {"message": "Some other error", "errors": []} + session.register( + "GET", + "/repos/foo/bar/compare/main...feature", + headers={"Accept": "application/vnd.github.v3.diff"}, + )(json=error_response, status_code=500) + + with pytest.raises(github_client.ApiError): + github.get_branch_diff( + github=gh, repository="foo/bar", base_branch="main", head_branch="feature" + ) + + +@pytest.mark.parametrize( + "error_str,expected", + [ + # Valid JSON with too_large error + ('{"errors": [{"code": "too_large"}]}', True), + # Valid JSON with too_large error and extra fields + ( + '{"message": "Diff too large", "errors": [{"resource": "PR", "code": "too_large"}]}', + True, + ), + # Valid JSON without too_large error + ('{"errors": [{"code": "other"}]}', False), + # Valid JSON with empty errors + ('{"errors": []}', False), + # Valid JSON with no errors key + ('{"message": "error"}', False), + # Non-JSON string (returns False, not a fallback match) + ("not valid json", False), + # Empty string + ("", False), + ], +) +def test__is_too_large_error(error_str, expected): + """Test the _is_too_large_error helper function with various inputs.""" + exc = github_client.ApiError(error_str) + assert github._is_too_large_error(exc) is expected diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index a7b9c610..4ae8368b 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -927,3 +927,76 @@ def test_action__workflow_run__post_comment( assert get_logs("INFO", "Comment file found in artifact, posting to PR") assert get_logs("INFO", "Comment posted in PR") assert summary_file.read_text() == "" + + +def test_action__pull_request__diff_too_large( + pull_request_config, + session, + in_integration_env, + output_file, + summary_file, + git, + get_logs, +): + """Test that when the diff is too large, a warning is shown in the comment.""" + session.register("GET", "/repos/py-cov-action/foobar")( + json={"default_branch": "main", "visibility": "public"} + ) + # No existing badge in this test + session.register("GET", "/repos/py-cov-action/foobar/contents/data.json")( + status_code=404 + ) + + # Who am I + session.register("GET", "/user")(json={"login": "foo"}) + # Are there already comments + session.register("GET", "/repos/py-cov-action/foobar/issues/2/comments")(json=[]) + + comment = None + + def checker(payload): + body = payload["body"] + assert "## Coverage report" in body + nonlocal comment + comment = body + return True + + # Post a new comment + session.register( + "POST", "/repos/py-cov-action/foobar/issues/2/comments", json=checker + )(status_code=200) + + # The diff is too large - returns 406 with error + error_response = { + "message": "Sorry, the diff exceeded the maximum number of files (300).", + "errors": [{"resource": "PullRequest", "field": "diff", "code": "too_large"}], + "documentation_url": "https://docs.github.com/rest/pulls/pulls#list-pull-requests-files", + "status": "406", + } + session.register("GET", "/repos/py-cov-action/foobar/pulls/2")( + json=error_response, status_code=406 + ) + + result = main.action( + config=pull_request_config( + GITHUB_OUTPUT=output_file, GITHUB_STEP_SUMMARY=summary_file + ), + github_session=session, + http_session=session, + git=git, + ) + assert result == 0 + + # Check that a warning was logged + assert get_logs("WARNING", "too large") + + # Comment was posted successfully, no fallback file should exist + assert not pathlib.Path("python-coverage-comment-action.txt").exists() + + # Check that the error message is in the comment + assert "too large" in comment + assert "maximum 300 files" in comment + # Check that the warning block is in the comment + assert "[!WARNING]" in comment + # Check the N/A badge is shown for PR coverage (URL-encoded in badge URL) + assert "PR%20Coverage-N/A-grey" in comment