add more compatibility for old-style steps

* BuildStep implements describe()
* Factor out command_to_string

master/buildbot/process/buildstep.py

@@ -754,6 +754,21 @@ def getStatistics(self):
     def setStatistic(self, name, value):
         self.statistics[name] = value
 
+    def _describe(self, done=False):
+        # old-style steps expect this function to exist
+        assert not self.isNewStyle()
+        return []
+
+    def describe(self, done=False):
+        # old-style steps expect this function to exist
+        assert not self.isNewStyle()
+        desc = self._describe(done)
+        if not desc:
+            return []
+        if self.descriptionSuffix:
+            desc = desc + u' ' + util.join_list(self.descriptionSuffix)
+        return desc
+
 
 components.registerAdapter(
     BuildStep._getStepFactory,

master/buildbot/steps/shell.py

@@ -25,6 +25,7 @@
 from buildbot.status.results import Results
 from buildbot.status.results import SUCCESS
 from buildbot.status.results import WARNINGS
+from buildbot.util import command_to_string
 from buildbot.util import flatten
 from buildbot.util import join_list
 from twisted.python import failure
@@ -211,35 +212,7 @@ def _getLegacySummary(self, done):
             else:
                 return None
 
-            words = command
-            if isinstance(words, (str, unicode)):
-                words = words.split()
-
-            try:
-                len(words)
-            except (AttributeError, TypeError):
-                # WithProperties and Property don't have __len__
-                # For old-style classes instances AttributeError raised,
-                # for new-style classes instances - TypeError.
-                return None
-
-            # flatten any nested lists
-            words = flatten(words, (list, tuple))
-
-            # strip instances and other detritus (which can happen if a
-            # description is requested before rendering)
-            words = [w for w in words if isinstance(w, (str, unicode))]
-
-            if len(words) < 1:
-                return None
-            if len(words) < 3:
-                rv = "'%s'" % (' '.join(words))
-            else:
-                rv = "'%s ...'" % (' '.join(words[:2]))
-
-            # cmd was a comand and thus probably a bytestring.  Be gentle in
-            # trying to covert it.
-            rv = rv.decode('ascii', errors='replace')
+            rv = command_to_string(command)
 
             # add the descriptionSuffix, if one was given
             if self.descriptionSuffix:

master/buildbot/steps/shellsequence.py

@@ -14,6 +14,7 @@
 # Copyright Buildbot Team Members
 
 from buildbot import config
+from buildbot import util
 from buildbot.process import buildstep
 from buildbot.status import results
 from twisted.internet import defer
@@ -61,6 +62,7 @@ def getRenderingFor(self, build):
 
 
 class ShellSequence(buildstep.ShellMixin, buildstep.BuildStep):
+    last_command = None
     renderables = ['commands']
 
     def __init__(self, commands=None, **kwargs):
@@ -93,8 +95,8 @@ def runShellSequence(self, commands):
             if not self.shouldRunTheCommand(command):
                 continue
 
-            # stick the command in self.command so that describe can use it
-            self.command = command
+            # keep the command around so we can describe it
+            self.last_command = command
 
             cmd = yield self.makeRemoteShellCommand(command=command,
                                                     stdioLogName=arg.logfile)
@@ -107,3 +109,10 @@ def runShellSequence(self, commands):
 
     def run(self):
         return self.runShellSequence(self.commands)
+
+    def getResultSummary(self):
+        if self.last_command:
+            summary = util.command_to_string(self.last_command)
+            if summary:
+                return {u'step': summary}
+        return super(ShellSequence, self).getResultSummary()

master/buildbot/test/unit/test_steps_shellsequence.py

@@ -54,7 +54,7 @@ def testShellArgsAreRendered(self):
                                         usePTY="slave-config")
                             + 0 + Expect.log('stdio make BUILDBOT-TEST'))
         # TODO: need to factor command-summary stuff into a utility method and use it here
-        self.expectOutcome(result=SUCCESS, status_text=["'make", "BUILDBOT-TEST'"])
+        self.expectOutcome(result=SUCCESS, state_string="'make BUILDBOT-TEST'")
         return self.runStep()
 
     def createDynamicRun(self, commands):
@@ -64,19 +64,19 @@ def createDynamicRun(self, commands):
     def testSanityChecksAreDoneInRuntimeWhenDynamicCmdIsNone(self):
         self.setupStep(self.createDynamicRun(None))
         self.expectOutcome(result=EXCEPTION,
-                           status_text=['commands == None'])
+                           state_string="commands == None")
         return self.runStep()
 
     def testSanityChecksAreDoneInRuntimeWhenDynamicCmdIsString(self):
         self.setupStep(self.createDynamicRun(["one command"]))
         self.expectOutcome(result=EXCEPTION,
-                           status_text=['one command', 'not', 'ShellArg'])
+                           state_string='one command not ShellArg')
         return self.runStep()
 
     def testSanityChecksAreDoneInRuntimeWhenDynamicCmdIsInvalidShellArg(self):
         self.setupStep(self.createDynamicRun([shellsequence.ShellArg(command=1)]))
         self.expectOutcome(result=EXCEPTION,
-                           status_text=[1, 'invalid', 'params'])
+                           state_string='1 invalid params')
         return self.runStep()
 
     def testMultipleCommandsAreRun(self):
