When doStepIf skips a step, finished() is called without start()

[gward]

I have a custom build step that looks something like this:

class MyStep(ShellCommand):
    def start(self):
        ShellCommand.start(self)
        self.__privatedata = ...

    def finished(self, results):
        self.__privatedata.something()
        ...

This worked fine under 0.7.9 for a year or so. In particular, it was safe for finished() to assume that self.__privatedata was set, because start() was always called. But I recently had to upgrade to use doStepIf. (Nice feature: thanks to whoever implemented it!)

Now the assumption behind my finished() is no longer correct. If a step is skipped because doStepIf returns false, start() is never called, but finished() is. So my step fails with AttributeError.

I've worked around it with try/except, but this seems bogus. It would be nice if we could go back to guaranteeing that start() is called, but I can see how that might be difficult. I would be perfectly happy to have a method in BuildStep that *is* guaranteed to be called before the step runs, regardless of whether start() is called. Then I can add private data attributes safely.

(I'm pretty sure that I cannot do this in __init__() because of the weird, semi-magical nature of BuildStep.)

[ccooper]

I'm hitting this too in 0.8.0. Specifically, if I don't specify a description for my build step, buildbot attempts to build one for me by walking the properties, some of which may not be set if doStepIf is false. (The workaround is, of course, to always provide a description.)

[dustin]

Let's see if we can get some tests to verify this. There are a lot of chicken-and-egg problems like this with the properties.

[dustin]

I don't think we can call start when skipping, since start actually starts stuff. However, it shouldn't be too hard to remove the finish invocation in this case.

[Dustin J. Mitchell]

Don't call finished() if start() is not called

Fixes #837

Changeset: f065671cfaa11e3c4e08de33995c135c740b43f0

[Dustin J. Mitchell]

Don't call finished() if start() is not called

Fixes #837

Changeset: f065671cfaa11e3c4e08de33995c135c740b43f0