Launchpad itself

Merge lp:~henninge/launchpad/bug-517080-pottery-split into lp:launchpad

  1. bug-517080-pottery-split
  2. Merge into devel
Proposed by Henning Eggers
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~henninge/launchpad/bug-517080-pottery-split
Merge into: lp:launchpad
Diff against target: 319 lines (+104/-47)
3 files modified
lib/lp/testing/__init__.py (+8/-8)
lib/lp/translations/pottery/detect_intltool.py (+35/-0)
lib/lp/translations/tests/test_pottery_detect_intltool.py (+61/-39)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-517080-pottery-split
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+19325@code.launchpad.net

Commit message

Provided check for possible intltool structure in a bzrlib.Tree.

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 517080 =

Although the bug talks about "splitting up", this branch really provides an extra function that simply checks for "POTFILES.in" existing in the tree. There was no code that could be re-used because the existing code works on the filesystem while this new function works directly on a bzrlib.Tree object.

== Implementation details ==

The code is straight forward - walk the tree and report success if a "POTFILES.in" is found.

This makes use of the new "with" statement with a context manager to acquire a lock on the tree. This seems to be a perfect use case for this new language feature and eventually the context manager should become part of bzrlib.Tree.

== Tests ==

The new test case uses the same packages as the tests that ran on the filesystem but uses BzrDir to create a working tree from the directories. The files in the trees don't actually get added and committed to the tree, simply because for the function to work this is not necessary.

To share some functionality between test cases, a mix-in class was created and some methods moved into it. This bloats the diff quite a bit because the method names lose a leading underscore.

bin/test -vvt pottery

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/translations/pottery/detect_intltool.py
  lib/lp/translations/tests/test_pottery_detect_intltool.py

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks for making the changes discussed in IRC. The new comments in the test are much clearer, and I think you've done a Good Thing™ in moving useTempDir up from TestCaseWithFactory into lp.testing.TestCase.

The code is good, with nice use of "with." Two more tiny things, both in the docstring for is_intltool_structure:

 * Say "whether" in this case, not "if." But that does make the line a bit long. Personally I prefer to phrase these descriptions as questions: "Does this source tree look like it's set up for intltool?"

 * Please make it state what "tree" is supposed to be.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/testing/__init__.py'
