← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/bi-dependencies-1 into lp:launchpad

 

William Grant has proposed merging lp:~wgrant/launchpad/bi-dependencies-1 into lp:launchpad.

Commit message:
Adjust various BuilderInteractor methods to take former attributes as arguments.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bi-dependencies-1/+merge/188489

This branch adjusts various BuilderInteractor methods to take the vitals, builder and slave as arguments. This is part of the push toward having SlaveScanner manage all of these potentially variable objects, and will make actual unit testing of the involved components less fugly.
-- 
https://code.launchpad.net/~wgrant/launchpad/bi-dependencies-1/+merge/188489
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bi-dependencies-1 into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py	2013-09-27 17:31:15 +0000
+++ lib/lp/buildmaster/interactor.py	2013-10-01 01:02:50 +0000
@@ -245,6 +245,15 @@
         # XXX wgrant: The BuilderVitals should be passed in.
         self.vitals = extract_vitals_from_db(builder)
 
+    @staticmethod
+    def makeSlaveFromVitals(vitals):
+        if vitals.virtualized:
+            timeout = config.builddmaster.virtualized_socket_timeout
+        else:
+            timeout = config.builddmaster.socket_timeout
+        return BuilderSlave.makeBuilderSlave(
+            vitals.url, vitals.vm_host, timeout)
+
     @property
     def slave(self):
         """See IBuilder."""
@@ -255,15 +264,16 @@
         new_slave_attrs = (
             self.vitals.url, self.vitals.vm_host, self.vitals.virtualized)
         if self._cached_slave_attrs != new_slave_attrs:
-            if self.vitals.virtualized:
-                timeout = config.builddmaster.virtualized_socket_timeout
-            else:
-                timeout = config.builddmaster.socket_timeout
-            self._cached_slave = BuilderSlave.makeBuilderSlave(
-                self.vitals.url, self.vitals.vm_host, timeout)
+            self._cached_slave = self.makeSlaveFromVitals(self.vitals)
             self._cached_slave_attrs = new_slave_attrs
         return self._cached_slave
 
+    @staticmethod
+    def getBuildBehavior(queue_item, builder, slave):
+        behavior = IBuildFarmJobBehavior(queue_item.specific_job)
+        behavior.setBuilder(builder, slave)
+        return behavior
+
     @property
     def _current_build_behavior(self):
         """Return the current build behavior."""
@@ -276,14 +286,14 @@
             self._cached_build_behavior = None
             self._cached_currentjob = None
         elif currentjob != self._cached_currentjob:
-            self._cached_build_behavior = IBuildFarmJobBehavior(
-                currentjob.specific_job)
-            self._cached_build_behavior.setBuilder(self.builder, self.slave)
+            self._cached_build_behavior = self.getBuildBehavior(
+                currentjob, self.builder, self.slave)
             self._cached_currentjob = currentjob
         return self._cached_build_behavior
 
+    @staticmethod
     @defer.inlineCallbacks
-    def slaveStatus(self):
+    def slaveStatus(slave):
         """Get the slave status for this builder.
 
         :return: A Deferred which fires when the slave dialog is complete.
@@ -291,7 +301,7 @@
             potentially other values included by the current build
             behavior.
         """
-        status_sentence = yield self.slave.status()
+        status_sentence = yield slave.status()
         status = {'builder_status': status_sentence[0]}
 
         # Extract detailed status and log information if present.
@@ -304,7 +314,8 @@
                 status['logtail'] = status_sentence[2]
         defer.returnValue((status_sentence, status))
 
-    def verifySlaveBuildCookie(self, behavior, slave_cookie):
+    @staticmethod
+    def verifySlaveBuildCookie(behavior, slave_cookie):
         """See `IBuildFarmJobBehavior`."""
         if behavior is None:
             if slave_cookie is not None:
@@ -316,8 +327,9 @@
                     "Invalid slave build cookie: got %r, expected %r."
                     % (slave_cookie, good_cookie))
 
+    @classmethod
     @defer.inlineCallbacks
