diff options
author | Simon Westphahl <simon.westphahl@bmw.de> | 2022-11-18 10:34:59 +0100 |
---|---|---|
committer | Simon Westphahl <simon.westphahl@bmw.de> | 2022-11-18 11:59:32 +0100 |
commit | c8aac6a118b84fd28ea454161d33f28184c4bd0b (patch) | |
tree | 5831afa0f8c2024add4010ce640bef38f5a57300 | |
parent | 4d555ca675d204b1d668a63fab2942a70f159143 (diff) | |
download | zuul-c8aac6a118b84fd28ea454161d33f28184c4bd0b.tar.gz |
Check if Github detected a merge conflict for a PR
Github uses libgit2 to compute merges without requiring a worktree [0].
In some cases this can lead to Github detecting a merge conflict while
for Zuul the PR merges fine.
To prevent such changes from entering dependent pipelines and e.g. cause
a gate reset, we'll also check if Github detected any merge conflicts.
[0] https://github.blog/2022-10-03-highlights-from-git-2-38/
Change-Id: I22275f24c903a8548bb0ef6c32a2e15ba9eadac8
-rw-r--r-- | tests/base.py | 7 | ||||
-rw-r--r-- | tests/fake_graphql.py | 4 | ||||
-rw-r--r-- | tests/fakegithub.py | 2 | ||||
-rw-r--r-- | tests/unit/test_github_driver.py | 12 | ||||
-rw-r--r-- | zuul/driver/github/githubconnection.py | 7 | ||||
-rw-r--r-- | zuul/driver/github/githubmodel.py | 3 | ||||
-rw-r--r-- | zuul/driver/github/graphql/__init__.py | 5 | ||||
-rw-r--r-- | zuul/driver/github/graphql/canmerge.graphql | 1 |
8 files changed, 37 insertions, 4 deletions
diff --git a/tests/base.py b/tests/base.py index 897e443d3..239b243a5 100644 --- a/tests/base.py +++ b/tests/base.py @@ -2340,7 +2340,7 @@ class FakeGithubPullRequest(object): def __init__(self, github, number, project, branch, subject, upstream_root, files=None, number_of_commits=1, writers=[], body=None, body_text=None, draft=False, - base_sha=None): + mergeable=True, base_sha=None): """Creates a new PR with several commits. Sends an event about opened PR. @@ -2356,6 +2356,7 @@ class FakeGithubPullRequest(object): self.body = body self.body_text = body_text self.draft = draft + self.mergeable = mergeable self.number_of_commits = 0 self.upstream_root = upstream_root # Dictionary of FakeFile -> content @@ -2911,12 +2912,12 @@ class FakeGithubConnection(githubconnection.GithubConnection): def openFakePullRequest(self, project, branch, subject, files=[], body=None, body_text=None, draft=False, - base_sha=None): + mergeable=True, base_sha=None): self.pr_number += 1 pull_request = FakeGithubPullRequest( self, self.pr_number, project, branch, subject, self.upstream_root, files=files, body=body, body_text=body_text, draft=draft, - base_sha=base_sha) + mergeable=mergeable, base_sha=base_sha) self.pull_requests[self.pr_number] = pull_request return pull_request diff --git a/tests/fake_graphql.py b/tests/fake_graphql.py index 6c7567910..9a3f07060 100644 --- a/tests/fake_graphql.py +++ b/tests/fake_graphql.py @@ -181,10 +181,14 @@ class FakeCommit(ObjectType): class FakePullRequest(ObjectType): isDraft = Boolean() reviewDecision = String() + mergeable = String() def resolve_isDraft(parent, info): return parent.draft + def resolve_mergeable(parent, info): + return "MERGEABLE" if parent.mergeable else "CONFLICTING" + def resolve_reviewDecision(parent, info): if hasattr(info.context, 'version') and info.context.version: if info.context.version < (2, 21, 0): diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 47cefd08d..4255d5022 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -547,7 +547,7 @@ class FakePull(object): 'login': 'octocat' }, 'draft': pr.draft, - 'mergeable': True, + 'mergeable': pr.mergeable, 'state': pr.state, 'head': { 'sha': pr.head_sha, diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 90d69a7bc..47cc6c624 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -785,6 +785,18 @@ class TestGithubDriver(ZuulTestCase): self.assertFalse(A.is_merged) self.assertHistory([]) + @simple_layout('layouts/dependent-github.yaml', driver='github') + def test_non_mergeable_pr(self): + # pipeline merges the pull request on success + A = self.fake_github.openFakePullRequest('org/project', 'master', + 'PR title', mergeable=False) + self.fake_github.emitEvent(A.addLabel('merge')) + self.waitUntilSettled() + + # A non-mergeable pull request must not enter gate + self.assertFalse(A.is_merged) + self.assertHistory([]) + @simple_layout('layouts/reporting-multiple-github.yaml', driver='github') def test_reporting_multiple_github(self): project = 'org/project1' diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 1e3af1e78..15a41a3b8 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1692,6 +1692,8 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): change.contexts = self._get_contexts(canmerge_data) change.draft = canmerge_data.get('isDraft', False) + change.mergeable = (canmerge_data.get('mergeable', 'MERGEABLE').lower() + in ('mergeable', 'unknown')) change.review_decision = canmerge_data['reviewDecision'] change.required_contexts = set( canmerge_data['requiredStatusCheckContexts'] @@ -1860,6 +1862,11 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): log.debug('Change %s can not merge because it is a draft', change) return False + if not change.mergeable: + log.debug('Change %s can not merge because Github detected a ' + 'merge conflict', change) + return False + missing_status_checks = self._getMissingStatusChecks( change, allow_needs) if missing_status_checks: diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 256333234..30610cf4e 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -38,6 +38,7 @@ class PullRequest(Change): self.files = [] self.labels = [] self.draft = None + self.mergeable = True self.review_decision = None self.required_contexts = set() self.contexts = set() @@ -74,6 +75,7 @@ class PullRequest(Change): "reviews": list(self.reviews), "labels": self.labels, "draft": self.draft, + "mergeable": self.mergeable, "review_decision": self.review_decision, "required_contexts": list(self.required_contexts), "contexts": list(self.contexts), @@ -90,6 +92,7 @@ class PullRequest(Change): self.reviews = data.get("reviews", []) self.labels = data.get("labels", []) self.draft = data.get("draft") + self.mergeable = data.get("mergeable", True) self.review_decision = data.get("review_decision") self.required_contexts = set(data.get("required_contexts", [])) self.contexts = set(tuple(c) for c in data.get("contexts", [])) diff --git a/zuul/driver/github/graphql/__init__.py b/zuul/driver/github/graphql/__init__.py index f0d17f5f0..babf780ee 100644 --- a/zuul/driver/github/graphql/__init__.py +++ b/zuul/driver/github/graphql/__init__.py @@ -116,6 +116,11 @@ class GraphQLClient: pull_request = nested_get(repository, 'pullRequest') result['isDraft'] = nested_get(pull_request, 'isDraft', default=False) + # Check if Github detected a merge conflict. Possible enum values + # are CONFLICTING, MERGEABLE and UNKNOWN. + result['mergeable'] = nested_get(pull_request, 'mergeable', + default='MERGEABLE') + # Get review decision. This is supported since GHE 2.21. Default to # None to signal if the field is not present. result['reviewDecision'] = nested_get( diff --git a/zuul/driver/github/graphql/canmerge.graphql b/zuul/driver/github/graphql/canmerge.graphql index 7b7e862fd..fdd72c083 100644 --- a/zuul/driver/github/graphql/canmerge.graphql +++ b/zuul/driver/github/graphql/canmerge.graphql @@ -20,6 +20,7 @@ query canMergeData( } pullRequest(number: $pull) { isDraft + mergeable reviewDecision } object(expression: $head_sha) { |