Skip to content

Commit

Permalink
be more flexible in getResponsibleUsers
Browse files Browse the repository at this point in the history
Normally properties['owner'] shall be a string.
We got some users reporting it is a list:
#2284
I couldn't find in the code any evidence where owner would be set to a list.

We don't have much more information, so lets log a message in that case in the hope we get more
reports
  • Loading branch information
Pierre Tardy committed Jun 28, 2016
1 parent 4076dbc commit 98ce116
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
13 changes: 12 additions & 1 deletion master/buildbot/reporters/utils.py
Expand Up @@ -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
Expand Down Expand Up @@ -163,7 +164,17 @@ def getResponsibleUsersForBuild(master, buildid):

# add owner from properties
if 'owner' in properties:
blamelist.update(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()
Expand Down
30 changes: 28 additions & 2 deletions master/buildbot/test/unit/test_reporters_utils.py
Expand Up @@ -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
Expand Down Expand Up @@ -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',
Expand All @@ -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)
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 98ce116

Please sign in to comment.