diff --git a/master/buildbot/reporters/utils.py b/master/buildbot/reporters/utils.py index ebf4300356a..fa12faea2fe 100644 --- a/master/buildbot/reporters/utils.py +++ b/master/buildbot/reporters/utils.py @@ -15,6 +15,7 @@ from UserList import UserList from twisted.internet import defer +from twisted.python import log from buildbot.data import resultspec from buildbot.process.properties import renderer @@ -30,7 +31,7 @@ def getPreviousBuild(master, build): n = build['number'] - 1 while n >= 0: prev = yield master.data.get(("builders", build['builderid'], "builds", n)) - if prev['results'] != RETRY: + if prev and prev['results'] != RETRY: defer.returnValue(prev) n -= 1 defer.returnValue(None) @@ -163,7 +164,17 @@ def getResponsibleUsersForBuild(master, buildid): # add owner from properties if 'owner' in properties: - blamelist.add(properties['owner'][0]) + owner = properties['owner'][0] + if isinstance(owner, basestring): + blamelist.add(owner) + else: + blamelist.update(owner) + log.msg("Warning: owner property is a list for buildid {}. ".format(buildid)) + log.msg("Please report a bug: changes: {}. properties: {}".format(changes, properties)) + + # add owner from properties + if 'owners' in properties: + blamelist.update(properties['owners'][0]) blamelist = list(blamelist) blamelist.sort() diff --git a/master/buildbot/test/unit/test_reporters_utils.py b/master/buildbot/test/unit/test_reporters_utils.py index 96ecd3e8160..405b38a6737 100644 --- a/master/buildbot/test/unit/test_reporters_utils.py +++ b/master/buildbot/test/unit/test_reporters_utils.py @@ -23,9 +23,10 @@ from buildbot.reporters import utils from buildbot.test.fake import fakedb from buildbot.test.fake import fakemaster +from buildbot.test.util import logging -class TestDataUtils(unittest.TestCase): +class TestDataUtils(unittest.TestCase, logging.LoggingMixin): LOGCONTENT = textwrap.dedent(u"""\ line zero line 1 @@ -70,6 +71,8 @@ def setupDb(self): buildid=_id, name="workername", value="sl"), fakedb.BuildProperty( buildid=_id, name="reason", value="because"), + fakedb.BuildProperty( + buildid=_id, name="owner", value="him"), fakedb.Step(id=100 + _id, buildid=_id, name="step1"), fakedb.Step(id=200 + _id, buildid=_id, name="step2"), fakedb.Log(id=60 + _id, stepid=100 + _id, name='stdio', slug='stdio', type='s', @@ -96,6 +99,7 @@ def test_getDetailsForBuildset(self): build2 = res['builds'][1] buildset = res['buildset'] self.assertEqual(build1['properties'], {u'reason': (u'because', u'fakedb'), + u'owner': (u'him', u'fakedb'), u'workername': (u'sl', u'fakedb')}) self.assertEqual(len(build1['steps']), 2) self.assertEqual(build1['buildid'], 20) @@ -132,7 +136,29 @@ def test_getResponsibleUsersFromPatch(self): def test_getResponsibleUsersForBuild(self): self.setupDb() res = yield utils.getResponsibleUsersForBuild(self.master, 20) - self.assertEqual(res, ["me@foo"]) + self.assertEqual(sorted(res), sorted(["me@foo", "him"])) + + @defer.inlineCallbacks + def test_getResponsibleUsersForBuildWithBadOwner(self): + self.setUpLogging() + self.setupDb() + self.db.insertTestData([ + fakedb.BuildProperty( + buildid=20, name="owner", value=["him"]), + ]) + res = yield utils.getResponsibleUsersForBuild(self.master, 20) + self.assertLogged("Please report a bug") + self.assertEqual(sorted(res), sorted(["me@foo", "him"])) + + @defer.inlineCallbacks + def test_getResponsibleUsersForBuildWithOwners(self): + self.setupDb() + self.db.insertTestData([ + fakedb.BuildProperty( + buildid=20, name="owners", value=["him", "her"]), + ]) + res = yield utils.getResponsibleUsersForBuild(self.master, 20) + self.assertEqual(sorted(res), sorted(["me@foo", "him", "her"])) @defer.inlineCallbacks def test_getPreviousBuild(self):