Skip to content

Commit 29517b6

Browse files
dhruvmaniladhruvmanila
committed
fix: do not check PRs made by bot
fixes: #127
1 parent 02fbaf7 commit 29517b6

File tree

3 files changed

+74
-22
lines changed

3 files changed

+74
-22
lines changed

Diff for: ‎algorithms_keeper/event/pull_request.py

+9-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,11 @@ async def close_invalid_or_additional_pr(
116116
"""
117117
pull_request = event.data["pull_request"]
118118

119-
if pull_request["author_association"].lower() not in {"owner", "member"}:
119+
if (
120+
pull_request["author_association"].lower() not in {"owner", "member"}
121+
# Don't check for invalid pull request made by a bot.
122+
and pull_request["user"]["type"].lower() != "bot"
123+
):
120124
pr_body = pull_request["body"]
121125
pr_author = pull_request["user"]["login"]
122126
comment = None
@@ -205,6 +209,10 @@ async def check_pr_files(
205209
gh, label=label, pr_or_issue=pull_request
206210
)
207211

212+
# Don't perform file checks if the pull request is made by a bot.
213+
if pull_request["user"]["type"].lower() == "bot":
214+
return None
215+
208216
# Default behavior is to ignore modified files but that can be changed.
209217
# This will come only from the commands module through the command:
210218
# ``@algorithms-keeper review-all``

Diff for: ‎tests/test_commands.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def test_command_regex_match(text: str, group: str) -> None:
162162
pr_url: {
163163
"url": pr_url,
164164
"html_url": html_pr_url,
165-
"user": {"login": user},
165+
"user": {"login": user, "type": "User"},
166166
"labels": [],
167167
"draft": False,
168168
},
@@ -200,7 +200,7 @@ def test_command_regex_match(text: str, group: str) -> None:
200200
"html_url": html_pr_url,
201201
"issue_url": issue_url,
202202
"head": {"sha": sha},
203-
"user": {"login": user},
203+
"user": {"login": user, "type": "User"},
204204
"comments_url": comments_url,
205205
"labels": [],
206206
"draft": False,

Diff for: ‎tests/test_pull_requests.py

+63-19
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes:
8181
"pull_request": {
8282
"url": pr_url,
8383
"body": CHECKBOX_TICKED_UPPER, # Case doesn't matter
84-
"user": {"login": user},
84+
"user": {"login": user, "type": "User"},
8585
"labels": [],
8686
"author_association": "NONE",
8787
"comments_url": comments_url,
@@ -130,7 +130,7 @@ async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes:
130130
"url": pr_url,
131131
"body": CHECKBOX_TICKED_UPPER, # Case doesn't matter
132132
"labels": [],
133-
"user": {"login": user},
133+
"user": {"login": user, "type": "User"},
134134
"author_association": "NONE",
135135
"comments_url": comments_url,
136136
"issue_url": issue_url,
@@ -186,7 +186,7 @@ async def test_max_pr_by_user(
186186
"pull_request": {
187187
"url": pr_url,
188188
"body": "",
189-
"user": {"login": user},
189+
"user": {"login": user, "type": "User"},
190190
"labels": [],
191191
"author_association": "NONE",
192192
"comments_url": comments_url,
@@ -225,7 +225,7 @@ async def test_max_pr_by_user(
225225
"pull_request": {
226226
"url": pr_url,
227227
"body": CHECKBOX_NOT_TICKED,
228-
"user": {"login": user},
228+
"user": {"login": user, "type": "User"},
229229
"labels": [],
230230
"author_association": "NONE",
231231
"comments_url": comments_url,
@@ -264,7 +264,7 @@ async def test_max_pr_by_user(
264264
"pull_request": {
265265
"url": pr_url,
266266
"body": "",
267-
"user": {"login": user},
267+
"user": {"login": user, "type": "User"},
268268
"labels": [],
269269
"author_association": "NONE",
270270
"comments_url": comments_url,
@@ -303,7 +303,7 @@ async def test_max_pr_by_user(
303303
"body": CHECKBOX_TICKED,
304304
"head": {"sha": sha},
305305
"labels": [],
306-
"user": {"login": user},
306+
"user": {"login": user, "type": "User"},
307307
"author_association": "NONE",
308308
"comments_url": comments_url,
309309
"issue_url": issue_url,
@@ -350,7 +350,7 @@ async def test_max_pr_by_user(
350350
"pull_request": {
351351
"url": pr_url,
352352
"body": CHECKBOX_TICKED,
353-
"user": {"login": user},
353+
"user": {"login": user, "type": "User"},
354354
"labels": [],
355355
"author_association": "NONE",
356356
"comments_url": comments_url,
@@ -385,7 +385,7 @@ async def test_max_pr_by_user(
385385
"pull_request": {
386386
"url": pr_url,
387387
"body": CHECKBOX_TICKED,
388-
"user": {"login": user},
388+
"user": {"login": user, "type": "User"},
389389
"labels": [],
390390
"author_association": "NONE",
391391
"comments_url": comments_url,
@@ -414,7 +414,7 @@ async def test_max_pr_by_user(
414414
"url": pr_url,
415415
"body": "", # body can be empty for member
416416
"labels": [],
417-
"user": {"login": user},
417+
"user": {"login": user, "type": "User"},
418418
"author_association": "MEMBER",
419419
"comments_url": comments_url,
420420
"issue_url": issue_url,
@@ -435,6 +435,50 @@ async def test_max_pr_by_user(
435435
post_data=[{"labels": [Label.REVIEW]}],
436436
),
437437
),
438+
# Pull request opened by a bot, so don't perform any validation checks nor any
439+
# file checks. This will be verified by keeping the pull request body empty.
440+
(
441+
Event(
442+
data={
443+
"action": "opened",
444+
"pull_request": {
445+
"url": pr_url,
446+
"body": "",
447+
"labels": [],
448+
"user": {"login": "bot", "type": "Bot"},
449+
"author_association": "NONE",
450+
"comments_url": comments_url,
451+
"issue_url": issue_url,
452+
"html_url": html_pr_url,
453+
"requested_reviewers": [],
454+
"draft": False,
455+
"mergeable": True,
456+
},
457+
"repository": {"full_name": repository},
458+
},
459+
event="pull_request",
460+
delivery_id="pr_opened_by_bot",
461+
),
462+
MockGitHubAPI(
463+
getiter={
464+
files_url: [
465+
{
466+
"filename": "annotation.py",
467+
"contents_url": "",
468+
"status": "added",
469+
},
470+
]
471+
}
472+
),
473+
# No file checks should be performed by the bot. This is validated by
474+
# passing an invalid file in the mock data and no comment/label added
475+
# to the pull request.
476+
ExpectedData(
477+
getiter_url=[files_url],
478+
post_url=[labels_url],
479+
post_data=[{"labels": [Label.REVIEW]}],
480+
),
481+
),
438482
# Pull request synchronize event to test whether the add labels function gets
439483
# called if there are any labels to be added, post review comments gets called
440484
# if there are comments to be posted and remove labels function gets called if
@@ -449,7 +493,7 @@ async def test_max_pr_by_user(
449493
# This got added when the pull request was opened.
450494
"labels": [{"name": Label.REVIEW}, {"name": Label.TYPE_HINT}],
451495
"head": {"sha": sha},
452-
"user": {"login": user},
496+
"user": {"login": user, "type": "User"},
453497
"author_association": "NONE",
454498
"comments_url": comments_url,
455499
"issue_url": issue_url,
@@ -496,7 +540,7 @@ async def test_max_pr_by_user(
496540
# The label was added when the PR was opened.
497541
"labels": [{"name": Label.REVIEW}],
498542
"head": {"sha": sha},
499-
"user": {"login": user},
543+
"user": {"login": user, "type": "User"},
500544
"author_association": "NONE",
501545
"comments_url": comments_url,
502546
"issue_url": issue_url,
@@ -538,7 +582,7 @@ async def test_max_pr_by_user(
538582
"body": CHECKBOX_TICKED,
539583
"head": {"sha": sha},
540584
"labels": [],
541-
"user": {"login": user},
585+
"user": {"login": user, "type": "User"},
542586
"author_association": "NONE",
543587
"comments_url": comments_url,
544588
"issue_url": issue_url,
@@ -741,7 +785,7 @@ async def test_max_pr_by_user(
741785
data={
742786
"action": "synchronize",
743787
"pull_request": {
744-
"user": {"login": user},
788+
"user": {"login": user, "type": "User"},
745789
"issue_url": issue_url,
746790
"labels": [{"name": Label.CHANGE}],
747791
"draft": True,
@@ -762,7 +806,7 @@ async def test_max_pr_by_user(
762806
"pull_request": {
763807
"url": pr_url,
764808
"html_url": html_pr_url,
765-
"user": {"login": user},
809+
"user": {"login": user, "type": "User"},
766810
"issue_url": issue_url,
767811
"labels": [{"name": Label.CHANGE}],
768812
"draft": False,
@@ -823,7 +867,7 @@ async def test_max_pr_by_user(
823867
"pull_request": {
824868
"url": pr_url,
825869
"html_url": html_pr_url,
826-
"user": {"login": user},
870+
"user": {"login": user, "type": "User"},
827871
"issue_url": issue_url,
828872
"labels": [{"name": Label.REVIEW}],
829873
"draft": False,
@@ -857,7 +901,7 @@ async def test_max_pr_by_user(
857901
"pull_request": {
858902
"url": pr_url,
859903
"html_url": html_pr_url,
860-
"user": {"login": user},
904+
"user": {"login": user, "type": "User"},
861905
"issue_url": issue_url,
862906
"labels": [{"name": Label.REVIEW}],
863907
"draft": False,
@@ -879,7 +923,7 @@ async def test_max_pr_by_user(
879923
"pull_request": {
880924
"url": pr_url,
881925
"html_url": html_pr_url,
882-
"user": {"login": user},
926+
"user": {"login": user, "type": "User"},
883927
"issue_url": issue_url,
884928
"labels": [
885929
{"name": Label.REVIEW},
@@ -907,7 +951,7 @@ async def test_max_pr_by_user(
907951
"pull_request": {
908952
"url": pr_url,
909953
"html_url": html_pr_url,
910-
"user": {"login": user},
954+
"user": {"login": user, "type": "User"},
911955
"issue_url": issue_url,
912956
"labels": [{"name": Label.REVIEW}],
913957
"draft": False,
@@ -933,7 +977,7 @@ async def test_max_pr_by_user(
933977
"pull_request": {
934978
"url": pr_url,
935979
"html_url": html_pr_url,
936-
"user": {"login": user},
980+
"user": {"login": user, "type": "User"},
937981
"issue_url": issue_url,
938982
"labels": [
939983
{"name": Label.REVIEW},

0 commit comments

Comments
 (0)