-    def rescueIfLost(self, logger=None):
+    def rescueIfLost(cls, vitals, slave, behavior, logger=None):
         """Reset the slave if its job information doesn't match the DB.
 
         This checks the build ID reported in the slave status against the
@@ -343,7 +355,7 @@
         # Determine the slave's current build cookie. For BUILDING, ABORTING
         # and WAITING we extract the string from the slave status
         # sentence, and for IDLE it is None.
-        status_sentence = yield self.slave.status()
+        status_sentence = yield slave.status()
         status = status_sentence[0]
         if status not in ident_position.keys():
             slave_cookie = None
@@ -355,24 +367,24 @@
         # verifying that the slave cookie is None iff we expect the
         # slave to be idle.
         try:
-            self.verifySlaveBuildCookie(
-                self._current_build_behavior, slave_cookie)
+            cls.verifySlaveBuildCookie(behavior, slave_cookie)
             defer.returnValue(False)
         except CorruptBuildCookie as reason:
             # An IDLE slave doesn't need rescuing (SlaveScanner.scan
             # will rescue the DB side instead), and we just have to wait
             # out an ABORTING one.
             if status == 'BuilderStatus.WAITING':
-                yield self.slave.clean()
+                yield slave.clean()
             elif status == 'BuilderStatus.BUILDING':
-                yield self.slave.abort()
+                yield slave.abort()
             if logger:
                 logger.info(
                     "Builder '%s' rescued from '%s': '%s'" %
-                    (self.vitals.name, slave_cookie, reason))
+                    (vitals.name, slave_cookie, reason))
             defer.returnValue(True)
 
-    def resumeSlaveHost(self):
+    @classmethod
+    def resumeSlaveHost(cls, vitals, slave):
         """Resume the slave host to a known good condition.
 
         Issues 'builddmaster.vm_resume_command' specified in the configuration
@@ -385,16 +397,16 @@
             whose value is a (stdout, stderr) tuple for success, or a Failure
             whose value is a CannotResumeHost exception.
         """
-        if not self.vitals.virtualized:
+        if not vitals.virtualized:
             return defer.fail(CannotResumeHost('Builder is not virtualized.'))
 
-        if not self.vitals.vm_host:
+        if not vitals.vm_host:
             return defer.fail(CannotResumeHost('Undefined vm_host.'))
 
-        logger = self._getSlaveScannerLogger()
-        logger.info("Resuming %s (%s)" % (self.vitals.name, self.vitals.url))
+        logger = cls._getSlaveScannerLogger()
+        logger.info("Resuming %s (%s)" % (vitals.name, vitals.url))
 
-        d = self.slave.resume()
+        d = slave.resume()
 
         def got_resume_ok((stdout, stderr, returncode)):
             return stdout, stderr
@@ -406,8 +418,10 @@
 
         return d.addCallback(got_resume_ok).addErrback(got_resume_bad)
 
+    @classmethod
     @defer.inlineCallbacks
-    def _startBuild(self, build_queue_item, behavior, logger):
+    def _startBuild(cls, build_queue_item, vitals, builder, slave, behavior,
+                    logger):
         """Start a build on this builder.
 
         :param build_queue_item: A BuildQueueItem to build.
@@ -421,7 +435,7 @@
         behavior.verifyBuildRequest(logger)
 
         # Set the build behavior depending on the provided build queue item.
-        if not self.builder.builderok:
+        if not builder.builderok:
             raise BuildDaemonError(
                 "Attempted to start a build on a known-bad builder.")
 
@@ -433,13 +447,14 @@
         # This could be a quirk of the Xen guest, we're not sure.  We
         # also don't care about the result from this message, just that
         # it's sent, hence the "addBoth".  See bug 586359.
-        if self.builder.virtualized:
-            yield self.resumeSlaveHost()
-            yield self.slave.echo("ping")
+        if builder.virtualized:
+            yield cls.resumeSlaveHost(vitals, slave)
+            yield slave.echo("ping")
 
         yield behavior.dispatchBuildToSlave(build_queue_item.id, logger)
 
-    def resetOrFail(self, logger, exception):
+    @classmethod
+    def resetOrFail(cls, vitals, slave, builder, logger, exception):
         """Handle "confirmed" build slave failures.
 
         Call this when there have been multiple failures that are not just
@@ -458,54 +473,57 @@
             or immediately if it's a non-virtual slave.
         """
         error_message = str(exception)
