use interface tests to ensure fake and real update methods' signature…

…s match

master/buildbot/test/fake/fakedata.py

@@ -95,6 +95,9 @@ def masterStopped(self, name, masterid):
         self.masterActive = False
         return defer.succeed(None)
 
+    def expireMasters(self):
+        return defer.succeed(None)
+
     @defer.inlineCallbacks
     def addBuildset(self, scheduler=None, sourcestamps=[], reason='',
             properties={}, builderNames=[], external_idstring=None):
@@ -133,6 +136,9 @@ def updateBuilderList(self, masterid, builderNames):
         self.builderNames = builderNames
         return defer.succeed(None)
 
+    def masterDeactivated(self, masterid):
+        return defer.succeed(None)
+
 
 class FakeDataConnector(object):
     # FakeDataConnector delegates to the real DataConnector so it can get all

master/buildbot/test/unit/test_data_builders.py

@@ -17,7 +17,7 @@
 from twisted.trial import unittest
 from twisted.internet import defer
 from buildbot.data import builders
-from buildbot.test.util import validation, endpoint
+from buildbot.test.util import validation, endpoint, interfaces
 from buildbot.test.fake import fakemaster, fakedb
 
 class Builder(endpoint.EndpointMixin, unittest.TestCase):
@@ -123,17 +123,24 @@ def test_startConsuming(self):
                 expected_filter=('builder', None, None))
 
 
-class BuilderResourceType(unittest.TestCase):
+class BuilderResourceType(interfaces.InterfaceTests, unittest.TestCase):
 
     def setUp(self):
-        self.master = fakemaster.make_master(wantMq=True, wantDb=True,
-                                                testcase=self)
+        self.master = fakemaster.make_master(testcase=self,
+                wantMq=True, wantDb=True, wantData=True)
         self.rtype = builders.BuildersResourceType(self.master)
         return self.master.db.insertTestData([
             fakedb.Master(id=13),
             fakedb.Master(id=14),
         ])
 
+    def test_signature_updateBuilderList(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.updateBuilderList, # fake
+            self.rtype.updateBuilderList) # real
+        def updateBuilderList(self, masterid, builderNames):
+            pass
+
     @defer.inlineCallbacks
     def test_updateBuilderList(self):
         # add one builder master
@@ -180,7 +187,7 @@ def test_updateBuilderList(self):
         ])
 
     @defer.inlineCallbacks
-    def test_masterDeactivated(self):
+    def test__masterDeactivated(self):
         # this method just calls updateBuilderList, so test that.
         self.rtype.updateBuilderList = mock.Mock(
                 spec=self.rtype.updateBuilderList)

master/buildbot/test/unit/test_data_buildsets.py

@@ -18,7 +18,7 @@
 from twisted.internet import task, defer
 from buildbot import interfaces
 from buildbot.data import buildsets
-from buildbot.test.util import validation, endpoint
+from buildbot.test.util import validation, endpoint, interfaces as util_interfaces
 from buildbot.test.fake import fakedb, fakemaster
 from buildbot.status.results import SUCCESS, FAILURE
 
@@ -124,11 +124,11 @@ def test_startConsuming(self):
 
 
 
-class BuildsetResourceType(unittest.TestCase):
+class BuildsetResourceType(util_interfaces.InterfaceTests, unittest.TestCase):
 
     def setUp(self):
