Merge pull request #3054 from tardyp/usePTY

fix the usePTY feature

master/buildbot/newsfragments/usePTY.bugfix

@@ -0,0 +1 @@
+``usePTY`` configuration of the :bb:step:`ShellCommand` now works as expected with recent version of buildbot-worker.

master/buildbot/steps/shell.py

@@ -248,7 +248,7 @@ def buildCommandKwargs(self, warnings):
 
         # check for the usePTY flag
         if 'usePTY' in kwargs and kwargs['usePTY'] is not None:
-            if self.workerVersionIsOlderThan("svn", "2.7"):
+            if self.workerVersionIsOlderThan("shell", "2.7"):
                 warnings.append(
                     "NOTE: worker does not allow master to override usePTY\n")
                 del kwargs['usePTY']

master/buildbot/test/integration/test_integration_template.py

@@ -22,14 +22,14 @@
 
 
 # This integration test creates a master and worker environment,
-# with one builders and a shellcommand step
+# with one builder and a shellcommand step
 # meant to be a template for integration steps
 class ShellMaster(RunMasterBase):
 
     @defer.inlineCallbacks
     def test_shell(self):
         yield self.setupConfig(masterConfig())
-
+        # if you don't need change, you can just remove this change, and useChange parameter
         change = dict(branch="master",
                       files=["foo.c"],
                       author="me@foo.com",
@@ -51,6 +51,9 @@ def masterConfig():
     c['schedulers'] = [
         schedulers.AnyBranchScheduler(
             name="sched",
+            builderNames=["testy"]),
+        schedulers.ForceScheduler(
+            name="force",
             builderNames=["testy"])]
 
     f = BuildFactory()

master/buildbot/test/integration/test_usePty.py

@@ -0,0 +1,70 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+from __future__ import absolute_import
+from __future__ import print_function
+
+from twisted.internet import defer
+
+from buildbot.test.util.decorators import skipUnlessPlatformIs
+from buildbot.test.util.integration import RunMasterBase
+
+
+# This integration test creates a master and worker environment,
+# with one builder and a shellcommand step, which use usePTY
+class ShellMaster(RunMasterBase):
+
+    @skipUnlessPlatformIs('posix')
+    @defer.inlineCallbacks
+    def test_usePTY(self):
+        yield self.setupConfig(masterConfig(usePTY=True))
+
+        build = yield self.doForceBuild(wantSteps=True, wantLogs=True)
+        self.assertEqual(build['buildid'], 1)
+        res = yield self.checkBuildStepLogExist(build, "in a terminal", onlyStdout=True)
+        self.assertTrue(res)
+
+    @skipUnlessPlatformIs('posix')
+    @defer.inlineCallbacks
+    def test_NOusePTY(self):
+        yield self.setupConfig(masterConfig(usePTY=False))
+
+        build = yield self.doForceBuild(wantSteps=True, wantLogs=True)
+        self.assertEqual(build['buildid'], 1)
+        res = yield self.checkBuildStepLogExist(build, "not a terminal", onlyStdout=True)
+        self.assertTrue(res)
+
+
+# master configuration
+def masterConfig(usePTY):
+    c = {}
+    from buildbot.config import BuilderConfig
+    from buildbot.process.factory import BuildFactory
+    from buildbot.plugins import steps, schedulers
+
+    c['schedulers'] = [
+        schedulers.ForceScheduler(
+            name="force",
+            builderNames=["testy"])]
+
+    f = BuildFactory()
+    f.addStep(steps.ShellCommand(
+        command='if [ -t 1 ] ; then echo in a terminal; else echo "not a terminal"; fi',
+        usePTY=usePTY))
+    c['builders'] = [
+        BuilderConfig(name="testy",
+                      workernames=["local1"],
+                      factory=f)]
+    return c

master/buildbot/test/unit/test_process_buildstep.py

@@ -1097,6 +1097,20 @@ def test_example_old_worker(self):
                          u'NOTE: worker does not allow master to override usePTY\n'
                          'NOTE: worker does not allow master to specify interruptSignal\n')
 
+    @defer.inlineCallbacks
+    def test_example_new_worker(self):
+        self.setupStep(ShellMixinExample(usePTY=False, interruptSignal='DIE'),
+                       worker_version={'*': "3.0"}, wantDefaultWorkdir=False)
+        self.expectCommands(
+            ExpectShell(workdir='build', usePTY=False, command=['./cleanup.sh'])
+            # note missing parameters
+            + 0,
+        )
+        self.expectOutcome(result=SUCCESS)
+        yield self.runStep()
+        self.assertEqual(self.step.getLog('stdio').header,
+                         u'')
+
     @defer.inlineCallbacks
     def test_description(self):
         self.setupStep(SimpleShellCommand(

master/buildbot/test/util/integration.py

@@ -232,11 +232,16 @@ def printBuild(self, build, out=sys.stdout, withLogs=False):
                     self.printLog(log, out)
 
     @defer.inlineCallbacks
-    def checkBuildStepLogExist(self, build, expectedLog):
+    def checkBuildStepLogExist(self, build, expectedLog, onlyStdout=False):
         yield self.enrichBuild(build, wantSteps=True, wantProperties=True, wantLogs=True)
         for step in build['steps']:
-            if expectedLog in step['state_string']:
-                defer.returnValue(True)
+            for log in step['logs']:
+                for line in log['contents']['content'].splitlines():
+                    if onlyStdout and line[0] != 'o':
+                        continue
+                    if expectedLog in line:
+                        defer.returnValue(True)
+        defer.returnValue(False)
 
     def printLog(self, log, out):
         print(u" " * 8 + "*********** LOG: %s *********" %