-        if self.vitals.virtualized:
+        if vitals.virtualized:
             # Virtualized/PPA builder: attempt a reset, unless the failure
             # was itself a failure to reset.  (In that case, the slave
             # scanner will try again until we reach the failure threshold.)
             if not isinstance(exception, CannotResumeHost):
                 logger.warn(
                     "Resetting builder: %s -- %s" % (
-                        self.vitals.url, error_message),
+                        vitals.url, error_message),
                     exc_info=True)
-                return self.resumeSlaveHost()
+                return cls.resumeSlaveHost(vitals, slave)
         else:
             # XXX: This should really let the failure bubble up to the
             # scan() method that does the failure counting.
             # Mark builder as 'failed'.
             logger.warn(
-                "Disabling builder: %s -- %s" % (
-                    self.vitals.url, error_message))
-            self.builder.failBuilder(error_message)
+                "Disabling builder: %s -- %s" % (vitals.url, error_message))
+            builder.failBuilder(error_message)
             transaction.commit()
         return defer.succeed(None)
 
+    @classmethod
     @defer.inlineCallbacks
-    def findAndStartJob(self):
+    def findAndStartJob(cls, vitals, builder, slave):
         """Find a job to run and send it to the buildd slave.
 
         :return: A Deferred whose value is the `IBuildQueue` instance
             found or None if no job was found.
         """
-        logger = self._getSlaveScannerLogger()
+        logger = cls._getSlaveScannerLogger()
         # XXX This method should be removed in favour of two separately
         # called methods that find and dispatch the job.  It will
         # require a lot of test fixing.
-        candidate = self.builder.acquireBuildCandidate()
+        candidate = builder.acquireBuildCandidate()
         if candidate is None:
             logger.debug("No build candidates available for builder.")
             defer.returnValue(None)
 
+        new_behavior = cls.getBuildBehavior(candidate, builder, slave)
         needed_bfjb = type(removeSecurityProxy(
             IBuildFarmJobBehavior(candidate.specific_job)))
-        if not zope_isinstance(self._current_build_behavior, needed_bfjb):
+        if not zope_isinstance(new_behavior, needed_bfjb):
             raise AssertionError(
                 "Inappropriate IBuildFarmJobBehavior: %r is not a %r" %
-                (self._current_build_behavior, needed_bfjb))
-        yield self._startBuild(candidate, self._current_build_behavior, logger)
+                (new_behavior, needed_bfjb))
+        yield cls._startBuild(
+            candidate, vitals, builder, slave, new_behavior, logger)
         defer.returnValue(candidate)
 
+    @classmethod
     @defer.inlineCallbacks
-    def updateBuild(self, queueItem):
+    def updateBuild(cls, queueItem, slave, behavior):
         """Verify the current build job status.
 
         Perform the required actions for each state.
@@ -517,20 +535,22 @@
         # the DB, and this method isn't called unless the DB says
         # there's a job.
         builder_status_handlers = {
-            'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
-            'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
-            'BuilderStatus.WAITING': self.updateBuild_WAITING,
+            'BuilderStatus.BUILDING': cls.updateBuild_BUILDING,
+            'BuilderStatus.ABORTING': cls.updateBuild_ABORTING,
+            'BuilderStatus.WAITING': cls.updateBuild_WAITING,
             }
-        statuses = yield self.slaveStatus()
+        statuses = yield cls.slaveStatus(slave)
         logger = logging.getLogger('slave-scanner')
         status_sentence, status_dict = statuses
         builder_status = status_dict['builder_status']
         if builder_status not in builder_status_handlers:
             raise AssertionError("Unknown status %s" % builder_status)
         method = builder_status_handlers[builder_status]
-        yield method(queueItem, status_sentence, status_dict, logger)
+        yield method(
+            queueItem, behavior, status_sentence, status_dict, logger)
 
-    def updateBuild_BUILDING(self, queueItem, status_sentence, status_dict,
+    @staticmethod
+    def updateBuild_BUILDING(queueItem, behavior, status_sentence, status_dict,
                              logger):
         """Build still building, collect the logtail"""
         if queueItem.job.status != JobStatus.RUNNING:
@@ -538,7 +558,8 @@
         queueItem.logtail = encoding.guess(str(status_dict.get('logtail')))
         transaction.commit()
 
-    def updateBuild_ABORTING(self, queueItem, status_sentence, status_dict,
+    @staticmethod
+    def updateBuild_ABORTING(queueItem, behavior, status_sentence, status_dict,
                              logger):
         """Build was ABORTED.
 