@@ -90,7 +90,7 @@ def testMultipleCommandsAreRun(self):
                             ExpectShell(workdir='build', command='deploy p1',
                                         usePTY="slave-config") + 0 +
                             Expect.log('stdio deploy p1'))
-        self.expectOutcome(result=SUCCESS, status_text=["'deploy", "p1'"])
+        self.expectOutcome(result=SUCCESS, state_string="'deploy p1'")
         return self.runStep()
 
     def testSkipWorks(self):
@@ -104,7 +104,7 @@ def testSkipWorks(self):
                                         usePTY="slave-config") + 0,
                             ExpectShell(workdir='build', command='deploy p1',
                                         usePTY="slave-config") + 0)
-        self.expectOutcome(result=SUCCESS, status_text=["'deploy", "p1'"])
+        self.expectOutcome(result=SUCCESS, state_string="'deploy p1'")
         return self.runStep()
 
     def testWarningWins(self):
@@ -119,7 +119,7 @@ def testWarningWins(self):
                                         usePTY="slave-config") + 1,
                             ExpectShell(workdir='build', command='deploy p1',
                                         usePTY="slave-config") + 0)
-        self.expectOutcome(result=WARNINGS, status_text=["'deploy", "p1'"])
+        self.expectOutcome(result=WARNINGS, state_string="'deploy p1'")
         return self.runStep()
 
     def testSequenceStopsOnHaltOnFailure(self):
@@ -131,5 +131,5 @@ def testSequenceStopsOnHaltOnFailure(self):
                                         workdir='build'))
         self.expectCommands(ExpectShell(workdir='build', command='make p1',
                                         usePTY="slave-config") + 1)
-        self.expectOutcome(result=FAILURE, status_text=["'make", "p1'"])
+        self.expectOutcome(result=FAILURE, state_string="'make p1'")
         return self.runStep()

master/buildbot/test/unit/test_util.py

@@ -329,3 +329,32 @@ def test_unicode(self):
 
     def test_nonascii(self):
         self.assertRaises(UnicodeDecodeError, lambda: util.join_list(['\xff']))
+
+
+class CommandToString(unittest.TestCase):
+
+    def test_short_string(self):
+        self.assertEqual(util.command_to_string("ab cd"), u"'ab cd'")
+
+    def test_long_string(self):
+        self.assertEqual(util.command_to_string("ab cd ef"), u"'ab cd ...'")
+
+    def test_list(self):
+        self.assertEqual(util.command_to_string(['ab', 'cd', 'ef']),
+                         u"'ab cd ...'")
+
+    def test_nested_list(self):
+        self.assertEqual(util.command_to_string(['ab', ['cd', ['ef']]]),
+                         u"'ab cd ...'")
+
+    def test_object(self):
+        # this looks like a renderable
+        self.assertEqual(util.command_to_string(object()), None)
+
+    def test_list_with_objects(self):
+        # the object looks like a renderable, and is skipped
+        self.assertEqual(util.command_to_string(['ab', object(), 'cd']),
+                         u"'ab cd'")
+
+    def test_invalid_ascii(self):
+        self.assertEqual(util.command_to_string('a\xffc'), u"'a\ufffdc'")

master/buildbot/test/util/steps.py

@@ -229,8 +229,9 @@ def check(result):
             self.assertEqual(result, self.exp_result, "expected result")
             if self.exp_state_strings:
                 stepStateStrings = self.master.data.updates.stepStateStrings
-                stepid = stepStateStrings.keys()[0]
-                self.assertEqual(stepStateStrings[stepid],
+                stepids = stepStateStrings.keys()
+                assert stepids, "no step state strings were set"
+                self.assertEqual(stepStateStrings[stepids[0]],
                                  self.exp_state_strings,
                                  "expected step state strings")
             for pn, (pv, ps) in self.exp_properties.iteritems():

master/buildbot/util/__init__.py

@@ -284,6 +284,40 @@ def join_list(maybeList):
         return ascii2unicode(maybeList)
 
 
+def command_to_string(command):
+    words = command
+    if isinstance(words, (str, unicode)):
+        words = words.split()
+
+    try:
+        len(words)
+    except (AttributeError, TypeError):
+        # WithProperties and Property don't have __len__
+        # For old-style classes instances AttributeError raised,
+        # for new-style classes instances - TypeError.
+        return None
+
+    # flatten any nested lists
+    words = flatten(words, (list, tuple))
+
+    # strip instances and other detritus (which can happen if a
+    # description is requested before rendering)
+    words = [w for w in words if isinstance(w, (str, unicode))]
+
+    if len(words) < 1:
+        return None
+    if len(words) < 3:
+        rv = "'%s'" % (' '.join(words))
+    else:
+        rv = "'%s ...'" % (' '.join(words[:2]))
+
+    # cmd was a comand and thus probably a bytestring.  Be gentle in
+    # trying to covert it.
+    rv = rv.decode('ascii', errors='replace')
+
+    return rv
+
+
 __all__ = [
     'naturalSort', 'now', 'formatInterval', 'ComparableMixin', 'json',
     'safeTranslate', 'none_or_str',