Create a _BuildStepFactory to capture the arguments to BuildStep subc…

…lasses.

We use an instance of this class, rather than a closure mostly to make it
easier to test that the right factories are getting created.

master/buildbot/process/buildstep.py

@@ -381,6 +381,26 @@ def _start(self):
     def __repr__(self):
         return "<RemoteShellCommand '%s'>" % repr(self.command)
 
+class _BuildStepFactory(util.ComparableMixin):
+    """
+    This is a wrapper to record the arguments passed to as BuildStep subclass.
+    We use an instance of this class, rather than a closure mostly to make it
+    easier to test that the right factories are getting created.
+    """
+    compare_attrs = ['factory', 'args', 'kwargs' ]
+    def __init__(self, factory, *args, **kwargs):
+        self.factory = factory
+        self.args = args
+        self.kwargs = kwargs
+
+    def __call__(self):
+        try:
+            return self.factory(*self.args, **self.kwargs)
+        except:
+            log.msg("error while creating step, factory=%s, args=%s, kwargs=%s"
+                    % (self.factory, self.args, self.kwargs))
+            raise
+
 class BuildStep(object, properties.PropertiesMixin):
 
     haltOnFailure = False
@@ -439,14 +459,7 @@ def __init__(self, **kwargs):
 
     def __new__(klass, *args, **kwargs):
         self = object.__new__(klass)
-        def factory():
-            try:
-                return klass(*args, **kwargs)
-            except:
-                log.msg("error while creating step, factory=%s, args=%s, kwargs=%s"
-                        % (klass, args, kwargs))
-                raise
-        self.factory = factory
+        self.factory = _BuildStepFactory(klass, *args, **kwargs)
         return self
 
     def describe(self, done=False):

master/buildbot/test/unit/test_process_factory.py

@@ -14,38 +14,33 @@
 # Copyright Buildbot Team Members
 
 from twisted.trial import unittest
-from mock import Mock
 
-from buildbot.process.factory import BuildFactory, ArgumentsInTheWrongPlace
-from buildbot.process.buildstep import BuildStep
+from buildbot.process.factory import BuildFactory
+from buildbot.process.buildstep import BuildStep, _BuildStepFactory
 
 class TestBuildFactory(unittest.TestCase):
 
     def test_init(self):
         step = BuildStep()
         factory = BuildFactory([step])
-        self.assertEqual(factory.steps, [(BuildStep, {})])
+        self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep)])
 
     def test_addStep(self):
         step = BuildStep()
         factory = BuildFactory()
         factory.addStep(step)
-        self.assertEqual(factory.steps, [(BuildStep, {})])
-
-    def test_addStep_deprecated(self):
-        factory = BuildFactory()
-        factory.addStep(BuildStep)
-        self.assertEqual(factory.steps, [(BuildStep, {})])
+        self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep)])
 
     def test_addStep_notAStep(self):
         factory = BuildFactory()
-        self.assertRaises(ValueError, factory.addStep, Mock())
+        # This fails because object doesn't have .getStepFactory
+        self.assertRaises(AttributeError, factory.addStep, object())
 
     def test_addStep_ArgumentsInTheWrongPlace(self):
         factory = BuildFactory()
-        self.assertRaises(ArgumentsInTheWrongPlace, factory.addStep, BuildStep(), name="name")
+        self.assertRaises(TypeError, factory.addStep, BuildStep(), name="name")
 
     def test_addSteps(self):
         factory = BuildFactory()
         factory.addSteps([BuildStep(), BuildStep()])
-        self.assertEqual(factory.steps, [(BuildStep, {}), (BuildStep, {})])
+        self.assertEqual(factory.steps, [_BuildStepFactory(BuildStep), _BuildStepFactory(BuildStep)])