Skip to content

Commit

Permalink
fix: fetch full PR details before executing merge checks
Browse files Browse the repository at this point in the history 
* ensure that we execute our checks against complete PR
  data and not some incomplete view on the pull request.
* add mandatory PR fields to fixtures
* do not drop mandatory PR fields during recording
  • Loading branch information
@nikku
nikku committed Nov 21, 2020
1 parent c70010d commit 3a69f91
Show file tree
Hide file tree
Showing 34 changed files with 142 additions and 35 deletions.
5 changes: 4 additions & 1 deletion lib/bot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ async function handlePullRequestReviewSubmitted(context) {
return;
}

// fetch pull request with full details
const pullRequest = await findPullRequestByShallowRef(context, pull_request);

// check, whether PR can be merged
await checkMerge(context, pull_request);
await checkMerge(context, pullRequest);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions lib/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ async function findPullRequestByStatus(context, status) {
return null;
}

return pullRequests[0];
return findPullRequestByShallowRef(context, pullRequests[0]);
}


Expand Down Expand Up @@ -490,13 +490,14 @@ async function canMerge(context, pullRequest) {
return false;
}

if ('merged' in pullRequest && pullRequest.merged) {
if (pullRequest.merged) {
context.log('skipping: pull request is already merged', PR(pullRequest));

return false;
}

if ('rebaseable' in pullRequest && pullRequest.rebaseable === false) {
// rebaseable => true, false or null
if (!pullRequest.rebaseable) {
context.log('skipping: pull request cannot be rebased', PR(pullRequest));

return false;
Expand Down
1 change: 1 addition & 0 deletions lib/recorder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ function filterStrip(key, value) {
// whitelist certain boolean attributes
typeof value === 'boolean' && !(
key === 'rebaseable' ||
key === 'mergeable' ||
key === 'draft' ||
key === 'merged'
)
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/ignore_queued_checks/001_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/ignore_queued_checks/005_pulls.get.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected/006_pull_request.opened.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected/015_pull_request.review_requested.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"requested_reviewer": {
"login": "nikku",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected/021_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"merged": true,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected_check_suite/000_pull_request.opened.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@
"commits": 1,
"additions": 1,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected_check_suite/003_pull_request.review_requested.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@
"commits": 1,
"additions": 1,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"requested_reviewer": {
"login": "nikku",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/protected_check_suite/010_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 1,
"additions": 1,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"merged": true,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
3 changes: 2 additions & 1 deletion test/fixtures/ready_for_review/009_pull_request.ready_for_review.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"mergeable": true
},
"repository": {
"name": "testtest-org",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/requested_teams/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/requested_teams/054_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 2
"changed_files": 2,
"rebaseable": false,
"mergeable": false
},
"repository": {
"name": "merge-me-test",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/requested_teams_no_review_teams/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/review_teams_approval_missing/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/review_teams_ignore_unmentioned/054_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 2
"changed_files": 2,
"rebaseable": false,
"mergeable": false
},
"repository": {
"name": "merge-me-test",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/review_teams_simple/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/review_teams_simple/054_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 2
"changed_files": 2,
"rebaseable": false,
"mergeable": false
},
"repository": {
"name": "merge-me-test",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/review_teams_user_in_multiple_teams/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/review_teams_user_in_multiple_teams/054_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 2
"changed_files": 2,
"rebaseable": false,
"mergeable": false
},
"repository": {
"name": "merge-me-test",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/review_teams_with_rejects/028_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "f9d55a5bff002d1e17967c5edc1fa979b9c243d6",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/skip_draft/000_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/skip_merged/000_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"draft": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
5 changes: 4 additions & 1 deletion test/fixtures/skip_non_rebaseable/000_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"mergeable": true,
"merged": false,
"draft": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_check_conclusions/019_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"merged": true,
"draft": false
},
"repository": {
"name": "testtest-checks",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_checks_only/002_pull_request.synchronize.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"before": "3831781b1c99d53a22dff46de4da83c4640916da",
"after": "ca7142df376bebb6edcc0c156ca6897d39f468e2",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_checks_only/019_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 2,
"additions": 2,
"deletions": 0,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"merged": true,
"draft": false
},
"repository": {
"name": "testtest-checks",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_rejected_reviews/013_pull_request.closed.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": false,
"mergeable": false,
"merged": true,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_status_only/000_pull_request.opened.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"repository": {
"name": "testtest",
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/unprotected_status_only/003_pull_request.review_requested.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
"commits": 1,
"additions": 1,
"deletions": 1,
"changed_files": 1
"changed_files": 1,
"rebaseable": true,
"mergeable": true,
"merged": false,
"draft": false
},
"requested_reviewer": {
"login": "waltaaa",
Expand Down
Loading

0 comments on commit 3a69f91

Please sign in to comment.