Don't pass HEAD to git describe with --dirty

The 'git describe' manual explicitly distinguishes --dirty as not
taking a commitish, so don't pass one.  Fixes #2764.

master/buildbot/steps/source/git.py

@@ -315,13 +315,18 @@ def parseCommitDescription(self, _=None):
             return
 
         cmd = ['describe']
+        descriptionOpts = {}
         if isinstance(self.getDescription, dict):
+            descriptionOpts = self.getDescription
             for opt, arg in git_describe_flags:
-                opt = self.getDescription.get(opt, None)
+                opt = descriptionOpts.get(opt, None)
                 arg = arg(opt)
                 if arg:
                     cmd.extend(arg)
-        cmd.append('HEAD')
+        # 'git describe' takes a commitish as an argument for all options
+        # *except* --dirty
+        if not any(arg.startswith('--dirty') for arg in cmd):
+            cmd.append('HEAD')
 
         try:
             stdout = yield self._dovccmd(cmd, collectStdout=True)

master/buildbot/test/unit/test_steps_source_git.py

@@ -1764,7 +1764,8 @@ def test_getDescription_failed(self):
         self.expectNoProperty('commit-description')
         return self.runStep()
 
-    def setup_getDescription_test(self, setup_args, output_args, codebase=None):
+    def setup_getDescription_test(self, setup_args, output_args,
+                                  expect_head=True, codebase=None):
         # clone of: test_mode_full_clobber
         # only difference is to set the getDescription property
 
@@ -1807,7 +1808,7 @@ def setup_getDescription_test(self, setup_args, output_args, codebase=None):
             ExpectShell(workdir='wkdir',
                         command=['git', 'describe'] +
                                 output_args +
-                                ['HEAD'])
+                                (['HEAD'] if expect_head else []))
             + ExpectShell.log('stdio',
                               stdout='Tag-1234')
             + 0,
@@ -1903,28 +1904,40 @@ def test_getDescription_abbrev_false(self):
     def test_getDescription_dirty(self):
         self.setup_getDescription_test(
             setup_args={'dirty': True},
-            output_args=['--dirty']
+            output_args=['--dirty'],
+            expect_head=False
         )
         return self.runStep()
 
     def test_getDescription_dirty_empty_str(self):
         self.setup_getDescription_test(
             setup_args={'dirty': ''},
-            output_args=['--dirty']
+            output_args=['--dirty'],
+            expect_head=False
         )
         return self.runStep()
 
     def test_getDescription_dirty_str(self):
         self.setup_getDescription_test(
             setup_args={'dirty': 'foo'},
-            output_args=['--dirty=foo']
+            output_args=['--dirty=foo'],
+            expect_head=False
         )
         return self.runStep()
 
     def test_getDescription_dirty_false(self):
         self.setup_getDescription_test(
             setup_args={'dirty': False},
-            output_args=[]
+            output_args=[],
+            expect_head=True
+        )
+        return self.runStep()
+
+    def test_getDescription_dirty_none(self):
+        self.setup_getDescription_test(
+            setup_args={'dirty': None},
+            output_args=[],
+            expect_head=True
         )
         return self.runStep()
 

master/buildbot/test/util/steps.py

@@ -13,6 +13,7 @@
 #
 # Copyright Buildbot Team Members
 
+from twisted.python import log
 import mock
 
 from buildbot import interfaces
@@ -272,7 +273,15 @@ def _remotecommand_run(self, command, step, remote):
             del got[1][arg]
 
         # first check any ExpectedRemoteReference instances
-        self.assertEqual((exp.remote_command, exp.args), got)
+        try:
+            self.assertEqual((exp.remote_command, exp.args), got)
+        except AssertionError:
+            # log this error, as the step may swallow the AssertionError or
+            # otherwise obscure the failure.  Trial will see the exception in
+            # the log and print an [ERROR].  This may result in
+            # double-reporting, but that's better than non-reporting!
+            log.err()
+            raise
 
         # let the Expect object show any behaviors that are required
         d = exp.runBehaviors(command)