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
diff --git a/tests/base.py b/tests/base.py
index 0f188bd..f137623 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -563,9 +563,43 @@
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 @@
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 @@
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 @@
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 @@
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
diff --git a/tests/fixtures/config/multi-driver/git/common-config/playbooks/project-gerrit.yaml b/tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project-gerrit.yaml
similarity index 100%
rename from tests/fixtures/config/multi-driver/git/common-config/playbooks/project-gerrit.yaml
rename to tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project-gerrit.yaml
diff --git a/tests/fixtures/config/multi-driver/git/common-config/playbooks/project1-github.yaml b/tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project1-github.yaml
similarity index 100%
rename from tests/fixtures/config/multi-driver/git/common-config/playbooks/project1-github.yaml
rename to tests/fixtures/config/multi-driver/git/org_common-config/playbooks/project1-github.yaml
diff --git a/tests/fixtures/config/multi-driver/git/common-config/zuul.yaml b/tests/fixtures/config/multi-driver/git/org_common-config/zuul.yaml
similarity index 100%
rename from tests/fixtures/config/multi-driver/git/common-config/zuul.yaml
rename to tests/fixtures/config/multi-driver/git/org_common-config/zuul.yaml
diff --git a/tests/fixtures/config/multi-driver/main.yaml b/tests/fixtures/config/multi-driver/main.yaml
index 301df38..4eed523 100644
--- a/tests/fixtures/config/multi-driver/main.yaml
+++ b/tests/fixtures/config/multi-driver/main.yaml
@@ -3,7 +3,7 @@
source:
github:
config-projects:
- - common-config
+ - org/common-config
untrusted-projects:
- org/project1
gerrit:
diff --git a/tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml b/tests/fixtures/config/push-reqs/git/org_common-config/playbooks/job1.yaml
similarity index 100%
rename from tests/fixtures/config/push-reqs/git/common-config/playbooks/job1.yaml
rename to tests/fixtures/config/push-reqs/git/org_common-config/playbooks/job1.yaml
diff --git a/tests/fixtures/config/push-reqs/git/common-config/zuul.yaml b/tests/fixtures/config/push-reqs/git/org_common-config/zuul.yaml
similarity index 100%
rename from tests/fixtures/config/push-reqs/git/common-config/zuul.yaml
rename to tests/fixtures/config/push-reqs/git/org_common-config/zuul.yaml
diff --git a/tests/fixtures/config/push-reqs/main.yaml b/tests/fixtures/config/push-reqs/main.yaml
index d9f1a42..b58db73 100644
--- a/tests/fixtures/config/push-reqs/main.yaml
+++ b/tests/fixtures/config/push-reqs/main.yaml
@@ -3,7 +3,7 @@
source:
github:
config-projects:
- - common-config
+ - org/common-config
untrusted-projects:
- org/project1
gerrit:
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index 68fbe29..d52e38e 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -258,14 +258,19 @@
@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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
@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 @@
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 @@
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 @@
@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'])
diff --git a/tests/unit/test_multi_driver.py b/tests/unit/test_multi_driver.py
index e40591b..1844c33 100644
--- a/tests/unit/test_multi_driver.py
+++ b/tests/unit/test_multi_driver.py
@@ -46,7 +46,8 @@
# 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'])