Replaced old PR quality comments when writing new ones.

2 files changed, 73 insertions, 10 deletions

diff --git a/scripts/pr_quality/check_pr.py b/scripts/pr_quality/check_pr.py
index 51d106e875..e1991e46d3 100644
--- a/scripts/pr_quality/check_pr.py
+++ b/scripts/pr_quality/check_pr.py
@@ -30,6 +30,7 @@ import urllib.error
 import urllib.parse
 import urllib.request
 from datetime import date, datetime, timedelta, timezone
+from http import HTTPStatus
 
 from pr_quality.errors import (
     CHECKS_FOOTER,
@@ -102,7 +103,8 @@ def github_request(method, path, token, repo, data=None, params=None):
         headers["Content-Type"] = "application/json"
     req = urllib.request.Request(url, data=body, headers=headers, method=method)
     with urllib.request.urlopen(req, timeout=URLOPEN_TIMEOUT_SECONDS) as response:
-        return json.loads(response.read())
+        if response.status != HTTPStatus.NO_CONTENT:
+            return json.loads(response.read())
 
 
 def get_recent_commit_count(pr_author, repo, token, since_days, max_count):
@@ -144,6 +146,26 @@ def get_pr_total_changes(pr_number, repo, token):
     return total_changes
 
 
+def get_comment_ids_to_delete(pr_number, repo, token):
+    ids = []
+    page = 1
+    while True:
+        comments = github_request(
+            "GET",
+            f"/issues/{pr_number}/comments",
+            token,
+            repo,
+            {"per_page": GITHUB_PER_PAGE, "page": page},
+        )
+        for comment in comments:
+            if CHECKS_HEADER in comment["body"]:
+                ids.append(comment["id"])
+        if len(comments) < GITHUB_PER_PAGE:
+            break
+        page += 1
+    return ids
+
+
 def strip_html_comments(text):
     """Return text with all HTML comments removed."""
     return re.sub(r"<!--.*?-->", "", text, flags=re.DOTALL)
@@ -506,6 +528,14 @@ def main(
         logger.info("PR #%s passed all quality checks.", pr_number)
         return
 
+    for id_to_delete in get_comment_ids_to_delete(pr_number, repo, token):
+        github_request(
+            "DELETE",
+            f"/issues/comments/{id_to_delete}",
+            token,
+            repo,
+        )
+
     github_request(
         "POST",
         f"/issues/{pr_number}/comments",
diff --git a/scripts/pr_quality/tests/test_check_pr.py b/scripts/pr_quality/tests/test_check_pr.py
index c7565df5ab..30bfa26512 100644
--- a/scripts/pr_quality/tests/test_check_pr.py
+++ b/scripts/pr_quality/tests/test_check_pr.py
@@ -781,8 +781,16 @@ class TestIntegration(BaseTestCase):
         body = make_pr_body(ticket="", checked_items=0)
         result, _, mock_gh = self.call_main(pr_body=body, commit_count=1)
         self.assertEqual(result, 1)
-        mock_gh.assert_called_once_with(
-            "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+        self.assertEqual(
+            mock_gh.call_args_list,
+            [
+                mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
+                    "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+            ],
         )
 
     def test_untrusted_author_failures_posts_comment_and_closes(self):
@@ -790,9 +798,12 @@ class TestIntegration(BaseTestCase):
         result, _, mock_gh = self.call_main(pr_body=body)
         self.assertEqual(result, 1)
         self.assertEqual(
-            mock_gh.mock_calls,
+            mock_gh.call_args_list,
             [
                 mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
                     "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
                 ),
                 mock.call(
@@ -806,9 +817,12 @@ class TestIntegration(BaseTestCase):
         result, _, mock_gh = self.call_main(pr_body=body, pr_author="")
         self.assertEqual(result, 1)
         self.assertEqual(
-            mock_gh.mock_calls,
+            mock_gh.call_args_list,
             [
                 mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
                     "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
                 ),
                 mock.call(
@@ -821,8 +835,16 @@ class TestIntegration(BaseTestCase):
         body = make_pr_body(ticket="", checked_items=0)
         result, _, mock_gh = self.call_main(pr_body=body, autoclose=False)
         self.assertEqual(result, 1)
-        mock_gh.assert_called_once_with(
-            "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+        self.assertEqual(
+            mock_gh.call_args_list,
+            [
+                mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
+                    "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+            ],
         )
 
     def test_warnings_only_posts_comment_does_not_close(self):
@@ -833,8 +855,16 @@ class TestIntegration(BaseTestCase):
             pr_body=VALID_PR_BODY, pr_title="", trac_data=trac_data
         )
         self.assertIsNone(result)
-        mock_gh.assert_called_once_with(
-            "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+        self.assertEqual(
+            mock_gh.call_args_list,
+            [
+                mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
+                    "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+            ],
         )
 
     def test_warnings_alongside_failures_still_closes(self):
@@ -844,9 +874,12 @@ class TestIntegration(BaseTestCase):
         result, _, mock_gh = self.call_main(pr_body=body, trac_data=trac_data)
         self.assertEqual(result, 1)
         self.assertEqual(
-            mock_gh.mock_calls,
+            mock_gh.call_args_list,
             [
                 mock.call(
+                    "GET", "/issues/10/comments", "test-token", "test/repo", mock.ANY
+                ),
+                mock.call(
                     "POST", "/issues/10/comments", "test-token", "test/repo", mock.ANY
                 ),
                 mock.call(