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
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2010-02-03 19:29:27 +0000
+++ lib/lp/testing/__init__.py 2010-02-15 15:45:30 +0000
@@ -368,6 +368,14 @@
368 % (expected_count, len(statements), "\n".join(statements)))368 % (expected_count, len(statements), "\n".join(statements)))
369 return ret369 return ret
370370
371 def useTempDir(self):
372 """Use a temporary directory for this test."""
373 tempdir = tempfile.mkdtemp()
374 self.addCleanup(lambda: shutil.rmtree(tempdir))
375 cwd = os.getcwd()
376 os.chdir(tempdir)
377 self.addCleanup(lambda: os.chdir(cwd))
378
371379
372class TestCaseWithFactory(TestCase):380class TestCaseWithFactory(TestCase):
373381
@@ -379,14 +387,6 @@
379 self.factory = LaunchpadObjectFactory()387 self.factory = LaunchpadObjectFactory()
380 self.real_bzr_server = False388 self.real_bzr_server = False
381389
382 def useTempDir(self):
383 """Use a temporary directory for this test."""
384 tempdir = tempfile.mkdtemp()
385 self.addCleanup(lambda: shutil.rmtree(tempdir))
386 cwd = os.getcwd()
387 os.chdir(tempdir)
388 self.addCleanup(lambda: os.chdir(cwd))
389
390 def getUserBrowser(self, url=None):390 def getUserBrowser(self, url=None):
391 """Return a Browser logged in as a fresh user, maybe opened at `url`.391 """Return a Browser logged in as a fresh user, maybe opened at `url`.
392 """392 """
393393
=== modified file 'lib/lp/translations/pottery/detect_intltool.py'
--- lib/lp/translations/pottery/detect_intltool.py 2009-12-23 18:10:37 +0000
+++ lib/lp/translations/pottery/detect_intltool.py 2010-02-15 15:45:30 +0000
@@ -6,6 +6,8 @@
6"""Functions to detect if intltool can be used to generate a POT file for the6"""Functions to detect if intltool can be used to generate a POT file for the
7package in the current directory."""7package in the current directory."""
88
9from __future__ import with_statement
10
9__metaclass__ = type11__metaclass__ = type
10__all__ = [12__all__ = [
11 'check_potfiles_in',13 'check_potfiles_in',
@@ -20,6 +22,39 @@
20from subprocess import call22from subprocess import call
2123
2224
25class ReadLockTree(object):
26 """Context manager to claim a read lock on a bzr tree."""
27
28 def __init__(self, tree):
29 self.tree = tree
30
31 def __enter__(self):
32 self.tree.lock_read()
33
34 def __exit__(self, exc_type, exc_val, exc_tb):
35 self.tree.unlock()
36 return False
37
38
39def is_intltool_structure(tree):
40 """Does this source tree look like it's set up for intltool?
41
42 Currently this just checks for the existence of POTFILES.in.
43
44 :param tree: A bzrlib.Tree object to search for the intltool structure.
45 :returns: True if signs of an intltool structure were found.
46 """
47 with ReadLockTree(tree):
48 for thedir, files in tree.walkdirs():
49 for afile in files:
50 file_path, file_name, file_type = afile[:3]
51 if file_type != 'file':
52 continue
53 if file_name == "POTFILES.in":
54 return True
55 return False
56
57
23def find_potfiles_in():58def find_potfiles_in():
24 """Search the current directory and its subdirectories for POTFILES.in.59 """Search the current directory and its subdirectories for POTFILES.in.
2560
2661
=== modified file 'lib/lp/translations/tests/test_pottery_detect_intltool.py'
--- lib/lp/translations/tests/test_pottery_detect_intltool.py 2010-01-28 09:05:00 +0000
+++ lib/lp/translations/tests/test_pottery_detect_intltool.py 2010-02-15 15:45:30 +0000
@@ -10,59 +10,56 @@
10from StringIO import StringIO10from StringIO import StringIO
11from textwrap import dedent11from textwrap import dedent
1212
13from bzrlib.bzrdir import BzrDir
13from canonical.launchpad.scripts.tests import run_script14from canonical.launchpad.scripts.tests import run_script
14from lp.translations.pottery.detect_intltool import (15from lp.translations.pottery.detect_intltool import (
15 ConfigFile, check_potfiles_in, find_intltool_dirs, find_potfiles_in,16 ConfigFile, check_potfiles_in, find_intltool_dirs, find_potfiles_in,
16 get_translation_domain)17 get_translation_domain, is_intltool_structure)
17from lp.testing import TestCase18from lp.testing import TestCase
1819
19class TestDetectIntltool(TestCase):20
2021class SetupTestPackageMixin(object):
21 def setUp(self):22
22 super(TestDetectIntltool, self).setUp()23 def prepare_package(self, packagename):
23 # Determine test directory and create temporary working directory.24 """Unpack the specified package in a temporary directory.
24 self.curdir = os.getcwd()25
25 self.testdir = os.path.join(self.curdir, os.path.dirname(__file__))26 Change into the package's directory.
26 self.workdir = tempfile.mkdtemp()27 """
27 os.chdir(self.workdir)28 # First build the path for the package.
2829 packagepath = os.path.join(
29 def tearDown(self):30 os.getcwd(), os.path.dirname(__file__), packagename + ".tar.bz2")
30 # Remove temporary directory.31 # Then change into the temporary directory and unpack it.
31 os.chdir(self.curdir)32 self.useTempDir()
32 shutil.rmtree(self.workdir)
33 super(TestDetectIntltool, self).tearDown()
34
35 def _prepare_package(self, packagename):
36 # Unpack the specified pacakge to run the test against it.
37 # Change to this directory.
38 packagepath = os.path.join(self.testdir, packagename+".tar.bz2")
39 tar = tarfile.open(packagepath, "r:bz2")33 tar = tarfile.open(packagepath, "r:bz2")
40 tar.extractall()34 tar.extractall()
41 tar.close()35 tar.close()
42 os.chdir(packagename)36 os.chdir(packagename)
4337
38
39class TestDetectIntltool(TestCase, SetupTestPackageMixin):
40
44 def test_detect_potfiles_in(self):41 def test_detect_potfiles_in(self):
45 # Find POTFILES.in in a package with multiple dirs when only one has42 # Find POTFILES.in in a package with multiple dirs when only one has
46 # POTFILES.in.43 # POTFILES.in.
47 self._prepare_package("intltool_POTFILES_in_1")44 self.prepare_package("intltool_POTFILES_in_1")
48 dirs = find_potfiles_in()45 dirs = find_potfiles_in()
49 self.assertContentEqual(["./po-intltool"], dirs)46 self.assertContentEqual(["./po-intltool"], dirs)
5047
51 def test_detect_potfiles_in_module(self):48 def test_detect_potfiles_in_module(self):
52 # Find POTFILES.in in a package with POTFILES.in at different levels.49 # Find POTFILES.in in a package with POTFILES.in at different levels.
53 self._prepare_package("intltool_POTFILES_in_2")50 self.prepare_package("intltool_POTFILES_in_2")
54 dirs = find_potfiles_in()51 dirs = find_potfiles_in()
55 self.assertContentEqual(["./po", "./module1/po"], dirs)52 self.assertContentEqual(["./po", "./module1/po"], dirs)
5653
57 def test_check_potfiles_in_content_ok(self):54 def test_check_potfiles_in_content_ok(self):
58 # Ideally all files listed in POTFILES.in exist in the source package.55 # Ideally all files listed in POTFILES.in exist in the source package.
59 self._prepare_package("intltool_single_ok")56 self.prepare_package("intltool_single_ok")
60 self.assertTrue(check_potfiles_in("./po")) 57 self.assertTrue(check_potfiles_in("./po"))
6158
62 def test_check_potfiles_in_content_ok_file_added(self):59 def test_check_potfiles_in_content_ok_file_added(self):
63 # If a file is not listed in POTFILES.in, the file is still good for60 # If a file is not listed in POTFILES.in, the file is still good for
64 # our purposes.61 # our purposes.
65 self._prepare_package("intltool_single_ok")62 self.prepare_package("intltool_single_ok")
66 added_file = file("./src/sourcefile_new.c", "w")63 added_file = file("./src/sourcefile_new.c", "w")
67 added_file.write("/* Test file. */")64 added_file.write("/* Test file. */")
68 added_file.close()65 added_file.close()
@@ -72,53 +69,53 @@
72 # If a file is missing that is listed in POTFILES.in, the file69 # If a file is missing that is listed in POTFILES.in, the file
73 # intltool structure is probably broken and cannot be used for70 # intltool structure is probably broken and cannot be used for
74 # our purposes.71 # our purposes.
75 self._prepare_package("intltool_single_ok")72 self.prepare_package("intltool_single_ok")
76 os.remove("./src/sourcefile1.c")73 os.remove("./src/sourcefile1.c")
77 self.assertFalse(check_potfiles_in("./po")) 74 self.assertFalse(check_potfiles_in("./po"))
7875
79 def test_check_potfiles_in_wrong_directory(self):76 def test_check_potfiles_in_wrong_directory(self):
80 # Passing in the wrong directory will cause the check to fail77 # Passing in the wrong directory will cause the check to fail
81 # gracefully and return False.78 # gracefully and return False.
82 self._prepare_package("intltool_single_ok")79 self.prepare_package("intltool_single_ok")
83 self.assertFalse(check_potfiles_in("./foo")) 80 self.assertFalse(check_potfiles_in("./foo"))
8481
85 def test_find_intltool_dirs(self):82 def test_find_intltool_dirs(self):
86 # Complete run: find all directories with intltool structure.83 # Complete run: find all directories with intltool structure.
87 self._prepare_package("intltool_full_ok")84 self.prepare_package("intltool_full_ok")
88 self.assertEqual(85 self.assertEqual(
89 ["./po-module1", "./po-module2"], find_intltool_dirs())86 ["./po-module1", "./po-module2"], find_intltool_dirs())
9087
91 def test_find_intltool_dirs_broken(self):88 def test_find_intltool_dirs_broken(self):
92 # Complete run: part of the intltool structure is broken.89 # Complete run: part of the intltool structure is broken.
93 self._prepare_package("intltool_full_ok")90 self.prepare_package("intltool_full_ok")
94 os.remove("./src/module1/sourcefile1.c")91 os.remove("./src/module1/sourcefile1.c")
95 self.assertEqual(92 self.assertEqual(
96 ["./po-module2"], find_intltool_dirs())93 ["./po-module2"], find_intltool_dirs())
9794
98 def test_get_translation_domain_makevars(self):95 def test_get_translation_domain_makevars(self):
99 # Find a translation domain in Makevars.96 # Find a translation domain in Makevars.
100 self._prepare_package("intltool_domain_makevars")97 self.prepare_package("intltool_domain_makevars")
101 self.assertEqual(98 self.assertEqual(
102 "translationdomain",99 "translationdomain",
103 get_translation_domain("po"))100 get_translation_domain("po"))
104101
105 def test_get_translation_domain_makefile_in_in(self):102 def test_get_translation_domain_makefile_in_in(self):
106 # Find a translation domain in Makefile.in.in.103 # Find a translation domain in Makefile.in.in.
107 self._prepare_package("intltool_domain_makefile_in_in")104 self.prepare_package("intltool_domain_makefile_in_in")
108 self.assertEqual(105 self.assertEqual(
109 "packagename-in-in",106 "packagename-in-in",
110 get_translation_domain("po"))107 get_translation_domain("po"))
111108
112 def test_get_translation_domain_configure_ac(self):109 def test_get_translation_domain_configure_ac(self):
113 # Find a translation domain in configure.ac.110 # Find a translation domain in configure.ac.
114 self._prepare_package("intltool_domain_configure_ac")111 self.prepare_package("intltool_domain_configure_ac")
115 self.assertEqual(112 self.assertEqual(
116 "packagename-ac",113 "packagename-ac",
117 get_translation_domain("po"))114 get_translation_domain("po"))
118115
119 def test_get_translation_domain_configure_in(self):116 def test_get_translation_domain_configure_in(self):
120 # Find a translation domain in configure.in.117 # Find a translation domain in configure.in.
121 self._prepare_package("intltool_domain_configure_in")118 self.prepare_package("intltool_domain_configure_in")
122 self.assertEqual(119 self.assertEqual(
123 "packagename-in",120 "packagename-in",
124 get_translation_domain("po"))121 get_translation_domain("po"))
@@ -126,7 +123,7 @@
126 def test_get_translation_domain_makefile_in_in_substitute(self):123 def test_get_translation_domain_makefile_in_in_substitute(self):
127 # Find a translation domain in Makefile.in.in with substitution from124 # Find a translation domain in Makefile.in.in with substitution from
128 # configure.ac.125 # configure.ac.
129 self._prepare_package("intltool_domain_makefile_in_in_substitute")126 self.prepare_package("intltool_domain_makefile_in_in_substitute")
130 self.assertEqual(127 self.assertEqual(
131 "domainname-ac-in-in",128 "domainname-ac-in-in",
132 get_translation_domain("po"))129 get_translation_domain("po"))
@@ -135,7 +132,7 @@
135 # Find a translation domain in Makefile.in.in with substitution from132 # Find a translation domain in Makefile.in.in with substitution from
136 # configure.ac from a variable with the same name as in133 # configure.ac from a variable with the same name as in
137 # Makefile.in.in.134 # Makefile.in.in.
138 self._prepare_package(135 self.prepare_package(
139 "intltool_domain_makefile_in_in_substitute_same_name")136 "intltool_domain_makefile_in_in_substitute_same_name")
140 self.assertEqual(137 self.assertEqual(
141 "packagename-ac-in-in",138 "packagename-ac-in-in",
@@ -144,7 +141,7 @@
144 def test_get_translation_domain_makefile_in_in_substitute_same_file(self):141 def test_get_translation_domain_makefile_in_in_substitute_same_file(self):
145 # Find a translation domain in Makefile.in.in with substitution from142 # Find a translation domain in Makefile.in.in with substitution from
146 # the same file.143 # the same file.
147 self._prepare_package(144 self.prepare_package(
148 "intltool_domain_makefile_in_in_substitute_same_file")145 "intltool_domain_makefile_in_in_substitute_same_file")
149 self.assertEqual(146 self.assertEqual(
150 "domain-in-in-in-in",147 "domain-in-in-in-in",
@@ -153,14 +150,14 @@
153 def test_get_translation_domain_makefile_in_in_substitute_broken(self):150 def test_get_translation_domain_makefile_in_in_substitute_broken(self):
154 # Find no translation domain in Makefile.in.in when the substitution151 # Find no translation domain in Makefile.in.in when the substitution
155 # cannot be fulfilled.152 # cannot be fulfilled.
156 self._prepare_package(153 self.prepare_package(
157 "intltool_domain_makefile_in_in_substitute_broken")154 "intltool_domain_makefile_in_in_substitute_broken")
158 self.assertIs(None, get_translation_domain("po"))155 self.assertIs(None, get_translation_domain("po"))
159156
160 def test_get_translation_domain_configure_in_substitute_version(self):157 def test_get_translation_domain_configure_in_substitute_version(self):
161 # Find a translation domain in configure.in with Makefile-style158 # Find a translation domain in configure.in with Makefile-style
162 # substitution from the same file.159 # substitution from the same file.
163 self._prepare_package(160 self.prepare_package(
164 "intltool_domain_configure_in_substitute_version")161 "intltool_domain_configure_in_substitute_version")
165 self.assertEqual(162 self.assertEqual(
166 "domainname-in42",163 "domainname-in42",
@@ -168,7 +165,7 @@
168165
169 def test_pottery_check_intltool_script(self):166 def test_pottery_check_intltool_script(self):
170 # Let the script run to see it works fine.167 # Let the script run to see it works fine.
171 self._prepare_package("intltool_full_ok")168 self.prepare_package("intltool_full_ok")
172169
173 return_code, stdout, stderr = run_script(170 return_code, stdout, stderr = run_script(
174 'scripts/rosetta/pottery-check-intltool.py', [])171 'scripts/rosetta/pottery-check-intltool.py', [])
@@ -179,6 +176,31 @@
179 """), stdout)176 """), stdout)
180177
181178
179class TestDetectIntltoolInBzrTree(TestCase, SetupTestPackageMixin):
180
181 def prepare_tree(self):
182 return BzrDir.create_standalone_workingtree(".")
183
184 def test_detect_intltool_structure(self):
185 # Detect a simple intltool structure.
186 self.prepare_package("intltool_POTFILES_in_1")
187 tree = self.prepare_tree()
188 self.assertTrue(is_intltool_structure(tree))
189
190 def test_detect_no_intltool_structure(self):
191 # If no POTFILES.in exists, no intltool structure is assumed.
192 self.prepare_package("intltool_POTFILES_in_1")
193 os.remove("./po-intltool/POTFILES.in")
194 tree = self.prepare_tree()
195 self.assertFalse(is_intltool_structure(tree))
196
197 def test_detect_intltool_structure_module(self):
198 # Detect an intltool structure in subdirectories.
199 self.prepare_package("intltool_POTFILES_in_2")
200 tree = self.prepare_tree()
201 self.assertTrue(is_intltool_structure(tree))
202
203
182class TestConfigFile(TestCase):204class TestConfigFile(TestCase):
183205
184 def setUp(self):206 def setUp(self):