@@ -562,8 +583,9 @@
 
         return status_string[len(lead_string):]
 
-    def updateBuild_WAITING(self, queueItem, status_sentence, status_dict,
-                            logger):
+    @classmethod
+    def updateBuild_WAITING(cls, queueItem, behavior, status_sentence,
+                            status_dict, logger):
         """Perform the actions needed for a slave in a WAITING state
 
         Buildslave can be WAITING in five situations:
@@ -577,13 +599,14 @@
           Librarian with getFileFromSlave() and then pass the binaries to
           the uploader for processing.
         """
-        self._current_build_behavior.updateSlaveStatus(
+        behavior.updateSlaveStatus(
             status_sentence, status_dict)
-        d = self._current_build_behavior.handleStatus(
-            queueItem, self.extractBuildStatus(status_dict), status_dict)
+        d = behavior.handleStatus(
+            queueItem, cls.extractBuildStatus(status_dict), status_dict)
         return d
 
-    def _getSlaveScannerLogger(self):
+    @staticmethod
+    def _getSlaveScannerLogger():
         """Return the logger instance from buildd-slave-scanner.py."""
         # XXX cprov 20071120: Ideally the Launchpad logging system
         # should be able to configure the root-logger instead of creating

=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py	2013-09-18 05:15:37 +0000
+++ lib/lp/buildmaster/manager.py	2013-10-01 01:02:50 +0000
@@ -104,7 +104,9 @@
         elif builder.failure_count % Builder.RESET_THRESHOLD == 0:
             # The builder is dead, but in the virtual case it might be worth
             # resetting it.
-            yield interactor.resetOrFail(logger, exception)
+            yield interactor.resetOrFail(
+                interactor.vitals, interactor.slave, interactor.builder,
+                logger, exception)
     else:
         # The job is the culprit!  Override its status to 'failed'
         # to make sure it won't get automatically dispatched again,
@@ -257,7 +259,9 @@
             self.date_cancel = None
             buildqueue.cancel()
             transaction.commit()
-            value = yield interactor.resetOrFail(self.logger, e)
+            value = yield interactor.resetOrFail(
+                interactor.vitals, interactor.slave, interactor.builder,
+                self.logger, e)
             # value is not None if we resumed a slave host.
             defer.returnValue(value is not None)
 
@@ -285,7 +289,9 @@
                 interactor.builder, interactor)
             if cancelled:
                 return
-            lost = yield interactor.rescueIfLost(self.logger)
+            lost = yield interactor.rescueIfLost(
+                vitals, interactor.slave, interactor._current_build_behavior,
+                self.logger)
             if lost:
                 lost_reason = '%s is lost' % vitals.name
 
@@ -308,15 +314,17 @@
         if vitals.build_queue is not None:
             # Scan the slave and get the logtail, or collect the build
             # if it's ready.  Yes, "updateBuild" is a bad name.
-            yield interactor.updateBuild(vitals.build_queue)
+            yield interactor.updateBuild(
+                vitals.build_queue, interactor.slave,
+                interactor._current_build_behavior)
         elif vitals.manual:
             # If the builder is in manual mode, don't dispatch anything.
             self.logger.debug(
                 '%s is in manual mode, not dispatching.' % vitals.name)
         else:
             # See if there is a job we can dispatch to the builder slave.
-            yield interactor.findAndStartJob()
             builder = self.builders_cache[self.builder_name]
+            yield interactor.findAndStartJob(vitals, builder, interactor.slave)
             if builder.currentjob is not None:
                 # After a successful dispatch we can reset the
                 # failure_count.

=== modified file 'lib/lp/buildmaster/tests/mock_slaves.py'
--- lib/lp/buildmaster/tests/mock_slaves.py	2013-09-02 12:45:50 +0000
+++ lib/lp/buildmaster/tests/mock_slaves.py	2013-10-01 01:02:50 +0000
@@ -47,11 +47,11 @@
     """Emulates a IBuilder class."""
 
     def __init__(self, name='mock-builder', builderok=True, manual=False,
-                 virtualized=True, vm_host=None):
+                 virtualized=True, vm_host=None, url='http://fake:0000'):
         self.currentjob = None
         self.builderok = builderok
         self.manual = manual
