zuul/zuul
Code Issues Proposed changes

Support for dependent pipelines with github

Browse Source 
Change-Id: I5e7937d0f524107572f8aec7945f94b620d77b61
Co-Authored-By: Jesse Keating <omgjlk@us.ibm.com>
This commit is contained in:
Jan Hruban
2015-11-19 14:30:49 +01:00
committed by Jesse Keating
parent 324ca5b744
commit 37615e5c6f
5 changed files with 280 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
tests/base.py
View File

@@ -547,7 +547,7 @@ class GithubChangeReference(git.Reference):
class FakeGithubPullRequest(object):
def __init__(self, github, number, project, branch,
upstream_root, number_of_commits=1):
subject, upstream_root, number_of_commits=1):
"""Creates a new PR with several commits.
Sends an event about opened PR."""
self.github = github
@@ -555,6 +555,8 @@ class FakeGithubPullRequest(object):
self.number = number
self.project = project
self.branch = branch
self.subject = subject
self.number_of_commits = 0
self.upstream_root = upstream_root
self.comments = []
self.labels = []
@@ -681,13 +683,15 @@ class FakeGithubPullRequest(object):
repo = self._getRepo()
ref = repo.references[self._getPRReference()]
if reset:
self.number_of_commits = 0
ref.set_object('refs/tags/init')
self.number_of_commits += 1
repo.head.reference = ref
zuul.merger.merger.reset_repo_to_head(repo)
repo.git.clean('-x', '-f', '-d')
fn = '%s-%s' % (self.branch.replace('/', '_'), self.number)
msg = 'test-%s' % self.number
msg = self.subject + '-' + str(self.number_of_commits)
fn = os.path.join(repo.working_dir, fn)
f = open(fn, 'w')
with open(fn, 'w') as f:
@@ -757,10 +761,10 @@ class FakeGithubConnection(githubconnection.GithubConnection):
self.merge_failure = False
self.merge_not_allowed_count = 0
def openFakePullRequest(self, project, branch):
def openFakePullRequest(self, project, branch, subject):
self.pr_number += 1
pull_request = FakeGithubPullRequest(
self, self.pr_number, project, branch, self.upstream_root)
self, self.pr_number, project, branch, subject, self.upstream_root)
self.pull_requests.append(pull_request)
return pull_request
@@ -803,6 +807,7 @@ class FakeGithubConnection(githubconnection.GithubConnection):
},
'ref': pr.branch,
},
'mergeable': True,
'head': {
'sha': pr.head_sha
}

35
tests/fixtures/layouts/dependent-github.yaml vendored Normal file
View File

@@ -0,0 +1,35 @@
- pipeline:
name: gate
description: Gatekeeping
manager: dependent
trigger:
github:
- event: pull_request
action: labeled
label: 'merge'
success:
github:
merge: true
unlabel: 'merge'
failure:
github:
unlabel: 'merge'
- job:
name: project-test1
- job:
name: project-test2
- job:
name: project-merge
failure-message: Unable to merge change
hold-following-changes: true
- project:
name: org/project
gate:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge

255
tests/unit/test_github_driver.py
View File

