From 1d1e5a48f4d05ed1d58b23e537a5ef59d2e2093d Mon Sep 17 00:00:00 2001
From: Brendan <2bndy5@gmail.com>
Date: Mon, 8 Jan 2024 03:12:47 -0800
Subject: [PATCH] do not review closed PRs

reduce test permutations to only what is needed
---
 cpp_linter/rest_api/github_api.py | 10 +++++----
 tests/reviews/pr_27.json          |  2 +-
 tests/reviews/test_pr_review.py   | 36 +++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/cpp_linter/rest_api/github_api.py b/cpp_linter/rest_api/github_api.py
index 7de052b..565e8a3 100644
--- a/cpp_linter/rest_api/github_api.py
+++ b/cpp_linter/rest_api/github_api.py
@@ -360,13 +360,15 @@ def post_review(
         url += "/reviews"
         is_draft = True
         if log_response_msg(response_buffer):
-            is_draft = cast(Dict[str, bool], response_buffer.json()).get("draft", False)
+            pr_payload = response_buffer.json()
+            is_draft = cast(Dict[str, bool], pr_payload).get("draft", False)
+            is_open = cast(Dict[str, str], pr_payload).get("state", "open") == "open"
         if "GITHUB_TOKEN" not in environ:
             logger.error("A GITHUB_TOKEN env var is required to post review comments")
-            return
+            sys.exit(self.set_exit_code(1))
         self._dismiss_stale_reviews(url)
-        if is_draft:
-            return  # don't post reviews for PRs marked as "draft"
+        if is_draft or not is_open:  # is PR open and ready for review
+            return  # don't post reviews
         body = f"{COMMENT_MARKER}## Cpp-linter Review\n"
         payload_comments = []
         total_changes = 0
diff --git a/tests/reviews/pr_27.json b/tests/reviews/pr_27.json
index 1ce3675..4648845 100644
--- a/tests/reviews/pr_27.json
+++ b/tests/reviews/pr_27.json
@@ -50,7 +50,7 @@
 
   ],
   "milestone": null,
-  "draft": true,
+  "draft": false,
   "commits_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/pulls/27/commits",
   "review_comments_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/pulls/27/comments",
   "review_comment_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/pulls/comments{/number}",
diff --git a/tests/reviews/test_pr_review.py b/tests/reviews/test_pr_review.py
index 0da4dca..6146124 100644
--- a/tests/reviews/test_pr_review.py
+++ b/tests/reviews/test_pr_review.py
@@ -12,14 +12,17 @@
 TEST_PR = 27
 
 
-@pytest.mark.parametrize("is_draft", [True, False], ids=["draft", "ready"])
-@pytest.mark.parametrize("with_token", [True, False], ids=["has_token", "no_token"])
-@pytest.mark.parametrize("tidy_review", [True, False], ids=["yes_tidy", "no_tidy"])
 @pytest.mark.parametrize(
-    "format_review", [True, False], ids=["yes_format", "no_format"]
-)
-@pytest.mark.parametrize(
-    "force_approved", [True, False], ids=["approved", "request_changes"]
+    "is_draft,is_closed,with_token,force_approved,tidy_review,format_review",
+    [
+        (True, False, True, False, False, True),
+        (False, True, True, False, False, True),
+        pytest.param(False, False, False, False, False, True, marks=pytest.mark.xfail),
+        (False, False, True, True, False, True),
+        (False, False, True, False, True, False),
+        (False, False, True, False, False, True),
+    ],
+    ids=["draft", "closed", "no_token", "approved", "tidy", "format"],
 )
 @pytest.mark.parametrize(
     "lines_changed_only", [0, 1, 2], ids=["all_lines", "lines_added", "diff_lines"]
@@ -28,6 +31,7 @@ def test_post_review(
     monkeypatch: pytest.MonkeyPatch,
     tmp_path: Path,
     is_draft: bool,
+    is_closed: bool,
     with_token: bool,
     tidy_review: bool,
     format_review: bool,
@@ -117,9 +121,14 @@ def test_post_review(
         cache_pr_response = (cache_path / f"pr_{TEST_PR}.json").read_text(
             encoding="utf-8"
         )
-        cache_pr_response = cache_pr_response.replace(
-            '  "draft": true,', f'  "draft": {str(is_draft).lower()},', 1
-        )
+        if is_draft:
+            cache_pr_response = cache_pr_response.replace(
+                '  "draft": false,', '  "draft": true,', 1
+            )
+        if is_closed:
+            cache_pr_response = cache_pr_response.replace(
+                '  "state": "open",', '  "state": "closed",', 1
+            )
         mock.get(
             base_url,
             headers={"Accept": "application/vnd.github.text+json"},
@@ -140,7 +149,12 @@ def test_post_review(
 
         # inspect the review payload for correctness
         last_request = mock.last_request
-        if (tidy_review or format_review) and not is_draft and with_token:
+        if (
+            (tidy_review or format_review)
+            and not is_draft
+            and with_token
+            and not is_closed
+        ):
             assert hasattr(last_request, "json")
             json_payload = last_request.json()
             assert "body" in json_payload