-        self.url = 'http://fake:0000'
+        self.url = url
         self.name = name
         self.virtualized = virtualized
         self.vm_host = vm_host

=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
--- lib/lp/buildmaster/tests/test_interactor.py	2013-09-27 17:31:15 +0000
+++ lib/lp/buildmaster/tests/test_interactor.py	2013-10-01 01:02:50 +0000
@@ -25,6 +25,7 @@
 from lp.buildmaster.interactor import (
     BuilderInteractor,
     BuilderSlave,
+    extract_vitals_from_db,
     )
 from lp.buildmaster.interfaces.builder import (
     CannotFetchFile,
@@ -102,8 +103,8 @@
         # A slave that's 'lost' should be aborted; when the slave is
         # broken then abort() should also throw a fault.
         slave = LostBuildingBrokenSlave()
-        interactor = BuilderInteractor(MockBuilder(), slave, TrivialBehavior())
-        d = interactor.rescueIfLost(DevNullLogger())
+        d = BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), slave, TrivialBehavior())
 
         def check_slave_status(failure):
             self.assertIn('abort', slave.call_log)
@@ -113,29 +114,32 @@
 
         return d.addBoth(check_slave_status)
 
+    def resumeSlaveHost(self, builder):
+        vitals = extract_vitals_from_db(builder)
+        return BuilderInteractor.resumeSlaveHost(
+            vitals, BuilderInteractor.makeSlaveFromVitals(vitals))
+
     def test_resumeSlaveHost_nonvirtual(self):
-        builder = MockBuilder(virtualized=False)
-        d = BuilderInteractor(builder).resumeSlaveHost()
+        d = self.resumeSlaveHost(MockBuilder(virtualized=False))
         return assert_fails_with(d, CannotResumeHost)
 
     def test_resumeSlaveHost_no_vmhost(self):
-        builder = MockBuilder(virtualized=True, vm_host=None)
-        d = BuilderInteractor(builder).resumeSlaveHost()
+        d = self.resumeSlaveHost(MockBuilder(virtualized=False, vm_host=None))
         return assert_fails_with(d, CannotResumeHost)
 
     def test_resumeSlaveHost_success(self):
         reset_config = """
             [builddmaster]
-            vm_resume_command: /bin/echo -n parp"""
+            vm_resume_command: /bin/echo -n snap %(buildd_name)s %(vm_host)s
+            """
         config.push('reset', reset_config)
         self.addCleanup(config.pop, 'reset')
 
-        builder = MockBuilder(virtualized=True, vm_host="pop")
-        d = BuilderInteractor(builder).resumeSlaveHost()
+        d = self.resumeSlaveHost(MockBuilder(
+            url="http://crackle.ppa/";, virtualized=True, vm_host="pop"))
 
         def got_resume(output):
-            self.assertEqual(('parp', ''), output)
-
+            self.assertEqual(('snap crackle pop', ''), output)
         return d.addCallback(got_resume)
 
     def test_resumeSlaveHost_command_failed(self):
@@ -144,8 +148,7 @@
             vm_resume_command: /bin/false"""
         config.push('reset fail', reset_fail_config)
         self.addCleanup(config.pop, 'reset fail')
-        builder = MockBuilder(virtualized=True, vm_host="pop")
-        d = BuilderInteractor(builder).resumeSlaveHost()
+        d = self.resumeSlaveHost(MockBuilder(virtualized=True, vm_host="pop"))
         return assert_fails_with(d, CannotResumeHost)
 
     def test_resetOrFail_resume_failure(self):
@@ -155,7 +158,9 @@
         config.push('reset fail', reset_fail_config)
         self.addCleanup(config.pop, 'reset fail')
         builder = MockBuilder(virtualized=True, vm_host="pop", builderok=True)
-        d = BuilderInteractor(builder).resetOrFail(
+        vitals = extract_vitals_from_db(builder)
+        d = BuilderInteractor.resetOrFail(
+            vitals, BuilderInteractor.makeSlaveFromVitals(vitals), builder,
             DevNullLogger(), Exception())
         return assert_fails_with(d, CannotResumeHost)
 
@@ -185,8 +190,8 @@
         # idle. SlaveScanner.scan() will clean up the DB side, because
         # we still report that it's lost.
         slave = OkSlave()
-        lost = yield BuilderInteractor(
-            MockBuilder(), slave, TrivialBehavior()).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), slave, TrivialBehavior())
         self.assertTrue(lost)
         self.assertEqual([], slave.call_log)
 
