Merge lp:~wgrant/launchpad/move-rescueiflost-tests into lp:launchpad

[William Grant]

Move the rescueIfLost tests out of buildd-slavescanner.txt into builder.txt, and simplify them considerably.

This branch moves buildd-slavescanner.txt's long-winded rescueIfLost tests to builder.txt, and refactors them to make them rather more succinct and less ugly.

It replaces a tonne of mock slaves with a couple of configurable ones, and fixes a couple of other tests around the tree that used the condemned mocks. It also replaces with a test of IdleBuildBehavior.verifySlaveBuildID the two tests that confirmed rescue of a BUILDING or WAITING slave marked as idle in the DB.

I also rewrote the start of builder.txt, since its style offended me.

[Brad Crittenden (bac)]

Hi William,

This branch looks great. As we discussed on IRC please run it past a Soyuz team member and I'll approve it.

> === modified file 'lib/lp/buildmaster/doc/builder.txt'
> --- lib/lp/buildmaster/doc/builder.txt 2010-03-24 10:24:22 +0000
> +++ lib/lp/buildmaster/doc/builder.txt 2010-04-03 04:39:18 +0000
> @@ -2,36 +2,34 @@
> Builder Class
> =============
>
> -This test aims to meet the requirements of
> -<https://launchpad.canonical.com/BasicTestCoverage> for the Builder class,
> -which represents the Buildd Slave entity.
> -
> -Need auxiliar methods from zope toolchain:
> +The Builder class represents a slave machine in the build farm. These
> +slaves are used to execute untrusted code -- for example when building
> +packages.
> +
> +There are several builders in the sample data. Let's examine the first.
> +
> + >>> from lp.buildmaster.model.builder import Builder
> + >>> builder = Builder.get(1)
> +
> +As expected, it implements IBuilder.

Nice changes.

> +Builders can take on different behaviors depending on the type of build
> +they are currently processing. Each builder provides an attribute
> +(current_build_behavior) to which all the build-type specific behavior
> +for the current build is delegated. In the sample data, bob's current
> +behavior is the behavior for dealing with binary packages

How about:
"behavior is dealing with binary packages."
(Note full-stop.)

> >>> from zope.security.proxy import isinstance
> >>> from lp.soyuz.model.binarypackagebuildbehavior import (
> @@ -40,85 +38,77 @@
> ... builder.current_build_behavior, BinaryPackageBuildBehavior)
> True
>
> -Confirm we can get the slave xmlrpc interface
> +A builder has an XML-RPC proxy in the 'slave' attribute, which allows
> +us to easily call methods on the slave machines.
>
> >>> s = builder.slave
>
> -Confirm that the urlbase is correct in that slave. (If the protocol changes,
> -this may change too)
> +The base URL of the proxy matches the builder's URL.
>
> >>> s.urlbase == builder.url
> True
>
> -Check if the instance corresponds to the declared interface:
> -
> - >>> verifyObject(IBuilder, builder)
> - True
> -
>
> BuilderSet
> ==========
>
> -Now perform the tests for the Builder ContentSet class, BuilderSet.
> -
> -Check if it can be imported:
> -
> +Builders and groups thereof are managed through a utility, IBuilderSet.
> +
> + >>> from zope.component import getUtility
> >>> from lp.buildmaster.interfaces.builder import IBuilderSet
> -
> -Check we can use the set as a utility:
> -
> >>> builderset = getUtility(IBuilderSet)
> -
> -Check if the instance returned as utility corresponds to its
> -respective interface:
> -
> >>> verifyObject(IBuilderSet, builderset)
> True
>
> -Check if the instance is iterable:
> +Iterating over a BuilderSet yields all registered builders.
>
> >>> for b in builderset:
> - ... b.id
> - 1
> + ... print b.name
> + bob
> + frog
> +
> +count() return the number of builder instance we have stored:

typo: instances

> +
> + >>> builderset.count()
> 2
>
> -Check if the __getitem__ method:
> -
...

review: Needs Information

[William Grant (wgrant)]

