Skip to content

Commit

Permalink
handle missing account properties for GerritChangeSource
Browse files Browse the repository at this point in the history
* create a separate function to convert account properties
* make use of the function in all cases where the situation might take place
* update test cases

See ticket:2918
  • Loading branch information
Mikhail Sobolev committed May 25, 2015
1 parent 730a53a commit be88a50
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
28 changes: 20 additions & 8 deletions master/buildbot/changes/gerritchangesource.py
Expand Up @@ -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
Expand Down Expand Up @@ -182,11 +197,9 @@ def addChangeFromEvent(self, properties, event):
if "change" in event and "patchSet" in event:
event_change = event["change"]
return self.addChange({
'author': "%s <%s>" % (
event_change["owner"]["name"],
event_change["owner"]["email"]),
'project': event_change["project"],
'repository': "ssh://%s@%s:%s/%s" % (
'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,
self.gerritport, event_change["project"]),
'branch': self.getGroupingPolicyFromEvent(event),
Expand All @@ -199,11 +212,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,
Expand Down
21 changes: 10 additions & 11 deletions master/buildbot/test/unit/test_changes_gerritchangesource.py
Expand Up @@ -23,7 +23,6 @@


class TestGerritHelpers(unittest.TestCase):

def test_proper_json(self):
self.assertEqual(u"Justin Case <justin.case@example.com>",
gerritchangesource._gerrit_user_to_author({
Expand All @@ -35,38 +34,38 @@ def test_proper_json(self):
def test_missing_username(self):
self.assertEqual(u"Justin Case <justin.case@example.com>",
gerritchangesource._gerrit_user_to_author({
"name": "Justin Case",
"email": "justin.case@example.com"
"name": "Justin Case",
"email": "justin.case@example.com"
}))

def test_missing_name(self):
self.assertEqual(u"unknown <justin.case@example.com>",
gerritchangesource._gerrit_user_to_author({
"email": "justin.case@example.com"
"email": "justin.case@example.com"
}))
self.assertEqual(u"gerrit <justin.case@example.com>",
gerritchangesource._gerrit_user_to_author({
"email": "justin.case@example.com"
"email": "justin.case@example.com"
}, u"gerrit"))
self.assertEqual(u"justincase <justin.case@example.com>",
gerritchangesource._gerrit_user_to_author({
"username": "justincase",
"email": "justin.case@example.com"
"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"
"username": "justincase",
"name": "Justin Case"
}))
self.assertEqual(u"Justin Case",
gerritchangesource._gerrit_user_to_author({
"name": "Justin Case"
"name": "Justin Case"
}))
self.assertEqual(u"justincase",
gerritchangesource._gerrit_user_to_author({
"username": "justincase"
"username": "justincase"
}))
self.assertEqual(u"unknown",
gerritchangesource._gerrit_user_to_author({
Expand Down

0 comments on commit be88a50

Please sign in to comment.