@@ -194,8 +199,8 @@
     def test_recover_ok_slave(self):
         # An idle slave that's meant to be idle is not rescued.
         slave = OkSlave()
-        lost = yield BuilderInteractor(
-            MockBuilder(), slave, None).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), slave, None)
         self.assertFalse(lost)
         self.assertEqual([], slave.call_log)
 
@@ -204,8 +209,9 @@
         # rescueIfLost does not attempt to abort or clean a builder that is
         # WAITING.
         waiting_slave = WaitingSlave(build_id='trivial')
-        lost = yield BuilderInteractor(
-            MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), waiting_slave,
+            TrivialBehavior())
         self.assertFalse(lost)
         self.assertEqual(['status'], waiting_slave.call_log)
 
@@ -217,8 +223,9 @@
         # builder is reset for a new build, and the corrupt build is
         # discarded.
         waiting_slave = WaitingSlave(build_id='non-trivial')
-        lost = yield BuilderInteractor(
-            MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), waiting_slave,
+            TrivialBehavior())
         self.assertTrue(lost)
         self.assertEqual(['status', 'clean'], waiting_slave.call_log)
 
@@ -227,8 +234,9 @@
         # rescueIfLost does not attempt to abort or clean a builder that is
         # BUILDING.
         building_slave = BuildingSlave(build_id='trivial')
-        lost = yield BuilderInteractor(
-            MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), building_slave,
+            TrivialBehavior())
         self.assertFalse(lost)
         self.assertEqual(['status'], building_slave.call_log)
 
@@ -237,8 +245,9 @@
         # If a slave is BUILDING with a build id we don't recognize, then we
         # abort the build, thus stopping it in its tracks.
         building_slave = BuildingSlave(build_id='non-trivial')
-        lost = yield BuilderInteractor(
-            MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()
+        lost = yield BuilderInteractor.rescueIfLost(
+            extract_vitals_from_db(MockBuilder()), building_slave,
+            TrivialBehavior())
         self.assertTrue(lost)
         self.assertEqual(['status', 'abort'], building_slave.call_log)
 
@@ -253,7 +262,7 @@
     def assertStatus(self, slave, builder_status=None,
                      build_status=None, logtail=False, filemap=None,
                      dependencies=None):
-        statuses = yield BuilderInteractor(MockBuilder(), slave).slaveStatus()
+        statuses = yield BuilderInteractor.slaveStatus(slave)
         status_dict = statuses[1]
 
         expected = {}
@@ -354,7 +363,8 @@
         # findAndStartJob delegates to it.
         removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
             result=candidate)
-        d = BuilderInteractor(builder).findAndStartJob()
+        vitals = extract_vitals_from_db(builder)
+        d = BuilderInteractor.findAndStartJob(vitals, builder, OkSlave())
         return d.addCallback(self.assertEqual, candidate)
 
     def test_findAndStartJob_starts_job(self):
@@ -365,7 +375,8 @@
         candidate = build.queueBuild()
         removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
             result=candidate)
-        d = BuilderInteractor(builder).findAndStartJob()
+        vitals = extract_vitals_from_db(builder)
+        d = BuilderInteractor.findAndStartJob(vitals, builder, OkSlave())
 
         def check_build_started(candidate):
             self.assertEqual(candidate.builder, builder)
@@ -380,11 +391,12 @@
         candidate = build.queueBuild()
         removeSecurityProxy(builder)._findBuildCandidate = FakeMethod(
             result=candidate)
-        interactor = BuilderInteractor(builder)
-        d = interactor.findAndStartJob()
+        vitals = extract_vitals_from_db(builder)
+        slave = OkSlave()
+        d = BuilderInteractor.findAndStartJob(vitals, builder, slave)
 
         def check_build_started(candidate):
-            self.assertIn(('echo', 'ping'), interactor.slave.call_log)
+            self.assertIn(('echo', 'ping'), slave.call_log)
 
         return d.addCallback(check_build_started)
 
@@ -402,10 +414,14 @@
 
         # At this point we should see a valid behaviour on the builder:
         self.assertIsNot(None, interactor._current_build_behavior)
