Launchpad itself

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

  1. bug-523449
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-523449
Merge into: lp:launchpad
Prerequisite: lp:~jtv/launchpad/branch-url
Diff against target: 88 lines (+30/-4)
3 files modified
lib/lp/translations/model/translationtemplatesbuildjob.py (+1/-1)
lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py (+15/-3)
lib/lp/translations/tests/test_translationtemplatesbuildjob.py (+14/-0)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-523449
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+19622@code.launchpad.net

Commit message

Pass branch_url to build slave.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= 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

Revision history for this message
Paul Hummer (rockstar) wrote :

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)
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
2--- lib/lp/translations/model/translationtemplatesbuildjob.py 2010-02-19 23:48:21 +0000
3+++ lib/lp/translations/model/translationtemplatesbuildjob.py 2010-02-19 23:48:22 +0000
4@@ -99,7 +99,7 @@
5 store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
6
7 # We don't have any JSON metadata for this BranchJob type.
8- metadata = {}
9+ metadata = {'branch_url': branch.composePublicURL()}
10 branch_job = BranchJob(
11 branch, BranchJobType.TRANSLATION_TEMPLATES_BUILD, metadata)
12 store.add(branch_job)
13
14=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
15--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-02-11 19:11:11 +0000
16+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-02-19 23:48:22 +0000
17@@ -37,9 +37,9 @@
18 class FakeSlave:
19 """Pretend build slave."""
20 def __init__(self, builderstatus):
21- self.build_started = False
22 self.status = {
23 'build_status': 'BuildStatus.%s' % builderstatus.name,
24+ 'test_build_started': False,
25 }
26
27 self.cacheFile = FakeMethod()
28@@ -49,7 +49,12 @@
29 """Pretend to start a build."""
30 self.status['build_id'] = buildid
31 self.status['filemap'] = filemap
32- self.build_started = True
33+
34+ # Chuck in some information that a real slave wouldn't store,
35+ # but which will allow tests to check up on the build call.
36+ self.status['test_build_type'] = build_type
37+ self.status['test_build_args'] = args
38+ self.status['test_build_started'] = True
39
40
41 class FakeBuilder:
42@@ -96,13 +101,20 @@
43 return getUtility(IBuildQueueSet).getByJob(job.id)
44
45 def test_dispatchBuildToSlave(self):
46+ # dispatchBuildToSlave ultimately causes the slave's build
47+ # method to be invoked. The slave receives the URL of the
48+ # branch it should build from.
49 behavior = self._makeBehavior()
50 behavior._getChroot = FakeChroot
51 buildqueue_item = self._getBuildQueueItem(behavior)
52
53 behavior.dispatchBuildToSlave(buildqueue_item, logging)
54
55- self.assertTrue(behavior._builder.slave.build_started)
56+ slave_status = behavior._builder.slaveStatus()
57+ self.assertTrue(slave_status['test_build_started'])
58+ self.assertEqual(
59+ 'translation-templates', slave_status['test_build_type'])
60+ self.assertIn('branch_url', slave_status['test_build_args'])
61
62 def test_getChroot(self):
63 # _getChroot produces the current chroot for the current Ubuntu
64
65=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
66--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-02-19 23:48:21 +0000
67+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2010-02-19 23:48:22 +0000
68@@ -227,6 +227,20 @@
69 self.assertEqual(1, len(branchjobs))
70 self.assertEqual(branch, branchjobs[0].branch)
71
72+ def test_create(self):
73+ branch = self._makeTranslationBranch(fake_pottery_compatible=True)
74+
75+ specific_job = self.jobsource.create(branch)
76+
77+ # A job is created with the branch URL in its metadata.
78+ metadata = specific_job.metadata
79+ self.assertIn('branch_url', metadata)
80+ url = metadata['branch_url']
81+ head = 'http://'
82+ self.assertEqual(head, url[:len(head)])
83+ tail = branch.name
84+ self.assertEqual(tail, url[-len(tail):])
85+
86
87 class TestTranslationTemplatesBuildBehavior(TestCaseWithFactory):
88 """Test `TranslationTemplatesBuildBehavior`."""