602180 - new UnpackTest step needs to set self.command

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[[e-mail] (:bhearsum)]

We're seeing tons of tracebacks that are rooted in the new UnpackTest step not setting self.command anymore; which ShellCommand requires for proper display/summary:
2010-10-06 05:44:01-0700 [-] acquireLocks(step <buildbotcustom.steps.misc.UnpackTest instance at 0x1437d86c>, locks [])
2010-10-06 05:44:01-0700 [-] Error describing step
2010-10-06 05:44:01-0700 [-] Unhandled Error
        Traceback (most recent call last):
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 190, in addCallback
            callbackKeywords=kw)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 181, in addCallbacks
            self._runCallbacks()
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/Twisted-9.0.0-py2.6-linux-i686.egg/twisted/internet/defer.py", line 323, in _runCallbacks
            self.result = callback(self.result, *args, **kw)
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/buildbot-0.8.1-py2.6.egg/buildbot/process/buildstep.py", line 744, in _startStep_2
            self.step_status.setText(self.describe(False))
        --- <exception caught here> ---
          File "/tools/buildbot-0.8.0/lib/python2.6/site-packages/buildbot-0.8.1-py2.6.egg/buildbot/steps/shell.py", line 144, in describe            if len(words) < 1:        exceptions.TypeError: object of type 'NoneType' has no len()


Syed, can you get to this today?

Comment 1

[Syed Albiz [:salbiz]]

Attachment: [tested]patch_unpacktests

This patch provides a workaround that provides an explicit description for the buildbot describe. The reason we were seeing this error is that the command is not set until the ShellCommand.start is called. Hence, the command executes just fine, but throws an exception on describe. One other way to deal with this might be to move the setCommand logic into the constructor

When I wrote this patch, I was following the design pattern used in other steps, like UnpackFile, Is there any particular reason we like to have it that way?

Comment 2

[[e-mail] (:bhearsum)]

(In reply to comment #1)
> Created attachment 481269 [details] [diff] [review]
> [tested]patch_unpacktests
> 
> This patch provides a workaround that provides an explicit description for the
> buildbot describe. The reason we were seeing this error is that the command is
> not set until the ShellCommand.start is called. Hence, the command executes
> just fine, but throws an exception on describe. One other way to deal with this
> might be to move the setCommand logic into the constructor
> 
> When I wrote this patch, I was following the design pattern used in other
> steps, like UnpackFile, Is there any particular reason we like to have it that
> way?

To be honest, I'm not completely sure. We usually err on the side of putting things in the constructor, because that code runs at config and run time, which means 'checkconfig' tests it. There must've been a good reason we didn't do that.

Anyways, r+!

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 481269 [details] [diff] [review]
[tested]patch_unpacktests

http://hg.mozilla.org/build/buildbotcustom/rev/c41141905a5f