From d550619f33075aa80845e2c7d6bb0e06bc7ec099 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Wed, 2 Nov 2022 18:48:31 +0600 Subject: [PATCH 1/2] Add Request Review Comment Trigger for Review Comments --- bedevere/stage.py | 19 ++++++-- tests/test_stage.py | 113 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 3 deletions(-) diff --git a/bedevere/stage.py b/bedevere/stage.py index ccd0a3a3..664c1adf 100644 --- a/bedevere/stage.py +++ b/bedevere/stage.py @@ -9,8 +9,11 @@ import enum import random +from typing import Any import gidgethub.routing +from gidgethub.abc import GitHubAPI +from gidgethub.sansio import Event from . import util @@ -227,12 +230,14 @@ async def dismissed_review(event, gh, *args, **kwargs): await stage(gh, await util.issue_for_PR(gh, pull_request), Blocker.review) +@router.register("pull_request_review_comment", action="created") @router.register("issue_comment", action="created") -async def new_comment(event, gh, *args, **kwargs): - issue = event.data["issue"] +async def new_comment(event: Event, gh: GitHubAPI, *args: Any, **kwargs: Any) -> None: + commented_on = event.data.get("issue") or event.data.get("pull_request") comment = event.data["comment"] comment_body = comment["body"].lower() - if util.user_login(issue) != util.user_login(comment): + + if util.user_login(commented_on) != util.user_login(comment): # Only care about the PR creator leaving a comment. return elif not any(trigger.lower() in comment_body for trigger in TRIGGERS): @@ -243,6 +248,14 @@ async def new_comment(event, gh, *args, **kwargs): thanks = FUN_THANKS else: thanks = BORING_THANKS + + # For `issue_comment` event, + # the required data is already present in the `issue` dict. + # But, for `pull_request_review_comment` event, + # we need to fetch the issue (pull_request) data. + issue = event.data.get("issue") or await util.issue_for_PR( + gh, event.data.get("pull_request") + ) await request_core_review( gh, issue, blocker=Blocker.change_review, greeting=thanks ) diff --git a/tests/test_stage.py b/tests/test_stage.py index 2f78dd20..12551a22 100644 --- a/tests/test_stage.py +++ b/tests/test_stage.py @@ -798,6 +798,119 @@ async def test_new_comment(): assert "not-core-dev" not in requested_reviewers +async def test_pull_request_review_comment(): + # Comment not from PR author. + data = { + "action": "created", + "pull_request": {"user": {"login": "andreamcinnes"}}, + "comment": { + "user": {"login": "brettcannon"}, + "body": awaiting.BORING_TRIGGER_PHRASE, + }, + } + event = sansio.Event(data, event="pull_request_review_comment", delivery_id="12345") + gh = FakeGH() + await awaiting.router.dispatch(event, gh) + assert not len(gh.post_) + + # Comment from PR author but missing trigger phrase. + data = { + "action": "created", + "pull_request": {"user": {"login": "andreamcinnes"}}, + "comment": { + "user": {"login": "andreamcinnes"}, + "body": "I DID expect the Spanish Inquisition", + }, + } + event = sansio.Event(data, event="pull_request_review_comment", delivery_id="12345") + gh = FakeGH() + await awaiting.router.dispatch(event, gh) + assert not len(gh.post_) + + # Everything is right with the world. + data = { + "action": "created", + "pull_request": { + "user": {"login": "andreamcinnes"}, + "issue_url": "https://api.github.com/issue/42", + }, + "comment": { + "user": {"login": "andreamcinnes"}, + "body": awaiting.BORING_TRIGGER_PHRASE, + }, + } + event = sansio.Event(data, event="pull_request_review_comment", delivery_id="12345") + items = { + "https://api.github.com/teams/6/memberships/brettcannon": True, + "https://api.github.com/teams/6/memberships/gvanrossum": True, + "https://api.github.com/teams/6/memberships/not-core-dev": gidgethub.BadRequest( + status_code=http.HTTPStatus(404) + ), + "https://api.github.com/issue/42": { + "user": {"login": "andreamcinnes"}, + "labels": [], + "labels_url": "https://api.github.com/labels/42", + "url": "https://api.github.com/issue/42", + "pull_request": {"url": "https://api.github.com/pr/42"}, + "comments_url": "https://api.github.com/comments/42", + } + } + iterators = { + "https://api.github.com/orgs/python/teams": [{"name": "python core", "id": 6}], + "https://api.github.com/pr/42/reviews": [ + {"user": {"login": "brettcannon"}, "state": "approved"}, + {"user": {"login": "gvanrossum"}, "state": "changes_requested"}, + {"user": {"login": "not-core-dev"}, "state": "approved"}, + ], + } + gh = FakeGH(getitem=items, getiter=iterators) + await awaiting.router.dispatch(event, gh) + assert len(gh.post_) == 3 + labeling, comment, review_request = gh.post_ + assert labeling[0] == "https://api.github.com/labels/42" + assert labeling[1] == [awaiting.Blocker.change_review.value] + assert comment[0] == "https://api.github.com/comments/42" + comment_body = comment[1]["body"] + assert "@brettcannon" in comment_body + assert "@gvanrossum" in comment_body + assert "not-core-dev" not in comment_body + assert review_request[0] == "https://api.github.com/pr/42/requested_reviewers" + requested_reviewers = review_request[1]["reviewers"] + assert "brettcannon" in requested_reviewers + assert "gvanrossum" in requested_reviewers + assert "not-core-dev" not in requested_reviewers + + # All is right with the Monty Python world. + data = { + "action": "created", + "pull_request": { + "user": {"login": "andreamcinnes"}, + "issue_url": "https://api.github.com/issue/42", + }, + "comment": { + "user": {"login": "andreamcinnes"}, + "body": awaiting.FUN_TRIGGER_PHRASE, + }, + } + event = sansio.Event(data, event="issue_comment", delivery_id="12345") + gh = FakeGH(getitem=items, getiter=iterators) + await awaiting.router.dispatch(event, gh) + assert len(gh.post_) == 3 + labeling, comment, review_request = gh.post_ + assert labeling[0] == "https://api.github.com/labels/42" + assert labeling[1] == [awaiting.Blocker.change_review.value] + assert comment[0] == "https://api.github.com/comments/42" + comment_body = comment[1]["body"] + assert "@brettcannon" in comment_body + assert "@gvanrossum" in comment_body + assert "not-core-dev" not in comment_body + assert review_request[0] == "https://api.github.com/pr/42/requested_reviewers" + requested_reviewers = review_request[1]["reviewers"] + assert "brettcannon" in requested_reviewers + assert "gvanrossum" in requested_reviewers + assert "not-core-dev" not in requested_reviewers + + async def test_change_requested_for_core_dev(): data = { "action": "submitted", From 62c87943b3593f0eaad491d18e119a370958e191 Mon Sep 17 00:00:00 2001 From: Maksudul Haque Date: Wed, 2 Nov 2022 19:03:10 +0600 Subject: [PATCH 2/2] test cleanup --- tests/test_stage.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/test_stage.py b/tests/test_stage.py index 12551a22..3ff2678c 100644 --- a/tests/test_stage.py +++ b/tests/test_stage.py @@ -827,7 +827,7 @@ async def test_pull_request_review_comment(): await awaiting.router.dispatch(event, gh) assert not len(gh.post_) - # Everything is right with the world. + # Comment with BORING_TRIGGER_PHRASE data = { "action": "created", "pull_request": { @@ -850,7 +850,6 @@ async def test_pull_request_review_comment(): "user": {"login": "andreamcinnes"}, "labels": [], "labels_url": "https://api.github.com/labels/42", - "url": "https://api.github.com/issue/42", "pull_request": {"url": "https://api.github.com/pr/42"}, "comments_url": "https://api.github.com/comments/42", } @@ -880,7 +879,7 @@ async def test_pull_request_review_comment(): assert "gvanrossum" in requested_reviewers assert "not-core-dev" not in requested_reviewers - # All is right with the Monty Python world. + # Comment with FUN_TRIGGER_PHRASE data = { "action": "created", "pull_request": { @@ -892,7 +891,7 @@ async def test_pull_request_review_comment(): "body": awaiting.FUN_TRIGGER_PHRASE, }, } - event = sansio.Event(data, event="issue_comment", delivery_id="12345") + event = sansio.Event(data, event="pull_request_review_comment", delivery_id="12345") gh = FakeGH(getitem=items, getiter=iterators) await awaiting.router.dispatch(event, gh) assert len(gh.post_) == 3