GerritStatusPush: Add per-buildset status updates

Currently GerritStatusPush will submit an update/review to gerrit
immediately when each builder in a buildset finishes building. It is
usually more useful to just get one update, containing information
about all of the builds in that buildset. So, allow GerritStatusPush
to do that, configurable via the new summaryCB constructor argument. A
default summaryCB is also provided: defaultSummaryCB.

If neither reviewCB nor summaryCB are explicitly provided in the
configuration, default to sending out the new "summary" reviews.
However, if reviewCB was explicitly provided, default to sending out
the old-style per-build reviews, and not sending out the new "summary"
reviews. This allows for older configurations to not see an unexpected
change in behavior, but completely new configurations are likely to
see the probably more useful "summary" reviews.

Since much of this functionality is already implemented in
MailNotifier, extract the relevant code into the new
BuildSetSummaryNotifierMixin, and make both MailNotifier and
GerritStatusPush use it. Document the new mixin class.

Also add tests for GerritStatusPush, to test the new behavior, and for
BuildSetSummaryNotifierMixin, to test that the mixin itself works
properly. Modify the unit tests for MailNotifier to accomodate the
refactoring.

master/buildbot/status/buildset.py

@@ -13,6 +13,7 @@
 #
 # Copyright Buildbot Team Members
 
+from twisted.internet import defer
 from buildbot import interfaces
 from buildbot.status.buildrequest import BuildRequestStatus
 from zope.interface import implements
@@ -69,3 +70,46 @@ def asDict(self):
         d = dict(self.bsdict)
         d["submitted_at"] = str(self.bsdict["submitted_at"])
         return d
+
+
+class BuildSetSummaryNotifierMixin:
+    def summarySubscribe(self):
+        self._buildSetSubscription = self.master.subscribeToBuildsetCompletions(self._buildsetComplete)
+
+    def summaryUnsubscribe(self):
+        if self._buildSetSubscription is not None:
+            self._buildSetSubscription.unsubscribe()
+            self._buildSetSubscription = None
+
+    def sendBuildSetSummary(self, buildset, builds):
+        raise NotImplementedError
+
+    @defer.inlineCallbacks
+    def _buildsetComplete(self, bsid, result):
+        # first, just get the buildset and all build requests for our buildset id
+        dl = [self.master.db.buildsets.getBuildset(bsid=bsid),
+              self.master.db.buildrequests.getBuildRequests(bsid=bsid)]
+        (buildset, breqs) = yield defer.gatherResults(dl)
+
+        # next, get the bdictlist for each build request
+        dl = []
+        for breq in breqs:
+            d = self.master.db.builds.getBuildsForRequest(breq['brid'])
+            dl.append(d)
+
+        buildinfo = yield defer.gatherResults(dl)
+
+        # next, get the builder for each build request, and for each bdict,
+        # look up the actual build object, using the bdictlist retrieved above
+        builds = []
+        for (breq, bdictlist) in zip(breqs, buildinfo):
+            builder = self.master_status.getBuilder(breq['buildername'])
+            for bdict in bdictlist:
+                build = builder.getBuild(bdict['number'])
+                if build is not None:
+                    builds.append(build)
+
+        if builds:
+            # We've received all of the information about the builds in this
+            # buildset; now send out the summary
+            self.sendBuildSetSummary(buildset, builds)

master/buildbot/status/mail.py

@@ -53,6 +53,7 @@
 from buildbot import util
 from buildbot.process.users import users
 from buildbot.status import base
+from buildbot.status import buildset
 from buildbot.status.results import EXCEPTION
 from buildbot.status.results import FAILURE
 from buildbot.status.results import Results
@@ -218,7 +219,7 @@ def defaultGetPreviousBuild(current_build):
         return current_build.getPreviousBuild()
 
 
