Fix manual dequeue of github items

Currently dequeuing a github item fails with [1]. This is caused
because the DequeueEvent has no change_url attribute. In order to fix
this we need to add a check and add the correct change_url if it
doesn't exist. This by accident fixes a second issue where manually
enqueued github items miss the change_url.

This adds a fix and test case for both use cases.

[1] Traceback:
2018-11-21 14:17:56,592 ERROR zuul.Scheduler: Exception in management event:
Traceback (most recent call last):
  File "/opt/zuul/lib/python3.6/site-packages/zuul/scheduler.py", line 1096, in process_management_queue
    self._doDequeueEvent(event)
  File "/opt/zuul/lib/python3.6/site-packages/zuul/scheduler.py", line 917, in _doDequeueEvent
    change = project.source.getChange(event, project)
  File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/github/githubsource.py", line 66, in getChange
    return self.connection.getChange(event, refresh)
  File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/github/githubconnection.py", line 789, in getChange
    change.url = event.change_url
AttributeError: 'DequeueEvent' object has no attribute 'change_url'

Change-Id: Ifbaa67dc06c671f9235545a3215aa337ab27697e
Story: 2003747
Task: 26432
This commit is contained in:
Tobias Henkel 2018-11-21 16:30:38 +01:00
parent 33699fa316
commit bc136cb40e
No known key found for this signature in database
GPG Key ID: 03750DEC158E5FA2
4 changed files with 76 additions and 6 deletions

View File

@ -1228,7 +1228,7 @@ class FakeGithubConnection(githubconnection.GithubConnection):
self.getGithubClient(project).addProject(project)
def _getPullReviews(self, owner, project, number):
pr = self.pull_requests[number]
pr = self.pull_requests[int(number)]
return pr.reviews
def getGitUrl(self, project):
@ -1246,13 +1246,13 @@ class FakeGithubConnection(githubconnection.GithubConnection):
def commentPull(self, project, pr_number, message):
# record that this got reported
self.reports.append((project, pr_number, 'comment'))
pull_request = self.pull_requests[pr_number]
pull_request = self.pull_requests[int(pr_number)]
pull_request.addComment(message)
def mergePull(self, project, pr_number, commit_message='', sha=None):
# record that this got reported
self.reports.append((project, pr_number, 'merge'))
pull_request = self.pull_requests[pr_number]
pull_request = self.pull_requests[int(pr_number)]
if self.merge_failure:
raise Exception('Pull request was not merged')
if self.merge_not_allowed_count > 0:
@ -1272,7 +1272,7 @@ class FakeGithubConnection(githubconnection.GithubConnection):
def labelPull(self, project, pr_number, label):
# record that this got reported
self.reports.append((project, pr_number, 'label', label))
pull_request = self.pull_requests[pr_number]
pull_request = self.pull_requests[int(pr_number)]
pull_request.addLabel(label)
def unlabelPull(self, project, pr_number, label):

View File

@ -379,7 +379,7 @@ class FakeGithubClient(object):
project_name, self._data)
def pull_request(self, owner, project, number):
fake_pr = self._data.pull_requests[number]
fake_pr = self._data.pull_requests[int(number)]
return FakePull(fake_pr)
def search_issues(self, query):

View File

@ -23,6 +23,8 @@ from unittest import mock, skip
import git
import github3.exceptions
import zuul.rpcclient
from tests.base import ZuulTestCase, simple_layout, random_sha1
from tests.base import ZuulWebFixture
@ -52,6 +54,8 @@ class TestGithubDriver(ZuulTestCase):
self.assertEqual(str(A.number), zuulvars['change'])
self.assertEqual(str(A.head_sha), zuulvars['patchset'])
self.assertEqual('master', zuulvars['branch'])
self.assertEquals('https://github.com/org/project/pull/1',
zuulvars['items'][0]['change_url'])
self.assertEqual(1, len(A.comments))
self.assertThat(
A.comments[0],
@ -935,6 +939,67 @@ class TestGithubDriver(ZuulTestCase):
new_sha='0' * 40,
modified_files=['README.md'])
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_client_dequeue_change_github(self):
"Test that the RPC client can dequeue a github pull request"
client = zuul.rpcclient.RPCClient('127.0.0.1',
self.gearman_server.port)
self.addCleanup(client.shutdown)
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
client.dequeue(
tenant='tenant-one',
pipeline='check',
project='org/project',
change='{},{}'.format(A.number, A.head_sha),
ref=None)
self.waitUntilSettled()
tenant = self.sched.abide.tenants.get('tenant-one')
check_pipeline = tenant.layout.pipelines['check']
self.assertEqual(check_pipeline.getAllItems(), [])
self.assertEqual(self.countJobResults(self.history, 'ABORTED'), 2)
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_client_enqueue_change_github(self):
"Test that the RPC client can enqueue a pull request"
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
client = zuul.rpcclient.RPCClient('127.0.0.1',
self.gearman_server.port)
self.addCleanup(client.shutdown)
r = client.enqueue(tenant='tenant-one',
pipeline='check',
project='org/project',
trigger='github',
change='{},{}'.format(A.number, A.head_sha))
self.waitUntilSettled()
self.assertEqual(self.getJobFromHistory('project-test1').result,
'SUCCESS')
self.assertEqual(self.getJobFromHistory('project-test2').result,
'SUCCESS')
self.assertEqual(r, True)
# check that change_url is correct
job1_params = self.getJobFromHistory('project-test1').parameters
job2_params = self.getJobFromHistory('project-test2').parameters
self.assertEquals('https://github.com/org/project/pull/1',
job1_params['zuul']['items'][0]['change_url'])
self.assertEquals('https://github.com/org/project/pull/1',
job2_params['zuul']['items'][0]['change_url'])
class TestGithubUnprotectedBranches(ZuulTestCase):
config_file = 'zuul-github-driver.conf'

View File

@ -786,7 +786,12 @@ class GithubConnection(BaseConnection):
if event.change_number:
change = self._getChange(project, event.change_number,
event.patch_number, refresh=refresh)
change.url = event.change_url
if hasattr(event, 'change_url') and event.change_url:
change.url = event.change_url
else:
# The event has no change url so just construct it
change.url = self.getPullUrl(
event.project_name, event.change_number)
change.uris = [
'%s/%s/pull/%s' % (self.server, project, change.number),
]