Fix branch parsing in _event_pull

We currently split the ref to get the branch name. However branches
can contain slashes like feature/x so we need to limit that to two
splits.

Change-Id: Ie936fcd13d5cf7d45bb464312810e8c3a558c51a
This commit is contained in:
Tobias Henkel 2018-06-22 10:52:51 +02:00
parent a784a4fbb7
commit 7ceb38bb4c
No known key found for this signature in database
GPG Key ID: 03750DEC158E5FA2
3 changed files with 48 additions and 19 deletions

View File

@ -104,6 +104,9 @@ class FakeRepository(object):
client = FakeGithubClient(self.data)
return client.session.get(url, headers)
def _create_branch(self, branch):
self._branches.append((FakeBranch(branch=branch)))
def create_status(self, sha, state, url, description, context,
user='zuul'):
# Since we're bypassing github API, which would require a user, we
@ -134,15 +137,14 @@ class FakeRepository(object):
def get_url_branches(self, path):
elements = path.split('/')
branch = elements[0]
if len(elements) == 1:
return self.get_url_branch(branch)
entity = elements[1]
entity = elements[-1]
if entity == 'protection':
branch = '/'.join(elements[0:-1])
return self.get_url_protection(branch)
else:
return None
# fall back to treat all elements as branch
branch = '/'.join(elements)
return self.get_url_branch(branch)
def get_url_branch(self, branch_name):
for branch in self._branches:

View File

@ -739,19 +739,21 @@ class TestGithubDriver(ZuulTestCase):
self.assertNotIn('merge', B.labels)
self.assertNotIn('merge', C.labels)
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_push_event_reconfigure(self):
pevent = self.fake_github.getPushEvent(project='org/common-config',
ref='refs/heads/master',
modified_files=['zuul.yaml'])
def _test_push_event_reconfigure(self, project, branch,
old_sha='0' * 40, expected_cat_jobs=1):
pevent = self.fake_github.getPushEvent(
project=project,
ref='refs/heads/%s' % branch,
old_rev=old_sha,
modified_files=['zuul.yaml'])
# record previous tenant reconfiguration time, which may not be set
old = self.sched.tenant_last_reconfigured.get('tenant-one', 0)
time.sleep(1)
self.waitUntilSettled()
# clear the gearman jobs history so we can count the cat jobs issued
# during reconfiguration
# clear the gearman jobs history so we can count the cat jobs
# issued during reconfiguration
self.gearman_server.jobs_history.clear()
self.fake_github.emitEvent(pevent)
@ -760,11 +762,36 @@ class TestGithubDriver(ZuulTestCase):
# New timestamp should be greater than the old timestamp
self.assertLess(old, new)
# We only expect one cat job here as the (empty) config of org/project
# should be cached.
cat_jobs = [job for job in self.gearman_server.jobs_history
if job.name == b'merger:cat']
self.assertEqual(1, len(cat_jobs))
# Check the expected number of cat jobs here as the (empty) config of
# org/project should be cached.
cat_jobs = set([job for job in self.gearman_server.jobs_history
if job.name == b'merger:cat'])
self.assertEqual(expected_cat_jobs, len(cat_jobs), cat_jobs)
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_push_event_reconfigure(self):
self._test_push_event_reconfigure('org/common-config', 'master')
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_push_event_reconfigure_complex_branch(self):
branch = 'feature/somefeature'
project = 'org/common-config'
# prepare an existing branch
self.create_branch(project, branch)
github = self.fake_github.getGithubClient()
repo = github.repo_from_project(project)
repo._create_branch(branch)
A = self.fake_github.openFakePullRequest(project, branch, 'A')
old_sha = A.head_sha
A.setMerged("merging A")
self._test_push_event_reconfigure(project, branch,
old_sha=old_sha,
expected_cat_jobs=2)
# TODO(jlk): Make this a more generic test for unknown project
@skip("Skipped for rewrite of webhook handler")

View File

@ -242,7 +242,7 @@ class GithubEventConnector(threading.Thread):
event.newrev = body.get('after')
event.commits = body.get('commits')
ref_parts = event.ref.split('/') # ie, ['refs', 'heads', 'master']
ref_parts = event.ref.split('/', 2) # ie, ['refs', 'heads', 'foo/bar']
if ref_parts[1] == "heads":
# necessary for the scheduler to match against particular branches