-        self.master = fakemaster.make_master(wantMq=True, wantDb=True,
-                                        wantData=True, testcase=self)
+        self.master = fakemaster.make_master(testcase=self,
+                wantMq=True, wantDb=True, wantData=True)
         self.rtype = buildsets.BuildsetResourceType(self.master)
         return self.master.db.insertTestData([
             fakedb.SourceStamp(id=234, branch='br', codebase='cb',
@@ -140,6 +140,14 @@ def setUp(self):
                   'project': u'pr', 'repository': u'rep', 'revision': u'rev',
                   'created_at': 89834834, 'ssid': 234}
 
+    def test_signature_addBuildset(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.addBuildset, # fake
+            self.rtype.addBuildset) # real
+        def addBuildset(self, scheduler=None, sourcestamps=[], reason='',
+            properties={}, builderNames=[], external_idstring=None):
+            pass
+
     def do_test_addBuildset(self, kwargs, expectedReturn,
             expectedMessages, expectedBuildset):
         """Run a test of addBuildset.
@@ -196,7 +204,6 @@ def _buildsetCompleteMessage(self, bsid, complete_at=A_TIMESTAMP,
                  results=results, submitted_at=submitted_at,
                  sourcestamps=[ ssmap[ssid] for ssid in sourcestampids ]))
 
-
     def test_addBuildset_two_builderNames(self):
         class FakeSched(object):
             implements(interfaces.IScheduler)
@@ -236,6 +243,13 @@ class FakeSched(object):
         return self.do_test_addBuildset(kwargs,
                 expectedReturn, expectedMessages, expectedBuildset)
 
+    def test_signature_maybeBuildsetComplete(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.maybeBuildsetComplete, # fake
+            self.rtype.maybeBuildsetComplete) # real
+        def maybeBuildsetComplete(self, bsid):
+            pass
+
     @defer.inlineCallbacks
     def do_test_maybeBuildsetComplete(self,
             buildRequestCompletions={},

master/buildbot/test/unit/test_data_changes.py

@@ -17,7 +17,7 @@
 from twisted.trial import unittest
 from twisted.internet import defer, task
 from buildbot.data import changes, exceptions
-from buildbot.test.util import validation, endpoint
+from buildbot.test.util import validation, endpoint, interfaces
 from buildbot.test.fake import fakedb, fakemaster
 from buildbot.process.users import users
 
@@ -128,13 +128,23 @@ def test_startConsuming(self):
                 expected_filter=('change', None, 'new'))
 
 
-class ChangeResourceType(unittest.TestCase):
+class ChangeResourceType(interfaces.InterfaceTests, unittest.TestCase):
 
     def setUp(self):
         self.master = fakemaster.make_master(wantMq=True, wantDb=True,
                                             wantData=True, testcase=self)
         self.rtype = changes.ChangeResourceType(self.master)
 
+    def test_signature_addChange(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.addChange, # fake
+            self.rtype.addChange) # real
+        def addChange(self, files=None, comments=None, author=None,
+            revision=None, when_timestamp=None, branch=None, category=None,
+            revlink=u'', properties={}, repository=u'', codebase=None,
+            project=u'', src=None):
+            pass
+
     def do_test_addChange(self, kwargs,
             expectedRoutingKey, expectedMessage, expectedRow,
             expectedChangeUsers=[]):

master/buildbot/test/unit/test_data_masters.py

@@ -18,7 +18,7 @@
 from twisted.internet import task, defer
 from buildbot.data import masters, builders
 from buildbot.util import epoch2datetime
-from buildbot.test.util import validation, endpoint
+from buildbot.test.util import validation, endpoint, interfaces
 from buildbot.test.fake import fakemaster, fakedb
 
 SOMETIME = 1349016870
@@ -139,7 +139,7 @@ def test_startConsuming(self):
                 expected_filter=('master', None, None))
 
 
-class MasterResourceType(unittest.TestCase):
+class MasterResourceType(interfaces.InterfaceTests, unittest.TestCase):
 
     def setUp(self):
         self.master = fakemaster.make_master(wantMq=True, wantDb=True,
@@ -152,6 +152,13 @@ def setUp(self):
 
         self.rtype = masters.MasterResourceType(self.master)
 
+    def test_signature_masterActive(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.masterActive, # fake
+            self.rtype.masterActive) # real
+        def masterActive(self, name, masterid):
+            pass
+
     @defer.inlineCallbacks
     def test_masterActive(self):
         clock = task.Clock()
@@ -192,6 +199,13 @@ def test_masterActive(self):
         ])
         self.master.mq.productions = []
 
+    def test_signature_masterStopped(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.masterStopped, # fake
+            self.rtype.masterStopped) # real
+        def masterStopped(self, name, masterid):
+            pass
+
     @defer.inlineCallbacks
     def test_masterStopped(self):
         clock = task.Clock()
@@ -221,6 +235,13 @@ def test_masterStopped_already(self):
         yield self.rtype.masterStopped(name=u'aname', masterid=13)
         self.assertEqual(self.master.mq.productions, [])
 
+    def test_signature_expireMasters(self):
+        @self.assertArgSpecMatches(
+            self.master.data.updates.expireMasters, # fake
+            self.rtype.expireMasters) # real
+        def expireMasters(self):
+            pass
+
     @defer.inlineCallbacks
     def test_expireMasters(self):
         clock = task.Clock()

master/buildbot/test/util/interfaces.py

@@ -19,7 +19,7 @@ class InterfaceTests(object):
 
     # assertions
 
-    def assertArgSpecMatches(self, actual, template=None):
+    def assertArgSpecMatches(self, *actualMethods):
         """Usage::
 
             @self.assertArgSpecMatches(obj.methodUnderTest)
@@ -56,17 +56,15 @@ def remove_decorators(func):
                 return func
 
         def wrap(template):
-            actual_argspec = filter(
-                    inspect.getargspec(remove_decorators(actual)))
-            template_argspec = filter(
-                    inspect.getargspec(remove_decorators(template)))
-            if actual_argspec != template_argspec:
-                msg = "Expected: %s; got: %s" % (
-                    inspect.formatargspec(*template_argspec),
-                    inspect.formatargspec(*actual_argspec))
-                self.fail(msg)
+            for actual in actualMethods:
+                actual_argspec = filter(
+                        inspect.getargspec(remove_decorators(actual)))
+                template_argspec = filter(
+                        inspect.getargspec(remove_decorators(template)))
+                if actual_argspec != template_argspec:
+                    msg = "Expected: %s; got: %s" % (
+                        inspect.formatargspec(*template_argspec),
+                        inspect.formatargspec(*actual_argspec))
+                    self.fail(msg)
             return template  # just in case it's useful
-        if template:
-            wrap(template)
-        else:
-            return wrap
+        return wrap

master/docs/developer/tests.rst

@@ -96,6 +96,8 @@ All of the test methods are defined here, segregated into tests that all impleme
 The ``test_signature_someMethod`` test above illustrates the :py:func:`buildbot.test.util.interfaces.assertArgSpecMatches` decorator, which can be used to compare the argument specification of a callable with a reference signature conveniently written as a nested function.
 Wherever possible, prefer to add tests to the ``Tests`` class, even if this means testing one method (e.g,. ``setFoo``) in terms of another (e.g., ``getFoo``).
 
+The ``assertArgSpecMatches`` method can take multiple methods to test; it will check each one in turn.
+
 At the bottom of the test module, a subclass is created for each implementation, implementing the setup methods that were stubbed out in the parent classes::
 
     class TestFakeThing(unittest.TestCase, Tests):