Merge lp:~jtv/launchpad/bug-507678 into lp:launchpad/db-devel

[Jeroen T. Vermeulen]

Retrieve results from translation-templates builds on the build farm.

[Jeroen T. Vermeulen (jtv)]

= Bug 507678 =

Continuing work on making the buildfarm generate translation templates from source branches. Here I'm jumping through the hoops to let the IBuildFarmJobBehavior implementation that we have for this kind of build-farm job retrieve a tarball produced by the slave.

The default BuildFarmJobBehaviorBase code for this assumed that there's an IBuildBase object (basically a generalization of soyuz's Build) attached to a build-farm job. It didn't seem worth it to implement IBuildBase just for this (I tried), I now override the only BuildFarmJobBehaviorBase method that requires it. I had to extend the infrastructure a bit along the way.

An important thing to note is that these pieces are still coming together; they're not actually a functional whole yet. But with the ongoing work on build-farm generalization, it's important to get and keep them integrated before bit-rot sets in.

Going through the diff step by step:

lib/lp/buildmaster/model/buildfarmjobbehavior.py

I extracted a method for reading and checking build status from a slave status dict. That makes it easy for my behavior class to reuse it.

lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py

Tests the method I abstracted.

lib/lp/testing/factory.py

New factory method for the type of build-farm job that generates translation templates.

lib/lp/testing/fakemethod.py

Provides a generic stub for all sorts of functions and methods. I got tired of writing one-liners, injecting ad-hoc status variables into objects, and building one-off mock classes. Need to override a method in an object to return a dummy value or fail in a particular way? Just assign it to a FakeMethod.

Pylint had a truly silly complaint about this file, so I silenced it. There's a comment to the effect.

lib/lp/testing/tests/test_fakemethod.py

Unit-tests FakeMethod.

lib/lp/translations/model/translationtemplatesbuildbehavior.py

Grew some methods for retrieving a tarball of results from the build-farm slave. As yet we do not have the code in place that will generate the real templates.

lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py

Tests the extensions to the behavior class. The starting point for this was an existing unit test that I ripped out of the next file in the diff.

Interaction with slaves makes testing this stuff very hard. About the best I can do is mock up all the infrastructure, unit-test my part, and hope the other classes do their job. It's not perfect, but it's a lot more automated testing than a lot of the other buildfarm work gets.

For this test I had to strike a balance black-box and white-box approaches: obviously the test needs a lot of inside knowledge about the classes it's dealing with, but I didn't make it check the fidgety details of what values are being passed around where. Those could change easily, as work on the build farm is still ongoing. Moreover, I have no real requirements for them other than "they should work."

lib/lp/translations/tests/test_translationtemplatesbuildjob.py

Lost a test case to the file above. What there was got simplified a bit, and setUp got replaced with explicit creation of objects.

To test these changes...

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-01-29 17:15:31 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-02-11 19:48:24 +0000
@@ -164,6 +164,20 @@
             queueItem.job.fail()
         queueItem.specific_job.jobAborted()
 
+    def extractBuildStatus(self, slave_status):
+        """Read build status name.
+
+        :param slave_status: build status dict as passed to the
+            updateBuild_* methods.
+        :return: the unqualified status name, e.g. "OK".
+        """
+        status_string = slave_status['build_status']
+        lead_string = 'BuildStatus.'
+        assert status_string.startswith(lead_string), (
+            "Malformed status string: '%s'" % status_string)
+
+        return status_string[len(lead_string):]
+
     def updateBuild_WAITING(self, queueItem, slave_status, logtail, logger):
         """Perform the actions needed for a slave in a WAITING state
 
@@ -178,13 +192,10 @@
           the uploader for processing.
         """
         librarian = getUtility(ILibrarianClient)
-        build_status = slave_status['build_status']
+        build_status = self.extractBuildStatus(slave_status)
 
         # XXX: dsilvers 2005-03-02: Confirm the builder has the right build?
-        assert build_status.startswith('BuildStatus.'), (
-            'Malformed status string: %s' % build_status)
 
-        build_status = build_status[len('BuildStatus.'):]
         queueItem.specific_job.build.handleStatus(
             build_status, librarian, slave_status)
 