-        yield interactor.rescueIfLost()
+        yield BuilderInteractor.rescueIfLost(
+            interactor.vitals, building_slave,
+            interactor._current_build_behavior)
         self.assertIsNot(None, interactor._current_build_behavior)
         candidate.destroySelf()
-        yield interactor.rescueIfLost()
+        yield BuilderInteractor.rescueIfLost(
+            interactor.vitals, building_slave,
+            interactor._current_build_behavior)
         self.assertIs(None, interactor._current_build_behavior)
 
 

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py'
--- lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py	2013-09-18 06:38:31 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py	2013-10-01 01:02:50 +0000
@@ -152,7 +152,8 @@
         bq.markAsBuilding(builder)
         interactor = BuilderInteractor(builder, slave)
         d = interactor._startBuild(
-            bq, interactor._current_build_behavior, BufferLogger())
+            bq, interactor.vitals, interactor.builder, interactor.slave,
+            interactor._current_build_behavior, BufferLogger())
         d.addCallback(
             self.assertExpectedInteraction, slave.call_log, interactor, build,
             lf, archive, ArchivePurpose.PRIMARY, 'universe')
@@ -174,7 +175,8 @@
         bq.markAsBuilding(builder)
         interactor = BuilderInteractor(builder, slave)
         d = interactor._startBuild(
-            bq, interactor._current_build_behavior, BufferLogger())
+            bq, interactor.vitals, interactor.builder, interactor.slave,
+            interactor._current_build_behavior, BufferLogger())
 
         def check_build(ignored):
             # We expect the first call to the slave to be a resume call,
@@ -200,7 +202,8 @@
         bq.markAsBuilding(builder)
         interactor = BuilderInteractor(builder, slave)
         d = interactor._startBuild(
-            bq, interactor._current_build_behavior, BufferLogger())
+            bq, interactor.vitals, interactor.builder, interactor.slave,
+            interactor._current_build_behavior, BufferLogger())
         d.addCallback(
             self.assertExpectedInteraction, slave.call_log, interactor, build,
             lf, archive, ArchivePurpose.PARTNER)
@@ -331,6 +334,11 @@
         # hang around between test runs.
         self.addCleanup(self._cleanup)
 
+    def updateBuild(self, candidate):
+        return self.interactor.updateBuild(
+            candidate, self.interactor.slave,
+            self.interactor._current_build_behavior)
+
     def assertBuildProperties(self, build):
         """Check that a build happened by making sure some of its properties
         are set."""
@@ -350,7 +358,7 @@
             self.assertBuildProperties(self.build)
             self.assertEqual(BuildStatus.FAILEDTOBUILD, self.build.status)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_depwait_collection(self):
@@ -365,7 +373,7 @@
             self.assertEqual(BuildStatus.MANUALDEPWAIT, self.build.status)
             self.assertEqual(DEPENDENCIES, self.build.dependencies)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_chrootfail_collection(self):
@@ -378,7 +386,7 @@
             self.assertBuildProperties(self.build)
             self.assertEqual(BuildStatus.CHROOTWAIT, self.build.status)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_builderfail_collection(self):
@@ -396,7 +404,7 @@
             job = self.candidate.specific_job.job
             self.assertEqual(JobStatus.WAITING, job.status)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_building_collection(self):
@@ -408,7 +416,7 @@
             # The fake log is returned from the BuildingSlave() mock.
             self.assertEqual("This is a build log", self.candidate.logtail)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_aborting_collection(self):
@@ -421,7 +429,7 @@
                 "Waiting for slave process to be terminated",
                 self.candidate.logtail)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_collection_for_deleted_source(self):
@@ -438,7 +446,7 @@
             self.assertEqual(
                 BuildStatus.SUPERSEDED, self.build.status)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_uploading_collection(self):
@@ -452,7 +460,7 @@
             # upload processing succeeded.
             self.assertIs(None, self.build.upload_log)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_givenback_collection(self):
@@ -469,7 +477,7 @@
             job = self.candidate.specific_job.job
             self.assertEqual(JobStatus.WAITING, job.status)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
     def test_log_file_collection(self):
@@ -540,7 +548,7 @@
             self.layer.txn.commit()
             self.assertTrue(self.build.log.restricted)
 
-        d = self.interactor.updateBuild(self.candidate)
+        d = self.updateBuild(self.candidate)
         return d.addCallback(got_update)
 
 


Follow ups