service.py: bugfix and adding objectid

When integrate the scheduler_manager into
BuildbotServiceManager the unit test
identified several issues
- objectid, that was previously set
  was not set in BuildbotServiceManager
- some 'yield'-s were missing
- reconfigServiceWithSibling was called
  twice (one for nothing) for services
  that just got added

This commit fixes these issues and factorizes
the some mocks needed for unit tests.

master/buildbot/schedulers/base.py

@@ -320,9 +320,3 @@ def addBuildsetForSourceStamps(self, waited_for=False, sourcestamps=None,
             waited_for=waited_for, properties=properties_dict, builderids=builderids,
             external_idstring=external_idstring, **kw)
         defer.returnValue((bsid, brids))
-
-    @defer.inlineCallbacks
-    def reconfigService(self, *args, **kwargs):
-        if self.running:
-            yield self.stopService()
-        yield self.startService()

master/buildbot/test/fake/fakemaster.py

@@ -14,6 +14,7 @@
 # Copyright Buildbot Team Members
 
 import os
+import mock
 import weakref
 
 from buildbot import config
@@ -166,6 +167,7 @@ class FakeMaster(service.MasterService):
     def __init__(self, master_id=fakedb.FakeBuildRequestsComponent.MASTER_ID):
         service.MasterService.__init__(self)
         self._master_id = master_id
+        self.objectids = []
         self.config = config.MasterConfig()
         self.caches = FakeCaches()
         self.pbmanager = pbmanager.FakePBManager()
@@ -178,6 +180,18 @@ def __init__(self, master_id=fakedb.FakeBuildRequestsComponent.MASTER_ID):
         self.masterid = master_id
         self.buildslaves = bslavemanager.FakeBuildslaveManager(self)
         self.log_rotation = FakeLogRotation()
+        self.db = mock.Mock()
+        self.next_objectid = 0
+
+        def getObjectId(sched_name, class_name):
+            k = (sched_name, class_name)
+            try:
+                rv = self.objectids[k]
+            except KeyError:
+                rv = self.objectids[k] = self.next_objectid
+                self.next_objectid += 1
+            return defer.succeed(rv)
+        self.db.state.getObjectId = getObjectId
 
     def getObjectId(self):
         return defer.succeed(self._master_id)

master/buildbot/test/unit/test_buildslave_manager.py

@@ -51,6 +51,7 @@ def setUp(self):
                                              wantMq=True, wantData=True)
         self.master.mq = self.master.mq
         self.buildslaves = bslavemanager.BuildslaveManager(self.master)
+        self.buildslaves.setServiceParent(self.master)
         # slaves expect a botmaster as well as a manager.
         self.botmaster = botmaster.BotMaster(self.master)
         self.master.botmaster = self.botmaster

master/buildbot/test/unit/test_schedulers_manager.py

@@ -146,27 +146,3 @@ def test_reconfigService_class_name_change(self):
         # instance
         self.assertIdentical(sch1_new.parent, self.sm)
         self.assertIdentical(sch1_new.master, self.master)
-
-    @defer.inlineCallbacks
-    def test_reconfigService_add_and_change_and_remove_no_reconfig(self):
-        sch1 = self.makeSched(self.Sched, 'sch1', attr='alpha')
-        self.new_config.schedulers = dict(sch1=sch1)
-
-        yield self.sm.reconfigServiceWithBuildbotConfig(self.new_config)
-
-        self.assertIdentical(sch1.parent, self.sm)
-        self.assertIdentical(sch1.master, self.master)
-
-        sch1_new = self.makeSched(self.Sched, 'sch1', attr='beta')
-        sch2 = self.makeSched(self.Sched, 'sch2', attr='alpha')
-        self.new_config.schedulers = dict(sch1=sch1_new, sch2=sch2)
-
-        yield self.sm.reconfigServiceWithBuildbotConfig(self.new_config)
-
-        # sch1 is not longer active, and sch1_new is
-        self.assertIdentical(sch1.parent, None)
-        self.assertIdentical(sch1.master, None)
-        self.assertIdentical(sch1_new.parent, self.sm)
-        self.assertIdentical(sch1_new.master, self.master)
-        self.assertIdentical(sch2.parent, self.sm)
-        self.assertIdentical(sch2.master, self.master)

master/buildbot/test/unit/test_schedulers_trysched.py

@@ -114,7 +114,7 @@ def setup_test_startService(self, jobdir, exp_jobdir):
     @defer.inlineCallbacks
     def do_test_startService(self):
         # start it
-        self.sched.startService()
+        yield self.sched.startService()
 
         # check that it has set the basedir correctly
         self.assertEqual(self.sched.watcher.basedir, self.jobdir)

master/buildbot/test/unit/test_util_service.py

@@ -553,10 +553,15 @@ class fakeMaster(service.MasterService, service.ReconfigurableServiceMixin):
     pass
 
 
+def makeFakeMaster():
+    m = fakeMaster()
+    m.db = mock.Mock()
+    return m
+
 class BuildbotService(unittest.TestCase):
 
     def setUp(self):
-        self.master = fakeMaster()
+        self.master = makeFakeMaster()
 
     @defer.inlineCallbacks
     def prepareService(self):
@@ -591,7 +596,7 @@ def testChecksDone(self):
 class BuildbotServiceManager(unittest.TestCase):
 
     def setUp(self):
-        self.master = fakeMaster()
+        self.master = makeFakeMaster()
 
     @defer.inlineCallbacks
     def prepareService(self):

master/buildbot/util/service.py

@@ -373,11 +373,22 @@ def reconfigServiceWithBuildbotConfig(self, new_config):
 
             for n in added_names:
                 child = new_by_name[n]
-                child.setServiceParent(self)
-
-        # get a list of child services to reconfigure
-        reconfigurable_services = [svc for svc in self]
-
+                # setup service's objectid
+                class_name = '%s.%s' % (child.__class__.__module__,
+                                        child.__class__.__name__)
+                objectid = yield self.master.db.state.getObjectId(
+                    child.name, class_name)
+                child.objectid = objectid
+                yield defer.maybeDeferred(child.setServiceParent, self)
+
+        # As the services that were just added got
+        # reconfigServiceWithSibling called by
+        # setServiceParent->startService,
+        # we avoid calling it again by selecting
+        # in reconfigurable_services, services
+        # that were not added just now
+        reconfigurable_services = [svc for svc in self
+                                   if svc.name not in added_names]
         # sort by priority
         reconfigurable_services.sort(key=lambda svc: -svc.reconfig_priority)