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
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2010-03-15 01:26:43 +0000
3+++ database/schema/security.cfg 2010-04-01 09:51:39 +0000
4@@ -860,6 +860,7 @@
5 public.packagesetinclusion = SELECT
6 public.flatpackagesetinclusion = SELECT
7 public.teamparticipation = SELECT
8+public.translationimportqueueentry = SELECT, INSERT, UPDATE
9
10 [sourcerer]
11 type=user
12
13=== modified file 'lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py'
14--- lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py 2010-03-18 13:27:47 +0000
15+++ lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py 2010-04-01 09:51:39 +0000
16@@ -43,6 +43,8 @@
17 def wasCalled(self, name):
18 return name in self._was_called
19
20+ addWaitingFile = FakeMethod()
21+
22
23 class MockBuildManager(TranslationTemplatesBuildManager):
24 def __init__(self, *args, **kwargs):
25@@ -53,8 +55,6 @@
26 self.commands.append([path]+command)
27 return 0
28
29- gatherResults = FakeMethod()
30-
31
32 class TestTranslationTemplatesBuildManagerIteration(TestCase):
33 """Run TranslationTemplatesBuildManager through its iteration steps."""
34@@ -110,6 +110,15 @@
35 self.assertEqual(expected_command, self.buildmanager.commands[-1])
36 self.assertFalse(self.slave.wasCalled('chrootFail'))
37
38+ outfile_path = os.path.join(
39+ self.chrootdir, self.buildmanager.home[1:],
40+ self.buildmanager._resultname)
41+ os.makedirs(os.path.dirname(outfile_path))
42+
43+ outfile = open(outfile_path, 'w')
44+ outfile.write("I am a template tarball. Seriously.")
45+ outfile.close()
46+
47 # The control returns to the DebianBuildManager in the REAP state.
48 self.buildmanager.iterate(0)
49 expected_command = [
50@@ -119,6 +128,8 @@
51 TranslationTemplatesBuildState.REAP, self.getState())
52 self.assertEqual(expected_command, self.buildmanager.commands[-1])
53 self.assertFalse(self.slave.wasCalled('buildFail'))
54+ self.assertEqual(
55+ [((outfile_path,), {})], self.slave.addWaitingFile.calls)
56
57 def test_iterate_fail_INSTALL(self):
58 # See that a failing INSTALL is handled properly.
59
60=== modified file 'lib/canonical/buildd/translationtemplates.py'
61--- lib/canonical/buildd/translationtemplates.py 2010-03-18 13:27:47 +0000
62+++ lib/canonical/buildd/translationtemplates.py 2010-04-01 09:51:39 +0000
63@@ -63,9 +63,11 @@
64 """Gather the results of the build and add them to the file cache."""
65 # The file is inside the chroot, in the home directory of the buildd
66 # user. Should be safe to assume the home dirs are named identically.
67- path = os.path.join(self._chroot_path, self.home, self._resultname)
68+ assert self.home.startswith('/'), "home directory must be absolute."
69+
70+ path = os.path.join(self._chroot_path, self.home[1:], self._resultname)
71 if os.access(path, os.F_OK):
72- self._slave.addWaiting(path)
73+ self._slave.addWaitingFile(path)
74
75 def iterate_INSTALL(self, success):
76 """Installation was done."""
77
78=== modified file 'lib/lp/testing/fakemethod.py'
79--- lib/lp/testing/fakemethod.py 2010-02-11 19:18:39 +0000
80+++ lib/lp/testing/fakemethod.py 2010-04-01 09:51:39 +0000
81@@ -21,9 +21,6 @@
82 systems.
83 """
84
85- # How many times has this fake method been called?
86- call_count = 0
87-
88 def __init__(self, result=None, failure=None):
89 """Set up a fake function or method.
90
91@@ -33,14 +30,19 @@
92 self.result = result
93 self.failure = failure
94
95+ # A log of arguments for each call to this method.
96+ self.calls = []
97+
98 def __call__(self, *args, **kwargs):
99- """Catch an invocation to the method. Increment `call_count`.
100+ """Catch an invocation to the method.
101+
102+ Increment `call_count`, and adds the arguments to `calls`.
103
104 Accepts any and all parameters. Raises the failure passed to
105 the constructor, if any; otherwise, returns the result value
106 passed to the constructor.
107 """
108- self.call_count += 1
109+ self.calls.append((args, kwargs))
110
111 if self.failure is None:
112 return self.result
113@@ -49,3 +51,7 @@
114 # possible. That's why this test disables pylint message
115 # E0702.
116 raise self.failure
117+
118+ @property
119+ def call_count(self):
120+ return len(self.calls)
121
122=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
123--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-03-24 12:26:27 +0000
124+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-04-01 09:51:39 +0000
125@@ -133,7 +133,7 @@
126 build_id))
127 else:
128 logger.debug("Uploading translation templates tarball.")
129- self._uploadTarball(tarball, logger)
130+ self._uploadTarball(queue_item.specific_job.branch, tarball, logger)
131 logger.debug("Upload complete.")
132
133 queue_item.builder.cleanSlave()
134
135=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
136--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-03-25 23:19:10 +0000
137+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-04-01 09:51:39 +0000
138@@ -68,7 +68,7 @@
139 'BuilderStatus.WAITING',
140 'BuildStatus.OK',
141 self._status.get('build_id'),
142- {},
143+ self._status.get('filemap'),
144 )
145
146
147@@ -90,19 +90,21 @@
148 Copies its builder from the behavior object.
149 """
150 self.builder = behavior._builder
151+ self.specific_job = behavior.buildfarmjob
152 self.destroySelf = FakeMethod()
153
154
155 class MakeBehaviorMixin(object):
156 """Provide common test methods."""
157
158- def makeBehavior(self):
159+ def makeBehavior(self, branch=None):
160 """Create a TranslationTemplatesBuildBehavior.
161
162 Anything that might communicate with build slaves and such
163 (which we can't really do here) is mocked up.
164 """
165- specific_job = self.factory.makeTranslationTemplatesBuildJob()
166+ specific_job = self.factory.makeTranslationTemplatesBuildJob(
167+ branch=branch)
168 behavior = IBuildFarmJobBehavior(specific_job)
169 slave = FakeSlave(BuildStatus.NEEDSBUILD)
170 behavior._builder = FakeBuilder(slave)
171@@ -270,6 +272,28 @@
172 entries = self.queue.getAllEntries(target=self.productseries)
173 self.assertEqual(self.branch.owner, entries[0].importer)
174
175+ def test_updateBuild_WAITING_uploads(self):
176+ behavior = self.makeBehavior(branch=self.branch)
177+ behavior._getChroot = FakeChroot
178+ queue_item = FakeBuildQueue(behavior)
179+ builder = behavior._builder
180+
181+ behavior.dispatchBuildToSlave(queue_item, logging)
182+
183+ builder.slave.getFile.result = open(self.dummy_tar)
184+ builder.slave._status['filemap'] = {
185+ 'translation-templates.tar.gz': 'foo'}
186+ slave_status = behavior.slaveStatus(builder.slave.status())
187+ behavior.updateBuild_WAITING(queue_item, slave_status, None, logging)
188+
189+ entries = self.queue.getAllEntries(target=self.productseries)
190+ expected_templates = [
191+ 'po/messages.pot',
192+ 'po-other/other.pot',
193+ 'po-thethird/templ3.pot'
194+ ]
195+ self.assertContentEqual(expected_templates, self._getPaths(entries))
196+
197
198 def test_suite():
199 return TestLoader().loadTestsFromName(__name__)