On Tue, 2010-04-06 at 15:51 +0000, Brad Crittenden wrote:
> Review: Needs Information
> Hi William,
>
> This branch looks great. As we discussed on IRC please run it past a Soyuz team member and I'll approve it.

Thanks for the review, Brad.

> > === modified file 'lib/lp/buildmaster/doc/builder.txt'
> > --- lib/lp/buildmaster/doc/builder.txt 2010-03-24 10:24:22 +0000
> > +++ lib/lp/buildmaster/doc/builder.txt 2010-04-03 04:39:18 +0000
> > @@ -2,36 +2,34 @@
> > Builder Class
> > =============
> >
> > -This test aims to meet the requirements of
> > -<https://launchpad.canonical.com/BasicTestCoverage> for the Builder class,
> > -which represents the Buildd Slave entity.
> > -
> > -Need auxiliar methods from zope toolchain:
> > +The Builder class represents a slave machine in the build farm. These
> > +slaves are used to execute untrusted code -- for example when building
> > +packages.
> > +
> > +There are several builders in the sample data. Let's examine the first.
> > +
> > + >>> from lp.buildmaster.model.builder import Builder
> > + >>> builder = Builder.get(1)
> > +
> > +As expected, it implements IBuilder.
>
> Nice changes.
>
> > +Builders can take on different behaviors depending on the type of build
> > +they are currently processing. Each builder provides an attribute
> > +(current_build_behavior) to which all the build-type specific behavior
> > +for the current build is delegated. In the sample data, bob's current
> > +behavior is the behavior for dealing with binary packages
>
> How about:
> "behavior is dealing with binary packages."
> (Note full-stop.)
>
> > >>> from zope.security.proxy import isinstance
> > >>> from lp.soyuz.model.binarypackagebuildbehavior import (
> > @@ -40,85 +38,77 @@
> > ... builder.current_build_behavior, BinaryPackageBuildBehavior)
> > True
> >
> > -Confirm we can get the slave xmlrpc interface
> > +A builder has an XML-RPC proxy in the 'slave' attribute, which allows
> > +us to easily call methods on the slave machines.
> >
> > >>> s = builder.slave
> >
> > -Confirm that the urlbase is correct in that slave. (If the protocol changes,
> > -this may change too)
> > +The base URL of the proxy matches the builder's URL.
> >
> > >>> s.urlbase == builder.url
> > True
> >
> > -Check if the instance corresponds to the declared interface:
> > -
> > - >>> verifyObject(IBuilder, builder)
> > - True
> > -
> >
> > BuilderSet
> > ==========
> >
> > -Now perform the tests for the Builder ContentSet class, BuilderSet.
> > -
> > -Check if it can be imported:
> > -
> > +Builders and groups thereof are managed through a utility, IBuilderSet.
> > +
> > + >>> from zope.component import getUtility
> > >>> from lp.buildmaster.interfaces.builder import IBuilderSet
> > -
> > -Check we can use the set as a utility:
> > -
> > >>> builderset = getUtility(IBuilderSet)
> > -
> > -Check if the instance returned as utility corresponds to its
> > -respective interface:
> > -
> > >>> verifyObject(IBuilderSet, builderset)
> > True
> >
> > -Check if the instance is iterable:
> > +Iterating over a BuilderSet yields all registered builders.
...

[Michael Nelson (michael.nelson)]