@@ -31,14 +31,14 @@ class TestGithubDriver(ZuulTestCase):
def test_pull_event(self):
self.executor_server.hold_jobs_in_build = True
pr = self.fake_github.openFakePullRequest('org/project', 'master')
self.fake_github.emitEvent(pr.getPullRequestOpenedEvent())
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
build_params = self.builds[0].parameters
self.assertEqual('master', build_params['ZUUL_BRANCH'])
self.assertEqual(str(pr.number), build_params['ZUUL_CHANGE'])
self.assertEqual(pr.head_sha, build_params['ZUUL_PATCHSET'])
self.assertEqual(str(A.number), build_params['ZUUL_CHANGE'])
self.assertEqual(A.head_sha, build_params['ZUUL_PATCHSET'])
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
@@ -51,20 +51,20 @@ class TestGithubDriver(ZuulTestCase):
job = self.getJobFromHistory('project-test2')
zuulvars = job.parameters['vars']['zuul']
self.assertEqual(pr.number, zuulvars['change'])
self.assertEqual(pr.head_sha, zuulvars['patchset'])
self.assertEqual(1, len(pr.comments))
self.assertEqual(A.number, zuulvars['change'])
self.assertEqual(A.head_sha, zuulvars['patchset'])
self.assertEqual(1, len(A.comments))
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_comment_event(self):
pr = self.fake_github.openFakePullRequest('org/project', 'master')
self.fake_github.emitEvent(pr.getCommentAddedEvent('test me'))
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getCommentAddedEvent('test me'))
self.waitUntilSettled()
self.assertEqual(2, len(self.history))
# Test an unmatched comment, history should remain the same
pr = self.fake_github.openFakePullRequest('org/project', 'master')
self.fake_github.emitEvent(pr.getCommentAddedEvent('casual comment'))
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
self.fake_github.emitEvent(B.getCommentAddedEvent('casual comment'))
self.waitUntilSettled()
self.assertEqual(2, len(self.history))
@@ -116,7 +116,7 @@ class TestGithubDriver(ZuulTestCase):
@simple_layout('layouts/labeling-github.yaml', driver='github')
def test_labels(self):
A = self.fake_github.openFakePullRequest('org/project', 'master')
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.addLabel('test'))
self.waitUntilSettled()
self.assertEqual(1, len(self.history))
@@ -124,7 +124,7 @@ class TestGithubDriver(ZuulTestCase):
self.assertEqual(['tests passed'], A.labels)
# test label removed
B = self.fake_github.openFakePullRequest('org/project', 'master')
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
B.addLabel('do not test')
self.fake_github.emitEvent(B.removeLabel('do not test'))
self.waitUntilSettled()
@@ -133,7 +133,7 @@ class TestGithubDriver(ZuulTestCase):
self.assertEqual(['tests passed'], B.labels)
# test unmatched label
C = self.fake_github.openFakePullRequest('org/project', 'master')
C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
self.fake_github.emitEvent(C.addLabel('other label'))
self.waitUntilSettled()
self.assertEqual(2, len(self.history))
@@ -143,16 +143,16 @@ class TestGithubDriver(ZuulTestCase):
def test_dequeue_pull_synchronized(self):
self.executor_server.hold_jobs_in_build = True
pr = self.fake_github.openFakePullRequest(
'org/one-job-project', 'master')
self.fake_github.emitEvent(pr.getPullRequestOpenedEvent())
A = self.fake_github.openFakePullRequest(
'org/one-job-project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
# event update stamp has resolution one second, wait so the latter
# one has newer timestamp
time.sleep(1)
pr.addCommit()
self.fake_github.emitEvent(pr.getPullRequestSynchronizeEvent())
A.addCommit()
self.fake_github.emitEvent(A.getPullRequestSynchronizeEvent())
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = False
@@ -166,11 +166,11 @@ class TestGithubDriver(ZuulTestCase):
def test_dequeue_pull_abandoned(self):
self.executor_server.hold_jobs_in_build = True
pr = self.fake_github.openFakePullRequest(
'org/one-job-project', 'master')
self.fake_github.emitEvent(pr.getPullRequestOpenedEvent())
A = self.fake_github.openFakePullRequest(
'org/one-job-project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
self.fake_github.emitEvent(pr.getPullRequestClosedEvent())
self.fake_github.emitEvent(A.getPullRequestClosedEvent())
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = False
@@ -196,54 +196,54 @@ class TestGithubDriver(ZuulTestCase):
def test_reporting(self):
# pipeline reports pull status both on start and success
self.executor_server.hold_jobs_in_build = True
pr = self.fake_github.openFakePullRequest('org/project', 'master')
self.fake_github.emitEvent(pr.getPullRequestOpenedEvent())
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
self.assertIn('check', pr.statuses)
check_status = pr.statuses['check']
self.assertIn('check', A.statuses)
check_status = A.statuses['check']
self.assertEqual('Standard check', check_status['description'])
self.assertEqual('pending', check_status['state'])
self.assertEqual('http://zuul.example.com/status', check_status['url'])
self.assertEqual(0, len(pr.comments))
self.assertEqual(0, len(A.comments))
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
check_status = pr.statuses['check']
check_status = A.statuses['check']
self.assertEqual('Standard check', check_status['description'])
self.assertEqual('success', check_status['state'])
self.assertEqual('http://zuul.example.com/status', check_status['url'])
self.assertEqual(1, len(pr.comments))
self.assertThat(pr.comments[0],
self.assertEqual(1, len(A.comments))
self.assertThat(A.comments[0],
MatchesRegex('.*Build succeeded.*', re.DOTALL))
# pipeline does not report any status but does comment
self.executor_server.hold_jobs_in_build = True
self.fake_github.emitEvent(
pr.getCommentAddedEvent('reporting check'))
A.getCommentAddedEvent('reporting check'))
self.waitUntilSettled()
self.assertNotIn('reporting', pr.statuses)
self.assertNotIn('reporting', A.statuses)
# comments increased by one for the start message
self.assertEqual(2, len(pr.comments))
self.assertThat(pr.comments[1],
self.assertEqual(2, len(A.comments))
self.assertThat(A.comments[1],
MatchesRegex('.*Starting reporting jobs.*', re.DOTALL))
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
self.assertNotIn('reporting', pr.statuses)
self.assertEqual(2, len(pr.comments))
self.assertNotIn('reporting', A.statuses)
self.assertEqual(2, len(A.comments))
@simple_layout('layouts/merging-github.yaml', driver='github')
def test_report_pull_merge(self):
# pipeline merges the pull request on success
A = self.fake_github.openFakePullRequest('org/project', 'master')
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertTrue(A.is_merged)
# pipeline merges the pull request on success after failure
self.fake_github.merge_failure = True
B = self.fake_github.openFakePullRequest('org/project', 'master')
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
self.fake_github.emitEvent(B.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertFalse(B.is_merged)
@@ -252,7 +252,7 @@ class TestGithubDriver(ZuulTestCase):
# pipeline merges the pull request on second run of merge
# first merge failed on 405 Method Not Allowed error
self.fake_github.merge_not_allowed_count = 1
C = self.fake_github.openFakePullRequest('org/project', 'master')
C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
self.fake_github.emitEvent(C.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertTrue(C.is_merged)
@@ -260,7 +260,180 @@ class TestGithubDriver(ZuulTestCase):
# pipeline does not merge the pull request
# merge failed on 405 Method Not Allowed error - twice
self.fake_github.merge_not_allowed_count = 2
D = self.fake_github.openFakePullRequest('org/project', 'master')
D = self.fake_github.openFakePullRequest('org/project', 'master', 'D')
self.fake_github.emitEvent(D.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertFalse(D.is_merged)
@simple_layout('layouts/dependent-github.yaml', driver='github')
def test_parallel_changes(self):
"Test that changes are tested in parallel and merged in series"
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
self.fake_github.emitEvent(A.addLabel('merge'))
self.fake_github.emitEvent(B.addLabel('merge'))
self.fake_github.emitEvent(C.addLabel('merge'))
self.waitUntilSettled()
self.assertEqual(len(self.builds), 1)
self.assertEqual(self.builds[0].name, 'project-merge')
self.assertTrue(self.builds[0].hasChanges(A))
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.assertEqual(len(self.builds), 3)
self.assertEqual(self.builds[0].name, 'project-test1')
self.assertTrue(self.builds[0].hasChanges(A))
self.assertEqual(self.builds[1].name, 'project-test2')
self.assertTrue(self.builds[1].hasChanges(A))
self.assertEqual(self.builds[2].name, 'project-merge')
self.assertTrue(self.builds[2].hasChanges(A, B))
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.assertEqual(len(self.builds), 5)
self.assertEqual(self.builds[0].name, 'project-test1')
self.assertTrue(self.builds[0].hasChanges(A))
self.assertEqual(self.builds[1].name, 'project-test2')
self.assertTrue(self.builds[1].hasChanges(A))
self.assertEqual(self.builds[2].name, 'project-test1')
self.assertTrue(self.builds[2].hasChanges(A))
self.assertEqual(self.builds[3].name, 'project-test2')
self.assertTrue(self.builds[3].hasChanges(A, B))
self.assertEqual(self.builds[4].name, 'project-merge')
self.assertTrue(self.builds[4].hasChanges(A, B, C))
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.assertEqual(len(self.builds), 6)
self.assertEqual(self.builds[0].name, 'project-test1')
self.assertTrue(self.builds[0].hasChanges(A))
self.assertEqual(self.builds[1].name, 'project-test2')
self.assertTrue(self.builds[1].hasChanges(A))
self.assertEqual(self.builds[2].name, 'project-test1')
self.assertTrue(self.builds[2].hasChanges(A, B))
self.assertEqual(self.builds[3].name, 'project-test2')
self.assertTrue(self.builds[3].hasChanges(A, B))
self.assertEqual(self.builds[4].name, 'project-test1')
self.assertTrue(self.builds[4].hasChanges(A, B, C))
self.assertEqual(self.builds[5].name, 'project-test2')
self.assertTrue(self.builds[5].hasChanges(A, B, C))
all_builds = self.builds[:]
self.release(all_builds[2])
self.release(all_builds[3])
self.waitUntilSettled()
self.assertFalse(A.is_merged)
self.assertFalse(B.is_merged)
self.assertFalse(C.is_merged)
self.release(all_builds[0])
self.release(all_builds[1])
self.waitUntilSettled()
self.assertTrue(A.is_merged)
self.assertTrue(B.is_merged)
self.assertFalse(C.is_merged)
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
self.assertEqual(len(self.builds), 0)
self.assertEqual(len(self.history), 9)
self.assertTrue(C.is_merged)
self.assertNotIn('merge', A.labels)
self.assertNotIn('merge', B.labels)
self.assertNotIn('merge', C.labels)
@simple_layout('layouts/dependent-github.yaml', driver='github')
def test_failed_changes(self):
"Test that a change behind a failed change is retested"
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
self.executor_server.failJob('project-test1', A)
self.fake_github.emitEvent(A.addLabel('merge'))
self.fake_github.emitEvent(B.addLabel('merge'))
self.waitUntilSettled()
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
# It's certain that the merge job for change 2 will run, but
# the test1 and test2 jobs may or may not run.
self.assertTrue(len(self.history) > 6)
self.assertFalse(A.is_merged)
self.assertTrue(B.is_merged)
self.assertNotIn('merge', A.labels)
self.assertNotIn('merge', B.labels)
@simple_layout('layouts/dependent-github.yaml', driver='github')
def test_failed_change_at_head(self):
"Test that if a change at the head fails, jobs behind it are canceled"
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
B = self.fake_github.openFakePullRequest('org/project', 'master', 'B')
C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
self.executor_server.failJob('project-test1', A)
self.fake_github.emitEvent(A.addLabel('merge'))
self.fake_github.emitEvent(B.addLabel('merge'))
self.fake_github.emitEvent(C.addLabel('merge'))
self.waitUntilSettled()
self.assertEqual(len(self.builds), 1)
self.assertEqual(self.builds[0].name, 'project-merge')
self.assertTrue(self.builds[0].hasChanges(A))
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.executor_server.release('.*-merge')
self.waitUntilSettled()
self.assertEqual(len(self.builds), 6)
self.assertEqual(self.builds[0].name, 'project-test1')
self.assertEqual(self.builds[1].name, 'project-test2')
self.assertEqual(self.builds[2].name, 'project-test1')
self.assertEqual(self.builds[3].name, 'project-test2')
self.assertEqual(self.builds[4].name, 'project-test1')
self.assertEqual(self.builds[5].name, 'project-test2')
self.release(self.builds[0])
self.waitUntilSettled()
# project-test2, project-merge for B
self.assertEqual(len(self.builds), 2)
self.assertEqual(self.countJobResults(self.history, 'ABORTED'), 4)
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
self.assertEqual(len(self.builds), 0)
self.assertEqual(len(self.history), 15)
self.assertFalse(A.is_merged)
self.assertTrue(B.is_merged)
self.assertTrue(C.is_merged)
self.assertNotIn('merge', A.labels)
self.assertNotIn('merge', B.labels)
self.assertNotIn('merge', C.labels)

13
zuul/driver/github/githubconnection.py
View File

@@ -293,6 +293,19 @@ class GithubConnection(BaseConnection):
owner, proj = project_name.split('/')
return self.github.pull_request(owner, proj, number).as_dict()
def canMerge(self, change, allow_needs):
# This API call may get a false (null) while GitHub is calculating
# if it can merge. The github3.py library will just return that as
# false. This could lead to false negatives.
# Additionally, this only checks if the PR code could merge
# cleanly to the target branch. It does not evaluate any branch
# protection merge requirements (such as reviews and status states)
# At some point in the future this may be available through the API
# or we can fetch the branch protection settings and evaluate within
# Zuul whether or not those protections have been met
# For now, just send back a True value.
return True
def commentPull(self, project, pr_number, message):
owner, proj = project.split('/')
repository = self.github.repository(owner, proj)

11
zuul/driver/github/githubsource.py
View File

@@ -38,11 +38,18 @@ class GithubSource(BaseSource):
def isMerged(self, change, head=None):
"""Determine if change is merged."""
raise NotImplementedError()
if not change.number:
# Not a pull request, considering merged.
return True
return change.is_merged
def canMerge(self, change, allow_needs):
"""Determine if change can merge."""
raise NotImplementedError()
if not change.number:
# Not a pull request, considering merged.
return True
return self.connection.canMerge(change, allow_needs)
def postConfig(self):
"""Called after configuration has been processed."""