bring back step statistics

README.md

@@ -108,6 +108,7 @@ Here are the remaining bits:
 * rewrite steps to new style
   * but not ShellCommand, yet, since user classes derive from it
   * merge rewrites to master
+  * remove `step_status` for statistics
 
 ## Data API ##
 

master/buildbot/process/build.py

@@ -74,6 +74,8 @@ class Build(properties.PropertiesMixin):
     set_runtime_properties = True
     subs = None
 
+    _sentinel = []  # used as a sentinel to indicate unspecified initial_value
+
     def __init__(self, requests):
         self.requests = requests
         self.locks = []
@@ -314,6 +316,7 @@ def setupBuild(self, expectations):
         # create the actual BuildSteps. If there are any name collisions, we
         # add a count to the loser until it is unique.
         self.steps = []
+        self.executedSteps = []
         self.stepStatuses = {}
         stepnames = {}
         sps = []
@@ -406,6 +409,7 @@ def startNextStep(self):
             s = None
         if not s:
             return self.allStepsDone()
+        self.executedSteps.append(s)
         self.currentStep = s
         d = defer.maybeDeferred(s.startStep, self.conn)
         d.addCallback(self._stepDone, s)
@@ -573,6 +577,16 @@ def releaseLocks(self):
                 # This should only happen if we've been interrupted
                 assert self.stopped
 
+    def getSummaryStatistic(self, name, summary_fn, initial_value=_sentinel):
+        step_stats_list = [
+            st.getStatistic(name)
+            for st in self.executedSteps
+            if st.hasStatistic(name)]
+        if initial_value is self._sentinel:
+            return reduce(summary_fn, step_stats_list)
+        else:
+            return reduce(summary_fn, step_stats_list, initial_value)
+
     # IBuildControl
 
     def getStatus(self):

master/buildbot/process/buildstep.py

@@ -243,6 +243,7 @@ def __init__(self, **kwargs):
         self._acquiringLock = None
         self.stopped = False
         self.master = None
+        self.statistics = {}
 
         self._start_unhandled_deferreds = None
 
@@ -330,7 +331,7 @@ def startStep(self, remote):
             if self.stopped:
                 raise BuildStepCancelled
 
-            # ste up progress
+            # set up progress
             if self.progress:
                 self.progress.start()
 
@@ -445,6 +446,11 @@ def run(self):
             self.step_status.setText = self.step_status.old_setText
             self.step_status.setText2 = self.step_status.old_setText2
 
+            # and monkey-patch in support for old statistics functions
+            self.step_status.setStatistic = self.setStatistic
+            self.step_status.getStatistic = self.getStatistic
+            self.step_status.hasStatistic = self.hasStatistic
+
             results = yield self.start()
             if results == SKIPPED:
                 yield self.setStateStrings(self.describe(True) + ['skipped'])
@@ -581,6 +587,19 @@ def runCommand(self, command):
         d = command.run(self, self.remote, self.build.builder.name)
         return d
 
+    def hasStatistic(self, name):
+        return name in self.statistics
+
+    def getStatistic(self, name, default=None):
+        return self.statistics.get(name, default)
+
+    def getStatistics(self):
+        return self.statistics.copy()
+
+    def setStatistic(self, name, value):
+        self.statistics[name] = value
+
+
 components.registerAdapter(
     BuildStep._getStepFactory,
     BuildStep, interfaces.IBuildStepFactory)

master/buildbot/steps/python.py

@@ -304,7 +304,7 @@ def createSummary(self, log):
         if self.warnings > 0:
             self.addCompleteLog('warnings', "\n".join(warnings))
 
-        self.setProperty('warnings', self.warnings, "sphinx")
+        self.step_status.setStatistic('warnings', self.warnings)
 
     def evaluateCommand(self, cmd):
         if self.success:

master/buildbot/steps/shell.py

@@ -592,6 +592,9 @@ def createSummary(self, log):
             self.addCompleteLog("warnings (%d)" % self.warnCount,
                                 "\n".join(warnings) + "\n")
 
+        warnings_stat = self.getStatistic('warnings', 0)
+        self.setStatistic('warnings', warnings_stat + self.warnCount)
+
         old_count = self.getProperty("warnings-count", 0)
         self.setProperty("warnings-count", old_count + self.warnCount, "WarningCountingShellCommand")
 
@@ -624,27 +627,27 @@ class Test(WarningCountingShellCommand):
 
     def setTestResults(self, total=0, failed=0, passed=0, warnings=0):
         """
-        Called by subclasses to set the relevant properties; this actually
-        adds to any properties already present
+        Called by subclasses to set the relevant statistics; this actually
+        adds to any statistics already present
         """
-        for prop, addend in [
-            ('tests-total', total),
-            ('tests-failed', failed),
-            ('tests-warnings', warnings),
-            ('tests-passed', passed),
-            ]:
-            addend += self.getProperty(prop, 0)
-            self.setProperty(prop, addend, 'test')
+        total += self.getStatistic('tests-total', 0)
+        self.setStatistic('tests-total', total)
+        failed += self.getStatistic('tests-failed', 0)
+        self.setStatistic('tests-failed', failed)
+        warnings += self.getStatistic('tests-warnings', 0)
+        self.setStatistic('tests-warnings', warnings)
+        passed += self.getStatistic('tests-passed', 0)
+        self.setStatistic('tests-passed', passed)
 
     def describe(self, done=False):
         description = WarningCountingShellCommand.describe(self, done)
         if done:
             description = description[:]  # make a private copy
