buildbot/buildbot — pull request #391 by Jc2k (2012-04-26 12:39:57+00:00)
Title: Make sure build steps can raise BuildStepFailed

I have a custom build step that calls multiple remote shell commands. It uses defer.inlineCallbacks and based on an example by @tomprince it raised BuildStepFailed when it detected an error condition.

However this exception is not honored. Worse, as far as I can tell proper cleanup for the step was not running at all and i was left with orphaned VM's and such.

I now check to see if self.start() is returning a deferred - if it is i add an errBack to it that calls self.failed.

Attached test case fails without my change and passes with it.

Another option is to use defer.inlineCallbacks on _startStep_3 and wait for a return value from self.start():

@defer.inlineCallbacks
def _startStep_3(self, doStep):
doStep = doStep[0]
try:
if doStep:
result = yield self.start()
if result == SKIPPED:
doStep = False
except:
log.msg("BuildStep.startStep exception in .start")
self.failed(Failure())

if not doStep:
self.step_status.setText(self.describe(True) + ['skipped'])
self.step_status.setSkipped(True)
# this return value from self.start is a shortcut to finishing
# the step immediately; we skip calling finished() as
# subclasses may have overridden that an expect it to be called
# after start() (bug #837)
reactor.callLater(0, self._finishFinished, SKIPPED)

However i'm not confident enough in the code to say whether this breaks any assumptions about how quickly the SKIPPED result from self.start is handled.