diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 4df7069..c626e91 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -8,7 +8,6 @@ import sys import tempfile import time -import typing import urllib.error import urllib.parse import urllib.request @@ -170,19 +169,32 @@ def report_size_deltas_from_workflow_artifacts(self) -> None: page_number += 1 page_count = api_data["page_count"] - def report_exists(self, pr_number: int, pr_head_sha: str) -> bool: + def report_exists(self, pr_number: int, pr_head_sha: str = "") -> str | None: """Return whether a report has already been commented to the pull request thread for the latest workflow run. - Keyword arguments: - pr_number -- number of the pull request to check - pr_head_sha -- PR's head branch hash + If `pr_head_sha` is a blank str, then all thread comments are traversed. Additionally, + any report comment found will be deleted if it is not the first report comment found. + This is designed to support the action input `update-comment` when asserted. + + If `pr_head_sha` is not a blank str, then thread comments are traversed + until a report comment that corresponds to the commit is found. + No comments are deleted in this scenario. + + Arguments: + pr_number: number of the pull request to check + pr_head_sha: PR's head branch hash + Returns: + - A URL str to use for PATCHing the first applicable report comment. + - None if no applicable report comments exist. """ - # Get the pull request's comments + comment_url: str | None = None + request_uri = f"repos/{self.repository_name}/issues/{pr_number}/comments" page_number = 1 page_count = 1 while page_number <= page_count: + # Get the pull request's comments api_data = self.api_request( - request="repos/" + self.repository_name + "/issues/" + str(pr_number) + "/comments", + request=request_uri, page_number=page_number, ) @@ -190,13 +202,20 @@ def report_exists(self, pr_number: int, pr_head_sha: str) -> bool: for comment_data in comments_data: # Check if the comment is a report for the PR's head SHA if comment_data["body"].startswith(self.report_key_beginning + pr_head_sha): - return True + if pr_head_sha: + return comment_data["url"] + # else: pr_head_sha == "" + if comment_url is None: + comment_url = comment_data["url"] + else: # found another report + # delete report comment if it is not the first report found + self.http_request(url=comment_data["url"], method="DELETE") page_number += 1 page_count = api_data["page_count"] # No reports found for the PR's head SHA - return False + return comment_url def get_artifacts_data_for_sha(self, pr_user_login: str, pr_head_ref: str, pr_head_sha: str): """Return the list of data objects for the report artifacts associated with the given head commit hash. @@ -526,43 +545,6 @@ def get_summary_value(self, show_emoji: bool, minimum, maximum) -> str: return value - def get_previous_comment(self, url: str) -> str | None: - """Get a previous comment to update. - - Arguments: - url -- The URL used to traverse existing comments of a PR thread. - To get the comment total, this str should be in the form: - "{GITHUB_API_URL}/repos/{GITHUB_REPOSITORY}/issues/{PR_NUMBER}" - Returns: - - A URL str to use for PATCHing the latest applicable comment. - - None if no previous/applicable comments exist. - """ - comment_url: str | None = None - - pr_info = self.get_json_response(url) - comment_count = typing.cast(typing.Dict[str, int], pr_info["json_data"])["comments"] - - comments_api_url = url + "/comments" - comments_response = self.get_json_response(url=comments_api_url) - while comment_count: - comments = typing.cast( - typing.List[typing.Dict[str, typing.Any]], - comments_response["json_data"], - ) - for comment in comments: - if typing.cast(str, comment["body"]).startswith(self.report_key_beginning): - if comment_url is not None: # we have more than 1 old comment - # delete old comment - self.http_request(url=comment_url, method="DELETE") - - # keep track of latest posted comment only - comment_url = typing.cast(str, comment["url"]) - comment_count -= len(comments) - next_page = comments_response["page"] + 1 - comments_response = self.get_json_response(url=f"{comments_api_url}/?page={next_page}") - - return comment_url - def comment_report(self, pr_number: int, report_markdown: str) -> None: """Submit the report as a comment on the PR thread. @@ -572,13 +554,12 @@ def comment_report(self, pr_number: int, report_markdown: str) -> None: """ print("::debug::Adding deltas report comment to pull request") report_data = json.dumps(obj={"body": report_markdown}).encode(encoding="utf-8") - url = f"https://api.github.com/repos/{self.repository_name}/issues/{pr_number}" + url = "https://api.github.com/repos/" + self.repository_name + "/issues/" + str(pr_number) + "/comments" - comment_url = None if not self.update_comment else self.get_previous_comment(url) - url = comment_url or url + "/comments" + comment_url = None if not self.update_comment else self.report_exists(pr_number=pr_number) method = "PATCH" if comment_url else None - self.http_request(url=url, data=report_data, method=method) + self.http_request(url=comment_url or url, data=report_data, method=method) def api_request(self, request: str, request_parameters: str = "", page_number: int = 1): """Do a GitHub API request. Return a dictionary containing: diff --git a/reportsizedeltas/tests/data/test_get_previous_comment/comments.json b/reportsizedeltas/tests/data/test_delete_previous_comment/comments.json similarity index 100% rename from reportsizedeltas/tests/data/test_get_previous_comment/comments.json rename to reportsizedeltas/tests/data/test_delete_previous_comment/comments.json diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index 7a93eff..6d8f3dd 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -367,18 +367,47 @@ def test_report_exists(): report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name) - json_data = [{"body": "foo123"}, {"body": report_size_deltas.report_key_beginning + pr_head_sha + "foo"}] + json_data = [ + {"body": "foo123"}, + {"body": report_size_deltas.report_key_beginning + pr_head_sha + "foo", "url": "some/url"}, + ] report_size_deltas.api_request = unittest.mock.MagicMock( return_value={"json_data": json_data, "additional_pages": False, "page_count": 1} ) - assert report_size_deltas.report_exists(pr_number=pr_number, pr_head_sha=pr_head_sha) + assert json_data[1]["url"] == report_size_deltas.report_exists(pr_number=pr_number, pr_head_sha=pr_head_sha) report_size_deltas.api_request.assert_called_once_with( request="repos/" + repository_name + "/issues/" + str(pr_number) + "/comments", page_number=1 ) - assert not report_size_deltas.report_exists(pr_number=pr_number, pr_head_sha="asdf") + assert report_size_deltas.report_exists(pr_number=pr_number, pr_head_sha="asdf") is None + + +def test_delete_previous_comment(): + pr_number = 42 + repository_name = "test_user/test_repo" + + report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name) + + json_comments = json.loads((test_data_path / "test_delete_previous_comment" / "comments.json").read_bytes()) + + report_size_deltas.http_request = unittest.mock.Mock() + report_size_deltas.get_json_response = unittest.mock.Mock( + return_value={"json_data": json_comments, "page_count": 1}, + ) + + comment_url = report_size_deltas.report_exists(pr_number=pr_number) + assert comment_url == json_comments[0]["url"] + + for comment in json_comments[1:4]: + if comment["body"].startswith(report_size_deltas.report_key_beginning): + report_size_deltas.http_request.assert_any_call(url=comment["url"], method="DELETE") + + # It would be nicer to assert that a call has not been made. + # Here, we just assert that only the last 3 out of 4 bot comments were deleted. + # Implicitly, this also means the non-bot comment (`json_comment[4]`) was not deleted. + assert report_size_deltas.http_request.call_count == 3 def test_get_artifacts_data_for_sha(): @@ -789,37 +818,6 @@ def test_comment_report(): ) -def test_get_previous_comment(): - pr_number = 42 - repository_name = "test_user/test_repo" - url = f"https://api.github.com/repos/{repository_name}/issues/{pr_number}" - - report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name) - - json_comments = json.loads((test_data_path / "test_get_previous_comment" / "comments.json").read_bytes()) - - def side_effect(url: str): - ret_val = {"page": 1} - if url.endswith("/comments"): - return {"json_data": json_comments, **ret_val} - return {"json_data": {"comments": len(json_comments)}, **ret_val} - - report_size_deltas.http_request = unittest.mock.Mock() - report_size_deltas.get_json_response = unittest.mock.Mock(side_effect=side_effect) - - comment_url = report_size_deltas.get_previous_comment(url=url) - assert comment_url == json_comments[3]["url"] - - for comment in json_comments[:3]: - if comment["body"].startswith(report_size_deltas.report_key_beginning): - report_size_deltas.http_request.assert_any_call(url=comment["url"], method="DELETE") - - # It would be nicer to assert that a call has not been made. - # Here, we just assert that the first 3 out of 4 bot comments were deleted. - # Implicitly, this also means the non-bot comment (`json_comment[4]`) was not deleted. - assert report_size_deltas.http_request.call_count == 3 - - def test_api_request(): response_data = {"json_data": {"foo": "bar"}, "additional_pages": False, "page_count": 1} request = "test_request"