Launchpad itself

Merge lp:~wgrant/launchpad/get-translations-jobs-working into lp:launchpad

  1. get-translations-jobs-working
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~wgrant/launchpad/get-translations-jobs-working
Merge into: lp:launchpad
Diff against target: 199 lines (+57/-13)
6 files modified
database/schema/security.cfg (+1/-0)
lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py (+13/-2)
lib/canonical/buildd/translationtemplates.py (+4/-2)
lib/lp/testing/fakemethod.py (+11/-5)
lib/lp/translations/model/translationtemplatesbuildbehavior.py (+1/-1)
lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py (+27/-3)
To merge this branch: bzr merge lp:~wgrant/launchpad/get-translations-jobs-working
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+22584@code.launchpad.net

Commit message

Fix the few remaining issues with translation template builds.

Description of the change

This branch fixes the last few issues that are preventing translation template builds from running end-to-end.

 - The slave used os.path.join() with the absolute home directory path as the second argument, which meant that the chroot part of the path was ignored.
 - The slave called addWaiting, when the correct name is addWaitingFile.
 - buildd-manager needed permission to create and update TranslationImportQueueEntries.
 - The build behaviour was forgetting to pass the branch when calling its own _uploadTarball method.

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

Thanks for extending the tests to cover your changes. Very comforting to know that tarball upload is now integration-tested. At the time of writing, just one more tiny change needed: a full stop at the end of the assertion string.

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-03-15 01:26:43 +0000
+++ database/schema/security.cfg 2010-04-01 09:51:39 +0000
@@ -860,6 +860,7 @@
860public.packagesetinclusion = SELECT860public.packagesetinclusion = SELECT
861public.flatpackagesetinclusion = SELECT861public.flatpackagesetinclusion = SELECT
862public.teamparticipation = SELECT862public.teamparticipation = SELECT
863public.translationimportqueueentry = SELECT, INSERT, UPDATE
863864
864[sourcerer]865[sourcerer]
865type=user866type=user
866867
=== modified file 'lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py'
--- lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py 2010-03-18 13:27:47 +0000
+++ lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py 2010-04-01 09:51:39 +0000
@@ -43,6 +43,8 @@
43 def wasCalled(self, name):43 def wasCalled(self, name):
44 return name in self._was_called44 return name in self._was_called
4545
46 addWaitingFile = FakeMethod()
47
4648
47class MockBuildManager(TranslationTemplatesBuildManager):49class MockBuildManager(TranslationTemplatesBuildManager):
48 def __init__(self, *args, **kwargs):50 def __init__(self, *args, **kwargs):
@@ -53,8 +55,6 @@
53 self.commands.append([path]+command)55 self.commands.append([path]+command)
54 return 056 return 0
5557
56 gatherResults = FakeMethod()
57
5858
59class TestTranslationTemplatesBuildManagerIteration(TestCase):59class TestTranslationTemplatesBuildManagerIteration(TestCase):
60 """Run TranslationTemplatesBuildManager through its iteration steps."""60 """Run TranslationTemplatesBuildManager through its iteration steps."""
@@ -110,6 +110,15 @@
110 self.assertEqual(expected_command, self.buildmanager.commands[-1])110 self.assertEqual(expected_command, self.buildmanager.commands[-1])
111 self.assertFalse(self.slave.wasCalled('chrootFail'))111 self.assertFalse(self.slave.wasCalled('chrootFail'))
112112
113 outfile_path = os.path.join(
114 self.chrootdir, self.buildmanager.home[1:],
115 self.buildmanager._resultname)
116 os.makedirs(os.path.dirname(outfile_path))
117
118 outfile = open(outfile_path, 'w')
119 outfile.write("I am a template tarball. Seriously.")
120 outfile.close()
121
113 # The control returns to the DebianBuildManager in the REAP state.122 # The control returns to the DebianBuildManager in the REAP state.
114 self.buildmanager.iterate(0)123 self.buildmanager.iterate(0)
115 expected_command = [124 expected_command = [
@@ -119,6 +128,8 @@
119 TranslationTemplatesBuildState.REAP, self.getState())128 TranslationTemplatesBuildState.REAP, self.getState())
120 self.assertEqual(expected_command, self.buildmanager.commands[-1])129 self.assertEqual(expected_command, self.buildmanager.commands[-1])
121 self.assertFalse(self.slave.wasCalled('buildFail'))130 self.assertFalse(self.slave.wasCalled('buildFail'))
131 self.assertEqual(
132 [((outfile_path,), {})], self.slave.addWaitingFile.calls)
122133
123 def test_iterate_fail_INSTALL(self):134 def test_iterate_fail_INSTALL(self):
124 # See that a failing INSTALL is handled properly.135 # See that a failing INSTALL is handled properly.
125136
=== modified file 'lib/canonical/buildd/translationtemplates.py'
--- lib/canonical/buildd/translationtemplates.py 2010-03-18 13:27:47 +0000
+++ lib/canonical/buildd/translationtemplates.py 2010-04-01 09:51:39 +0000
@@ -63,9 +63,11 @@
63 """Gather the results of the build and add them to the file cache."""63 """Gather the results of the build and add them to the file cache."""
64 # The file is inside the chroot, in the home directory of the buildd64 # The file is inside the chroot, in the home directory of the buildd
65 # user. Should be safe to assume the home dirs are named identically.65 # user. Should be safe to assume the home dirs are named identically.
66 path = os.path.join(self._chroot_path, self.home, self._resultname)66 assert self.home.startswith('/'), "home directory must be absolute."
67
68 path = os.path.join(self._chroot_path, self.home[1:], self._resultname)
67 if os.access(path, os.F_OK):69 if os.access(path, os.F_OK):
68 self._slave.addWaiting(path)70 self._slave.addWaitingFile(path)
6971
70 def iterate_INSTALL(self, success):72 def iterate_INSTALL(self, success):
71 """Installation was done."""73 """Installation was done."""
7274
=== modified file 'lib/lp/testing/fakemethod.py'
--- lib/lp/testing/fakemethod.py 2010-02-11 19:18:39 +0000
+++ lib/lp/testing/fakemethod.py 2010-04-01 09:51:39 +0000
@@ -21,9 +21,6 @@
21 systems.21 systems.
22 """22 """
2323
24 # How many times has this fake method been called?
25 call_count = 0
26
27 def __init__(self, result=None, failure=None):24 def __init__(self, result=None, failure=None):
28 """Set up a fake function or method.25 """Set up a fake function or method.
2926
@@ -33,14 +30,19 @@
33 self.result = result30 self.result = result
34 self.failure = failure31 self.failure = failure
3532
33 # A log of arguments for each call to this method.
34 self.calls = []
35
36 def __call__(self, *args, **kwargs):36 def __call__(self, *args, **kwargs):
37 """Catch an invocation to the method. Increment `call_count`.37 """Catch an invocation to the method.
38
39 Increment `call_count`, and adds the arguments to `calls`.
3840
39 Accepts any and all parameters. Raises the failure passed to41 Accepts any and all parameters. Raises the failure passed to
40 the constructor, if any; otherwise, returns the result value42 the constructor, if any; otherwise, returns the result value
41 passed to the constructor.43 passed to the constructor.
42 """44 """
43 self.call_count += 145 self.calls.append((args, kwargs))
4446
45 if self.failure is None:47 if self.failure is None:
46 return self.result48 return self.result
@@ -49,3 +51,7 @@
49 # possible. That's why this test disables pylint message51 # possible. That's why this test disables pylint message
50 # E0702.52 # E0702.
51 raise self.failure53 raise self.failure
54
55 @property
56 def call_count(self):
57 return len(self.calls)
5258
=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-03-24 12:26:27 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-04-01 09:51:39 +0000
@@ -133,7 +133,7 @@
133 build_id))133 build_id))
134 else:134 else:
135 logger.debug("Uploading translation templates tarball.")135 logger.debug("Uploading translation templates tarball.")
136 self._uploadTarball(tarball, logger)136 self._uploadTarball(queue_item.specific_job.branch, tarball, logger)
137 logger.debug("Upload complete.")137 logger.debug("Upload complete.")
138138
139 queue_item.builder.cleanSlave()139 queue_item.builder.cleanSlave()
140140
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-03-25 23:19:10 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-04-01 09:51:39 +0000
@@ -68,7 +68,7 @@
68 'BuilderStatus.WAITING',68 'BuilderStatus.WAITING',
69 'BuildStatus.OK',69 'BuildStatus.OK',
70 self._status.get('build_id'),70 self._status.get('build_id'),
71 {},71 self._status.get('filemap'),
72 )72 )
7373
7474
@@ -90,19 +90,21 @@
90 Copies its builder from the behavior object.90 Copies its builder from the behavior object.
91 """91 """
92 self.builder = behavior._builder92 self.builder = behavior._builder
93 self.specific_job = behavior.buildfarmjob
93 self.destroySelf = FakeMethod()94 self.destroySelf = FakeMethod()
9495
9596
96class MakeBehaviorMixin(object):97class MakeBehaviorMixin(object):
97 """Provide common test methods."""98 """Provide common test methods."""
9899
99 def makeBehavior(self):100 def makeBehavior(self, branch=None):
100 """Create a TranslationTemplatesBuildBehavior.101 """Create a TranslationTemplatesBuildBehavior.
101102
102 Anything that might communicate with build slaves and such103 Anything that might communicate with build slaves and such
103 (which we can't really do here) is mocked up.104 (which we can't really do here) is mocked up.
104 """105 """
105 specific_job = self.factory.makeTranslationTemplatesBuildJob()106 specific_job = self.factory.makeTranslationTemplatesBuildJob(
107 branch=branch)
106 behavior = IBuildFarmJobBehavior(specific_job)108 behavior = IBuildFarmJobBehavior(specific_job)
107 slave = FakeSlave(BuildStatus.NEEDSBUILD)109 slave = FakeSlave(BuildStatus.NEEDSBUILD)
108 behavior._builder = FakeBuilder(slave)110 behavior._builder = FakeBuilder(slave)
@@ -270,6 +272,28 @@
270 entries = self.queue.getAllEntries(target=self.productseries)272 entries = self.queue.getAllEntries(target=self.productseries)
271 self.assertEqual(self.branch.owner, entries[0].importer)273 self.assertEqual(self.branch.owner, entries[0].importer)
272274
275 def test_updateBuild_WAITING_uploads(self):
276 behavior = self.makeBehavior(branch=self.branch)
277 behavior._getChroot = FakeChroot
278 queue_item = FakeBuildQueue(behavior)
279 builder = behavior._builder
280
281 behavior.dispatchBuildToSlave(queue_item, logging)
282
283 builder.slave.getFile.result = open(self.dummy_tar)
284 builder.slave._status['filemap'] = {
285 'translation-templates.tar.gz': 'foo'}
286 slave_status = behavior.slaveStatus(builder.slave.status())
287 behavior.updateBuild_WAITING(queue_item, slave_status, None, logging)
288
289 entries = self.queue.getAllEntries(target=self.productseries)
290 expected_templates = [
291 'po/messages.pot',
292 'po-other/other.pot',
293 'po-thethird/templ3.pot'
294 ]
295 self.assertContentEqual(expected_templates, self._getPaths(entries))
296
273297
274def test_suite():298def test_suite():
275 return TestLoader().loadTestsFromName(__name__)299 return TestLoader().loadTestsFromName(__name__)