Merge pull request #1509 from tomprince/render-path-in-master-shell-c…

…ommand

Make ``path`` renderable in ``MasterShellCommand``.

master/buildbot/steps/master.py

@@ -38,7 +38,10 @@ class MasterShellCommand(BuildStep):
     description = 'Running'
     descriptionDone = 'Ran'
     descriptionSuffix = None
-    renderables = ['command', 'env', 'description', 'descriptionDone', 'descriptionSuffix']
+    renderables = [
+        'command', 'env', 'path',
+        'description', 'descriptionDone', 'descriptionSuffix',
+    ]
     haltOnFailure = True
     flunkOnFailure = True
 
@@ -107,14 +110,19 @@ def start(self):
             else:
                 argv = command
 
+        if self.path is not None:
+            path = os.path.join(os.getcwd(), self.path)
+        else:
+            path = os.getcwd()
+
         self.stdio_log = stdio_log = self.addLog("stdio")
 
         if type(command) in types.StringTypes:
             stdio_log.addHeader(command.strip() + "\n\n")
         else:
             stdio_log.addHeader(" ".join(command) + "\n\n")
         stdio_log.addHeader("** RUNNING ON BUILDMASTER **\n")
-        stdio_log.addHeader(" in dir %s\n" % os.getcwd())
+        stdio_log.addHeader(" in dir %s\n" % (path,))
         stdio_log.addHeader(" argv: %s\n" % (argv,))
         self.step_status.setText(self.describe())
 
@@ -149,7 +157,7 @@ def subst(match):
 
         # TODO add a timeout?
         self.process = reactor.spawnProcess(self.LocalPP(self), argv[0], argv,
-                                            path=self.path, usePTY=self.usePTY, env=env)
+                                            path=path, usePTY=self.usePTY, env=env)
         # (the LocalPP object will call processEnded for us)
 
     def processEnded(self, status_object):

master/buildbot/test/unit/test_steps_master.py

@@ -29,6 +29,7 @@
 from twisted.python import failure
 from twisted.python import runtime
 from twisted.trial import unittest
+from twisted.python.filepath import FilePath
 
 
 class TestMasterShellCommand(steps.BuildStepMixin, unittest.TestCase):
@@ -103,7 +104,7 @@ def test_real_cmd_fails(self):
     def test_constr_args(self):
         self.setupStep(
             master.MasterShellCommand(description='x', descriptionDone='y',
-                                      env={'a': 'b'}, path=['/usr/bin'], usePTY=True,
+                                      env={'a': 'b'}, path='/path/to/working/directory', usePTY=True,
                                       command='true'))
 
         self.assertEqual(self.step.describe(), ['x'])
@@ -114,7 +115,7 @@ def test_constr_args(self):
             exp_argv = ['/bin/sh', '-c', 'true']
         self.patchSpawnProcess(
             exp_cmd=exp_argv[0], exp_argv=exp_argv,
-            exp_path=['/usr/bin'], exp_usePTY=True, exp_env={'a': 'b'},
+            exp_path='/path/to/working/directory', exp_usePTY=True, exp_env={'a': 'b'},
             outputs=[
                 ('out', 'hello!\n'),
                 ('err', 'world\n'),
@@ -178,11 +179,70 @@ def test_prop_rendering(self):
         self.expectOutcome(result=SUCCESS, status_text=["Ran"])
         return self.runStep()
 
+    def test_path_is_renderable(self):
+        """
+        The ``path`` argument of ``MasterShellCommand`` is renderable.`
+        """
+        path = FilePath(self.mktemp())
+        path.createDirectory()
+        cmd = [sys.executable, '-c', 'import os, sys; sys.stdout.write(os.getcwd())']
+        self.setupStep(
+            master.MasterShellCommand(command=cmd, path=Interpolate(path.path)))
+        self.expectLogfile('stdio', path.path)
+        self.expectOutcome(result=SUCCESS, status_text=["Ran"])
+        return self.runStep()
+
+    def test_path_absolute(self):
+        """
+        If the ``path`` argument is absolute, the command is executed in that directory,
+        and that directory is logged.
+        """
+        path = FilePath(self.mktemp())
+        path.createDirectory()
+        cmd = [sys.executable, '-c', 'import os, sys; sys.stdout.write(os.getcwd())']
+        self.setupStep(
+            master.MasterShellCommand(command=cmd, path=path.path))
+        self.expectLogfile('stdio', path.path)
+        self.expectOutcome(result=SUCCESS, status_text=["Ran"])
+        d = self.runStep()
+
+        def check(_):
+            headers = self.step_status.logs['stdio'].headers.splitlines()
+            self.assertIn(" in dir %s" % (path.path,), headers)
+        return d
+
+    def test_path_relative(self):
+        """
+        If the ``path`` argument is relative, the path is combined with the
+        current working directory. The command is executed in that directory,
+        and that directory is logged.
+        """
+        base_path = FilePath(self.mktemp())
+        base_path.createDirectory()
+        child_path = base_path.child('child')
+        child_path.createDirectory()
+        cmd = [sys.executable, '-c', 'import os, sys; sys.stdout.write(os.getcwd())']
+
+        old_cwd = os.getcwd()
+        os.chdir(base_path.path)
+        self.addCleanup(os.chdir, old_cwd)
+
+        self.setupStep(
+            master.MasterShellCommand(command=cmd, path="child"))
+        self.expectLogfile('stdio', child_path.path)
+        self.expectOutcome(result=SUCCESS, status_text=["Ran"])
+        d = self.runStep()
+
+        def check(_):
+            headers = self.step_status.logs['stdio'].headers.splitlines()
+            self.assertIn(" in dir %s" % (child_path.path,), headers)
+        return d
+
     def test_constr_args_descriptionSuffix(self):
         self.setupStep(
             master.MasterShellCommand(description='x', descriptionDone='y',
                                       descriptionSuffix='z',
-                                      env={'a': 'b'}, path=['/usr/bin'], usePTY=True,
+                                      env={'a': 'b'}, path='/path/to/working/directory', usePTY=True,
                                       command='true'))
 
         # call twice to make sure the suffix doesn't get double added
@@ -195,7 +255,7 @@ def test_constr_args_descriptionSuffix(self):
             exp_argv = ['/bin/sh', '-c', 'true']
         self.patchSpawnProcess(
             exp_cmd=exp_argv[0], exp_argv=exp_argv,
-            exp_path=['/usr/bin'], exp_usePTY=True, exp_env={'a': 'b'},
+            exp_path='/path/to/working/directory', exp_usePTY=True, exp_env={'a': 'b'},
             outputs=[
                 ('out', 'hello!\n'),
                 ('err', 'world\n'),

master/docs/relnotes/index.rst

@@ -72,11 +72,14 @@ Features
 
 * :bb:chsrc:`GitPoller` now supports detecting new branches
 
+* :bb:step:`MasterShellCommand` now renders the ``path`` argument.
+
 Fixes
 ~~~~~
 
 * GitHub change hook now correctly responds to ping events.
 * ``buildbot.steps.http`` steps now correctly have ``url`` parameter renderable
+* :bb:step:`MasterShellCommand` now correctly logs the working directory where it was run.
 
 Deprecations, Removals, and Non-Compatible Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~