I *think* Jelmer is actually working on refactoring a lot of buildd-slavescanner into unit-tests (but I can't see the branch), so it'd be worthwhile chatting with him about it (he might just handle the conflicts). Other than that, I'm all for it :)

review: Approve (code)

=== modified file 'lib/lp/buildmaster/doc/builder.txt'
--- lib/lp/buildmaster/doc/builder.txt	2010-03-24 10:24:22 +0000
+++ lib/lp/buildmaster/doc/builder.txt	2010-04-09 00:40:18 +0000
@@ -2,36 +2,34 @@
 Builder Class
 =============
 
-This test aims to meet the requirements of
-<https://launchpad.canonical.com/BasicTestCoverage> for the Builder class,
-which represents the Buildd Slave entity.
-
-Need auxiliar methods from zope toolchain:
+The Builder class represents a slave machine in the build farm. These
+slaves are used to execute untrusted code -- for example when building
+packages.
+
+There are several builders in the sample data. Let's examine the first.
+
+    >>> from lp.buildmaster.model.builder import Builder
+    >>> builder = Builder.get(1)
+
+As expected, it implements IBuilder.
 
     >>> from canonical.launchpad.webapp.testing import verifyObject
-    >>> from zope.component import getUtility
-
-Importing Builder content class and its interface:
-
-    >>> from lp.buildmaster.model.builder import Builder
     >>> from lp.buildmaster.interfaces.builder import IBuilder
-
-Get an instance of Builder from the current sampledata:
-
-    >>> builder = Builder.get(1)
-
-Test some attributes:
+    >>> verifyObject(IBuilder, builder)
+    True
 
     >>> print builder.name
     bob
-
-    >>> builder.builderok = True
-    >>> builder.failnotes = None
-
-A builder provides a current_build_behavior attribute to which all the
-build-type specific behavior for the current build is delegated. For
-our sample data, the behavior is specifically a behavior for dealing
-with binary packages
+    >>> print builder.builderok
+    True
+    >>> print builder.failnotes
+    None
+
+Builders can take on different behaviors depending on the type of build
+they are currently processing. Each builder provides an attribute
+(current_build_behavior) to which all the build-type specific behavior
+for the current build is delegated. In the sample data, bob's current
+behavior is dealing with binary packages.
 
     >>> from zope.security.proxy import isinstance
     >>> from lp.soyuz.model.binarypackagebuildbehavior import (
@@ -40,86 +38,78 @@
     ...     builder.current_build_behavior, BinaryPackageBuildBehavior)
     True
 
-Confirm we can get the slave xmlrpc interface
+A builder has an XML-RPC proxy in the 'slave' attribute, which allows
+us to easily call methods on the slave machines.
 
     >>> s = builder.slave
 
-Confirm that the urlbase is correct in that slave. (If the protocol changes,
-this may change too)
+The base URL of the proxy matches the builder's URL.
 
     >>> s.urlbase == builder.url
     True
 
-Check if the instance corresponds to the declared interface:
-
-    >>> verifyObject(IBuilder, builder)
-    True
-
 
 BuilderSet
 ==========
 
-Now perform the tests for the Builder ContentSet class, BuilderSet.
-
-Check if it can be imported:
-
+Builders and groups thereof are managed through a utility, IBuilderSet.
+
+    >>> from zope.component import getUtility
     >>> from lp.buildmaster.interfaces.builder import IBuilderSet
-
-Check we can use the set as a utility:
-
     >>> builderset = getUtility(IBuilderSet)
-
-Check if the instance returned as utility corresponds to its
-respective interface:
-
     >>> verifyObject(IBuilderSet, builderset)
     True
 
-Check if the instance is iterable:
+Iterating over a BuilderSet yields all registered builders.
 
     >>> for b in builderset:
-    ...     b.id
-    1
+    ...     print b.name
+    bob
+    frog
+
+count() return the number of builders registered:
+
+    >>> builderset.count()
     2
 
-Check if the __getitem__ method:
-
-    >>> builderset['bob'].name
-    u'bob'
-
-Check now the specific method in the utility as new():
+Builders can be retrieved by name.
+
+    >>> print builderset['bob'].name
+    bob
+    >>> print builderset['bad']
+    None
+
+And also by ID.
+
+    >>> print builderset.get(2).name
+    frog
+    >>> print builderset.get(100).name
+    Traceback (most recent call last):
+    ...
+    SQLObjectNotFound: Object not found
+
+The 'new' method will create a new builder in the database.
 
     >>> bnew = builderset.new(1, 'http://dummy.com:8221/', 'dummy',
     ...	                   'Dummy Title', 'eh ?', 1)
     >>> bnew.name
     u'dummy'
 
-Check get() which returns a correspondent Builder instance to a given id:
-
-    >>> builderset.get(bnew.id).name
-    u'dummy'
-
-Or raises an SQLObjectNotFound exception:
-
-    >>> builderset.get(100)
-    Traceback (most recent call last):
-    ...
-    SQLObjectNotFound: Object not found
-
-count() return the number of builder instance we have stored:
-
-    >>> builderset.count()
-    3
-
-getBuilder() method returns all the builders available. It seems the
-same than the own instance but we have plans to turn it aware of some
-attributes of builder instance as: builderok and trust.
-
-    >>> for b in builderset.getBuilders():
-    ...     b.name
-    u'bob'
-    u'dummy'
-    u'frog'
+'getBuilders' returns builders with the 'active' flag set, ordered by
+virtualization status, architecture, then name.
+
+    >>> for b in builderset.getBuilders():
+    ...     print b.name
+    bob
+    dummy
+    frog
+    >>> login('[e-mail]')
+    >>> bnew.active = False
+    >>> login(ANONYMOUS)
+    >>> for b in builderset.getBuilders():
+    ...     print b.name
+    bob
+    frog
 
 'getBuildQueueSizeForProcessor' returns the number of pending builds
 for a given Processor. The callsites can also control which build-farm
@@ -246,24 +236,76 @@
 A fictitious i386 variant is rejected, since there are no DASes with that
 tag.
 
-   >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
-   >>> bob.setSlaveForTesting(OkSlave('i387'))
-   >>> bob.checkSlaveArchitecture()
-   Traceback (most recent call last):
-   ...
-   BuildDaemonError: Bad slave architecture tag: i387 (registered family: x86)
+    >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
+    >>> bob.setSlaveForTesting(OkSlave('i387'))
+    >>> bob.checkSlaveArchitecture()
+    Traceback (most recent call last):
+    ...
+    BuildDaemonError: Bad slave architecture tag: i387 (registered family: x86)
 
 hppa isn't in the x86 family, so it too is rejected.
 
-   >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
-   >>> bob.setSlaveForTesting(OkSlave('hppa'))
-   >>> bob.checkSlaveArchitecture()
-   Traceback (most recent call last):
-   ...
-   BuildDaemonError: Bad slave architecture tag: hppa (registered family: x86)
+    >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
+    >>> bob.setSlaveForTesting(OkSlave('hppa'))
+    >>> bob.checkSlaveArchitecture()
+    Traceback (most recent call last):
+    ...
+    BuildDaemonError: Bad slave architecture tag: hppa (registered family: x86)
 
 But i386, a real x86 variant, passes without objection.
 
-   >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
-   >>> bob.setSlaveForTesting(OkSlave('i386'))
-   >>> bob.checkSlaveArchitecture()
+    >>> from lp.soyuz.tests.soyuzbuilddhelpers import OkSlave
+    >>> bob.setSlaveForTesting(OkSlave('i386'))
+    >>> bob.checkSlaveArchitecture()
+
+
+Rescuing lost slaves
+====================
+
+Builder.rescueIfLost() checks the build ID reported in the slave status
+against the database. If it isn't building what we think it should be,
+the current build will be aborted and the slave cleaned in preparation
+for a new task. The decision about the slave's correctness is left up
+to IBuildFarmJobBehavior.verifySlaveBuildID -- for these examples we
+will use a special behavior that just checks if the slave ID is 'good'.
+
+    >>> import logging
+    >>> from lp.buildmaster.interfaces.builder import CorruptBuildID
+    >>> from lp.soyuz.tests.soyuzbuilddhelpers import (
+    ...    BuildingSlave, MockBuilder, OkSlave, WaitingSlave)
+
+    >>> class TestBuildBehavior:
+    ...     def verifySlaveBuildID(self, build_id):
+    ...         if build_id != 'good':
+    ...             raise CorruptBuildID('Bad value')
+
+    >>> def rescue_slave_if_lost(slave):
+    ...     builder = MockBuilder('mock', slave, TestBuildBehavior())
+    ...     builder.rescueIfLost(logging.getLogger())
+
+An idle slave is not rescued.
+
+    >>> rescue_slave_if_lost(OkSlave())
+
+Slaves building or having built the correct build are not rescued
+either.
+
+    >>> rescue_slave_if_lost(BuildingSlave(build_id='good'))
+    >>> rescue_slave_if_lost(WaitingSlave(build_id='good'))
+
+But if a slave is building the wrong ID, it is declared lost and
+an abort is attempted. MockSlave prints out a message when it is aborted
+or cleaned.
+
+    >>> rescue_slave_if_lost(BuildingSlave(build_id='bad'))
+    Aborting slave
+    WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'
+
+Slaves having completed an incorrect build are also declared lost,
+but there's no need to abort a completed build. Such builders are
+instead simply cleaned, ready for the next build.
+
+    >>> rescue_slave_if_lost(WaitingSlave(build_id='bad'))
+    Cleaning slave 
+    WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'
+

=== modified file 'lib/lp/buildmaster/doc/buildfarmjobbehavior.txt'
--- lib/lp/buildmaster/doc/buildfarmjobbehavior.txt	2010-01-13 20:19:43 +0000
+++ lib/lp/buildmaster/doc/buildfarmjobbehavior.txt	2010-04-09 00:40:18 +0000
@@ -111,3 +111,11 @@
      ...
     BuildBehaviorMismatch: Builder was idle when asked to log the start of a
     build.
+
+If a slave is working on a job while we think it is idle, it will always be
+aborted.
+
+    >>> bob.current_build_behavior.verifySlaveBuildID('foo')
+    Traceback (most recent call last):
+    ...
+    CorruptBuildID: No job assigned to builder

=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py	2010-04-08 04:00:30 +0000
+++ lib/lp/buildmaster/tests/test_manager.py	2010-04-09 00:40:18 +0000
@@ -31,7 +31,7 @@
 from lp.buildmaster.tests.harness import BuilddManagerTestSetup
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.soyuz.interfaces.build import IBuildSet
-from lp.soyuz.tests.soyuzbuilddhelpers import SaneBuildingSlave
+from lp.soyuz.tests.soyuzbuilddhelpers import BuildingSlave
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 
 
@@ -671,7 +671,7 @@
         self.assertTrue(builder.builderok)
 
         job = getUtility(IBuildQueueSet).get(job.id)
-        self.assertBuildingJob(job, builder, logtail='Doing something ...')
+        self.assertBuildingJob(job, builder, logtail='This is a build log')
 
     def testScanUpdatesBuildingJobs(self):
         # The job assigned to a broken builder is rescued.
@@ -682,7 +682,7 @@
 
         login('[e-mail]')
         builder.builderok = True
-        builder.setSlaveForTesting(SaneBuildingSlave())
+        builder.setSlaveForTesting(BuildingSlave(build_id='8-1'))
         transaction.commit()
         login(ANONYMOUS)
 

=== modified file 'lib/lp/code/tests/test_recipebuilder.py'
--- lib/lp/code/tests/test_recipebuilder.py	2010-03-25 18:34:07 +0000
+++ lib/lp/code/tests/test_recipebuilder.py	2010-04-09 00:40:18 +0000
@@ -20,8 +20,8 @@
     SourcePackageRecipeBuild)
 from lp.soyuz.adapters.archivedependencies import get_sources_list_for_building
 from lp.soyuz.model.processor import ProcessorFamilySet
