diff --git a/tests/base.py b/tests/base.py index 1757a7e8af..27479b0ef2 100755 --- a/tests/base.py +++ b/tests/base.py @@ -569,13 +569,11 @@ class FakeGithubPullRequest(object): """Adds a commit on top of the actual PR head.""" self._addCommitToRepo(files=files) self._updateTimeStamp() - self._clearStatuses() def forcePush(self, files=[]): """Clears actual commits and add a commit on top of the base.""" self._addCommitToRepo(files=files, reset=True) self._updateTimeStamp() - self._clearStatuses() def getPullRequestOpenedEvent(self): return self._getPullRequestEvent('opened') @@ -695,7 +693,10 @@ class FakeGithubPullRequest(object): } }, 'head': { - 'sha': self.head_sha + 'sha': self.head_sha, + 'repo': { + 'full_name': self.project + } } }, 'label': { @@ -742,6 +743,9 @@ class FakeGithubPullRequest(object): repo.index.add([fn]) self.head_sha = repo.index.commit(msg).hexsha + # Create an empty set of statuses for the given sha, + # each sha on a PR may have a status set on it + self.statuses[self.head_sha] = [] repo.head.reference = 'master' zuul.merger.merger.reset_repo_to_head(repo) repo.git.clean('-x', '-f', '-d') @@ -754,15 +758,21 @@ class FakeGithubPullRequest(object): repo = self._getRepo() return repo.references[self._getPRReference()].commit.hexsha - def setStatus(self, state, url, description, context): - self.statuses[context] = { + def setStatus(self, sha, state, url, description, context): + # Since we're bypassing github API, which would require a user, we + # hard set the user as 'zuul' here. + user = 'zuul' + # insert the status at the top of the list, to simulate that it + # is the most recent set status + self.statuses[sha].insert(0, ({ 'state': state, 'url': url, - 'description': description - } - - def _clearStatuses(self): - self.statuses = {} + 'description': description, + 'context': context, + 'creator': { + 'login': user + } + })) def _getPRReference(self): return '%s/head' % self.number @@ -783,7 +793,10 @@ class FakeGithubPullRequest(object): } }, 'head': { - 'sha': self.head_sha + 'sha': self.head_sha, + 'repo': { + 'full_name': self.project + } } }, 'sender': { @@ -857,7 +870,10 @@ class FakeGithubConnection(githubconnection.GithubConnection): }, 'mergeable': True, 'head': { - 'sha': pr.head_sha + 'sha': pr.head_sha, + 'repo': { + 'full_name': pr.project + } } } return data @@ -901,6 +917,14 @@ class FakeGithubConnection(githubconnection.GithubConnection): pull_request.is_merged = True pull_request.merge_message = commit_message + def getCommitStatuses(self, project, sha): + owner, proj = project.split('/') + for pr in self.pull_requests: + pr_owner, pr_project = pr.project.split('/') + if (pr_owner == owner and pr_project == proj and + pr.head_sha == sha): + return pr.statuses[sha] + def setCommitStatus(self, project, sha, state, url='', description='', context=''): owner, proj = project.split('/') @@ -908,7 +932,7 @@ class FakeGithubConnection(githubconnection.GithubConnection): pr_owner, pr_project = pr.project.split('/') if (pr_owner == owner and pr_project == proj and pr.head_sha == sha): - pr.setStatus(state, url, description, context) + pr.setStatus(sha, state, url, description, context) def labelPull(self, project, pr_number, label): pull_request = self.pull_requests[pr_number - 1] diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml index bcbac1bde9..c939f39950 100644 --- a/tests/fixtures/layouts/reporting-github.yaml +++ b/tests/fixtures/layouts/reporting-github.yaml @@ -28,6 +28,7 @@ success: github: comment: false + status: 'success' failure: github: comment: false diff --git a/tests/fixtures/layouts/requirements-github.yaml b/tests/fixtures/layouts/requirements-github.yaml new file mode 100644 index 0000000000..cacc54f294 --- /dev/null +++ b/tests/fixtures/layouts/requirements-github.yaml @@ -0,0 +1,23 @@ +- pipeline: + name: pipeline + manager: independent + require: + github: + status: "zuul:check:success" + trigger: + github: + - event: pull_request + action: comment + comment: 'test me' + success: + github: + comment: true + +- job: + name: project1-pipeline + +- project: + name: org/project1 + pipeline: + jobs: + - project1-pipeline diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index f9182186c7..f2877903c5 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -253,10 +253,14 @@ class TestGithubDriver(ZuulTestCase): A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) self.waitUntilSettled() - self.assertIn('check', A.statuses) - check_status = A.statuses['check'] + # We should have a status container for the head sha + self.assertIn(A.head_sha, A.statuses.keys()) + # We should only have one status for the head sha + self.assertEqual(1, len(A.statuses[A.head_sha])) + check_status = A.statuses[A.head_sha][0] check_url = ('http://zuul.example.com/status/#%s,%s' % (A.number, A.head_sha)) + self.assertEqual('check', check_status['context']) self.assertEqual('Standard check', check_status['description']) self.assertEqual('pending', check_status['state']) self.assertEqual(check_url, check_status['url']) @@ -265,8 +269,12 @@ class TestGithubDriver(ZuulTestCase): self.executor_server.hold_jobs_in_build = False self.executor_server.release() self.waitUntilSettled() - check_status = A.statuses['check'] - self.assertEqual('Standard check', check_status['description']) + # We should only have two statuses for the head sha + self.assertEqual(2, len(A.statuses[A.head_sha])) + check_status = A.statuses[A.head_sha][0] + check_url = ('http://zuul.example.com/status/#%s,%s' % + (A.number, A.head_sha)) + self.assertEqual('check', check_status['context']) self.assertEqual('success', check_status['state']) self.assertEqual(check_url, check_status['url']) self.assertEqual(1, len(A.comments)) @@ -278,7 +286,7 @@ class TestGithubDriver(ZuulTestCase): self.fake_github.emitEvent( A.getCommentAddedEvent('reporting check')) self.waitUntilSettled() - self.assertNotIn('reporting', A.statuses) + self.assertEqual(2, len(A.statuses[A.head_sha])) # comments increased by one for the start message self.assertEqual(2, len(A.comments)) self.assertThat(A.comments[1], @@ -286,7 +294,11 @@ class TestGithubDriver(ZuulTestCase): self.executor_server.hold_jobs_in_build = False self.executor_server.release() self.waitUntilSettled() - self.assertNotIn('reporting', A.statuses) + # pipeline reports success status + self.assertEqual(3, len(A.statuses[A.head_sha])) + report_status = A.statuses[A.head_sha][0] + self.assertEqual('reporting', report_status['context']) + self.assertEqual('success', report_status['state']) self.assertEqual(2, len(A.comments)) @simple_layout('layouts/merging-github.yaml', driver='github') diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py new file mode 100644 index 0000000000..bb9993e71f --- /dev/null +++ b/tests/unit/test_github_requirements.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python +# Copyright (c) 2017 IBM Corp. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from tests.base import ZuulTestCase, simple_layout + + +class TestGithubRequirements(ZuulTestCase): + """Test pipeline and trigger requirements""" + config_file = 'zuul-github-driver.conf' + + @simple_layout('layouts/requirements-github.yaml', driver='github') + def test_pipeline_require_status(self): + "Test pipeline requirement: status" + A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A') + # A comment event that we will keep submitting to trigger + comment = A.getCommentAddedEvent('test me') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + # No status from zuul so should not be enqueued + self.assertEqual(len(self.history), 0) + + # An error status should not cause it to be enqueued + A.setStatus(A.head_sha, 'error', 'null', 'null', 'check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A success status goes in + A.setStatus(A.head_sha, 'success', 'null', 'null', 'check') + self.fake_github.emitEvent(comment) + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, 'project1-pipeline') diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 4b945a53aa..2513569575 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -315,6 +315,7 @@ class GithubConnection(BaseConnection): change.patchset = event.patch_number change.files = self.getPullFileNames(project, change.number) change.title = event.title + change.status = self._get_statuses(project, event.patch_number) change.source_event = event elif event.ref: change = Ref(project) @@ -408,6 +409,17 @@ class GithubConnection(BaseConnection): if not result: raise Exception('Pull request was not merged') + def getCommitStatuses(self, project, sha): + owner, proj = project.split('/') + repository = self.github.repository(owner, proj) + commit = repository.commit(sha) + # make a list out of the statuses so that we complete our + # API transaction + statuses = [status.as_dict() for status in commit.statuses()] + + log_rate_limit(self.log, self.github) + return statuses + def setCommitStatus(self, project, sha, state, url='', description='', context=''): owner, proj = project.split('/') @@ -430,6 +442,30 @@ class GithubConnection(BaseConnection): def _ghTimestampToDate(self, timestamp): return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ') + def _get_statuses(self, project, sha): + # A ref can have more than one status from each context, + # however the API returns them in order, newest first. + # So we can keep track of which contexts we've already seen + # and throw out the rest. Our unique key is based on + # the user and the context, since context is free form and anybody + # can put whatever they want there. We want to ensure we track it + # by user, so that we can require/trigger by user too. + seen = [] + statuses = [] + for status in self.getCommitStatuses(project.name, sha): + # creator can be None if the user has been removed. + creator = status.get('creator') + if not creator: + continue + user = creator.get('login') + context = status.get('context') + state = status.get('state') + if "%s:%s" % (user, context) not in seen: + statuses.append("%s:%s:%s" % (user, context, state)) + seen.append("%s:%s" % (user, context)) + + return statuses + def log_rate_limit(log, github): try: diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 0d77cae349..22f549fa9b 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -16,7 +16,7 @@ import re -from zuul.model import Change, TriggerEvent, EventFilter +from zuul.model import Change, TriggerEvent, EventFilter, RefFilter EMPTY_GIT_REF = '0' * 40 # git sha of all zeros, used during creates/deletes @@ -161,3 +161,31 @@ class GithubEventFilter(EventFilter): return False return True + + +class GithubRefFilter(RefFilter): + def __init__(self, statuses=[]): + RefFilter.__init__(self) + + self.statuses = statuses + + def __repr__(self): + ret = '