2--- lib/lp/testing/__init__.py 2010-02-03 19:29:27 +0000
3+++ lib/lp/testing/__init__.py 2010-02-15 15:45:30 +0000
4@@ -368,6 +368,14 @@
5 % (expected_count, len(statements), "\n".join(statements)))
6 return ret
7
8+ def useTempDir(self):
9+ """Use a temporary directory for this test."""
10+ tempdir = tempfile.mkdtemp()
11+ self.addCleanup(lambda: shutil.rmtree(tempdir))
12+ cwd = os.getcwd()
13+ os.chdir(tempdir)
14+ self.addCleanup(lambda: os.chdir(cwd))
15+
16
17 class TestCaseWithFactory(TestCase):
18
19@@ -379,14 +387,6 @@
20 self.factory = LaunchpadObjectFactory()
21 self.real_bzr_server = False
22
23- def useTempDir(self):
24- """Use a temporary directory for this test."""
25- tempdir = tempfile.mkdtemp()
26- self.addCleanup(lambda: shutil.rmtree(tempdir))
27- cwd = os.getcwd()
28- os.chdir(tempdir)
29- self.addCleanup(lambda: os.chdir(cwd))
30-
31 def getUserBrowser(self, url=None):
32 """Return a Browser logged in as a fresh user, maybe opened at `url`.
33 """
34
35=== modified file 'lib/lp/translations/pottery/detect_intltool.py'
36--- lib/lp/translations/pottery/detect_intltool.py 2009-12-23 18:10:37 +0000
37+++ lib/lp/translations/pottery/detect_intltool.py 2010-02-15 15:45:30 +0000
38@@ -6,6 +6,8 @@
39 """Functions to detect if intltool can be used to generate a POT file for the
40 package in the current directory."""
41
42+from __future__ import with_statement
43+
44 __metaclass__ = type
45 __all__ = [
46 'check_potfiles_in',
47@@ -20,6 +22,39 @@
48 from subprocess import call
49
50
51+class ReadLockTree(object):
52+ """Context manager to claim a read lock on a bzr tree."""
53+
54+ def __init__(self, tree):
55+ self.tree = tree
56+
57+ def __enter__(self):
58+ self.tree.lock_read()
59+
60+ def __exit__(self, exc_type, exc_val, exc_tb):
61+ self.tree.unlock()
62+ return False
63+
64+
65+def is_intltool_structure(tree):
66+ """Does this source tree look like it's set up for intltool?
67+
68+ Currently this just checks for the existence of POTFILES.in.
69+
70+ :param tree: A bzrlib.Tree object to search for the intltool structure.
71+ :returns: True if signs of an intltool structure were found.
72+ """
73+ with ReadLockTree(tree):
74+ for thedir, files in tree.walkdirs():
75+ for afile in files:
76+ file_path, file_name, file_type = afile[:3]
77+ if file_type != 'file':
78+ continue
79+ if file_name == "POTFILES.in":
80+ return True
81+ return False
82+
83+
84 def find_potfiles_in():
85 """Search the current directory and its subdirectories for POTFILES.in.
86
87
88=== modified file 'lib/lp/translations/tests/test_pottery_detect_intltool.py'
89--- lib/lp/translations/tests/test_pottery_detect_intltool.py 2010-01-28 09:05:00 +0000
90+++ lib/lp/translations/tests/test_pottery_detect_intltool.py 2010-02-15 15:45:30 +0000
91@@ -10,59 +10,56 @@
92 from StringIO import StringIO
93 from textwrap import dedent
94
95+from bzrlib.bzrdir import BzrDir
96 from canonical.launchpad.scripts.tests import run_script
97 from lp.translations.pottery.detect_intltool import (
98 ConfigFile, check_potfiles_in, find_intltool_dirs, find_potfiles_in,
99- get_translation_domain)
100+ get_translation_domain, is_intltool_structure)
101 from lp.testing import TestCase
102
103-class TestDetectIntltool(TestCase):
104-
105- def setUp(self):
106- super(TestDetectIntltool, self).setUp()
107- # Determine test directory and create temporary working directory.
108- self.curdir = os.getcwd()
109- self.testdir = os.path.join(self.curdir, os.path.dirname(__file__))
110- self.workdir = tempfile.mkdtemp()
111- os.chdir(self.workdir)
112-
113- def tearDown(self):
114- # Remove temporary directory.
115- os.chdir(self.curdir)
116- shutil.rmtree(self.workdir)
117- super(TestDetectIntltool, self).tearDown()
118-
119- def _prepare_package(self, packagename):
120- # Unpack the specified pacakge to run the test against it.
121- # Change to this directory.
122- packagepath = os.path.join(self.testdir, packagename+".tar.bz2")
123+
124+class SetupTestPackageMixin(object):
125+
126+ def prepare_package(self, packagename):
127+ """Unpack the specified package in a temporary directory.
128+
129+ Change into the package's directory.
130+ """
131+ # First build the path for the package.
132+ packagepath = os.path.join(
133+ os.getcwd(), os.path.dirname(__file__), packagename + ".tar.bz2")
134+ # Then change into the temporary directory and unpack it.
135+ self.useTempDir()
136 tar = tarfile.open(packagepath, "r:bz2")
137 tar.extractall()
138 tar.close()
139 os.chdir(packagename)
140
141+
142+class TestDetectIntltool(TestCase, SetupTestPackageMixin):
143+
144 def test_detect_potfiles_in(self):
145 # Find POTFILES.in in a package with multiple dirs when only one has
146 # POTFILES.in.
147- self._prepare_package("intltool_POTFILES_in_1")
148+ self.prepare_package("intltool_POTFILES_in_1")
149 dirs = find_potfiles_in()
150 self.assertContentEqual(["./po-intltool"], dirs)
151
152 def test_detect_potfiles_in_module(self):
153 # Find POTFILES.in in a package with POTFILES.in at different levels.
154- self._prepare_package("intltool_POTFILES_in_2")
155+ self.prepare_package("intltool_POTFILES_in_2")
156 dirs = find_potfiles_in()
157 self.assertContentEqual(["./po", "./module1/po"], dirs)
158
159 def test_check_potfiles_in_content_ok(self):
160 # Ideally all files listed in POTFILES.in exist in the source package.
161- self._prepare_package("intltool_single_ok")
162+ self.prepare_package("intltool_single_ok")
163 self.assertTrue(check_potfiles_in("./po"))
164
165 def test_check_potfiles_in_content_ok_file_added(self):
166 # If a file is not listed in POTFILES.in, the file is still good for
167 # our purposes.
168- self._prepare_package("intltool_single_ok")
169+ self.prepare_package("intltool_single_ok")
170 added_file = file("./src/sourcefile_new.c", "w")
171 added_file.write("/* Test file. */")
172 added_file.close()
173@@ -72,53 +69,53 @@
174 # If a file is missing that is listed in POTFILES.in, the file
175 # intltool structure is probably broken and cannot be used for
176 # our purposes.
177- self._prepare_package("intltool_single_ok")
178+ self.prepare_package("intltool_single_ok")
179 os.remove("./src/sourcefile1.c")
180 self.assertFalse(check_potfiles_in("./po"))
181
182 def test_check_potfiles_in_wrong_directory(self):
183 # Passing in the wrong directory will cause the check to fail
184 # gracefully and return False.
185- self._prepare_package("intltool_single_ok")
186+ self.prepare_package("intltool_single_ok")
187 self.assertFalse(check_potfiles_in("./foo"))
188
189 def test_find_intltool_dirs(self):
190 # Complete run: find all directories with intltool structure.
191- self._prepare_package("intltool_full_ok")
192+ self.prepare_package("intltool_full_ok")
193 self.assertEqual(
194 ["./po-module1", "./po-module2"], find_intltool_dirs())
195
196 def test_find_intltool_dirs_broken(self):
197 # Complete run: part of the intltool structure is broken.
198- self._prepare_package("intltool_full_ok")
199+ self.prepare_package("intltool_full_ok")
200 os.remove("./src/module1/sourcefile1.c")
201 self.assertEqual(
202 ["./po-module2"], find_intltool_dirs())
203
204 def test_get_translation_domain_makevars(self):
205 # Find a translation domain in Makevars.
206- self._prepare_package("intltool_domain_makevars")
207+ self.prepare_package("intltool_domain_makevars")
208 self.assertEqual(
209 "translationdomain",
210 get_translation_domain("po"))
211
212 def test_get_translation_domain_makefile_in_in(self):
213 # Find a translation domain in Makefile.in.in.
214- self._prepare_package("intltool_domain_makefile_in_in")
215+ self.prepare_package("intltool_domain_makefile_in_in")
216 self.assertEqual(
217 "packagename-in-in",
218 get_translation_domain("po"))
219
220 def test_get_translation_domain_configure_ac(self):
221 # Find a translation domain in configure.ac.
222- self._prepare_package("intltool_domain_configure_ac")
223+ self.prepare_package("intltool_domain_configure_ac")
224 self.assertEqual(
225 "packagename-ac",
226 get_translation_domain("po"))
227
228 def test_get_translation_domain_configure_in(self):
229 # Find a translation domain in configure.in.
230- self._prepare_package("intltool_domain_configure_in")
231+ self.prepare_package("intltool_domain_configure_in")
232 self.assertEqual(
233 "packagename-in",
234 get_translation_domain("po"))
235@@ -126,7 +123,7 @@
236 def test_get_translation_domain_makefile_in_in_substitute(self):
237 # Find a translation domain in Makefile.in.in with substitution from
238 # configure.ac.
239- self._prepare_package("intltool_domain_makefile_in_in_substitute")
240+ self.prepare_package("intltool_domain_makefile_in_in_substitute")
241 self.assertEqual(
242 "domainname-ac-in-in",
243 get_translation_domain("po"))
244@@ -135,7 +132,7 @@
245 # Find a translation domain in Makefile.in.in with substitution from
246 # configure.ac from a variable with the same name as in
247 # Makefile.in.in.
248- self._prepare_package(
249+ self.prepare_package(
250 "intltool_domain_makefile_in_in_substitute_same_name")
251 self.assertEqual(
252 "packagename-ac-in-in",
253@@ -144,7 +141,7 @@
254 def test_get_translation_domain_makefile_in_in_substitute_same_file(self):
255 # Find a translation domain in Makefile.in.in with substitution from
256 # the same file.
257- self._prepare_package(
258+ self.prepare_package(
259 "intltool_domain_makefile_in_in_substitute_same_file")
260 self.assertEqual(
261 "domain-in-in-in-in",
262@@ -153,14 +150,14 @@
263 def test_get_translation_domain_makefile_in_in_substitute_broken(self):
264 # Find no translation domain in Makefile.in.in when the substitution
265 # cannot be fulfilled.
266- self._prepare_package(
267+ self.prepare_package(
268 "intltool_domain_makefile_in_in_substitute_broken")
269 self.assertIs(None, get_translation_domain("po"))
270
271 def test_get_translation_domain_configure_in_substitute_version(self):
272 # Find a translation domain in configure.in with Makefile-style
273 # substitution from the same file.
274- self._prepare_package(
275+ self.prepare_package(
276 "intltool_domain_configure_in_substitute_version")
277 self.assertEqual(
278 "domainname-in42",
279@@ -168,7 +165,7 @@
280
281 def test_pottery_check_intltool_script(self):
282 # Let the script run to see it works fine.
283- self._prepare_package("intltool_full_ok")
284+ self.prepare_package("intltool_full_ok")
285
286 return_code, stdout, stderr = run_script(
287 'scripts/rosetta/pottery-check-intltool.py', [])
288@@ -179,6 +176,31 @@
289 """), stdout)
290
291
292+class TestDetectIntltoolInBzrTree(TestCase, SetupTestPackageMixin):
293+
294+ def prepare_tree(self):
295+ return BzrDir.create_standalone_workingtree(".")
296+
297+ def test_detect_intltool_structure(self):
298+ # Detect a simple intltool structure.
299+ self.prepare_package("intltool_POTFILES_in_1")
300+ tree = self.prepare_tree()
301+ self.assertTrue(is_intltool_structure(tree))
302+
303+ def test_detect_no_intltool_structure(self):
304+ # If no POTFILES.in exists, no intltool structure is assumed.
305+ self.prepare_package("intltool_POTFILES_in_1")
306+ os.remove("./po-intltool/POTFILES.in")
307+ tree = self.prepare_tree()
308+ self.assertFalse(is_intltool_structure(tree))
309+
310+ def test_detect_intltool_structure_module(self):
311+ # Detect an intltool structure in subdirectories.
312+ self.prepare_package("intltool_POTFILES_in_2")
313+ tree = self.prepare_tree()
314+ self.assertTrue(is_intltool_structure(tree))
315+
316+
317 class TestConfigFile(TestCase):
318
319 def setUp(self):