zuul
/
zuul
Code Issues Proposed changes

Remove status handling from FakeGithubConnection 

The FakeGithubConnection redefines the methods getCommitStatuses and
setCommitStatus for test purposes. This implies that the original
methods of GithubConnections are untested. This is an attempt to
remove these from FakeGithubConnection and pushing the test handling
into a FakeGithub object. This way we can do the test handling and at
the same time use the original methods of GithubConnection.

This also uncovered some test fixtures which are invalid in Github
context as there projects require the form <owner>/<project> which is
not matched by the 'common-config' which was used in many fixtures.

Change-Id: Ib3badca63b77166c1d69332121d78ef05bd899fe
This commit is contained in:
Tobias Henkel 2017-08-03 11:46:54 +02:00
parent eca4620efa
commit 3c17d5f351
10 changed files with 100 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
tests/base.py
View File

@ -563,9 +563,43 @@ class FakeGithub(object):
def __init__(self, branch='master'):
self.name = branch
class FakeStatus(object):
def __init__(self, state, url, description, context, user):
self._state = state
self._url = url
self._description = description
self._context = context
self._user = user
def as_dict(self):
return {
'state': self._state,
'url': self._url,
'description': self._description,
'context': self._context,
'creator': {
'login': self._user
}
}
class FakeCommit(object):
def __init__(self):
self._statuses = []
def set_status(self, state, url, description, context, user):
status = FakeGithub.FakeStatus(
state, url, description, context, user)
# always insert a status to the front of the list, to represent
# the last status provided for a commit.
self._statuses.insert(0, status)
def statuses(self):
return self._statuses
class FakeRepository(object):
def __init__(self):
self._branches = [FakeGithub.FakeBranch()]
self._commits = {}
def branches(self, protected=False):
if protected:
@ -573,12 +607,40 @@ class FakeGithub(object):
return []
return self._branches
def create_status(self, sha, state, url, description, context,
user='zuul'):
# Since we're bypassing github API, which would require a user, we
# default the user as 'zuul' here.
commit = self._commits.get(sha, None)
if commit is None:
commit = FakeGithub.FakeCommit()
self._commits[sha] = commit
commit.set_status(state, url, description, context, user)
def commit(self, sha):
commit = self._commits.get(sha, None)
if commit is None:
commit = FakeGithub.FakeCommit()
self._commits[sha] = commit
return commit
def __init__(self):
self._repos = {}
def user(self, login):
return self.FakeUser(login)
def repository(self, owner, proj):
repo = self.FakeRepository()
return repo
return self._repos.get((owner, proj), None)
def repo_from_project(self, project):
# This is a convenience method for the tests.
owner, proj = project.split('/')
return self.repository(owner, proj)
def addProject(self, project):
owner, proj = project.name.split('/')
self._repos[(owner, proj)] = self.FakeRepository()
class FakeGithubPullRequest(object):
@ -964,6 +1026,12 @@ class FakeGithubConnection(githubconnection.GithubConnection):
data=payload, headers=headers)
return urllib.request.urlopen(req)
def addProject(self, project):
# use the original method here and additionally register it in the
# fake github
super(FakeGithubConnection, self).addProject(project)
self.getGithubClient(project).addProject(project)
def getPull(self, project, number):
pr = self.pull_requests[number - 1]
data = {
@ -1037,27 +1105,13 @@ class FakeGithubConnection(githubconnection.GithubConnection):
pull_request.is_merged = True
pull_request.merge_message = commit_message
def getCommitStatuses(self, project, sha):
return self.statuses.get(project, {}).get(sha, [])
def setCommitStatus(self, project, sha, state, url='', description='',
context='default', user='zuul'):
# record that this got reported
# record that this got reported and call original method
self.reports.append((project, sha, 'status', (user, context, state)))
# always insert a status to the front of the list, to represent
# the last status provided for a commit.
# Since we're bypassing github API, which would require a user, we
# default the user as 'zuul' here.
self.statuses.setdefault(project, {}).setdefault(sha, [])
self.statuses[project][sha].insert(0, {
'state': state,
'url': url,
'description': description,
'context': context,
'creator': {
'login': user
}
})
super(FakeGithubConnection, self).setCommitStatus(
project, sha, state,
url=url, description=description, context=context)
def labelPull(self, project, pr_number, label):
# record that this got reported
@ -2145,15 +2199,15 @@ class ZuulTestCase(BaseTestCase):
config = [{'tenant':
{'name': 'tenant-one',
'source': {driver:
{'config-projects': ['common-config'],
{'config-projects': ['org/common-config'],
'untrusted-projects': untrusted_projects}}}}]
f.write(yaml.dump(config).encode('utf8'))
f.close()
self.config.set('scheduler', 'tenant_config',
os.path.join(FIXTURE_DIR, f.name))
self.init_repo('common-config')
self.addCommitToRepo('common-config', 'add content from fixture',
self.init_repo('org/common-config')
self.addCommitToRepo('org/common-config', 'add content from fixture',
files, branch='master', tag='init')
return True

0
tests/fixtures/config/multi-driver/git/common-config/playbooks/project-gerrit.yaml → tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project-gerrit.yaml vendored
View File

0
tests/fixtures/config/multi-driver/git/common-config/playbooks/project1-github.yaml → tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project1-github.yaml vendored
View File

0
tests/fixtures/config/multi-driver/git/common-config/zuul.yaml → tests/fixtures/config/multi-driver/git/org_common-config/zuul.yaml vendored
View File

2
tests/fixtures/config/multi-driver/main.yaml vendored
View File

@ -3,7 +3,7 @@
source:
github:
config-projects:
- common-config
- org/common-config
untrusted-projects:
- org/project1
gerrit:

0
tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml → tests/fixtures/config/push-reqs/git/org_common-config/playbooks/job1.yaml vendored
View File

0
tests/fixtures/config/push-reqs/git/common-config/zuul.yaml → tests/fixtures/config/push-reqs/git/org_common-config/zuul.yaml vendored
View File

2
tests/fixtures/config/push-reqs/main.yaml vendored
View File

@ -3,7 +3,7 @@
source:
github:
config-projects:
- common-config
- org/common-config
untrusted-projects:
- org/project1
gerrit:

30
tests/unit/test_github_driver.py
View File

@ -258,14 +258,19 @@ class TestGithubDriver(ZuulTestCase):
@simple_layout('layouts/reporting-github.yaml', driver='github')
def test_reporting(self):
project = 'org/project'
github = self.fake_github.github_client
# pipeline reports pull status both on start and success
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest(project, 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
# We should have a status container for the head sha
statuses = self.fake_github.statuses[project][A.head_sha]
self.assertIn(A.head_sha, self.fake_github.statuses[project].keys())
self.assertIn(
A.head_sha, github.repo_from_project(project)._commits.keys())
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
# We should only have one status for the head sha
self.assertEqual(1, len(statuses))
check_status = statuses[0]
@ -282,7 +287,7 @@ class TestGithubDriver(ZuulTestCase):
self.executor_server.release()
self.waitUntilSettled()
# We should only have two statuses for the head sha
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(2, len(statuses))
check_status = statuses[0]
check_url = ('http://zuul.example.com/status/#%s,%s' %
@ -301,7 +306,7 @@ class TestGithubDriver(ZuulTestCase):
self.fake_github.emitEvent(
A.getCommentAddedEvent('reporting check'))
self.waitUntilSettled()
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(2, len(statuses))
# comments increased by one for the start message
self.assertEqual(2, len(A.comments))
@ -311,7 +316,7 @@ class TestGithubDriver(ZuulTestCase):
self.executor_server.release()
self.waitUntilSettled()
# pipeline reports success status
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(3, len(statuses))
report_status = statuses[0]
self.assertEqual('tenant-one/reporting', report_status['context'])
@ -343,7 +348,7 @@ class TestGithubDriver(ZuulTestCase):
self.fake_github.emitEvent(
A.getCommentAddedEvent('long pipeline'))
self.waitUntilSettled()
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(1, len(statuses))
check_status = statuses[0]
# Status is truncated due to long pipeline name
@ -354,7 +359,7 @@ class TestGithubDriver(ZuulTestCase):
self.executor_server.release()
self.waitUntilSettled()
# We should only have two statuses for the head sha
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(2, len(statuses))
check_status = statuses[0]
# Status is truncated due to long pipeline name
@ -441,6 +446,8 @@ class TestGithubDriver(ZuulTestCase):
@simple_layout('layouts/reporting-multiple-github.yaml', driver='github')
def test_reporting_multiple_github(self):
project = 'org/project1'
github = self.fake_github.github_client
# pipeline reports pull status both on start and success
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest(project, 'master', 'A')
@ -451,8 +458,9 @@ class TestGithubDriver(ZuulTestCase):
self.fake_github.emitEvent(B.getPullRequestOpenedEvent())
self.waitUntilSettled()
# We should have a status container for the head sha
statuses = self.fake_github.statuses[project][A.head_sha]
self.assertIn(A.head_sha, self.fake_github.statuses[project].keys())
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertIn(
A.head_sha, github.repo_from_project(project)._commits.keys())
# We should only have one status for the head sha
self.assertEqual(1, len(statuses))
check_status = statuses[0]
@ -468,7 +476,7 @@ class TestGithubDriver(ZuulTestCase):
self.executor_server.release()
self.waitUntilSettled()
# We should only have two statuses for the head sha
statuses = self.fake_github.statuses[project][A.head_sha]
statuses = self.fake_github.getCommitStatuses(project, A.head_sha)
self.assertEqual(2, len(statuses))
check_status = statuses[0]
check_url = ('http://zuul.example.com/status/#%s,%s' %
@ -656,7 +664,7 @@ class TestGithubDriver(ZuulTestCase):
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_push_event_reconfigure(self):
pevent = self.fake_github.getPushEvent(project='common-config',
pevent = self.fake_github.getPushEvent(project='org/common-config',
ref='refs/heads/master',
modified_files=['zuul.yaml'])

3
tests/unit/test_multi_driver.py
View File

@ -46,7 +46,8 @@ class TestGerritAndGithub(ZuulTestCase):
# Check on reporting results
# github should have a success status (only).
statuses = self.fake_github.statuses['org/project1'][B.head_sha]
statuses = self.fake_github.getCommitStatuses(
'org/project1', B.head_sha)
self.assertEqual(1, len(statuses))
self.assertEqual('success', statuses[0]['state'])