adapt report_exists() for new input

update tests accordingly

replaces `get_previous_comment()` which was almost identical to `report_exists()`

Author: 2bndy5

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,33 +169,53 @@ 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,
             )
 
             comments_data = api_data["json_data"]
             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:

reportsizedeltas/tests/data/test_get_previous_comment/comments.json renamed to reportsizedeltas/tests/data/test_delete_previous_comment/comments.json

@@ -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"