-from lp.soyuz.tests.soyuzbuilddhelpers import (MockBuilder,
-    SaneBuildingSlave,)
+from lp.soyuz.tests.soyuzbuilddhelpers import (
+    MockBuilder, OkSlave)
 from lp.soyuz.tests.test_binarypackagebuildbehavior import (
     BaseTestVerifySlaveBuildID)
 from lp.soyuz.tests.test_publishing import (
@@ -84,7 +84,7 @@
         # VerifyBuildRequest won't raise any exceptions when called with a
         # valid builder set.
         job = self.makeJob()
-        builder = MockBuilder("bob-de-bouwer", SaneBuildingSlave())
+        builder = MockBuilder("bob-de-bouwer", OkSlave())
         job.setBuilder(builder)
         logger = BufferLogger()
         job.verifyBuildRequest(logger)
@@ -136,7 +136,7 @@
         # dispatchBuildToSlave will fail when there is not chroot tarball
         # available for the distroseries to build for.
         job = self.makeJob()
-        builder = MockBuilder("bob-de-bouwer", SaneBuildingSlave())
+        builder = MockBuilder("bob-de-bouwer", OkSlave())
         processorfamily = ProcessorFamilySet().getByProcessorName('386')
         builder.processor = processorfamily.processors[0]
         job.setBuilder(builder)

=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
--- lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-04-08 15:29:39 +0000
+++ lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-04-09 00:40:18 +0000
@@ -32,90 +32,8 @@
 Import MockBuilder and a series of MockSlaves to be used in this test.
 
     >>> from lp.soyuz.tests.soyuzbuilddhelpers import (
-    ...    AbortedSlave, AbortingSlave, BuildingSlave, InsaneWaitingSlave,
-    ...    LostBuildingBrokenSlave, LostBuildingSlave, LostWaitingSlave,
-    ...    MockBuilder, OkSlave, SaneBuildingSlave, SaneWaitingSlave,
-    ...    WaitingSlave)
-
-Let's play with a Builder method designed to rescue build slaves
-that are processing unknown jobs. In real conditions, this situation
-only happens if the slave is processing deleted or modified BuildQueue
-entry, since Build entries are never removed. It might be caused by
-exceptions in slavescanner or queuebuilder scripts.
-
-When we figured this situation out, the procedure to rescue is to
-request the slave XMLRPC method 'clean', reseting the slave completely.
-
-We figured out if the building information is correct and sane by
-checking the job identifier field from status message information,
-which consists of "<Build.id>-<BuildQueue.id>".
-
-First let's emulate a sane and a lost slave. The SaneSlave returns a
-job identifier that exists in our sampledata, but the LostSlave
-returns a completely bogus one.
-
-The the mock slave.clean() method is modified to print a message for
-testing purposes.
-
-Initializing the sane_builder. It was not rescued, since the job
-identifier is sane (Build.id == 8 and BuildQueue.id == 1 exist):
-
-    >>> sanebuilding_builder = MockBuilder(
-    ...     'Sane Building Slave', SaneBuildingSlave())
-    >>> sanebuilding_builder.rescueIfLost(logger) is None
-    True
-
-A sane WAITING slave:
-
-    >>> sanewaiting_builder = MockBuilder(
-    ...     'Sane Waiting Slave', SaneWaitingSlave())
-    >>> sanewaiting_builder.rescueIfLost(logger) is None
-    True
-
-A insane WAITING slave, with wrong BuildQueue/Build relation:
-
-    >>> insanewaiting_builder = MockBuilder(
-    ...     'Insane Waiting Slave', InsaneWaitingSlave())
-    >>> insanewaiting_builder.rescueIfLost(logger)
-    WARNING:root:Builder 'Insane Waiting Slave' rescued from '7-1': 'Job build entry mismatch'
-
-It was rescued because the BuildQueue.id == 1 isn't related to
-Build.id == 7, so this pair relation is wrong.
-
-Let's test slaves with job identifier pointing non-existent
-Build/BuildQueue entries. first a lost slave in status 'BUILDING':
-
-    >>> lostbuilding_builder = MockBuilder(
-    ...     'Lost Building Slave', LostBuildingSlave())
-    >>> lostbuilding_builder.rescueIfLost(logger)
-    WARNING:root:Builder 'Lost Building Slave' rescued from '1000-10000':
-    'Build 1000 is not available: 'Object not found''
-
-Then a lost slave in status 'WAITING':
-
-    >>> lostwaiting_builder = MockBuilder(
-    ...     'Lost Waiting Slave', LostWaitingSlave())
-    >>> lostwaiting_builder.rescueIfLost(logger)
-    WARNING:root:Builder 'Lost Waiting Slave' rescued from '1000-10000':
-    'Build 1000 is not available: 'Object not found''
-
-Both got rescued, as expected.
-
-A BUILDING or WAITING slave without a build assigned in the DB will also be rescued.
-
-    >>> from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
-
-    >>> lostbuilding_builder = MockBuilder(
-    ...     'Lost Building Slave', LostBuildingSlave())
-    >>> lostbuilding_builder.current_build_behavior = IdleBuildBehavior()
-    >>> lostbuilding_builder.rescueIfLost(logger)
-    WARNING:root:Builder 'Lost Building Slave' rescued from '1000-10000': 'No job assigned to builder'
-
-    >>> lostwaiting_builder = MockBuilder(
-    ...     'Lost Waiting Slave', LostWaitingSlave())
-    >>> lostwaiting_builder.current_build_behavior = IdleBuildBehavior()
-    >>> lostwaiting_builder.rescueIfLost(logger)
-    WARNING:root:Builder 'Lost Waiting Slave' rescued from '1000-10000': 'No job assigned to builder'
+    ...    AbortedSlave, AbortingSlave, BuildingSlave,
+    ...    LostBuildingBrokenSlave, MockBuilder, OkSlave, WaitingSlave)
 
 Slave-scanner will deactivate a 'lost-building' builder that could not
 be aborted appropriately.
