Skip to content

Commit

Permalink
do not review closed PRs
Browse files Browse the repository at this point in the history
reduce test permutations to only what is needed
  • Loading branch information
2bndy5 committed Jan 8, 2024
1 parent 34710e2 commit 1d1e5a4
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
10 changes: 6 additions & 4 deletions cpp_linter/rest_api/github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/reviews/pr_27.json
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down
36 changes: 25 additions & 11 deletions tests/reviews/test_pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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,
Expand Down Expand Up @@ -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"},
Expand All @@ -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
Expand Down

0 comments on commit 1d1e5a4

Please sign in to comment.