-            if self.hasProperty('tests-total'):
-                total = self.getProperty("tests-total", 0)
-                failed = self.getProperty("tests-failed", 0)
-                passed = self.getProperty("tests-passed", 0)
-                warnings = self.getProperty("tests-warnings", 0)
+            if self.hasStatistic('tests-total'):
+                total = self.getStatistic("tests-total", 0)
+                failed = self.getStatistic("tests-failed", 0)
+                passed = self.getStatistic("tests-passed", 0)
+                warnings = self.getStatistic("tests-warnings", 0)
                 if not total:
                     total = failed + passed + warnings
 

master/buildbot/steps/vstudio.py

@@ -164,21 +164,21 @@ def setupEnvironment(self, cmd):
     def describe(self, done=False):
         description = ShellCommand.describe(self, done)
         if done:
-            description.append('%d projects' % self.getProperty('projects', 0))
-            description.append('%d files' % self.getProperty('files', 0))
-            warnings = self.getProperty('warnings', 0)
+            description.append('%d projects' % self.step_status.getStatistic('projects', 0))
+            description.append('%d files' % self.step_status.getStatistic('files', 0))
+            warnings = self.step_status.getStatistic('warnings', 0)
             if warnings > 0:
                 description.append('%d warnings' % warnings)
-            errors = self.getProperty('errors', 0)
+            errors = self.step_status.getStatistic('errors', 0)
             if errors > 0:
                 description.append('%d errors' % errors)
         return description
 
     def createSummary(self, log):
-        self.setProperty('projects', self.logobserver.nbProjects, 'Visual Studio')
-        self.setProperty('files', self.logobserver.nbFiles, 'Visual Studio')
-        self.setProperty('warnings', self.logobserver.nbWarnings, 'Visual Studio')
-        self.setProperty('errors', self.logobserver.nbErrors, 'Visual Studio')
+        self.step_status.setStatistic('projects', self.logobserver.nbProjects)
+        self.step_status.setStatistic('files', self.logobserver.nbFiles)
+        self.step_status.setStatistic('warnings', self.logobserver.nbWarnings)
+        self.step_status.setStatistic('errors', self.logobserver.nbErrors)
 
     def evaluateCommand(self, cmd):
         if cmd.didFail():

master/buildbot/test/unit/test_process_build.py

@@ -13,11 +13,13 @@
 #
 # Copyright Buildbot Team Members
 
+import operator
+
 from buildbot import config
 from buildbot import interfaces
 from buildbot.locks import SlaveLock
 from buildbot.process.build import Build
-from buildbot.process.buildstep import LoggingBuildStep
+from buildbot.process.buildstep import BuildStep, LoggingBuildStep
 from buildbot.process.properties import Properties
 from buildbot.status.results import CANCELLED
 from buildbot.status.results import EXCEPTION
@@ -636,6 +638,21 @@ def testStepDoneRetryOverridesAnythingElse(self):
         self.assertEqual(terminate, True)
         self.assertEqual(b.result, RETRY)
 
+    def test_getSummaryStatistic(self):
+        b = self.build
+
+        b.executedSteps = [
+            BuildStep(),
+            BuildStep(),
+            BuildStep()
+        ]
+        b.executedSteps[0].setStatistic('casualties', 7)
+        b.executedSteps[2].setStatistic('casualties', 4)
+
+        add = operator.add
+        self.assertEqual(b.getSummaryStatistic('casualties', add), 11)
+        self.assertEqual(b.getSummaryStatistic('casualties', add, 10), 21)
+
 
 class TestMultipleSourceStamps(unittest.TestCase):
 

master/buildbot/test/unit/test_process_buildstep.py

@@ -313,6 +313,30 @@ def test_step_renders_flunkOnFailure(self):
         yield self.runStep()
         self.assertEqual(self.step.flunkOnFailure, 'yes')
 
+    def test_hasStatistic(self):
+        step = buildstep.BuildStep()
+        self.assertFalse(step.hasStatistic('rbi'))
+        step.setStatistic('rbi', 13)
+        self.assertTrue(step.hasStatistic('rbi'))
+
+    def test_setStatistic(self):
+        step = buildstep.BuildStep()
+        step.setStatistic('rbi', 13)
+        self.assertEqual(step.getStatistic('rbi'), 13)
+
+    def test_getStatistic(self):
+        step = buildstep.BuildStep()
+        self.assertEqual(step.getStatistic('rbi', 99), 99)
+        self.assertEqual(step.getStatistic('rbi'), None)
+        step.setStatistic('rbi', 13)
+        self.assertEqual(step.getStatistic('rbi'), 13)
+
+    def test_getStatistics(self):
+        step = buildstep.BuildStep()
+        step.setStatistic('rbi', 13)
+        step.setStatistic('ba', 0.298)
+        self.assertEqual(step.getStatistics(), {'rbi': 13, 'ba': 0.298})
+
 
 class TestLoggingBuildStep(unittest.TestCase):
 