-class MailNotifier(base.StatusReceiverMultiService):
+class MailNotifier(base.StatusReceiverMultiService, buildset.BuildSetSummaryNotifierMixin):
 
     """This is a status notifier which sends email to a list of recipients
     upon the completion of each build. It can be configured to only send out
@@ -460,15 +461,12 @@ def setServiceParent(self, parent):
 
     def startService(self):
         if self.buildSetSummary:
-            self.buildSetSubscription = \
-                self.master.subscribeToBuildsetCompletions(self.buildsetFinished)
+            self.summarySubscribe()
 
         base.StatusReceiverMultiService.startService(self)
 
     def stopService(self):
-        if self.buildSetSubscription is not None:
-            self.buildSetSubscription.unsubscribe()
-            self.buildSetSubscription = None
+        self.summaryUnsubscribe()
 
         return base.StatusReceiverMultiService.stopService(self)
 
@@ -523,6 +521,12 @@ def isMailNeeded(self, build, results):
 
         return False
 
+    def sendBuildSetSummary(self, buildset, builds):
+        # only include builds for which isMailNeeded returns true
+        builds = [build for build in builds if self.isMailNeeded(build, build.getResults())]
+        if builds:
+            self.buildMessage("(whole buildset)", builds, buildset['results'])
+
     def buildFinished(self, name, build, results):
         if (not self.buildSetSummary and
                 self.isMailNeeded(build, results)):
@@ -535,39 +539,6 @@ def buildFinished(self, name, build, results):
             return self.buildMessage(name, [build], results)
         return None
 
-    def _gotBuilds(self, res, buildset):
-        builds = []
-        for (builddictlist, builder) in res:
-            for builddict in builddictlist:
-                build = builder.getBuild(builddict['number'])
-                if build is not None and self.isMailNeeded(build, build.results):
-                    builds.append(build)
-
-        if builds:
-            self.buildMessage("(whole buildset)", builds, buildset['results'])
-
-    def _gotBuildRequests(self, breqs, buildset):
-        dl = []
-        for breq in breqs:
-            buildername = breq['buildername']
-            builder = self.master_status.getBuilder(buildername)
-            d = self.master.db.builds.getBuildsForRequest(breq['brid'])
-            d.addCallback(lambda builddictlist, builder=builder:
-                          (builddictlist, builder))
-            dl.append(d)
-        d = defer.gatherResults(dl)
-        d.addCallback(self._gotBuilds, buildset)
-
-    def _gotBuildSet(self, buildset, bsid):
-        d = self.master.db.buildrequests.getBuildRequests(bsid=bsid)
-        d.addCallback(self._gotBuildRequests, buildset)
-
-    def buildsetFinished(self, bsid, result):
-        d = self.master.db.buildsets.getBuildset(bsid=bsid)
-        d.addCallback(self._gotBuildSet, bsid)
-
-        return d
-
     def getCustomMesgData(self, mode, name, build, results, master_status):
         #
         # logs is a list of tuples that contain the log

master/buildbot/status/status_gerrit.py

@@ -18,13 +18,18 @@
 
 ."""
 
+from buildbot.status import buildset
 from buildbot.status.base import StatusReceiverMultiService
+from buildbot.status.builder import EXCEPTION
+from buildbot.status.builder import FAILURE
 from buildbot.status.builder import RETRY
 from buildbot.status.builder import Results
 from buildbot.status.builder import SUCCESS
+from buildbot.status.builder import WARNINGS
 from twisted.internet import reactor
 from twisted.internet.protocol import ProcessProtocol
 from distutils.version import LooseVersion
+import time
 
 # Cache the version that the gerrit server is running for this many seconds
 GERRIT_VERSION_CACHE_TIMEOUT = 600
@@ -42,13 +47,53 @@ def defaultReviewCB(builderName, build, result, status, arg):
     return message, (result == SUCCESS or -1), 0
 
 
-class GerritStatusPush(StatusReceiverMultiService):
+def defaultSummaryCB(buildInfoList, results, status, arg):
+    success = False
+    failure = False
+
+    msgs = []
+
+    for buildInfo in buildInfoList:
+        msg = "Builder %(name)s %(resultText)s (%(text)s)" % buildInfo
+        link = buildInfo.get('url', None)
+        if link:
+            msg += " - " + link
+        else:
+            msg += "."
+        msgs.append(msg)
+
+        if buildInfo['result'] == SUCCESS:
+            success = True
+        else:
+            failure = True
+
+    msg = '\n\n'.join(msgs)
+
+    if success and not failure:
+        verified = 1
+    else:
+        verified = -1
+
+    reviewed = 0
+    return (msg, verified, reviewed)
+
+
+# These are just sentinel values for GerritStatusPush.__init__ args
+class DEFAULT_REVIEW(object):
+    pass
+
+
+class DEFAULT_SUMMARY(object):
+    pass
+
+
+class GerritStatusPush(StatusReceiverMultiService, buildset.BuildSetSummaryNotifierMixin):
 
     """Event streamer to a gerrit ssh server."""
 