=== added file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	1970-01-01 00:00:00 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	2010-02-11 19:48:24 +0000
@@ -0,0 +1,41 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Unit tests for BuildFarmJobBehaviorBase."""
+
+from unittest import TestCase, TestLoader
+
+from lp.buildmaster.model.buildfarmjobbehavior import BuildFarmJobBehaviorBase
+from lp.soyuz.interfaces.build import BuildStatus
+
+
+class FakeBuildFarmJob:
+    """Dummy BuildFarmJob."""
+    pass
+
+
+class TestBuildFarmJobBehaviorBase(TestCase):
+    """Test very small, basic bits of BuildFarmJobBehaviorBase."""
+
+    def setUp(self):
+        self.behavior = BuildFarmJobBehaviorBase(FakeBuildFarmJob())
+
+    def test_extractBuildStatus_baseline(self):
+        # extractBuildStatus picks the name of the build status out of a
+        # dict describing the slave's status.
+        slave_status = {'build_status': 'BuildStatus.BUILDING'}
+        self.assertEqual(
+            BuildStatus.BUILDING.name,
+            self.behavior.extractBuildStatus(slave_status))
+
+    def test_extractBuildStatus_malformed(self):
+        # extractBuildStatus errors out when the status string is not
+        # of the form it expects.
+        slave_status = {'build_status': 'BUILDING'}
+        self.assertRaises(
+            AssertionError,
+            self.behavior.extractBuildStatus, slave_status)
+
+
+def test_suite():
+    return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-02-11 05:08:47 +0000
+++ lib/lp/testing/factory.py	2010-02-11 19:48:24 +0000
@@ -129,6 +129,7 @@
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.model.buildqueue import BuildQueue
+<<<<<<< TREE
 from lp.soyuz.model.processor import ProcessorFamilySet
 from lp.soyuz.model.publishing import (
     SecureSourcePackagePublishingHistory,
@@ -142,6 +143,12 @@
     ITranslationGroupSet)
 from lp.translations.interfaces.translationsperson import ITranslationsPerson
 from lp.translations.interfaces.translator import ITranslatorSet
+=======
+from lp.testing import run_with_login, time_counter
+from lp.translations.interfaces.translationtemplatesbuildjob import (
+    ITranslationTemplatesBuildJobSource)
+
+>>>>>>> MERGE-SOURCE
 
 SPACE = ' '
 
@@ -1714,6 +1721,18 @@
         store.add(bq)
         return bq
 
+    def makeTranslationTemplatesBuildJob(self, branch=None):
+        """Make a new `TranslationTemplatesBuildJob`.
+
+        :param branch: The branch that the job should be for.  If none
+            is given, one will be created.
+        """
+        if branch is None:
+            branch = self.makeBranch()
+
+        jobset = getUtility(ITranslationTemplatesBuildJobSource)
+        return jobset.create(branch)
+
     def makePOTemplate(self, productseries=None, distroseries=None,
                        sourcepackagename=None, owner=None, name=None,
                        translation_domain=None, path=None):

=== added file 'lib/lp/testing/fakemethod.py'
--- lib/lp/testing/fakemethod.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/fakemethod.py	2010-02-11 19:48:24 +0000
@@ -0,0 +1,51 @@
+# Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# pylint: disable-msg=E0702
+
+
+__metaclass__ = type
+__all__ = [
+    'FakeMethod',
+    ]
+
+
+class FakeMethod:
+    """Catch any function or method call, and record the fact.
+
+    Use this for easy stubbing.  The call operator can return a fixed
+    value, or raise a fixed exception object.
+
+    This is useful when unit-testing code that does things you don't
+    want to integration-test, e.g. because it wants to talk to remote
+    systems.
+    """
+
+    # How many times has this fake method been called?
+    call_count = 0
+
+    def __init__(self, result=None, failure=None):
+        """Set up a fake function or method.
+
+        :param result: Value to return.
+        :param failure: Exception to raise.
+        """
+        self.result = result
+        self.failure = failure
+
+    def __call__(self, *args, **kwargs):
+        """Catch an invocation to the method.  Increment `call_count`.
+
+        Accepts any and all parameters.  Raises the failure passed to
+        the constructor, if any; otherwise, returns the result value
+        passed to the constructor.
+        """
+        self.call_count += 1
+
+        if self.failure is None:
+            return self.result
+        else:
+            # pylint thinks this raises None, which is clearly not
+            # possible.  That's why this test disables pylint message
+            # E0702.
+            raise self.failure

=== added file 'lib/lp/testing/tests/test_fakemethod.py'
--- lib/lp/testing/tests/test_fakemethod.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/tests/test_fakemethod.py	2010-02-11 19:48:24 +0000
@@ -0,0 +1,85 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from unittest import TestCase, TestLoader
+
+from lp.testing.fakemethod import FakeMethod
+
+
+class RealActualClass:
+    """A class that's hard to test."""
+
+    doing_impossible_stuff = False
+    doing_possible_stuff = False
+
+    def impossibleMethod(self):
+        """A method that you can't afford to invoke in your test.
+
+        This is the method you're going to want to avoid calling.  The
+        way to do that is to replace this method with a fake method.
+        """
+        self.doing_impossible_stuff = True
+        raise AssertionError("Trying to do impossible stuff.")
+
+    def testableMethod(self):
+        """This part of the class logic that you do want to exercise."""
+        self.doing_possible_stuff = True
+
+    def doComplicatedThings(self, argument):
+        """This is the top-level method you want to test.
+
+        Unfortunately this invokes impossibleMethod, making it hard.
+        """
+        self.impossibleMethod()
+        self.testableMethod()
+        return argument
+
+
+class CustomException(Exception):
+    """Some specific error that you want raised."""
+
+
+class TestFakeMethod(TestCase):
+    def test_fakeMethod(self):
+        # A class that you're testing can continue normally despite some
+        # of its methods being stubbed.
+        thing = RealActualClass()
+        thing.impossibleMethod = FakeMethod()
+
+        result = thing.doComplicatedThings(99)
+
+        self.assertEqual(99, result)
+        self.assertFalse(thing.doing_impossible_stuff)
+        self.assertTrue(thing.doing_possible_stuff)
+
+    def test_raiseFailure(self):
+        # A FakeMethod can raise an exception you specify.
+        ouch = CustomException("Ouch!")
+        func = FakeMethod(failure=ouch)
+        self.assertRaises(CustomException, func)
+
+    def test_returnResult(self):
+        # A FakeMethod can return a value you specify.
+        value = "Fixed return value."
+        func = FakeMethod(result=value)
+        self.assertEqual(value, func())
+
+    def test_countCalls(self):
+        # A FakeMethod counts the number of times it's been invoked.
+        func = FakeMethod()
+        for count in xrange(3):
+            self.assertEqual(count, func.call_count)
+            func()
+            self.assertEqual(count + 1, func.call_count)
+
+    def test_takeArguments(self):
+        # A FakeMethod invocation accepts any arguments it gets.
+        func = FakeMethod()
+        func()
+        func(1)
+        func(2, kwarg=3)
+        self.assertEqual(3, func.call_count)
+
+
+def test_suite():
+    return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py	2010-01-15 01:20:20 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py	2010-02-11 19:48:24 +0000
@@ -13,6 +13,7 @@
 
 from zope.component import getUtility
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces import ILaunchpadCelebrities
 
