Launchpad itself

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

  1. bug-520651
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Muharem Hrnjadovic
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-520651
Merge into: lp:launchpad
Diff against target: 132 lines (+56/-21)
2 files modified
lib/canonical/buildd/generate_translation_templates.py (+7/-2)
lib/canonical/buildd/tests/test_generate_translation_templates.py (+49/-19)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-520651
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) code Approve
Review via email: mp+20632@code.launchpad.net

Commit message

Let TranslationTemplatesBuild slaves retrieve branch contents.

Description of the change

= Bug 520651 =

This ticks off one to-do item on the project of using the build farm to generate translation templates based on source code from a bzr branch. The item is: to make the slave code retrieve the source code from the branch.

I used the equivalent of a "bzr export" for this, which is the lightest-weight operation I could find that did the job. All it retrieves is the contents of the branch, in its current revision.

The test is not entirely realistic: it uses the pull URL for the branch (which is implemented using a Launchpad custom protocol that maps to file://) instead of the http public URL that the real build slave will get. The custom protocol requires the Launchpad bzr plugins, and special access to the bzr server. The real slave will go through plain http. The http way is more manageable when it comes to firewall holes, but as far as I've been able to find out from various knowledgeable developers, impossible to integration-test. Our test setup doesn't include http access to branches.

No lint. For Q/A we'll need some intimate time with dogfood and instructions on its use; if it fails we'll get TranslationTemplatesBuildJobs failing with "not a branch" errors.

To test,
{{{
./bin/test -vv -t test_generate_translation_templates
}}}

You'll notice that the test no longer needs a MockGenerateTranslationTemplates class. It wouldn't have suited the newly added test case anyway. Applying the new FakeMethod helper is enough.

In case the extraction of a method to create and populate a branch seems overweight: I have a feeling it may be a candidate for future reuse in the same test, but isn't quite worth moving into the factory either.

Jeroen

To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Looks good!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/buildd/generate_translation_templates.py'
2--- lib/canonical/buildd/generate_translation_templates.py 2010-02-25 13:43:22 +0000
3+++ lib/canonical/buildd/generate_translation_templates.py 2010-03-04 09:44:21 +0000
4@@ -7,8 +7,12 @@
5 import os.path
6 import sys
7
8+from bzrlib.branch import Branch
9+from bzrlib.export import export
10+
11 from lp.translations.pottery.buildd import generate_pots
12
13+
14 class GenerateTranslationTemplates:
15 """Script to generate translation templates from a branch."""
16
17@@ -39,8 +43,9 @@
18 The branch is checked out to the location specified by
19 `self.branch_dir`.
20 """
21- # XXX JeroenVermeulen 2010-02-11 bug=520651: Read the contents
22- # of the branch using bzrlib.
23+ branch = Branch.open(branch_url)
24+ rev_tree = branch.basis_tree()
25+ export(rev_tree, self.branch_dir)
26
27 def generate(self):
28 """Do It. Generate templates."""
29
30=== modified file 'lib/canonical/buildd/tests/test_generate_translation_templates.py'
31--- lib/canonical/buildd/tests/test_generate_translation_templates.py 2010-01-22 06:07:27 +0000
32+++ lib/canonical/buildd/tests/test_generate_translation_templates.py 2010-03-04 09:44:21 +0000
33@@ -4,37 +4,31 @@
34 import os
35 from unittest import TestLoader
36
37-from lp.testing import TestCase
38+from lp.testing.fakemethod import FakeMethod
39
40 from canonical.buildd.generate_translation_templates import (
41 GenerateTranslationTemplates)
42
43 from canonical.launchpad.ftests.script import run_script
44 from canonical.testing.layers import ZopelessDatabaseLayer
45-
46-
47-class MockGenerateTranslationTemplates(GenerateTranslationTemplates):
48- """A GenerateTranslationTemplates with mocked _checkout."""
49- # Records, for testing purposes, whether this object checked out a
50- # branch.
51- checked_out_branch = False
52-
53- def _checkout(self, branch_url):
54- assert not self.checked_out_branch, "Checking out branch again!"
55- self.checked_out_branch = True
56-
57-
58-class TestGenerateTranslationTemplates(TestCase):
59+from lp.code.model.directbranchcommit import DirectBranchCommit
60+from lp.testing import TestCaseWithFactory
61+
62+
63+class TestGenerateTranslationTemplates(TestCaseWithFactory):
64 """Test slave-side generate-translation-templates script."""
65+ layer = ZopelessDatabaseLayer
66+
67 def test_getBranch_url(self):
68 # If passed a branch URL, the template generation script will
69 # check out that branch into a directory called "source-tree."
70 branch_url = 'lp://~my/translation/branch'
71
72- generator = MockGenerateTranslationTemplates(branch_url)
73+ generator = GenerateTranslationTemplates(branch_url)
74+ generator._checkout = FakeMethod()
75 generator._getBranch()
76
77- self.assertTrue(generator.checked_out_branch)
78+ self.assertEqual(1, generator._checkout.call_count)
79 self.assertTrue(generator.branch_dir.endswith('source-tree'))
80
81 def test_getBranch_dir(self):
82@@ -42,12 +36,48 @@
83 # works directly in that directory.
84 branch_dir = '/home/me/branch'
85
86- generator = MockGenerateTranslationTemplates(branch_dir)
87+ generator = GenerateTranslationTemplates(branch_dir)
88+ generator._checkout = FakeMethod()
89 generator._getBranch()
90
91- self.assertFalse(generator.checked_out_branch)
92+ self.assertEqual(0, generator._checkout.call_count)
93 self.assertEqual(branch_dir, generator.branch_dir)
94
95+ def _createBranch(self, content_map=None):
96+ """Create a working branch.
97+
98+ :param content_map: optional dict mapping file names to file contents.
99+ Each of these files with their contents will be written to the
100+ branch.
101+
102+ :return: a fresh lp.code.model.Branch backed by a real bzr branch.
103+ """
104+ db_branch, tree = self.create_branch_and_tree(hosted=True)
105+ populist = DirectBranchCommit(db_branch)
106+ last_revision = populist.bzrbranch.last_revision()
107+ db_branch.last_scanned_id = populist.last_scanned_id = last_revision
108+
109+ if content_map is not None:
110+ for name, contents in content_map.iteritems():
111+ populist.writeFile(name, contents)
112+ populist.commit("Populating branch.")
113+
114+ return db_branch
115+
116+ def test_getBranch_bzr(self):
117+ # _getBranch can retrieve branch contents from a branch URL.
118+ self.useBzrBranches()
119+ marker_text = "Ceci n'est pas cet branch."
120+ branch = self._createBranch({'marker.txt': marker_text})
121+ branch_url = branch.getPullURL()
122+
123+ generator = GenerateTranslationTemplates(branch_url)
124+ generator.branch_dir = self.makeTemporaryDirectory()
125+ generator._getBranch()
126+
127+ marker_file = file(os.path.join(generator.branch_dir, 'marker.txt'))
128+ self.assertEqual(marker_text, marker_file.read())
129+
130 def test_script(self):
131 tempdir = self.makeTemporaryDirectory()
132 (retval, out, err) = run_script(