From fd7cb7c891b3e866513e60b29c07c3523876c00f Mon Sep 17 00:00:00 2001 From: Pierre Tardy Date: Mon, 20 Mar 2017 11:16:53 +0100 Subject: [PATCH 1/3] add more tests for the usePTY feature some people have reported the usePTY feature to not work. This is a unit test and integration test to make sure it actually works test is only for posix --- master/buildbot/steps/shell.py | 2 +- .../buildbot/test/integration/test_usePty.py | 71 +++++++++++++++++++ .../test/unit/test_process_buildstep.py | 14 ++++ master/buildbot/test/util/integration.py | 11 ++- 4 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 master/buildbot/test/integration/test_usePty.py diff --git a/master/buildbot/steps/shell.py b/master/buildbot/steps/shell.py index 95264e6680d..be7bd0b73db 100644 --- a/master/buildbot/steps/shell.py +++ b/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'] diff --git a/master/buildbot/test/integration/test_usePty.py b/master/buildbot/test/integration/test_usePty.py new file mode 100644 index 00000000000..05ded536c95 --- /dev/null +++ b/master/buildbot/test/integration/test_usePty.py @@ -0,0 +1,71 @@ +# 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 builders and a shellcommand step +# meant to be a template for integration steps +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 diff --git a/master/buildbot/test/unit/test_process_buildstep.py b/master/buildbot/test/unit/test_process_buildstep.py index 3c4fdbfff65..553913cb6ef 100644 --- a/master/buildbot/test/unit/test_process_buildstep.py +++ b/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( diff --git a/master/buildbot/test/util/integration.py b/master/buildbot/test/util/integration.py index 3930833a3b4..a41f3ded190 100644 --- a/master/buildbot/test/util/integration.py +++ b/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 *********" % From 173ecaad69053635bbd38b9658fbe4f0463a6e88 Mon Sep 17 00:00:00 2001 From: Pierre Tardy Date: Mon, 20 Mar 2017 14:33:23 +0100 Subject: [PATCH 2/3] typo, and improvments of the template --- .../buildbot/test/integration/test_integration_template.py | 7 +++++-- master/buildbot/test/integration/test_usePty.py | 3 +-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/master/buildbot/test/integration/test_integration_template.py b/master/buildbot/test/integration/test_integration_template.py index 496b07131a9..9fb552b243d 100644 --- a/master/buildbot/test/integration/test_integration_template.py +++ b/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() diff --git a/master/buildbot/test/integration/test_usePty.py b/master/buildbot/test/integration/test_usePty.py index 05ded536c95..010066bc890 100644 --- a/master/buildbot/test/integration/test_usePty.py +++ b/master/buildbot/test/integration/test_usePty.py @@ -23,8 +23,7 @@ # This integration test creates a master and worker environment, -# with one builders and a shellcommand step -# meant to be a template for integration steps +# with one builder and a shellcommand step, which use usePTY class ShellMaster(RunMasterBase): @skipUnlessPlatformIs('posix') From a3da1d9a7ba1b5bada0a1d08e8d87e9399b1870d Mon Sep 17 00:00:00 2001 From: Pierre Tardy Date: Tue, 21 Mar 2017 10:59:36 +0100 Subject: [PATCH 3/3] add relnotes about the bugfix --- master/buildbot/newsfragments/usePTY.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 master/buildbot/newsfragments/usePTY.bugfix diff --git a/master/buildbot/newsfragments/usePTY.bugfix b/master/buildbot/newsfragments/usePTY.bugfix new file mode 100644 index 00000000000..b175c889802 --- /dev/null +++ b/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.