diff --git a/master/buildbot/changes/gerritchangesource.py b/master/buildbot/changes/gerritchangesource.py index 4d082fc1f52..ecf91720b07 100644 --- a/master/buildbot/changes/gerritchangesource.py +++ b/master/buildbot/changes/gerritchangesource.py @@ -42,6 +42,21 @@ def __init__(self, del self.checks["branch"] +def _gerrit_user_to_author(props, username=u"unknown"): + """ + Convert Gerrit account properties to Buildbot format + + Take into account missing values + """ + username = props.get("username", username) + + result = [props.get("name", username)] + if "email" in props: + result.append(u"<%s>" % props["email"]) + + return u" ".join(result) + + class GerritChangeSource(base.ChangeSource): """This source will maintain a connection to gerrit ssh server @@ -187,11 +202,8 @@ def addChangeFromEvent(self, properties, event): if "change" in event and "patchSet" in event: event_change = event["change"] - username = event_change["owner"].get("username", u"unknown") return self.addChange({ - 'author': "%s <%s>" % ( - event_change["owner"].get("name", username), - event_change["owner"].get("email", u'unknown@example.com')), + 'author': _gerrit_user_to_author(event_change["owner"]), 'project': util.ascii2unicode(event_change["project"]), 'repository': u"ssh://%s@%s:%s/%s" % ( self.username, self.gerritserver, @@ -206,11 +218,10 @@ def addChangeFromEvent(self, properties, event): def eventReceived_ref_updated(self, properties, event): ref = event["refUpdate"] - author = "gerrit" + author = u"gerrit" if "submitter" in event: - author = "%s <%s>" % ( - event["submitter"]["name"], event["submitter"]["email"]) + author = _gerrit_user_to_author(event["submitter"], author) return self.addChange(dict( author=author, diff --git a/master/buildbot/test/unit/test_changes_gerritchangesource.py b/master/buildbot/test/unit/test_changes_gerritchangesource.py index e0e2d3b5da2..2a3af73827c 100644 --- a/master/buildbot/test/unit/test_changes_gerritchangesource.py +++ b/master/buildbot/test/unit/test_changes_gerritchangesource.py @@ -21,6 +21,59 @@ from twisted.trial import unittest +class TestGerritHelpers(unittest.TestCase): + def test_proper_json(self): + self.assertEqual(u"Justin Case ", + gerritchangesource._gerrit_user_to_author({ + "username": "justincase", + "name": "Justin Case", + "email": "justin.case@example.com" + })) + + def test_missing_username(self): + self.assertEqual(u"Justin Case ", + gerritchangesource._gerrit_user_to_author({ + "name": "Justin Case", + "email": "justin.case@example.com" + })) + + def test_missing_name(self): + self.assertEqual(u"unknown ", + gerritchangesource._gerrit_user_to_author({ + "email": "justin.case@example.com" + })) + self.assertEqual(u"gerrit ", + gerritchangesource._gerrit_user_to_author({ + "email": "justin.case@example.com" + }, u"gerrit")) + self.assertEqual(u"justincase ", + gerritchangesource._gerrit_user_to_author({ + "username": "justincase", + "email": "justin.case@example.com" + }, u"gerrit")) + + def test_missing_email(self): + self.assertEqual(u"Justin Case", + gerritchangesource._gerrit_user_to_author({ + "username": "justincase", + "name": "Justin Case" + })) + self.assertEqual(u"Justin Case", + gerritchangesource._gerrit_user_to_author({ + "name": "Justin Case" + })) + self.assertEqual(u"justincase", + gerritchangesource._gerrit_user_to_author({ + "username": "justincase" + })) + self.assertEqual(u"unknown", + gerritchangesource._gerrit_user_to_author({ + })) + self.assertEqual(u"gerrit", + gerritchangesource._gerrit_user_to_author({ + }, u"gerrit")) + + class TestGerritChangeSource(changesource.ChangeSourceMixin, unittest.TestCase): diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst index ebc17ab1bd3..019977c2ab0 100644 --- a/master/docs/relnotes/index.rst +++ b/master/docs/relnotes/index.rst @@ -149,6 +149,8 @@ Fixes treeStableTimer=None, builderNames=['tag_build'])) +* Missing "name" and "email" properties received from Gerrit are now handled properly + Deprecations, Removals, and Non-Compatible Changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~