-    def __init__(self, server, username, reviewCB=defaultReviewCB,
+    def __init__(self, server, username, reviewCB=DEFAULT_REVIEW,
                  startCB=None, port=29418, reviewArg=None,
-                 startArg=None, **kwargs):
+                 startArg=None, summaryCB=DEFAULT_SUMMARY, summaryArg=None, **kwargs):
         """
         @param server:    Gerrit SSH server's address to use for push event notifications.
         @param username:  Gerrit SSH server's username.
@@ -59,8 +104,24 @@ def __init__(self, server, username, reviewCB=defaultReviewCB,
         @param port:      Gerrit SSH server's port.
         @param reviewArg: Optional argument passed to the review callback.
         @param startArg:  Optional argument passed to the start callback.
+        @param summaryCB:  Callback that is called each time a buildset finishes, and that is used
+                           to define a message and review approvals depending on the build result.
+        @param summaryArg: Optional argument passed to the summary callback.
         """
         StatusReceiverMultiService.__init__(self)
+
+        # If neither reviewCB nor summaryCB were specified, default to sending
+        # out "summary" reviews. But if we were given a reviewCB and only a
+        # reviewCB, disable the "summary" reviews, so we don't send out both
+        # by default.
+        if reviewCB is DEFAULT_REVIEW and summaryCB is DEFAULT_SUMMARY:
+            reviewCB = None
+            summaryCB = defaultSummaryCB
+        if reviewCB is DEFAULT_REVIEW:
+            reviewCB = None
+        if summaryCB is DEFAULT_SUMMARY:
+            summaryCB = None
+
         # Parameters.
         self.gerrit_server = server
         self.gerrit_username = username
@@ -71,6 +132,8 @@ def __init__(self, server, username, reviewCB=defaultReviewCB,
         self.reviewArg = reviewArg
         self.startCB = startCB
         self.startArg = startArg
+        self.summaryCB = summaryCB
+        self.summaryArg = summaryArg
 
     def _gerritCmd(self, *args):
         return ["ssh", self.gerrit_username + "@" + self.gerrit_server, "-p %d" % self.gerrit_port, "gerrit"] + list(args)
@@ -135,11 +198,24 @@ def processEnded(self, status_object):
             else:
                 print "gerrit status: OK"
 
+    def setServiceParent(self, parent):
+        """
+        @type  parent: L{buildbot.master.BuildMaster}
+        """
+        StatusReceiverMultiService.setServiceParent(self, parent)
+        self.master_status = self.parent
+        self.master_status.subscribe(self)
+        self.master = self.master_status.master
+
     def startService(self):
         print """Starting up."""
+        if self.summaryCB:
+            self.summarySubscribe()
+
         StatusReceiverMultiService.startService(self)
-        self.status = self.parent.getStatus()
-        self.status.subscribe(self)
+
+    def stopService(self):
+        self.summaryUnsubscribe()
 
     def builderAdded(self, name, builder):
         return self  # subscribe to this builder
@@ -151,8 +227,31 @@ def buildStarted(self, builderName, build):
 
     def buildFinished(self, builderName, build, result):
         """Do the SSH gerrit verify command to the server."""
-        message, verified, reviewed = self.reviewCB(builderName, build, result, self.status, self.reviewArg)
-        self.sendCodeReviews(build, message, verified, reviewed)
+        if self.reviewCB:
+            message, verified, reviewed = self.reviewCB(builderName, build, result, self.master_status, self.reviewArg)
+            self.sendCodeReviews(build, message, verified, reviewed)
+
+    def sendBuildSetSummary(self, buildset, builds):
+        if self.summaryCB:
+            def getBuildInfo(build):
+                result = build.getResults()
+                resultText = {
+                    SUCCESS: "succeeded",
+                    FAILURE: "failed",
+                    WARNINGS: "completed with warnings",
+                    EXCEPTION: "encountered an exception",
+                }.get(result, "completed with unknown result %d" % result)
+
+                return {'name': build.getBuilder().getName(),
+                        'result': result,
+                        'resultText': resultText,
+                        'text': ' '.join(build.getText()),
+                        'url': self.master_status.getURLForThing(build),
+                        }
+            buildInfoList = sorted([getBuildInfo(build) for build in builds], key=lambda bi: bi['name'])
+
+            message, verified, reviewed = self.summaryCB(buildInfoList, Results[buildset['results']], self.master_status, self.summaryArg)
+            self.sendCodeReviews(builds[0], message, verified, reviewed)
 
     def sendCodeReviews(self, build, message, verified=0, reviewed=0):
         if message is None: