Merge lp:~jtv/launchpad/bug-523449 into lp:launchpad

[Jeroen T. Vermeulen]

Pass branch_url to build slave.

[Jeroen T. Vermeulen (jtv)]

= Bug 523449 =

We're farming out the generation of translation templates based on a branch of source code to the build farm. One of the pieces of the puzzle is to tell the slave where it can find the branch. The code you see here takes care of that. It relies on a new method, IBranch.composePublicURL, which was implemented as bug 523467 but has not landed yet because of review backlog.

This feature is not enabled yet because it is not yet complete. Q/A for this particular part of the job does not make sense; what's needed is end-to-end manual testing of the entire feature.

Test:
{{{
./bin/test -vv -t translationtemplatesbuild
}}}

There is some lint, but none related to my code and none that I can reasonably fix. Mostly pylint being pitifully confused for no good reason.

Jeroen

[Paul Hummer (rockstar)]

This diff had some changes from the pre-req branch, which is sub-optimal. I wonder why. Did you use a pipe or just merge in some of the revisions from the pre-req branch. It's really odd.

Other than that, this all looks good, assuming that the one single change in the pre-req branch is fixed as well.

review: Approve (code)

[Jeroen T. Vermeulen (jtv)]

Thanks, Paul! Yes, I'm working in a pipe and so in the regular flow of things, I found that I wanted to do one thing differently—move more of the trigger function's logic into the utility. I can backport that change, or leave it in and just land the 4 branches that have accumulated here (all but the first reviewed...) as one. If you're reviewing the prerequisite branch as well, perhaps the easiest thing is to keep things as they are.

=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
--- lib/lp/translations/model/translationtemplatesbuildjob.py	2010-02-19 23:48:21 +0000
+++ lib/lp/translations/model/translationtemplatesbuildjob.py	2010-02-19 23:48:22 +0000
@@ -99,7 +99,7 @@
         store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
 
         # We don't have any JSON metadata for this BranchJob type.
-        metadata = {}
+        metadata = {'branch_url': branch.composePublicURL()}
         branch_job = BranchJob(
             branch, BranchJobType.TRANSLATION_TEMPLATES_BUILD, metadata)
         store.add(branch_job)

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	2010-02-11 19:11:11 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py	2010-02-19 23:48:22 +0000
@@ -37,9 +37,9 @@
 class FakeSlave:
     """Pretend build slave."""
     def __init__(self, builderstatus):
-        self.build_started = False
         self.status = {
             'build_status': 'BuildStatus.%s' % builderstatus.name,
+            'test_build_started': False,
         }
 
         self.cacheFile = FakeMethod()
@@ -49,7 +49,12 @@
         """Pretend to start a build."""
         self.status['build_id'] = buildid
         self.status['filemap'] = filemap
-        self.build_started = True
+
+        # Chuck in some information that a real slave wouldn't store,
+        # but which will allow tests to check up on the build call.
+        self.status['test_build_type'] = build_type
+        self.status['test_build_args'] = args
+        self.status['test_build_started'] = True
 
 
 class FakeBuilder:
@@ -96,13 +101,20 @@
         return getUtility(IBuildQueueSet).getByJob(job.id)
 
     def test_dispatchBuildToSlave(self):
+        # dispatchBuildToSlave ultimately causes the slave's build
+        # method to be invoked.  The slave receives the URL of the
+        # branch it should build from.
         behavior = self._makeBehavior()
         behavior._getChroot = FakeChroot
         buildqueue_item = self._getBuildQueueItem(behavior)
 
         behavior.dispatchBuildToSlave(buildqueue_item, logging)
 
-        self.assertTrue(behavior._builder.slave.build_started)
+        slave_status = behavior._builder.slaveStatus()
+        self.assertTrue(slave_status['test_build_started'])
+        self.assertEqual(
+            'translation-templates', slave_status['test_build_type'])
+        self.assertIn('branch_url', slave_status['test_build_args'])
 
     def test_getChroot(self):
         # _getChroot produces the current chroot for the current Ubuntu

=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-02-19 23:48:21 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py	2010-02-19 23:48:22 +0000
@@ -227,6 +227,20 @@
         self.assertEqual(1, len(branchjobs))
         self.assertEqual(branch, branchjobs[0].branch)
 
+    def test_create(self):
+        branch = self._makeTranslationBranch(fake_pottery_compatible=True)
+
+        specific_job = self.jobsource.create(branch)
+
+        # A job is created with the branch URL in its metadata.
+        metadata = specific_job.metadata
+        self.assertIn('branch_url', metadata)
+        url = metadata['branch_url']
+        head = 'http://'
+        self.assertEqual(head, url[:len(head)])
+        tail = branch.name
+        self.assertEqual(tail, url[-len(tail):])
+
 
 class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
     """Test `TranslationTemplatesBuildBehavior`."""