launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/ttb-no-chroot into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/ttb-no-chroot/+merge/91015

Handle the lack of chroot in TTBJ.dispatchBuildToSlave(). I removed the _getChroot() method as useless cruft, IDistroArchSeries.getChroot() is well-tested enough, so I removed the test for TTBJ._getChroot(). It was used as a stub function for testing, sneakily enough, but I figured I could just add a LFA to the DAS in question.
-- 
https://code.launchpad.net/~stevenk/launchpad/ttb-no-chroot/+merge/91015
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/ttb-no-chroot into lp:launchpad.

=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py	2012-01-11 08:52:22 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py	2012-02-01 04:43:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehavior` for `TranslationTemplatesBuildJob`.
@@ -23,6 +23,7 @@
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.interfaces.builder import CannotBuild
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior,
     )
@@ -46,7 +47,11 @@
 
     def dispatchBuildToSlave(self, build_queue_item, logger):
         """See `IBuildFarmJobBehavior`."""
-        chroot = self._getChroot()
+        distroarchseries = self._getDistroArchSeries()
+        chroot = distroarchseries.getChroot()
+        if chroot is None:
+            raise CannotBuild("Unable to find a chroot for %s" %
+                              distroarchseries.displayname)
         chroot_sha1 = chroot.content.sha1
         d = self._builder.slave.cacheFile(logger, chroot)
 
@@ -64,9 +69,6 @@
                 cookie, self.build_type, chroot_sha1, filemap, args)
         return d.addCallback(got_cache_file)
 
-    def _getChroot(self):
-        return self._getDistroArchSeries().getChroot()
-
     def _getDistroArchSeries(self):
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
         return ubuntu.currentseries.nominatedarchindep

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	2012-01-20 15:42:44 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	2012-02-01 04:43:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for TranslationTemplatesBuildBehavior."""
@@ -15,6 +15,7 @@
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.interfaces.builder import CannotBuild
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior,
     )
@@ -24,7 +25,6 @@
     WaitingSlave,
     )
 from lp.services.config import config
-from lp.services.librarian.interfaces import ILibraryFileAliasSet
 from lp.services.librarian.utils import copy_and_close
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import switch_dbuser
@@ -82,7 +82,9 @@
         if use_fake_chroot:
             lf = self.factory.makeLibraryFileAlias()
             self.layer.txn.commit()
-            behavior._getChroot = lambda: lf
+            ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+            das = ubuntu.currentseries.nominatedarchindep
+            das.addOrUpdateChroot(lf)
         return behavior
 
     def makeProductSeriesWithBranchForTranslation(self):
@@ -111,6 +113,16 @@
         job = removeSecurityProxy(behavior.buildfarmjob.job)
         return getUtility(IBuildQueueSet).getByJob(job.id)
 
+    def test_dispatchBuildToSlave_no_chroot_fails(self):
+        # dispatchBuildToSlave will fail if the chroot does not exist.
+        behavior = self.makeBehavior(use_fake_chroot=False)
+        buildqueue_item = self._getBuildQueueItem(behavior)
+
+        switch_dbuser(config.builddmaster.dbuser)
+        self.assertRaises(
+            CannotBuild, behavior.dispatchBuildToSlave, buildqueue_item,
+            logging)
+
     def test_dispatchBuildToSlave(self):
         # dispatchBuildToSlave ultimately causes the slave's build
         # method to be invoked.  The slave receives the URL of the
@@ -136,24 +148,6 @@
                 behavior.buildfarmjob.branch.composePublicURL())
         return d.addCallback(got_dispatch)
 
-    def test_getChroot(self):
-        # _getChroot produces the current chroot for the current Ubuntu
-        # release, on the nominated architecture for
-        # architecture-independent builds.
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        current_ubuntu = ubuntu.currentseries
-        distroarchseries = current_ubuntu.nominatedarchindep
-
-        # Set an arbitrary chroot file.
-        fake_chroot_file = getUtility(ILibraryFileAliasSet)[1]
-        distroarchseries.addOrUpdateChroot(fake_chroot_file)
-
-        behavior = self.makeBehavior(use_fake_chroot=False)
-        chroot = behavior._getChroot()
-
-        self.assertNotEqual(None, chroot)
-        self.assertEqual(fake_chroot_file, chroot)
-
     def test_readTarball(self):
         behavior = self.makeBehavior()
         buildqueue = FakeBuildQueue(behavior)