@@ -124,6 +42,7 @@
     ...     'Lost Building Broken Slave', LostBuildingBrokenSlave())
 
     >>> lostbuilding_builder.updateStatus(logger)
+    Aborting slave
     WARNING:root:Lost Building Broken Slave (http://fake:0000) marked as failed due to: <Fault 8002: 'Could not abort'>
     Traceback (most recent call last):
     ...

=== modified file 'lib/lp/soyuz/tests/soyuzbuilddhelpers.py'
--- lib/lp/soyuz/tests/soyuzbuilddhelpers.py	2010-04-03 03:38:55 +0000
+++ lib/lp/soyuz/tests/soyuzbuilddhelpers.py	2010-04-09 00:40:18 +0000
@@ -7,11 +7,6 @@
 
 __all__ = [
     'MockBuilder',
-    'SaneBuildingSlave',
-    'SaneWaitingSlave',
-    'InsaneWaitingSlave',
-    'LostBuildingSlave',
-    'LostWaitingSlave',
     'LostBuildingBrokenSlave',
     'BrokenSlave',
     'OkSlave',
@@ -36,9 +31,12 @@
 class MockBuilder:
     """Emulates a IBuilder class."""
 
-    current_build_behavior = BinaryPackageBuildBehavior(None)
+    def __init__(self, name, slave, behavior=None):
+        if behavior is None:
+            self.current_build_behavior = BinaryPackageBuildBehavior(None)
+        else:
+            self.current_build_behavior = behavior
 
-    def __init__(self, name, slave):
         self.slave = slave
         self.builderok = True
         self.manual = False
@@ -58,9 +56,11 @@
         return self.current_build_behavior.verifySlaveBuildID(slave_build_id)
 
     def cleanSlave(self):
+        print 'Cleaning slave'
         return self.slave.clean()
 
     def requestAbort(self):
+        print 'Aborting slave'
         return self.slave.abort()
 
     def resumeSlave(self, logger):
@@ -79,88 +79,6 @@
         updateBuilderStatus(self, logger)
 
 
-class SaneBuildingSlave:
-    """A mock slave that is currently building build 8 and buildqueue 1."""
-
-    def status(self):
-        return ('BuilderStatus.BUILDING', '8-1', 'Doing something ...')
-
-    def clean(self):
-        print 'Rescuing SaneSlave'
-
-    def echo(self, *args):
-        return args
-
-    def info(self):
-        return ['1.0', 'i386', ['debian']]
-
-    def build(self, buildid, builder_type, chroot_sha1, filemap, args):
-        return ('BuildStatus.Building', buildid)
-
-
-class SaneWaitingSlave:
-    """A mock slave that is currently waiting.
-
-    Uses build 8 and buildqueue 1.
-    """
-
-    def status(self):
-        return ('BuilderStatus.WAITING', 'BuildStatus.OK', '8-1')
-
-    def clean(self):
-        print 'Rescuing SaneSlave'
-
-
-class InsaneWaitingSlave:
-    """A mock slave waiting with a bogus Build/BuildQueue relation."""
-
-    def status(self):
-        return ('BuilderStatus.WAITING', 'BuildStatus.OK', '7-1')
-
-    def clean(self):
-        pass
-
-
-class LostBuildingSlave:
-    """A mock slave building bogus Build/BuildQueue IDs."""
-
-    def status(self):
-        return ('BuilderStatus.BUILDING', '1000-10000')
-
-    def abort(self):
-        pass
-
-
-class LostWaitingSlave:
-    """A mock slave waiting with bogus Build/BuildQueue IDs."""
-
-    def status(self):
-        return ('BuilderStatus.WAITING', 'BuildStatus.OK', '1000-10000')
-
-    def clean(self):
-        pass
-
-
-class LostBuildingBrokenSlave:
-    """A mock slave building bogus Build/BuildQueue IDs that can't be aborted.
-
-    When 'aborted' it raises an xmlrpclib.Fault(8002, 'Could not abort')
-    """
-
-    def status(self):
-        return ('BuilderStatus.BUILDING', '1000-10000')
-
-    def abort(self):
-        raise xmlrpclib.Fault(8002, "Could not abort")
-
-
-class BrokenSlave:
-    """A mock slave that reports that it is broken."""
-
-    def status(self):
-        raise xmlrpclib.Fault(8001, "Broken slave")
-
-
 class OkSlave:
     """An idle mock slave that prints information about itself.
 
@@ -196,9 +114,15 @@
     def fetchlogtail(self, size):
         return 'BOGUS'
 
+    def echo(self, *args):
+        return args
+
     def clean(self):
         pass
 
+    def abort(self):
+        pass
+
     def info(self):
         return ('1.0', self.arch_tag, 'debian')
 
@@ -223,9 +147,13 @@
 class BuildingSlave(OkSlave):
     """A mock slave that looks like it's currently building."""
 
+    def __init__(self, build_id='1-1'):
+        super(BuildingSlave, self).__init__()
+        self.build_id = build_id
+
     def status(self):
         buildlog = xmlrpclib.Binary("This is a build log")
-        return ('BuilderStatus.BUILDING', '1-1', buildlog)
+        return ('BuilderStatus.BUILDING', self.build_id, buildlog)
 
     def getFile(self, sum):
         if sum == "buildlog":
@@ -234,24 +162,19 @@
             return s
 
 
-class AbortedSlave(OkSlave):
-    """A mock slave that looks like it's aborted."""
-
-    def status(self):
-        return ('BuilderStatus.ABORTED', '1-1')
-
-
 class WaitingSlave(OkSlave):
     """A mock slave that looks like it's currently waiting."""
 
-    def __init__(self, state, dependencies=None):
+    def __init__(self, state='BuildStatus.OK', dependencies=None,
+                 build_id='1-1'):
         super(WaitingSlave, self).__init__()
         self.state = state
         self.dependencies = dependencies
+        self.build_id = build_id
 
     def status(self):
-        return ('BuilderStatus.WAITING', self.state, '1-1', {},
-                self.dependencies )
+        return ('BuilderStatus.WAITING', self.state, self.build_id, {},
+                self.dependencies)
 
     def getFile(self, sum):
         if sum == "buildlog":
@@ -259,8 +182,36 @@
             s.headers = {'content-length':19}
             return s
 
+
 class AbortingSlave(OkSlave):
     """A mock slave that looks like it's in the process of aborting."""
 
     def status(self):
         return ('BuilderStatus.ABORTING', '1-1')
+
+
+class AbortedSlave(OkSlave):
+    """A mock slave that looks like it's aborted."""
+
+    def status(self):
+        return ('BuilderStatus.ABORTED', '1-1')
+
+
+class LostBuildingBrokenSlave:
+    """A mock slave building bogus Build/BuildQueue IDs that can't be aborted.
+
+    When 'aborted' it raises an xmlrpclib.Fault(8002, 'Could not abort')
+    """
+
+    def status(self):
+        return ('BuilderStatus.BUILDING', '1000-10000')
+
+    def abort(self):
+        raise xmlrpclib.Fault(8002, "Could not abort")
+
+
+class BrokenSlave:
+    """A mock slave that reports that it is broken."""
+
+    def status(self):
+        raise xmlrpclib.Fault(8001, "Broken slave")