address code comments for StashStatusPush
This commit is contained in:
Andrew Hammond
parent
d67f89e1a1
commit
a61405461c
|
|
@ -22,9 +22,11 @@ try:
|
|||
from twisted.web.client import readBody
|
||||
except ImportError:
|
||||
def readBody(*args, **kwargs):
|
||||
return defer.succeed('StatusStashPush requires twisted.web.client.readBody() '
|
||||
return defer.succeed('StatusStashPush requires '
|
||||
'twisted.web.client.readBody() '
|
||||
'to report the body of Stash API errors. '
|
||||
'Please upgrade to Twisted 13.1.0 or newer if you need more verbose error messages.')
|
||||
'Please upgrade to Twisted 13.1.0 or newer '
|
||||
'if you need more verbose error messages.')
|
||||
|
||||
# Magic words understood by Stash REST API
|
||||
INPROGRESS = 'INPROGRESS'
|
||||
|
|
@ -72,20 +74,19 @@ class StashStatusPush(StatusReceiverMultiService,
|
|||
implements(IStatusReceiver)
|
||||
|
||||
def __init__(self, base_url, user, password,
|
||||
key_format='%(builderName)s',
|
||||
key_format='%(prop:builderName)s',
|
||||
name_format=None):
|
||||
"""
|
||||
:param base_url: The base url of the stash host, up to the path.
|
||||
For example, https://stash.example.com/
|
||||
:param user: The stash user to log in as using http basic auth.
|
||||
:param password: The password to use with the stash user.
|
||||
:param key_format: A python format string used to create
|
||||
:param key_format: A rendered string used as
|
||||
the build status key sent to Stash.
|
||||
Currently supports builderName, branch and buildNumber.
|
||||
Defaults to '%(builderName)s' for backwards compatability.
|
||||
:param name_format: An optional python format string used to create
|
||||
the build status name sent to stash. Supports same keywords as
|
||||
key_format. Defaults to None, which disables sending it.
|
||||
Defaults to '%(prop:builderName)s' for backwards compatability.
|
||||
:param name_format: A rendered string used as
|
||||
the build status name sent to stash.
|
||||
Defaults to None, which disables sending it.
|
||||
:return:
|
||||
"""
|
||||
StatusReceiverMultiService.__init__(self)
|
||||
|
|
@ -93,39 +94,31 @@ class StashStatusPush(StatusReceiverMultiService,
|
|||
base_url += '/'
|
||||
self.base_url = '%srest/build-status/1.0/commits/' % (base_url, )
|
||||
self.auth = b64encode('%s:%s' % (user, password))
|
||||
self.key_format = key_format
|
||||
self.key_interpolation = Interpolate(key_format)
|
||||
self.name_interpolation = Interpolate(name_format or '')
|
||||
self.name_format = name_format
|
||||
self._sha = Interpolate('%(src::revision)s')
|
||||
self._branch = Interpolate('%(src::branch)s')
|
||||
self._buildnumber = Interpolate('%(prop:buildnumber)s')
|
||||
self.master_status = None
|
||||
|
||||
@defer.inlineCallbacks
|
||||
def _send(self, request_kwargs, body, error_message):
|
||||
request_kwargs['bodyProducer'] = StringProducer(body)
|
||||
agent = Agent(reactor)
|
||||
d = agent.request(**request_kwargs)
|
||||
d.addCallback(logIfNot2XX)
|
||||
d.addErrback(log.err, error_message)
|
||||
yield d
|
||||
return d
|
||||
|
||||
@defer.inlineCallbacks
|
||||
def send(self, builderName, build, status):
|
||||
sha, branch, buildNumber = yield defer.gatherResults([
|
||||
sha, key, name_string = yield defer.gatherResults([
|
||||
build.render(self._sha),
|
||||
build.render(self._branch),
|
||||
build.render(self._buildnumber),
|
||||
build.render(self.key_interpolation),
|
||||
build.render(self.name_interpolation),
|
||||
])
|
||||
build_url = build.builder.status.getURLForThing(build)
|
||||
format_dict = {
|
||||
'builderName': builderName,
|
||||
'branch': branch,
|
||||
'buildNumber': buildNumber,
|
||||
}
|
||||
key = self.key_format % format_dict
|
||||
body_dict = {'state': status, 'key': key, 'url': build_url}
|
||||
if self.name_format is not None:
|
||||
body_dict['name'] = self.name_format % format_dict
|
||||
body_dict['name'] = name_string
|
||||
body = dumps(body_dict)
|
||||
stash_uri = self.base_url + sha
|
||||
request_kwargs = dict(
|
||||
|
|
|
|||
|
|
@ -20,13 +20,12 @@ from buildbot.status.status_stash import StashStatusPush, INPROGRESS, SUCCESSFUL
|
|||
from buildbot.test.fake.fakebuild import FakeBuild
|
||||
from buildbot.test.util import logging
|
||||
from json import loads
|
||||
from mock import patch
|
||||
from mock import Mock
|
||||
from twisted.internet import defer
|
||||
from twisted.trial import unittest
|
||||
from twisted.web.http_headers import Headers
|
||||
|
||||
|
||||
@patch.object(StashStatusPush, 'send')
|
||||
class TestStashStatusPush(unittest.TestCase, logging.LoggingMixin):
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -35,33 +34,30 @@ class TestStashStatusPush(unittest.TestCase, logging.LoggingMixin):
|
|||
self.setUpLogging()
|
||||
self.build = FakeBuild()
|
||||
self.status = StashStatusPush('fake host', 'fake user', 'fake password')
|
||||
self.status.send = Mock()
|
||||
|
||||
def tearDown(self):
|
||||
pass
|
||||
|
||||
def test_buildStarted_sends_inprogress(self, mock_send):
|
||||
def test_buildStarted_sends_inprogress(self):
|
||||
"""
|
||||
buildStarted should send INPROGRESS
|
||||
"""
|
||||
self.status.buildStarted('fakeBuilderName', self.build)
|
||||
mock_send.assert_called_with('fakeBuilderName', self.build, INPROGRESS)
|
||||
self.status.send.assert_called_with('fakeBuilderName', self.build, INPROGRESS)
|
||||
|
||||
def test_buildFinished_sends_successful_on_success(self, mock_send):
|
||||
def test_buildFinished_sends_successful_on_success(self):
|
||||
"""
|
||||
buildFinished should send SUCCESSFUL if called with SUCCESS
|
||||
"""
|
||||
self.status.buildFinished('fakeBuilderName', self.build, SUCCESS)
|
||||
mock_send.assert_called_with('fakeBuilderName', self.build, SUCCESSFUL)
|
||||
self.status.send.assert_called_with('fakeBuilderName', self.build, SUCCESSFUL)
|
||||
|
||||
def test_buildFinished_sends_failed_on_failure(self, mock_send):
|
||||
def test_buildFinished_sends_failed_on_failure(self):
|
||||
"""
|
||||
buildFinished should send FAILED if called with anything other than SUCCESS
|
||||
"""
|
||||
self.status.buildFinished('fakeBuilderName', self.build, FAILURE)
|
||||
mock_send.assert_called_with('fakeBuilderName', self.build, FAILED)
|
||||
self.status.send.assert_called_with('fakeBuilderName', self.build, FAILED)
|
||||
|
||||
|
||||
@patch.object(StashStatusPush, '_send')
|
||||
class TestStashStatusSend(unittest.TestCase, logging.LoggingMixin):
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -70,27 +66,27 @@ class TestStashStatusSend(unittest.TestCase, logging.LoggingMixin):
|
|||
self.setUpLogging()
|
||||
self.build = FakeBuild()
|
||||
self.build.builder.status.getURLForThing.return_value = 'fake_build_url'
|
||||
self.my_key_format = '%(builderName)s-%(branch)s'
|
||||
self.my_name_format = '%(builderName)s-%(branch)s-%(buildNumber)s'
|
||||
self.my_key_format = '%(prop:builderName)s-%(src::branch)s'
|
||||
self.my_name_format = '%(prop:builderName)s-%(src::branch)s-%(prop:buildNumber)s'
|
||||
self.status = StashStatusPush('fake host', 'fake user', 'fake password',
|
||||
key_format=self.my_key_format,
|
||||
name_format=self.my_name_format)
|
||||
self.status._sha = Interpolate('fake_sha')
|
||||
self.status._branch = Interpolate('fake_branch')
|
||||
self.status._buildnumber = Interpolate('fake_buildNumber')
|
||||
self.status.key_interpolation = Interpolate('fakeBuilderName-fake_branch')
|
||||
self.status.name_interpolation = Interpolate('fakeBuilderName-fake_branch-fake_buildNumber')
|
||||
self.status._send = Mock()
|
||||
|
||||
@defer.inlineCallbacks
|
||||
def test_send(self, mock_send):
|
||||
d = yield self.status.send('fakeBuilderName', self.build, INPROGRESS)
|
||||
self.assertEqual(1, mock_send.call_count)
|
||||
args, kwargs = mock_send.call_args
|
||||
def test_send(self):
|
||||
_ = yield self.status.send('fakeBuilderName', self.build, INPROGRESS)
|
||||
self.assertEqual(1, self.status._send.call_count)
|
||||
args, kwargs = self.status._send.call_args
|
||||
self.assertEqual({}, kwargs)
|
||||
self.assertEqual(3, len(args))
|
||||
request_kwargs, body, error_message = args
|
||||
self.assertEqual('fake host/rest/build-status/1.0/commits/fake_sha',
|
||||
request_kwargs.get('uri', 'No uri present'))
|
||||
self.assertEqual('POST', request_kwargs.get('method', 'No method present'))
|
||||
headers = request_kwargs.get('headers', {})
|
||||
self.assertEqual(Headers({
|
||||
'content-type': ['application/json; charset=utf-8'],
|
||||
'accept-language': ['en-US, en;q=0.8'],
|
||||
|
|
|
|||
Loading…
Reference in New Issue