@@ -29,6 +30,9 @@
     # Identify the type of job to the slave.
     build_type = 'translation-templates'
 
+    # Filename for the tarball of templates that the slave builds.
+    templates_tarball_path = 'translation-templates.tar.gz'
+
     def dispatchBuildToSlave(self, build_queue_item, logger):
         """See `IBuildFarmJobBehavior`."""
         chroot = self._getChroot()
@@ -39,7 +43,7 @@
         args = { 'branch_url': self.buildfarmjob.branch.url }
         filemap = {}
 
-        status, info = self._builder.slave.build(
+        self._builder.slave.build(
             buildid, self.build_type, chroot_sha1, filemap, args)
 
     def _getChroot(self):
@@ -49,3 +53,48 @@
     def logStartBuild(self, logger):
         """See `IBuildFarmJobBehavior`."""
         logger.info("Starting templates build.")
+
+    def _readTarball(self, buildqueue, filemap, logger):
+        """Read tarball with generated translation templates from slave."""
+        slave_filename = filemap.get(self.templates_tarball_path)
+        if slave_filename is None:
+            logger.error("Did not find templates tarball in slave output.")
+            return None
+
+        slave = removeSecurityProxy(buildqueue.builder.slave)
+        return slave.getFile(slave_filename).read()
+
+    def _uploadTarball(self, branch, tarball, logger):
+        """Upload tarball to productseries that want it."""
+        # XXX JeroenVermeulen 2010-01-28 bug=507680: Find productseries
+        # that want these templates, and upload to there.
+
+    def updateBuild_WAITING(self, queue_item, slave_status, logtail, logger):
+        """Deal with a finished ("WAITING" state, perversely) build job.
+
+        Retrieves tarball and logs from the slave, then cleans up the
+        slave so it's ready for a next job and destroys the queue item.
+
+        If this fails for whatever unforeseen reason, a future run will
+        retry it.
+        """
+        build_status = self.extractBuildStatus(slave_status)
+        build_id = slave_status['build_id']
+        filemap = slave_status['filemap']
+
+        logger.debug("Templates generation job %s finished with status %s" % (
+            build_id, build_status))
+
+        if build_status == 'OK':
+            logger.debug("Processing templates build %s" % build_id)
+            tarball = self._readTarball(queue_item, filemap, logger)
+            if tarball is None:
+                logger.error("Successful build %s produced no tarball." % (
+                    build_id))
+            else:
+                logger.debug("Uploading translation templates tarball.")
+                self._uploadTarball(tarball, logger)
+                logger.debug("Upload complete.")
+
+        queue_item.builder.cleanSlave()
+        queue_item.destroySelf()

=== added file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	2010-02-11 19:48:24 +0000
@@ -0,0 +1,157 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Unit tests for TranslationTemplatesBuildBehavior."""
+
+import logging
+from StringIO import StringIO
+from unittest import TestLoader
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.testing import ZopelessDatabaseLayer
+
+from canonical.launchpad.interfaces import ILaunchpadCelebrities
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from lp.buildmaster.interfaces.buildfarmjobbehavior import (
+    IBuildFarmJobBehavior)
+from lp.soyuz.interfaces.build import BuildStatus
+from lp.soyuz.interfaces.buildqueue import IBuildQueueSet
+from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
+
+
+class FakeChrootContent:
+    """Pretend chroot contents."""
+    sha1 = "shasha"
+
+
+class FakeChroot:
+    """Pretend chroot."""
+    def __init__(self, *args, **kwargs):
+        """Constructor acts as a fake _getChroot."""
+        self.content = FakeChrootContent()
+
+
+class FakeSlave:
+    """Pretend build slave."""
+    def __init__(self, builderstatus):
+        self.build_started = False
+        self.status = {
+            'build_status': 'BuildStatus.%s' % builderstatus.name,
+        }
+
+        self.cacheFile = FakeMethod()
+        self.getFile = FakeMethod(result=StringIO("File from the slave."))
+
+    def build(self, buildid, build_type, chroot_sha1, filemap, args):
+        """Pretend to start a build."""
+        self.status['build_id'] = buildid
+        self.status['filemap'] = filemap
+        self.build_started = True
+
+
+class FakeBuilder:
+    """Pretend `Builder`."""
+    def __init__(self, slave):
+        self.slave = slave
+        self.cleanSlave = FakeMethod()
+
+    def slaveStatus(self):
+        return self.slave.status
+
+
+class FakeBuildQueue:
+    """Pretend `BuildQueue`."""
+    def __init__(self, behavior):
+        """Pretend to be a BuildQueue item for the given build behavior.
+
+        Copies its builder from the behavior object.
+        """
+        self.builder = behavior._builder
+        self.destroySelf = FakeMethod()
+
+
+class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
+    """Test `TranslationTemplatesBuildBehavior`."""
+
+    layer = ZopelessDatabaseLayer
+
+    def _makeBehavior(self):
+        """Create a TranslationTemplatesBuildBehavior.
+
+        Anything that might communicate with build slaves and such
+        (which we can't really do here) is mocked up.
+        """
+        specific_job = self.factory.makeTranslationTemplatesBuildJob()
+        behavior = IBuildFarmJobBehavior(specific_job)
+        slave = FakeSlave(BuildStatus.NEEDSBUILD)
+        behavior._builder = FakeBuilder(slave)
+        return behavior
+
+    def _getBuildQueueItem(self, behavior):
+        """Get `BuildQueue` for an `IBuildFarmJobBehavior`."""
+        job = removeSecurityProxy(behavior.buildfarmjob.job)
+        return getUtility(IBuildQueueSet).getByJob(job.id)
+
+    def test_dispatchBuildToSlave(self):
+        behavior = self._makeBehavior()
+        behavior._getChroot = FakeChroot
+        buildqueue_item = self._getBuildQueueItem(behavior)
+
+        behavior.dispatchBuildToSlave(buildqueue_item, logging)
+
+        self.assertTrue(behavior._builder.slave.build_started)
+
+    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()
+        chroot = behavior._getChroot()
+
+        self.assertNotEqual(None, chroot)
+        self.assertEqual(fake_chroot_file, chroot)
+
+    def test_readTarball(self):
+        behavior = self._makeBehavior()
+        buildqueue = FakeBuildQueue(behavior)
+        path = behavior.templates_tarball_path
+        self.assertEqual(
+            "File from the slave.",
+            behavior._readTarball(buildqueue, {path: path}, logging))
+
+    def test_updateBuild_WAITING(self):
+        behavior = self._makeBehavior()
+        behavior._getChroot = FakeChroot
+        behavior._uploadTarball = FakeMethod()
+        queue_item = FakeBuildQueue(behavior)
+        slave_status = behavior._builder.slave.status
+        builder = behavior._builder
+        slave = builder.slave
+
+        behavior.dispatchBuildToSlave(queue_item, logging)
+
+        self.assertEqual(0, queue_item.destroySelf.call_count)
+        self.assertEqual(0, builder.cleanSlave.call_count)
+        self.assertEqual(0, behavior._uploadTarball.call_count)
+
+        behavior.updateBuild_WAITING(queue_item, slave_status, None, logging)
+
+        self.assertEqual(1, queue_item.destroySelf.call_count)
+        self.assertEqual(1, builder.cleanSlave.call_count)
+        self.assertEqual(0, behavior._uploadTarball.call_count)
+
+
+def test_suite():
+    return TestLoader().loadTestsFromName(__name__)
+

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-01-15 01:20:20 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-02-11 19:48:24 +0000
@@ -8,8 +8,6 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.launchpad.interfaces import (
-    ILaunchpadCelebrities, ILibraryFileAliasSet)
 from canonical.launchpad.webapp.testing import verifyObject
 from canonical.testing import ZopelessDatabaseLayer
 
@@ -17,8 +15,6 @@
 
 from lp.buildmaster.interfaces.buildfarmjob import (
     IBuildFarmJob, ISpecificBuildFarmJobClass)
-from lp.buildmaster.interfaces.buildfarmjobbehavior import (
-    IBuildFarmJobBehavior)
 from lp.code.interfaces.branchjob import IBranchJob
 from lp.services.job.model.job import Job
 from lp.soyuz.interfaces.buildqueue import IBuildQueueSet
@@ -102,35 +98,5 @@
         self.assertEqual(1000, self.specific_job.score())
 
 
-class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
-    """Test `TranslationTemplatesBuildBehavior`."""
-
-    layer = ZopelessDatabaseLayer
-
-    def setUp(self):
-        super(TestTranslationTemplatesBuildBehavior, self).setUp()
-        self.jobset = getUtility(ITranslationTemplatesBuildJobSource)
-        self.branch = self.factory.makeBranch()
-        self.specific_job = self.jobset.create(self.branch)
-        self.behavior = IBuildFarmJobBehavior(self.specific_job)
-
-    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)
-
-        chroot = self.behavior._getChroot()
-
-        self.assertNotEqual(None, chroot)
-        self.assertEqual(fake_chroot_file, chroot)
-
-
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)