master/buildbot/test/unit/test_steps_python.py

@@ -420,8 +420,12 @@ def test_warnings(self):
         self.expectOutcome(result=WARNINGS,
                            status_text=["sphinx", "2 warnings", "warnings"])
         self.expectLogfile("warnings", warnings)
-        self.expectProperty('warnings', 2)
-        return self.runStep()
+        d = self.runStep()
+
+        def check(_):
+            self.assertEqual(self.step_statistics, {'warnings': 2})
+        d.addCallback(check)
+        return d
 
     def test_constr_args(self):
         self.setupStep(python.Sphinx(sphinx_sourcedir='src',

master/buildbot/test/unit/test_steps_shell.py

@@ -828,19 +828,19 @@ def tearDown(self):
     def test_setTestResults(self):
         step = self.setupStep(shell.Test())
         step.setTestResults(total=10, failed=3, passed=5, warnings=3)
-        self.assertEqual(self.properties.asDict(), {
-            'tests-total': (10, 'test'),
-            'tests-failed': (3, 'test'),
-            'tests-passed': (5, 'test'),
-            'tests-warnings': (3, 'test'),
+        self.assertEqual(self.step_statistics, {
+            'tests-total': 10,
+            'tests-failed': 3,
+            'tests-passed': 5,
+            'tests-warnings': 3,
         })
         # ensure that they're additive
         step.setTestResults(total=1, failed=2, passed=3, warnings=4)
-        self.assertEqual(self.properties.asDict(), {
-            'tests-total': (11, 'test'),
-            'tests-failed': (5, 'test'),
-            'tests-passed': (8, 'test'),
-            'tests-warnings': (7, 'test'),
+        self.assertEqual(self.step_statistics, {
+            'tests-total': 11,
+            'tests-failed': 5,
+            'tests-passed': 8,
+            'tests-warnings': 7,
         })
 
     def test_describe_not_done(self):
@@ -849,19 +849,19 @@ def test_describe_not_done(self):
 
     def test_describe_done(self):
         step = self.setupStep(shell.Test())
-        step.setProperty('tests-total', 93, 'test')
-        step.setProperty('tests-failed', 10, 'test')
-        step.setProperty('tests-passed', 20, 'test')
-        step.setProperty('tests-warnings', 30, 'test')
+        self.step_statistics['tests-total'] = 93
+        self.step_statistics['tests-failed'] = 10
+        self.step_statistics['tests-passed'] = 20
+        self.step_statistics['tests-warnings'] = 30
         self.assertEqual(step.describe(done=True), ['test', '93 tests',
                                                     '20 passed', '30 warnings', '10 failed'])
 
     def test_describe_done_no_total(self):
         step = self.setupStep(shell.Test())
-        step.setProperty('tests-total', 0, 'test')
-        step.setProperty('tests-failed', 10, 'test')
-        step.setProperty('tests-passed', 20, 'test')
-        step.setProperty('tests-warnings', 30, 'test')
+        self.step_statistics['tests-total'] = 0
+        self.step_statistics['tests-failed'] = 10
+        self.step_statistics['tests-passed'] = 20
+        self.step_statistics['tests-warnings'] = 30
         # describe calculates 60 = 10+20+30
         self.assertEqual(step.describe(done=True), ['test', '60 tests',
                                                     '20 passed', '30 warnings', '10 failed'])

master/buildbot/test/util/steps.py

@@ -127,6 +127,11 @@ def ss_setText2(strings):
 
         ss.getLogs = lambda: ss.logs.values()
 
+        self.step_statistics = {}
+        self.step.setStatistic = self.step_statistics.__setitem__
+        self.step.getStatistic = self.step_statistics.get
+        self.step.hasStatistic = self.step_statistics.__contains__
+
         self.step.setStepStatus(ss)
 
         # step overrides

master/docs/developer/classes.rst

@@ -11,6 +11,7 @@ The sections contained here document classes that can be used or subclassed.
 .. toctree::
     :maxdepth: 1
 
+    cls-build
     cls-buildfactory
     cls-remotecommands
     cls-buildsteps

master/docs/developer/cls-build.rst

@@ -0,0 +1,23 @@
+Builds
+======
+
+.. py:module:: buildbot.process.build
+
+The :py:class:`Build` class represents a running build, with associated steps.
+
+Build
+-----
+
+.. py:class:: Build
+
+    .. py:method:: getSummaryStatistic(name, summary_fn, initial_value=None)
+
+        :param name: statistic name to summarize
+        :param summary_fn: callable with two arguments that will combine two values
+        :param initial_value: first value to pass to ``summary_fn``
+        :returns: summarized result
+
+        This method summarizes the named step statistic over all steps in which it exists, using ``combination_fn`` and ``initial_value`` to combine multiple results into a single result.
+        This translates to a call to Python's ``reduce``::
+
+            return reduce(summary_fn, step_stats_list, initial_value)