do not review closed PRs

reduce test permutations to only what is needed

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

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}",

tests/reviews/test_pr_review.py

@@ -12,27 +12,41 @@
 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"]
-)
-@pytest.mark.parametrize(
-    "lines_changed_only", [0, 1, 2], ids=["all_lines", "lines_added", "diff_lines"]
+    "is_draft,is_closed,with_token,force_approved,tidy_review,format_review,changes",
+    [
+        (True, False, True, False, False, True, 2),
+        (False, True, True, False, False, True, 2),
+        pytest.param(
+            False, False, False, False, False, True, 2, marks=pytest.mark.xfail
+        ),
+        (False, False, True, True, False, True, 2),
+        (False, False, True, False, True, False, 2),
+        (False, False, True, False, False, True, 2),
+        (False, False, True, False, True, True, 1),
+        (False, False, True, False, True, True, 0),
+    ],
+    ids=[
+        "draft",
+        "closed",
+        "no_token",
+        "approved",
+        "tidy",  # changes == diff_chunks only
+        "format",  # changes == diff_chunks only
+        "lines_added",
+        "all_lines",
+    ],
 )
 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,
     force_approved: bool,
-    lines_changed_only: int,
+    changes: int,
 ):
     """A mock test of posting PR reviews"""
     # patch env vars
@@ -90,7 +104,7 @@ def test_post_review(
             extensions=["cpp", "hpp"],
             ignored=[],
             not_ignored=[],
-            lines_changed_only=lines_changed_only,
+            lines_changed_only=changes,
         )
         assert files
         for file_obj in files:
@@ -103,7 +117,7 @@ def test_post_review(
             version=environ.get("CLANG_VERSION", "16"),
             checks="",
             style="file",
-            lines_changed_only=lines_changed_only,
+            lines_changed_only=changes,
             database="",
             extra_args=[],
             tidy_review=tidy_review,
@@ -117,9 +131,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 +159,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