last cleanups and unit test fixes

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>

master/buildbot/buildslave/manager.py

@@ -66,7 +66,7 @@ def __init__(self, master):
         service.AsyncMultiService.__init__(self)
 
         self.pb = bbpb.Listener(master)
-        self.pb.setServiceParent(self)
+        self.pb.setServiceParent(master)
 
         # BuildslaveRegistration instances keyed by buildslave name
         self.registrations = {}

master/buildbot/buildslave/protocols/pb.py

@@ -23,6 +23,7 @@
 
 
 class Listener(base.Listener):
+    name = "pbListener"
 
     def __init__(self, master):
         assert master

master/buildbot/test/unit/test_buildslave_base.py

@@ -110,9 +110,7 @@ def callAttached(self):
         self.buildslaves = bslavemanager.FakeBuildslaveManager(self.master)
         self.master.botmaster = self.botmaster
         self.master.buildslaves = self.buildslaves
-        self.sl.parent = self.master
-        self.sl.botmaster = self.botmaster
-        self.sl.manager = self.buildslaves
+        self.sl.setServiceParent(self.master.buildslaves)
         self.conn = fakeprotocol.FakeConnection(self.master, self.sl)
         return self.sl.attached(self.conn)
 
@@ -133,17 +131,16 @@ class TestAbstractBuildSlave(unittest.TestCase):
     def setUp(self):
         self.master = fakemaster.make_master(wantDb=True, wantData=True,
                                              testcase=self)
-        self.botmaster = botmaster.FakeBotMaster(self.master)
+        self.botmaster = self.master.botmaster
         self.buildslaves = bslavemanager.FakeBuildslaveManager(self.master)
         self.clock = task.Clock()
         self.patch(reactor, 'callLater', self.clock.callLater)
         self.patch(reactor, 'seconds', self.clock.seconds)
 
-    def createBuildslave(self, name='bot', password='pass', attached=False, **kwargs):
+    def createBuildslave(self, name='bot', password='pass', attached=False, configured=True, **kwargs):
         slave = ConcreteBuildSlave(name, password, **kwargs)
-        slave.parent = self.master
-        slave.botmaster = self.botmaster
-        slave.manager = self.buildslaves
+        if configured:
+            slave.setServiceParent(self.buildslaves)
         if attached:
             slave.conn = fakeprotocol.FakeConnection(self.master, slave)
         return slave
@@ -201,7 +198,7 @@ def test_reconfigService_attrs(self):
                                     notify_on_missing=['me@me.com'],
                                     missing_timeout=120,
                                     properties={'a': 'b'})
-        new = self.createBuildslave('bot', 'pass',
+        new = self.createBuildslave('bot', 'pass', configured=False,
                                     max_builds=3,
                                     notify_on_missing=['her@me.com'],
                                     missing_timeout=121,
@@ -395,6 +392,7 @@ def test_attached_remoteGetSlaveInfo(self):
     def test_attached_callsMaybeStartBuildsForSlave(self):
         slave = self.createBuildslave()
         yield slave.startService()
+        yield slave.reconfigServiceWithSibling(slave)
 
         conn = fakeprotocol.FakeConnection(slave.master, slave)
         conn.info = {}

master/buildbot/test/unit/test_util_service.py

@@ -560,10 +560,9 @@ def setUp(self):
     def prepareService(self):
         self.master.config = fakeConfig()
         serv = MyService(1, a=2, name="basic")
-        self.master.config.services = {"basic": serv}
         yield serv.setServiceParent(self.master)
         yield self.master.startService()
-        yield self.master.reconfigServiceWithBuildbotConfig(self.master.config)
+        yield serv.reconfigServiceWithSibling(serv)
         defer.returnValue(serv)
 
     @defer.inlineCallbacks
@@ -580,43 +579,6 @@ def testConfigDict(self):
             'kwargs': {'a': 2},
             'name': 'basic'})
 
-    @defer.inlineCallbacks
-    def testReconfigNoChange(self):
-        serv = yield self.prepareService()
-        serv.config = None  # 'de-configure' the service
-        # reconfigure with the same config
-        serv2 = MyService(1, a=2, name="basic")
-        self.master.config.services = {"basic": serv2}
-
-        # reconfigure the master
-        yield self.master.reconfigServiceWithBuildbotConfig(self.master.config)
-        # the first service is still used
-        self.assertIdentical(self.master.namedServices["basic"], serv)
-        # the second service is not used
-        self.assertNotIdentical(self.master.namedServices["basic"], serv2)
-
-        # reconfigServiceWithConstructorArgs was not called
-        self.assertEqual(serv.config, None)
-
-    @defer.inlineCallbacks
-    def testReconfigWithChanges(self):
-        serv = yield self.prepareService()
-        serv.config = None  # 'de-configure' the service
-
-        # reconfigure with the differnt config
-        serv2 = MyService(1, a=4, name="basic")
-        self.master.config.services = {"basic": serv2}
-
-        # reconfigure the master
-        yield self.master.reconfigServiceWithBuildbotConfig(self.master.config)
-        # the first service is still used
-        self.assertIdentical(self.master.namedServices["basic"], serv)
-        # the second service is not used
-        self.assertNotIdentical(self.master.namedServices["basic"], serv2)
-
-        # reconfigServiceWithConstructorArgs was called with new config
-        self.assertEqual(serv.config, ((1,), dict(a=4)))
-
     def testNoName(self):
         self.assertRaises(ValueError, lambda: MyService(1, a=2))
 

master/buildbot/test/util/changesource.py

@@ -52,8 +52,11 @@ def tearDownChangeSource(self):
     def attachChangeSource(self, cs):
         "Set up a change source for testing; sets its .master attribute"
         self.changesource = cs
-        self.changesource.master = self.master
-
+        # FIXME some changesource does not have master property yet but mailchangesource has :-/
+        try:
+            self.changesource.master = self.master
+        except AttributeError:
+            self.changesource.setServiceParent(self.master)
         # also, now that changesources are ClusteredServices, setting up
         # the clock here helps in the unit tests that check that behavior
         self.changesource.clock = task.Clock()

master/buildbot/util/service.py

@@ -348,5 +348,7 @@ def reconfigServiceWithBuildbotConfig(self, new_config):
         reconfigurable_services.sort(key=lambda svc: -svc.reconfig_priority)
 
         for svc in reconfigurable_services:
-            config_sibling = new_by_name[svc.name]
+            if not svc.name:
+                raise ValueError("%r: child %r should have a defined name attribute", self, svc)
+            config_sibling = new_by_name.get(svc.name)
             yield svc.reconfigServiceWithSibling(config_sibling)