From 14fb28ae7ca7cdf41262048242e5575384457b96 Mon Sep 17 00:00:00 2001 From: Daniel Anya Date: Thu, 15 May 2025 11:51:30 -0400 Subject: [PATCH 1/9] Updates the action to support saving coverage artifacts on pull request merge to the default branch. This is functionally equivalent to the currently supported pattern of pushing to the default branch. --- coverage_comment/activity.py | 3 ++- coverage_comment/main.py | 12 ++++++++++++ coverage_comment/settings.py | 1 + tests/unit/test_activity.py | 18 ++++++++++-------- tests/unit/test_settings.py | 5 +++++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/coverage_comment/activity.py b/coverage_comment/activity.py index dee9ea8b..3fdfaaca 100644 --- a/coverage_comment/activity.py +++ b/coverage_comment/activity.py @@ -16,12 +16,13 @@ class ActivityNotFound(Exception): def find_activity( event_name: str, is_default_branch: bool, + event_action: str | None = None, ) -> str: """Find the activity to perform based on the event type and payload.""" if event_name == "workflow_run": return "post_comment" - if (event_name == "push" and is_default_branch) or event_name == "schedule": + if (event_name == "push" and is_default_branch) or (event_name == "pull_request" and event_action == "merged" and is_default_branch) or event_name == "schedule": return "save_coverage_data_files" if event_name not in {"pull_request", "push"}: diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 5b679f8a..cad1fa40 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -4,6 +4,7 @@ import logging import os import sys +import json import httpx @@ -69,12 +70,23 @@ def action( log.debug(f"Operating on {config.GITHUB_REF}") gh = github_client.GitHub(session=github_session) event_name = config.GITHUB_EVENT_NAME + event_path = config.GITHUB_EVENT_PATH + event_action = None + + if event_path and os.path.exists(event_path): + with open(event_path, "r") as event_file: + event_payload = json.load(event_file) + is_merged_pr_action = event_payload.get("pull_request", {}).get("merged", False) + if is_merged_pr_action: + event_action = "merged" + repo_info = github.get_repository_info( github=gh, repository=config.GITHUB_REPOSITORY ) try: activity = activity_module.find_activity( event_name=event_name, + event_action=event_action, is_default_branch=repo_info.is_default_branch(ref=config.GITHUB_REF), ) except activity_module.ActivityNotFound: diff --git a/coverage_comment/settings.py b/coverage_comment/settings.py index 12473c7d..1617c5b8 100644 --- a/coverage_comment/settings.py +++ b/coverage_comment/settings.py @@ -47,6 +47,7 @@ class Config: # (from https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables ) GITHUB_REF: str GITHUB_EVENT_NAME: str + GITHUB_EVENT_PATH: pathlib.Path | None = None GITHUB_PR_RUN_ID: int | None GITHUB_STEP_SUMMARY: pathlib.Path COMMENT_TEMPLATE: str | None = None diff --git a/tests/unit/test_activity.py b/tests/unit/test_activity.py index 06d267c2..e2d61437 100644 --- a/tests/unit/test_activity.py +++ b/tests/unit/test_activity.py @@ -6,18 +6,20 @@ @pytest.mark.parametrize( - "event_name, is_default_branch, expected_activity", + "event_name, event_action, is_default_branch, expected_activity", [ - ("workflow_run", True, "post_comment"), - ("push", True, "save_coverage_data_files"), - ("push", False, "process_pr"), - ("pull_request", True, "process_pr"), - ("pull_request", False, "process_pr"), + ("workflow_run", None, True, "post_comment"), + ("push", None, True, "save_coverage_data_files"), + ("push", None, False, "process_pr"), + ("pull_request", "merged", True, "save_coverage_data_files"), + ("pull_request", None, True, "process_pr"), + ("pull_request", None, False, "process_pr"), + ("schedule", None, False, "save_coverage_data_files"), ], ) -def test_find_activity(event_name, is_default_branch, expected_activity): +def test_find_activity(event_name, event_action, is_default_branch, expected_activity): result = activity.find_activity( - event_name=event_name, is_default_branch=is_default_branch + event_name=event_name, event_action=event_action, is_default_branch=is_default_branch ) assert result == expected_activity diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index b283d991..720263c7 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -33,6 +33,7 @@ def test_config__from_environ__ok(): "GITHUB_REF": "master", "GITHUB_OUTPUT": "foo.txt", "GITHUB_EVENT_NAME": "pull", + "GITHUB_EVENT_PATH": pathlib.Path("test_event_path"), "GITHUB_PR_RUN_ID": "123", "GITHUB_STEP_SUMMARY": "step_summary", "COMMENT_ARTIFACT_NAME": "baz", @@ -56,6 +57,7 @@ def test_config__from_environ__ok(): GITHUB_REF="master", GITHUB_OUTPUT=pathlib.Path("foo.txt"), GITHUB_EVENT_NAME="pull", + GITHUB_EVENT_PATH=pathlib.Path("test_event_path"), GITHUB_PR_RUN_ID=123, GITHUB_STEP_SUMMARY=pathlib.Path("step_summary"), COMMENT_ARTIFACT_NAME="baz", @@ -82,6 +84,7 @@ def test_config__verbose_deprecated(get_logs): "GITHUB_REPOSITORY": "owner/repo", "GITHUB_REF": "master", "GITHUB_EVENT_NAME": "pull", + "GITHUB_EVENT_PATH": pathlib.Path("test_event_path"), "GITHUB_PR_RUN_ID": "123", "GITHUB_STEP_SUMMARY": "step_summary", "VERBOSE": "true", @@ -92,6 +95,7 @@ def test_config__verbose_deprecated(get_logs): GITHUB_REPOSITORY="owner/repo", GITHUB_REF="master", GITHUB_EVENT_NAME="pull", + GITHUB_EVENT_PATH=pathlib.Path("test_event_path"), GITHUB_PR_RUN_ID=123, GITHUB_STEP_SUMMARY=pathlib.Path("step_summary"), VERBOSE=False, @@ -107,6 +111,7 @@ def config(): "GITHUB_REPOSITORY": "owner/repo", "GITHUB_REF": "master", "GITHUB_EVENT_NAME": "pull", + "GITHUB_EVENT_PATH": pathlib.Path("test_event_path"), "GITHUB_PR_RUN_ID": 123, "GITHUB_STEP_SUMMARY": pathlib.Path("step_summary"), "COMMENT_ARTIFACT_NAME": "baz", From 97351324837637d2dbe6f2dc81cc230e798c66ac Mon Sep 17 00:00:00 2001 From: Daniel Anya Date: Thu, 15 May 2025 12:25:00 -0400 Subject: [PATCH 2/9] Adds additional support for GitHub Enterprise in coverage artifacts Adds support for figuring out a github host from the `GITHUB_BASE_URL` config and using this value in generated coverage artifacts rather than hardcoding them to "github.com" --- coverage_comment/github.py | 36 +++++++++++++++++++++++ coverage_comment/main.py | 6 ++++ coverage_comment/storage.py | 24 +++++++++------- coverage_comment/template.py | 6 ++-- tests/unit/test_storage.py | 55 ++++++++++++++++++++++-------------- tests/unit/test_template.py | 10 +++++++ 6 files changed, 104 insertions(+), 33 deletions(-) diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 56124983..8655a064 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -6,6 +6,7 @@ import pathlib import sys import zipfile +from urllib.parse import urlparse from coverage_comment import github_client, log @@ -45,6 +46,41 @@ def get_repository_info( default_branch=response.default_branch, visibility=response.visibility ) +def extract_github_host(api_url: str) -> str: + """ + Extracts the base GitHub web host URL from a GitHub API URL. + + Args: + api_url: The GitHub API URL (e.g., 'https://api.github.com/...', + 'https://my-ghe.company.com/api/v3/...'). + + Returns: + The base GitHub web host URL (e.g., 'https://github.com', + 'https://my-ghe.company.com'). + """ + try: + parsed_url = urlparse(api_url) + scheme = parsed_url.scheme + netloc = parsed_url.netloc # This includes the domain and potentially the port + + # Special case for GitHub.com API + if netloc == 'api.github.com': + host_domain = 'github.com' + # Special case for GitHub.com with port (less common but good practice) + elif netloc.startswith('api.github.com:'): + # Remove 'api.' prefix but keep the port + host_domain = netloc.replace('api.', '', 1) + # General case for GitHub Enterprise (netloc is already the host:port) + else: + host_domain = netloc + + # Reconstruct the host URL + host_url = f"{scheme}://{host_domain}" + + return host_url + except Exception as e: + log.error(f"Error parsing URL {api_url}: {e}") + return "" def download_artifact( github: github_client.GitHub, diff --git a/coverage_comment/main.py b/coverage_comment/main.py index cad1fa40..831d7a58 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -201,6 +201,7 @@ def process_pr( max_files=config.MAX_FILES_IN_COMMENT, minimum_green=config.MINIMUM_GREEN, minimum_orange=config.MINIMUM_ORANGE, + github_host=github.extract_github_host(config.GITHUB_BASE_URL), repo_name=config.GITHUB_REPOSITORY, pr_number=config.GITHUB_PR_NUMBER, base_template=template.read_template_file("comment.md.j2"), @@ -220,6 +221,7 @@ def process_pr( max_files=None, minimum_green=config.MINIMUM_GREEN, minimum_orange=config.MINIMUM_ORANGE, + github_host=github.extract_github_host(config.GITHUB_BASE_URL), repo_name=config.GITHUB_REPOSITORY, pr_number=config.GITHUB_PR_NUMBER, base_template=template.read_template_file("comment.md.j2"), @@ -411,17 +413,21 @@ def save_coverage_data_files( github_step_summary=config.GITHUB_STEP_SUMMARY, ) + github_host = github.extract_github_host(config.GITHUB_BASE_URL) url_getter = functools.partial( storage.get_raw_file_url, + github_host=github_host, is_public=is_public, repository=config.GITHUB_REPOSITORY, branch=config.FINAL_COVERAGE_DATA_BRANCH, ) readme_url = storage.get_repo_file_url( + github_host=github_host, branch=config.FINAL_COVERAGE_DATA_BRANCH, repository=config.GITHUB_REPOSITORY, ) html_report_url = storage.get_html_report_url( + github_host=github_host, branch=config.FINAL_COVERAGE_DATA_BRANCH, repository=config.GITHUB_REPOSITORY, ) diff --git a/coverage_comment/storage.py b/coverage_comment/storage.py index eb8e49a0..246fb1b7 100644 --- a/coverage_comment/storage.py +++ b/coverage_comment/storage.py @@ -132,16 +132,17 @@ def get_datafile_contents( def get_raw_file_url( + github_host: str, repository: str, branch: str, path: pathlib.Path, is_public: bool, ): - if not is_public: - # If the repository is private, then the real links to raw.githubusercontents.com - # will be short-lived. In this case, it's better to keep an URL that will - # redirect to the correct URL just when asked. - return f"https://github.com/{repository}/raw/{branch}/{path}" + if (not is_public) or (not github_host.endswith("github.com")): + # If the repository is private or hosted on a github enterprise instance, + # then the real links to raw.githubusercontents.com will be short-lived. + # In this case, it's better to keep an URL that will redirect to the correct URL just when asked. + return f"{github_host}/{repository}/raw/{branch}/{path}" # Otherwise, we can access the file directly. (shields.io doesn't like the # github.com domain) @@ -154,7 +155,7 @@ def get_raw_file_url( # seconds. -def get_repo_file_url(repository: str, branch: str, path: str = "/") -> str: +def get_repo_file_url(github_host: str, repository: str, branch: str, path: str = "/") -> str: """ Computes the GitHub Web UI URL for a given path: If the path is empty or ends with a slash, it will be interpreted as a folder, @@ -166,11 +167,14 @@ def get_repo_file_url(repository: str, branch: str, path: str = "/") -> str: # See test_get_repo_file_url for precise specifications path = "/" + path.lstrip("/") part = "tree" if path.endswith("/") else "blob" - return f"https://github.com/{repository}/{part}/{branch}{path}".rstrip("/") + return f"{github_host}/{repository}/{part}/{branch}{path}".rstrip("/") -def get_html_report_url(repository: str, branch: str) -> str: +def get_html_report_url(github_host: str, repository: str, branch: str) -> str: readme_url = get_repo_file_url( - repository=repository, branch=branch, path="/htmlcov/index.html" + github_host, repository=repository, branch=branch, path="/htmlcov/index.html" ) - return f"https://htmlpreview.github.io/?{readme_url}" + if github_host.endswith("github.com"): + return f"https://htmlpreview.github.io/?{readme_url}" + return readme_url + diff --git a/coverage_comment/template.py b/coverage_comment/template.py index ed680cf5..0db685aa 100644 --- a/coverage_comment/template.py +++ b/coverage_comment/template.py @@ -130,6 +130,7 @@ def get_comment_markdown( count_files: int, minimum_green: decimal.Decimal, minimum_orange: decimal.Decimal, + github_host: str, repo_name: str, pr_number: int, base_template: str, @@ -148,7 +149,7 @@ def get_comment_markdown( env.filters["pluralize"] = pluralize env.filters["compact"] = compact env.filters["file_url"] = functools.partial( - get_file_url, repo_name=repo_name, pr_number=pr_number + get_file_url, github_host=github_host, repo_name=repo_name, pr_number=pr_number ) env.filters["get_badge_color"] = functools.partial( badge.get_badge_color, @@ -304,11 +305,12 @@ def get_file_url( filename: pathlib.Path, lines: tuple[int, int] | None = None, *, + github_host: str, repo_name: str, pr_number: int, ) -> str: # To link to a file in a PR, GitHub uses the link to the file overview combined with a SHA256 hash of the file path - s = f"https://github.com/{repo_name}/pull/{pr_number}/files#diff-{hashlib.sha256(str(filename).encode('utf-8')).hexdigest()}" + s = f"{github_host}/{repo_name}/pull/{pr_number}/files#diff-{hashlib.sha256(str(filename).encode('utf-8')).hexdigest()}" if lines is not None: # R stands for Right side of the diff. But since we generate these links for new code we only need the right side. diff --git a/tests/unit/test_storage.py b/tests/unit/test_storage.py index 2ba72a80..be36b249 100644 --- a/tests/unit/test_storage.py +++ b/tests/unit/test_storage.py @@ -154,14 +154,16 @@ def test_get_datafile_contents(gh, session): @pytest.mark.parametrize( - "is_public, expected", + "github_host, is_public, expected", [ - (False, "https://github.com/foo/bar/raw/baz/qux"), - (True, "https://raw.githubusercontent.com/foo/bar/baz/qux"), + ("https://github.com", False, "https://github.com/foo/bar/raw/baz/qux"), + ("https://github.com", True, "https://raw.githubusercontent.com/foo/bar/baz/qux"), + ("https://github.mycompany.com", True, "https://github.mycompany.com/foo/bar/raw/baz/qux"), ], ) -def test_get_raw_file_url(is_public, expected): +def test_get_raw_file_url(github_host, is_public, expected): result = storage.get_raw_file_url( + github_host=github_host, repository="foo/bar", branch="baz", path=pathlib.Path("qux"), @@ -171,29 +173,40 @@ def test_get_raw_file_url(is_public, expected): @pytest.mark.parametrize( - "path, expected", + "github_host, path, expected", [ - ("", "https://github.com/foo/bar/tree/baz"), - ("/", "https://github.com/foo/bar/tree/baz"), - ("qux", "https://github.com/foo/bar/blob/baz/qux"), # blob - ("qux/", "https://github.com/foo/bar/tree/baz/qux"), - ("/qux", "https://github.com/foo/bar/blob/baz/qux"), # blob - ("/qux/", "https://github.com/foo/bar/tree/baz/qux"), + ("https://github.com", "", "https://github.com/foo/bar/tree/baz"), + ("https://github.com", "/", "https://github.com/foo/bar/tree/baz"), + ("https://github.com", "qux", "https://github.com/foo/bar/blob/baz/qux"), # blob + ("https://github.com", "qux/", "https://github.com/foo/bar/tree/baz/qux"), + ("https://github.mycompany.com", "/qux", "https://github.mycompany.com/foo/bar/blob/baz/qux"), # blob + ("https://github.mycompany.com", "/qux/", "https://github.mycompany.com/foo/bar/tree/baz/qux"), ], ) -def test_get_repo_file_url(path, expected): - result = storage.get_repo_file_url(repository="foo/bar", branch="baz", path=path) +def test_get_repo_file_url(github_host, path, expected): + result = storage.get_repo_file_url(github_host=github_host, repository="foo/bar", branch="baz", path=path) assert result == expected +@pytest.mark.parametrize( + "github_host", + [ + "https://github.com", + "https://github.mycompany.com", + ], +) +def test_get_repo_file_url__no_path(github_host): + result = storage.get_repo_file_url(github_host=github_host, repository="foo/bar", branch="baz") -def test_get_repo_file_url__no_path(): - result = storage.get_repo_file_url(repository="foo/bar", branch="baz") - - assert result == "https://github.com/foo/bar/tree/baz" - + assert result == f"{github_host}/foo/bar/tree/baz" -def test_get_html_report_url(): - result = storage.get_html_report_url(repository="foo/bar", branch="baz") - expected = "https://htmlpreview.github.io/?https://github.com/foo/bar/blob/baz/htmlcov/index.html" +@pytest.mark.parametrize( + "github_host, expected", + [ + ("https://github.com", "https://htmlpreview.github.io/?https://github.com/foo/bar/blob/baz/htmlcov/index.html"), + ("https://github.mycompany.com", "https://github.mycompany.com/foo/bar/blob/baz/htmlcov/index.html"), + ], +) +def test_get_html_report_url(github_host, expected): + result = storage.get_html_report_url(github_host=github_host, repository="foo/bar", branch="baz") assert result == expected diff --git a/tests/unit/test_template.py b/tests/unit/test_template.py index 521b5dc8..f516baad 100644 --- a/tests/unit/test_template.py +++ b/tests/unit/test_template.py @@ -27,6 +27,7 @@ def test_get_comment_markdown(coverage_obj, diff_coverage_obj): minimum_green=decimal.Decimal("100"), minimum_orange=decimal.Decimal("70"), marker="", + github_host="https://github.com", repo_name="org/repo", pr_number=1, base_template=""" @@ -66,6 +67,7 @@ def test_template(coverage_obj, diff_coverage_obj): files=files, count_files=total, max_files=25, + github_host="https://github.com", repo_name="org/repo", pr_number=5, base_template=template.read_template_file("comment.md.j2"), @@ -195,6 +197,7 @@ def test_template_full(make_coverage, make_coverage_and_diff): minimum_green=decimal.Decimal("100"), minimum_orange=decimal.Decimal("70"), marker="", + github_host="https://github.com", repo_name="org/repo", pr_number=12, base_template=template.read_template_file("comment.md.j2"), @@ -257,6 +260,7 @@ def test_template__no_previous(coverage_obj_no_branch, diff_coverage_obj): minimum_green=decimal.Decimal("100"), minimum_orange=decimal.Decimal("70"), marker="", + github_host="https://github.com", repo_name="org/repo", pr_number=3, base_template=template.read_template_file("comment.md.j2"), @@ -309,6 +313,7 @@ def test_template__max_files(coverage_obj_more_files, diff_coverage_obj_more_fil previous_coverage_rate=decimal.Decimal("0.92"), minimum_green=decimal.Decimal("79"), minimum_orange=decimal.Decimal("40"), + github_host="https://github.com", repo_name="org/repo", pr_number=5, max_files=1, @@ -340,6 +345,7 @@ def test_template__no_max_files(coverage_obj_more_files, diff_coverage_obj_more_ previous_coverage_rate=decimal.Decimal("0.92"), minimum_green=decimal.Decimal("79"), minimum_orange=decimal.Decimal("40"), + github_host="https://github.com", repo_name="org/repo", pr_number=5, max_files=None, @@ -374,6 +380,7 @@ def test_template__no_files(coverage_obj, diff_coverage_obj_more_files): previous_coverage_rate=decimal.Decimal("0.92"), minimum_green=decimal.Decimal("79"), minimum_orange=decimal.Decimal("40"), + github_host="https://github.com", repo_name="org/repo", pr_number=5, max_files=25, @@ -412,6 +419,7 @@ def test_template__no_marker(coverage_obj, diff_coverage_obj): previous_coverage_rate=decimal.Decimal("0.92"), minimum_green=decimal.Decimal("100"), minimum_orange=decimal.Decimal("70"), + github_host="https://github.com", repo_name="org/repo", pr_number=1, base_template=template.read_template_file("comment.md.j2"), @@ -432,6 +440,7 @@ def test_template__broken_template(coverage_obj, diff_coverage_obj): previous_coverage_rate=decimal.Decimal("0.92"), minimum_green=decimal.Decimal("100"), minimum_orange=decimal.Decimal("70"), + github_host="https://github.com", repo_name="org/repo", pr_number=1, base_template=template.read_template_file("comment.md.j2"), @@ -496,6 +505,7 @@ def test_get_file_url(filepath, lines, expected): result = template.get_file_url( filename=filepath, lines=lines, + github_host="https://github.com", repo_name="py-cov-action/python-coverage-comment-action", pr_number=33, ) From 4f95ed0fe1dd403c6991f2ffd3db1b12d12a408e Mon Sep 17 00:00:00 2001 From: Daniel Anya Date: Thu, 15 May 2025 12:26:50 -0400 Subject: [PATCH 3/9] Small fixes to dev-env script --- coverage_comment/activity.py | 10 ++++++- coverage_comment/github.py | 12 ++++---- coverage_comment/main.py | 4 +-- coverage_comment/storage.py | 5 ++-- dev-env | 4 +-- tests/unit/test_activity.py | 4 ++- tests/unit/test_storage.py | 54 +++++++++++++++++++++++++++++------- 7 files changed, 70 insertions(+), 23 deletions(-) diff --git a/coverage_comment/activity.py b/coverage_comment/activity.py index 3fdfaaca..1b5507e1 100644 --- a/coverage_comment/activity.py +++ b/coverage_comment/activity.py @@ -22,7 +22,15 @@ def find_activity( if event_name == "workflow_run": return "post_comment" - if (event_name == "push" and is_default_branch) or (event_name == "pull_request" and event_action == "merged" and is_default_branch) or event_name == "schedule": + if ( + (event_name == "push" and is_default_branch) + or ( + event_name == "pull_request" + and event_action == "merged" + and is_default_branch + ) + or event_name == "schedule" + ): return "save_coverage_data_files" if event_name not in {"pull_request", "push"}: diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 8655a064..57fdc27c 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -46,6 +46,7 @@ def get_repository_info( default_branch=response.default_branch, visibility=response.visibility ) + def extract_github_host(api_url: str) -> str: """ Extracts the base GitHub web host URL from a GitHub API URL. @@ -61,15 +62,15 @@ def extract_github_host(api_url: str) -> str: try: parsed_url = urlparse(api_url) scheme = parsed_url.scheme - netloc = parsed_url.netloc # This includes the domain and potentially the port + netloc = parsed_url.netloc # This includes the domain and potentially the port # Special case for GitHub.com API - if netloc == 'api.github.com': - host_domain = 'github.com' + if netloc == "api.github.com": + host_domain = "github.com" # Special case for GitHub.com with port (less common but good practice) - elif netloc.startswith('api.github.com:'): + elif netloc.startswith("api.github.com:"): # Remove 'api.' prefix but keep the port - host_domain = netloc.replace('api.', '', 1) + host_domain = netloc.replace("api.", "", 1) # General case for GitHub Enterprise (netloc is already the host:port) else: host_domain = netloc @@ -82,6 +83,7 @@ def extract_github_host(api_url: str) -> str: log.error(f"Error parsing URL {api_url}: {e}") return "" + def download_artifact( github: github_client.GitHub, repository: str, diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 831d7a58..6b2b4697 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -1,10 +1,10 @@ from __future__ import annotations import functools +import json import logging import os import sys -import json import httpx @@ -74,7 +74,7 @@ def action( event_action = None if event_path and os.path.exists(event_path): - with open(event_path, "r") as event_file: + with open(event_path) as event_file: event_payload = json.load(event_file) is_merged_pr_action = event_payload.get("pull_request", {}).get("merged", False) if is_merged_pr_action: diff --git a/coverage_comment/storage.py b/coverage_comment/storage.py index 246fb1b7..4d0f9c93 100644 --- a/coverage_comment/storage.py +++ b/coverage_comment/storage.py @@ -155,7 +155,9 @@ def get_raw_file_url( # seconds. -def get_repo_file_url(github_host: str, repository: str, branch: str, path: str = "/") -> str: +def get_repo_file_url( + github_host: str, repository: str, branch: str, path: str = "/" +) -> str: """ Computes the GitHub Web UI URL for a given path: If the path is empty or ends with a slash, it will be interpreted as a folder, @@ -177,4 +179,3 @@ def get_html_report_url(github_host: str, repository: str, branch: str) -> str: if github_host.endswith("github.com"): return f"https://htmlpreview.github.io/?{readme_url}" return readme_url - diff --git a/dev-env b/dev-env index 102d3146..ddcda718 100755 --- a/dev-env +++ b/dev-env @@ -47,7 +47,7 @@ function create-repo(){ repo_dirname=$(basename ${GITHUB_REPOSITORY}) mv "${repo_dirname}/"{*,.*} . rmdir "${repo_dirname}" - git pull --ff-only origin master + git pull --ff-only origin main } function delete-repo(){ @@ -142,7 +142,7 @@ function help(){ echo " coverage_comment" >&2 echo " Launch the action locally (no argument)" >&2 echo " pytest" >&2 - echo " Launch the the tests on the example repo (generates the coverage data that the action uses)" >&2 + echo " Launch the tests on the example repo (generates the coverage data that the action uses)" >&2 echo "" >&2 echo "Change configuration:" >&2 diff --git a/tests/unit/test_activity.py b/tests/unit/test_activity.py index e2d61437..d54b49e9 100644 --- a/tests/unit/test_activity.py +++ b/tests/unit/test_activity.py @@ -19,7 +19,9 @@ ) def test_find_activity(event_name, event_action, is_default_branch, expected_activity): result = activity.find_activity( - event_name=event_name, event_action=event_action, is_default_branch=is_default_branch + event_name=event_name, + event_action=event_action, + is_default_branch=is_default_branch, ) assert result == expected_activity diff --git a/tests/unit/test_storage.py b/tests/unit/test_storage.py index be36b249..82bd5c29 100644 --- a/tests/unit/test_storage.py +++ b/tests/unit/test_storage.py @@ -157,8 +157,16 @@ def test_get_datafile_contents(gh, session): "github_host, is_public, expected", [ ("https://github.com", False, "https://github.com/foo/bar/raw/baz/qux"), - ("https://github.com", True, "https://raw.githubusercontent.com/foo/bar/baz/qux"), - ("https://github.mycompany.com", True, "https://github.mycompany.com/foo/bar/raw/baz/qux"), + ( + "https://github.com", + True, + "https://raw.githubusercontent.com/foo/bar/baz/qux", + ), + ( + "https://github.mycompany.com", + True, + "https://github.mycompany.com/foo/bar/raw/baz/qux", + ), ], ) def test_get_raw_file_url(github_host, is_public, expected): @@ -177,17 +185,32 @@ def test_get_raw_file_url(github_host, is_public, expected): [ ("https://github.com", "", "https://github.com/foo/bar/tree/baz"), ("https://github.com", "/", "https://github.com/foo/bar/tree/baz"), - ("https://github.com", "qux", "https://github.com/foo/bar/blob/baz/qux"), # blob + ( + "https://github.com", + "qux", + "https://github.com/foo/bar/blob/baz/qux", + ), # blob ("https://github.com", "qux/", "https://github.com/foo/bar/tree/baz/qux"), - ("https://github.mycompany.com", "/qux", "https://github.mycompany.com/foo/bar/blob/baz/qux"), # blob - ("https://github.mycompany.com", "/qux/", "https://github.mycompany.com/foo/bar/tree/baz/qux"), + ( + "https://github.mycompany.com", + "/qux", + "https://github.mycompany.com/foo/bar/blob/baz/qux", + ), # blob + ( + "https://github.mycompany.com", + "/qux/", + "https://github.mycompany.com/foo/bar/tree/baz/qux", + ), ], ) def test_get_repo_file_url(github_host, path, expected): - result = storage.get_repo_file_url(github_host=github_host, repository="foo/bar", branch="baz", path=path) + result = storage.get_repo_file_url( + github_host=github_host, repository="foo/bar", branch="baz", path=path + ) assert result == expected + @pytest.mark.parametrize( "github_host", [ @@ -196,17 +219,28 @@ def test_get_repo_file_url(github_host, path, expected): ], ) def test_get_repo_file_url__no_path(github_host): - result = storage.get_repo_file_url(github_host=github_host, repository="foo/bar", branch="baz") + result = storage.get_repo_file_url( + github_host=github_host, repository="foo/bar", branch="baz" + ) assert result == f"{github_host}/foo/bar/tree/baz" + @pytest.mark.parametrize( "github_host, expected", [ - ("https://github.com", "https://htmlpreview.github.io/?https://github.com/foo/bar/blob/baz/htmlcov/index.html"), - ("https://github.mycompany.com", "https://github.mycompany.com/foo/bar/blob/baz/htmlcov/index.html"), + ( + "https://github.com", + "https://htmlpreview.github.io/?https://github.com/foo/bar/blob/baz/htmlcov/index.html", + ), + ( + "https://github.mycompany.com", + "https://github.mycompany.com/foo/bar/blob/baz/htmlcov/index.html", + ), ], ) def test_get_html_report_url(github_host, expected): - result = storage.get_html_report_url(github_host=github_host, repository="foo/bar", branch="baz") + result = storage.get_html_report_url( + github_host=github_host, repository="foo/bar", branch="baz" + ) assert result == expected From fdd2bd20738d959c7e233aef70eaf3675403f531 Mon Sep 17 00:00:00 2001 From: Daniel Anya Date: Fri, 16 May 2025 15:59:05 -0400 Subject: [PATCH 4/9] Updates `extract_github_host` function + test --- coverage_comment/github.py | 38 ++++++++++++++------------------ tests/integration/test_github.py | 13 +++++++++++ 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 57fdc27c..992c38fd 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -59,29 +59,25 @@ def extract_github_host(api_url: str) -> str: The base GitHub web host URL (e.g., 'https://github.com', 'https://my-ghe.company.com'). """ - try: - parsed_url = urlparse(api_url) - scheme = parsed_url.scheme - netloc = parsed_url.netloc # This includes the domain and potentially the port - - # Special case for GitHub.com API - if netloc == "api.github.com": - host_domain = "github.com" - # Special case for GitHub.com with port (less common but good practice) - elif netloc.startswith("api.github.com:"): - # Remove 'api.' prefix but keep the port - host_domain = netloc.replace("api.", "", 1) - # General case for GitHub Enterprise (netloc is already the host:port) - else: - host_domain = netloc + parsed_url = urlparse(api_url) + scheme = parsed_url.scheme + netloc = parsed_url.netloc # This includes the domain and potentially the port + + # Special case for GitHub.com API + if netloc == "api.github.com": + host_domain = "github.com" + # Special case for GitHub.com with port (less common but good practice) + elif netloc.startswith("api.github.com:"): + # Remove 'api.' prefix but keep the port + host_domain = netloc.replace("api.", "", 1) + # General case for GitHub Enterprise (netloc is already the host:port) + else: + host_domain = netloc - # Reconstruct the host URL - host_url = f"{scheme}://{host_domain}" + # Reconstruct the host URL + host_url = f"{scheme}://{host_domain}" - return host_url - except Exception as e: - log.error(f"Error parsing URL {api_url}: {e}") - return "" + return host_url def download_artifact( diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 39fb3c48..8964bbd2 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -45,6 +45,19 @@ def test_get_repository_info(gh, session): assert info == github.RepositoryInfo(default_branch="baz", visibility="public") +@pytest.mark.parametrize( + "api_url, expected", + [ + ("https://api.github.com/repos/foo/bar", "https://github.com"), + ("https://api.github.com:8080/repos/foo/bar", "https://github.com:8080"), + ("https://api.github.com/repos/foo/bar/issues", "https://github.com"), + ("https://my-ghe.company.com/api/v3/repos/foo/bar", "https://my-ghe.company.com"), + ("https://my-ghe.company.com/api/v3/repos/foo/bar/issues", "https://my-ghe.company.com"), + ] +) +def test_extract_github_host(api_url, expected): + result = github.extract_github_host(api_url=api_url) + assert result == expected def test_download_artifact(gh, session, zip_bytes): artifacts = [ From 717fea2ecab20d833ea55d8e522ae02eaadddd09 Mon Sep 17 00:00:00 2001 From: Daniel Anya Date: Fri, 16 May 2025 18:38:50 -0400 Subject: [PATCH 5/9] Adds support for Github pages html url coverage report --- action.yml | 4 ++++ coverage_comment/main.py | 1 + coverage_comment/settings.py | 1 + coverage_comment/storage.py | 32 +++++++++++++++++++++++++++++--- tests/integration/test_github.py | 14 +++++++++++--- tests/unit/test_storage.py | 21 ++++++++++++++++++--- 6 files changed, 64 insertions(+), 9 deletions(-) diff --git a/action.yml b/action.yml index a6fa986f..c06ce5e6 100644 --- a/action.yml +++ b/action.yml @@ -102,6 +102,10 @@ inputs: notice, warning and error as annotation type. For more information look here: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message default: warning + USE_GH_PAGES_HTML_URL: + description: > + If true, will use the GitHub Pages URL for the coverage report instead of the raw URL or an htmlpreview.github.io link. + default: false VERBOSE: description: > Deprecated, see https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/enabling-debug-logging diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 6b2b4697..b49e9749 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -430,6 +430,7 @@ def save_coverage_data_files( github_host=github_host, branch=config.FINAL_COVERAGE_DATA_BRANCH, repository=config.GITHUB_REPOSITORY, + use_gh_pages_html_url=config.USE_GH_PAGES_HTML_URL, ) readme_file, log_message = communication.get_readme_and_log( is_public=is_public, diff --git a/coverage_comment/settings.py b/coverage_comment/settings.py index 1617c5b8..8c5ed59b 100644 --- a/coverage_comment/settings.py +++ b/coverage_comment/settings.py @@ -63,6 +63,7 @@ class Config: ANNOTATE_MISSING_LINES: bool = False ANNOTATION_TYPE: str = "warning" MAX_FILES_IN_COMMENT: int = 25 + USE_GH_PAGES_HTML_URL: bool = False VERBOSE: bool = False # Only for debugging, not exposed in the action: FORCE_WORKFLOW_RUN: bool = False diff --git a/coverage_comment/storage.py b/coverage_comment/storage.py index 4d0f9c93..655466d3 100644 --- a/coverage_comment/storage.py +++ b/coverage_comment/storage.py @@ -172,10 +172,36 @@ def get_repo_file_url( return f"{github_host}/{repository}/{part}/{branch}{path}".rstrip("/") -def get_html_report_url(github_host: str, repository: str, branch: str) -> str: +def get_html_report_url( + github_host: str, + repository: str, + branch: str, + use_gh_pages_html_url: bool = False, +) -> str: + """ + Computes the URL for an HTML report: + - If use_gh_pages_html_url is True: + * GitHub.com => https://.github.io// + * GitHub Enterprise => https:///pages/// + - If use_gh_pages_html_url is False: + * GitHub.com => https://htmlpreview.github.io/? + * GitHub Enterprise => + """ + html_report_path = "htmlcov/index.html" readme_url = get_repo_file_url( - github_host, repository=repository, branch=branch, path="/htmlcov/index.html" + github_host, repository=repository, branch=branch, path=html_report_path ) + if github_host.endswith("github.com"): - return f"https://htmlpreview.github.io/?{readme_url}" + if use_gh_pages_html_url: + user, repo = repository.split("/", 1) + return f"https://{user}.github.io/{repo}/{html_report_path}" + else: + return f"https://htmlpreview.github.io/?{readme_url}" + else: + # Assume GitHub Enterprise + if use_gh_pages_html_url: + return f"{github_host}/pages/{repository}/{html_report_path}" + + # Always fallback to the raw readme_url return readme_url diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py index 8964bbd2..1f6c9092 100644 --- a/tests/integration/test_github.py +++ b/tests/integration/test_github.py @@ -45,20 +45,28 @@ def test_get_repository_info(gh, session): assert info == github.RepositoryInfo(default_branch="baz", visibility="public") + @pytest.mark.parametrize( "api_url, expected", [ ("https://api.github.com/repos/foo/bar", "https://github.com"), ("https://api.github.com:8080/repos/foo/bar", "https://github.com:8080"), ("https://api.github.com/repos/foo/bar/issues", "https://github.com"), - ("https://my-ghe.company.com/api/v3/repos/foo/bar", "https://my-ghe.company.com"), - ("https://my-ghe.company.com/api/v3/repos/foo/bar/issues", "https://my-ghe.company.com"), - ] + ( + "https://my-ghe.company.com/api/v3/repos/foo/bar", + "https://my-ghe.company.com", + ), + ( + "https://my-ghe.company.com/api/v3/repos/foo/bar/issues", + "https://my-ghe.company.com", + ), + ], ) def test_extract_github_host(api_url, expected): result = github.extract_github_host(api_url=api_url) assert result == expected + def test_download_artifact(gh, session, zip_bytes): artifacts = [ {"name": "bar", "id": 456}, diff --git a/tests/unit/test_storage.py b/tests/unit/test_storage.py index 82bd5c29..dc359491 100644 --- a/tests/unit/test_storage.py +++ b/tests/unit/test_storage.py @@ -227,20 +227,35 @@ def test_get_repo_file_url__no_path(github_host): @pytest.mark.parametrize( - "github_host, expected", + "github_host,use_gh_pages_html_url,expected", [ ( "https://github.com", + True, + "https://foo.github.io/bar/htmlcov/index.html", + ), + ( + "https://github.com", + False, "https://htmlpreview.github.io/?https://github.com/foo/bar/blob/baz/htmlcov/index.html", ), ( "https://github.mycompany.com", + True, + "https://github.mycompany.com/pages/foo/bar/htmlcov/index.html", + ), + ( + "https://github.mycompany.com", + False, "https://github.mycompany.com/foo/bar/blob/baz/htmlcov/index.html", ), ], ) -def test_get_html_report_url(github_host, expected): +def test_get_html_report_url(github_host, use_gh_pages_html_url, expected): result = storage.get_html_report_url( - github_host=github_host, repository="foo/bar", branch="baz" + github_host=github_host, + repository="foo/bar", + branch="baz", + use_gh_pages_html_url=use_gh_pages_html_url, ) assert result == expected From 19a6297885530097c26ad6a8422feb3fc892bcd6 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Wed, 20 Aug 2025 21:35:15 +0200 Subject: [PATCH 6/9] Add mention in the doc on supporting pull_request/closed --- README.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/README.md b/README.md index 31ea33b9..d9df4e60 100644 --- a/README.md +++ b/README.md @@ -391,6 +391,35 @@ the `push` events instead. This is most likely only useful for setups not accepting external PRs and you will not have the best user experience. If that's something you need to do, please have a look at [this issue](https://github.com/py-cov-action/python-coverage-comment-action/issues/234). +### Updating the coverage information on the `pull_request/closed` event + +Usually, the coverage data for the repository is updated on `push` events to the default +branch, but it can also work to do it on `pull_request/closed` events, especially if +you require all changes to go through a pull request. + +In this case, your workflow's `on:` clause should look like this: + +```yaml +on: + pull_request: + # opened, synchronize, reopened are the default value + # closed will trigger when the PR is closed (merged or not) + types: [opened, synchronize, reopened, closed] + +jobs: + build: + # Optional: if you want to avoid doing the whole build on PRs closed without + # merging, add the following clause. Note that this action won't update the + # coverage data even if you don't specify this (it will raise an error instead), + # but it can help you avoid a useless build. + if: github.event.action != "closed" || github.event.pull_request.merged == true + runs-on: ubuntu-latest + ... +``` + +> [!TIP] +> The action will also save repository coverage data on `schedule` workflows. + ## Overriding the template By default, comments are generated from a From 8f59450d817bd9665666ec8bd354c3380d774b92 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Wed, 20 Aug 2025 23:26:30 +0200 Subject: [PATCH 7/9] Move logic of reading the event payload file to settings.py --- coverage_comment/activity.py | 11 ++++---- coverage_comment/main.py | 13 ++------- coverage_comment/settings.py | 19 +++++++++++++ tests/conftest.py | 11 +++++++- tests/integration/conftest.py | 6 ++--- tests/integration/test_main.py | 49 ++++++++++++++++++++++++++++++++++ tests/unit/test_activity.py | 40 +++++++++++++++++++-------- tests/unit/test_main.py | 9 ++++--- tests/unit/test_settings.py | 25 +++++++++++++++-- 9 files changed, 145 insertions(+), 38 deletions(-) diff --git a/coverage_comment/activity.py b/coverage_comment/activity.py index 1b5507e1..a134cefe 100644 --- a/coverage_comment/activity.py +++ b/coverage_comment/activity.py @@ -16,7 +16,8 @@ class ActivityNotFound(Exception): def find_activity( event_name: str, is_default_branch: bool, - event_action: str | None = None, + event_type: str, + is_pr_merged: bool, ) -> str: """Find the activity to perform based on the event type and payload.""" if event_name == "workflow_run": @@ -24,13 +25,11 @@ def find_activity( if ( (event_name == "push" and is_default_branch) - or ( - event_name == "pull_request" - and event_action == "merged" - and is_default_branch - ) or event_name == "schedule" + or (event_name == "pull_request" and event_type == "closed") ): + if event_name == "pull_request" and event_type == "closed" and not is_pr_merged: + raise ActivityNotFound return "save_coverage_data_files" if event_name not in {"pull_request", "push"}: diff --git a/coverage_comment/main.py b/coverage_comment/main.py index b49e9749..4d5ce97a 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -1,7 +1,6 @@ from __future__ import annotations import functools -import json import logging import os import sys @@ -70,15 +69,6 @@ def action( log.debug(f"Operating on {config.GITHUB_REF}") gh = github_client.GitHub(session=github_session) event_name = config.GITHUB_EVENT_NAME - event_path = config.GITHUB_EVENT_PATH - event_action = None - - if event_path and os.path.exists(event_path): - with open(event_path) as event_file: - event_payload = json.load(event_file) - is_merged_pr_action = event_payload.get("pull_request", {}).get("merged", False) - if is_merged_pr_action: - event_action = "merged" repo_info = github.get_repository_info( github=gh, repository=config.GITHUB_REPOSITORY @@ -86,8 +76,9 @@ def action( try: activity = activity_module.find_activity( event_name=event_name, - event_action=event_action, is_default_branch=repo_info.is_default_branch(ref=config.GITHUB_REF), + event_type=config.GITHUB_EVENT_TYPE, + is_pr_merged=config.IS_PR_MERGED, ) except activity_module.ActivityNotFound: log.error( diff --git a/coverage_comment/settings.py b/coverage_comment/settings.py index 8c5ed59b..fbfff6e1 100644 --- a/coverage_comment/settings.py +++ b/coverage_comment/settings.py @@ -2,7 +2,9 @@ import dataclasses import decimal +import functools import inspect +import json import pathlib from collections.abc import MutableMapping from typing import Any @@ -139,6 +141,23 @@ def GITHUB_BRANCH_NAME(self) -> str | None: return self.GITHUB_REF.split("/", 2)[2] return None + @functools.cached_property + def GITHUB_EVENT_PAYLOAD(self) -> dict: + if not self.GITHUB_EVENT_PATH: + return {} + return json.loads(self.GITHUB_EVENT_PATH.read_text()) + + @property + def GITHUB_EVENT_TYPE(self) -> str | None: + return self.GITHUB_EVENT_PAYLOAD.get("action") + + @property + def IS_PR_MERGED(self) -> bool: + try: + return self.GITHUB_EVENT_PAYLOAD["pull_request"]["merged"] + except KeyError: + return False + @property def FINAL_COMMENT_FILENAME(self): filename = self.COMMENT_FILENAME diff --git a/tests/conftest.py b/tests/conftest.py index 49c2b2c6..82de9fca 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,7 @@ import os import pathlib import zipfile +from collections.abc import Callable import httpx import pytest @@ -47,7 +48,7 @@ def _(**kwargs): @pytest.fixture -def pull_request_config(base_config): +def pull_request_config(base_config) -> Callable[..., settings.Config]: def _(**kwargs): defaults = { # GitHub stuff @@ -250,6 +251,14 @@ def summary_file(tmp_path): return file +@pytest.fixture +def pull_request_event_payload(tmp_path): + file = tmp_path / "event.json" + file.touch() + + return file + + _is_failed = [] diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index d6aace95..f8e0e775 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -9,11 +9,9 @@ @pytest.fixture -def in_integration_env(integration_env, integration_dir): - curdir = os.getcwd() - os.chdir(integration_dir) +def in_integration_env(integration_env, integration_dir, monkeypatch): + monkeypatch.chdir(integration_dir) yield integration_dir - os.chdir(curdir) @pytest.fixture diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py index e98c949e..3211ad13 100644 --- a/tests/integration/test_main.py +++ b/tests/integration/test_main.py @@ -581,6 +581,55 @@ def test_action__push__default_branch( assert "Missing" in summary_content +def test_action__pull_request_closed_merged( + pull_request_config, + session, + in_integration_env, + get_logs, + git, + summary_file, + pull_request_event_payload, +): + session.register("GET", "/repos/py-cov-action/foobar")( + json={"default_branch": "main", "visibility": "public"} + ) + session.register( + "GET", + "https://img.shields.io/static/v1?label=Coverage&message=77%25&color=orange", + )(text="") + + git.register("git branch --show-current")(stdout="foo") + git.register("git reset --hard")() + git.register("git fetch origin python-coverage-comment-action-data")() + git.register("git switch python-coverage-comment-action-data")() + git.register("git add endpoint.json")() + git.register("git add data.json")() + git.register("git add badge.svg")() + git.register("git add htmlcov")() + git.register("git add README.md")() + git.register("git diff --staged --exit-code")(exit_code=1) + git.register("git commit --message Update coverage data")() + git.register("git push origin python-coverage-comment-action-data")() + git.register("git switch foo")() + + pull_request_event_payload.write_text( + """{"action": "closed", "pull_request": {"merged": true}}""" + ) + + result = main.action( + config=pull_request_config( + GITHUB_STEP_SUMMARY=summary_file, + GITHUB_EVENT_PATH=pull_request_event_payload, + ), + github_session=session, + http_session=session, + git=git, + ) + assert result == 0 + + assert get_logs("INFO", "Saving coverage files") + + def test_action__push__default_branch__private( push_config, session, in_integration_env, get_logs, git ): diff --git a/tests/unit/test_activity.py b/tests/unit/test_activity.py index d54b49e9..e1390f0b 100644 --- a/tests/unit/test_activity.py +++ b/tests/unit/test_activity.py @@ -6,26 +6,44 @@ @pytest.mark.parametrize( - "event_name, event_action, is_default_branch, expected_activity", + "event_name, is_default_branch, event_type, is_pr_merged, expected_activity", [ - ("workflow_run", None, True, "post_comment"), - ("push", None, True, "save_coverage_data_files"), - ("push", None, False, "process_pr"), - ("pull_request", "merged", True, "save_coverage_data_files"), - ("pull_request", None, True, "process_pr"), - ("pull_request", None, False, "process_pr"), - ("schedule", None, False, "save_coverage_data_files"), + ("workflow_run", True, None, False, "post_comment"), + ("push", True, None, False, "save_coverage_data_files"), + ("push", False, None, False, "process_pr"), + ("pull_request", True, "closed", True, "save_coverage_data_files"), + ("pull_request", True, None, False, "process_pr"), + ("pull_request", False, None, False, "process_pr"), + ("schedule", False, None, False, "save_coverage_data_files"), ], ) -def test_find_activity(event_name, event_action, is_default_branch, expected_activity): +def test_find_activity( + event_name, is_default_branch, event_type, is_pr_merged, expected_activity +): result = activity.find_activity( event_name=event_name, - event_action=event_action, is_default_branch=is_default_branch, + event_type=event_type, + is_pr_merged=is_pr_merged, ) assert result == expected_activity def test_find_activity_not_found(): with pytest.raises(activity.ActivityNotFound): - activity.find_activity(event_name="not_found", is_default_branch=False) + activity.find_activity( + event_name="not_found", + is_default_branch=False, + event_type="not_found", + is_pr_merged=False, + ) + + +def test_find_activity_pr_closed_not_merged(): + with pytest.raises(activity.ActivityNotFound): + activity.find_activity( + event_name="pull_request", + is_default_branch=False, + event_type="closed", + is_pr_merged=False, + ) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 36e29df1..d140f796 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -3,6 +3,7 @@ import os import httpx +import pytest from coverage_comment import main, settings, subprocess @@ -14,7 +15,6 @@ def test_main(mocker, get_logs): # We could also accept not to test this function but if we've come this # far and have 98% coverage, we can as well have 100%. - exit = mocker.patch("sys.exit") action = mocker.patch("coverage_comment.main.action") os.environ.update( @@ -26,11 +26,14 @@ def test_main(mocker, get_logs): "GITHUB_BASE_REF": "", "GITHUB_EVENT_NAME": "push", "GITHUB_STEP_SUMMARY": "step_summary", + "GITHUB_EVENT_PATH": "foo/bar", } ) - main.main() - exit.assert_called_with(action.return_value) + with pytest.raises(SystemExit) as exc_data: + main.main() + + assert exc_data.value.code == action.return_value kwargs = action.call_args_list[0].kwargs assert isinstance(kwargs["config"], settings.Config) assert isinstance(kwargs["git"], subprocess.Git) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 720263c7..1589f61a 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -1,7 +1,9 @@ from __future__ import annotations import decimal +import json import pathlib +from collections.abc import Callable import pytest @@ -104,7 +106,7 @@ def test_config__verbose_deprecated(get_logs): @pytest.fixture -def config(): +def config() -> Callable[..., settings.Config]: defaults = { "GITHUB_BASE_REF": "master", "GITHUB_TOKEN": "foo", @@ -122,7 +124,7 @@ def config(): "MERGE_COVERAGE_FILES": True, } - def _(**kwargs): + def _(**kwargs) -> settings.Config: return settings.Config(**(defaults | kwargs)) return _ @@ -196,3 +198,22 @@ def test_final_coverage_data_branch(config): SUBPROJECT_ID="bar", ) assert config_obj.FINAL_COVERAGE_DATA_BRANCH == "foo-bar" + + +@pytest.mark.parametrize("merged", [True, False]) +def test_is_pr_merged(tmp_path, config, merged): + path = tmp_path / "event.json" + path.write_text( + json.dumps({"action": "closed", "pull_request": {"merged": merged}}) + ) + config_obj = config(GITHUB_EVENT_PATH=path) + + assert config_obj.IS_PR_MERGED is merged + + +def test_is_pr_merged__other_payload(tmp_path, config): + path = tmp_path / "event.json" + path.write_text(json.dumps({"action": "other"})) + config_obj = config(GITHUB_EVENT_PATH=path) + + assert config_obj.IS_PR_MERGED is False From 627d6dd31a2730514cabed7152eae20556480c95 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Wed, 20 Aug 2025 23:27:13 +0200 Subject: [PATCH 8/9] Simplify extract_github_host --- coverage_comment/github.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/coverage_comment/github.py b/coverage_comment/github.py index 992c38fd..b4b79507 100644 --- a/coverage_comment/github.py +++ b/coverage_comment/github.py @@ -4,6 +4,7 @@ import io import json import pathlib +import re import sys import zipfile from urllib.parse import urlparse @@ -63,13 +64,10 @@ def extract_github_host(api_url: str) -> str: scheme = parsed_url.scheme netloc = parsed_url.netloc # This includes the domain and potentially the port - # Special case for GitHub.com API - if netloc == "api.github.com": - host_domain = "github.com" - # Special case for GitHub.com with port (less common but good practice) - elif netloc.startswith("api.github.com:"): + # Special case for GitHub.com API (including possible port) + if re.match(r"api\.github\.com(:|$)", netloc): # Remove 'api.' prefix but keep the port - host_domain = netloc.replace("api.", "", 1) + host_domain = netloc.removeprefix("api.") # General case for GitHub Enterprise (netloc is already the host:port) else: host_domain = netloc From 370c909e5f1ab393c34e150d4d006884a3ee9743 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Wed, 20 Aug 2025 23:48:53 +0200 Subject: [PATCH 9/9] Missing cleaner for event path --- coverage_comment/settings.py | 4 ++++ tests/unit/test_settings.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/coverage_comment/settings.py b/coverage_comment/settings.py index fbfff6e1..eebf8140 100644 --- a/coverage_comment/settings.py +++ b/coverage_comment/settings.py @@ -127,6 +127,10 @@ def clean_coverage_path(cls, value: str) -> pathlib.Path: def clean_github_output(cls, value: str) -> pathlib.Path: return pathlib.Path(value) + @classmethod + def clean_github_event_path(cls, value: str) -> pathlib.Path: + return pathlib.Path(value) + @property def GITHUB_PR_NUMBER(self) -> int | None: # "refs/pull/2/merge" diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 1589f61a..314a1dcf 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -35,7 +35,7 @@ def test_config__from_environ__ok(): "GITHUB_REF": "master", "GITHUB_OUTPUT": "foo.txt", "GITHUB_EVENT_NAME": "pull", - "GITHUB_EVENT_PATH": pathlib.Path("test_event_path"), + "GITHUB_EVENT_PATH": "test_event_path", "GITHUB_PR_RUN_ID": "123", "GITHUB_STEP_SUMMARY": "step_summary", "COMMENT_ARTIFACT_NAME": "baz",