Launchpad itself

Merge lp://qastaging/~jtv/launchpad/bug-445511 into lp://qastaging/launchpad

  1. bug-445511
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://qastaging/~jtv/launchpad/bug-445511
Merge into: lp://qastaging/launchpad
Diff against target: 153 lines
3 files modified
lib/lp/translations/interfaces/translationimportqueue.py (+1/-1)
lib/lp/translations/scripts/reupload_translations.py (+6/-1)
lib/lp/translations/scripts/tests/test_reupload_translations.py (+60/-17)
To merge this branch: bzr merge lp://qastaging/~jtv/launchpad/bug-445511
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Review via email: mp+13122@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= Bug 445511 =

This is a candidate for cherry-picking, so some small things are left
unimproved: the _filter_ubuntu_translation_file function shouldn't have
the leading underscore in its name, but that'd drag in irrelevant diff.

We have a script for re-uploading the latest translations tarball for a
given source package. But it neglected to run the tarball's translation
files through the same filename filter that the normal Ubuntu uploads go
through. This filter takes care of stripping out non-translation files,
stripping out translation files in the wrong locations, and removing the
unwanted path prefix that is introduced by the build process.

There is one small free bonus: the interface for the translations import
queue failed to declare a parameter that is actually implemented.

Test with
{{{
./bin/test -vv -t reupload
}}}

No lint.

Jeroen

Revision history for this message
Данило Шеган (danilo) wrote :

Looks good.

review: Approve
Revision history for this message
Данило Шеган (danilo) wrote :

Please QA this on staging as well, before asking for a CP.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
2--- lib/lp/translations/interfaces/translationimportqueue.py 2009-10-01 16:10:42 +0000
3+++ lib/lp/translations/interfaces/translationimportqueue.py 2009-10-09 13:35:20 +0000
4@@ -333,7 +333,7 @@
5
6 def addOrUpdateEntriesFromTarball(content, is_published, importer,
7 sourcepackagename=None, distroseries=None, productseries=None,
8- potemplate=None):
9+ potemplate=None, filename_filter=None):
10 """Add all .po or .pot files from the tarball at :content:.
11
12 :arg content: is a tarball stream.
13
14=== modified file 'lib/lp/translations/scripts/reupload_translations.py'
15--- lib/lp/translations/scripts/reupload_translations.py 2009-09-30 16:50:06 +0000
16+++ lib/lp/translations/scripts/reupload_translations.py 2009-10-09 13:35:20 +0000
17@@ -89,6 +89,10 @@
18
19 def _processPackage(self, package):
20 """Get translations for `package` re-uploaded."""
21+ # Avoid circular imports.
22+ from lp.soyuz.model.sourcepackagerelease import (
23+ _filter_ubuntu_translation_file)
24+
25 self.logger.info("Processing %s" % package.displayname)
26 tarball_aliases = package.getLatestTranslationsUploads()
27 queue = getUtility(ITranslationImportQueue)
28@@ -102,7 +106,8 @@
29 queue.addOrUpdateEntriesFromTarball(
30 alias.read(), True, rosetta_team,
31 sourcepackagename=package.sourcepackagename,
32- distroseries=self.distroseries)
33+ distroseries=self.distroseries,
34+ filename_filter=_filter_ubuntu_translation_file)
35
36 if not have_uploads:
37 self.logger.warn(
38
39=== modified file 'lib/lp/translations/scripts/tests/test_reupload_translations.py'
40--- lib/lp/translations/scripts/tests/test_reupload_translations.py 2009-09-30 16:50:06 +0000
41+++ lib/lp/translations/scripts/tests/test_reupload_translations.py 2009-10-09 13:35:20 +0000
42@@ -22,6 +22,8 @@
43
44 from canonical.launchpad.database.librarian import LibraryFileAliasSet
45 from lp.registry.model.sourcepackage import SourcePackage
46+from lp.soyuz.model.sourcepackagerelease import (
47+ _filter_ubuntu_translation_file)
48 from lp.translations.model.translationimportqueue import (
49 TranslationImportQueue)
50
51@@ -30,6 +32,12 @@
52
53
54 class UploadInjector:
55+ """Mockup for `SourcePackage.getLatestTranslationsUploads`.
56+
57+ If patched into a `SourcePackage` object, makes its
58+ getLatestTranslationsUploads method return the given library file
59+ alias.
60+ """
61 def __init__(self, script, tar_alias):
62 self.tar_alias = tar_alias
63 self.script = script
64@@ -76,6 +84,23 @@
65 return dict((entry.path, entry.content.read()) for entry in entries)
66
67
68+def filter_paths(files_dict):
69+ """Apply `_filter_ubuntu_translation_file` to each file in `files_dict.`
70+
71+ :param files_dict: A dict mapping translation file names to their
72+ contents.
73+ :return: A similar dict, but with `_filter_ubuntu_translation_file`
74+ applied to each file's path, and non-Ubuntu files left out.
75+ """
76+ filtered_dict = {}
77+ for original_path, content in files_dict.iteritems():
78+ new_path = _filter_ubuntu_translation_file(original_path)
79+ if new_path:
80+ filtered_dict[new_path] = content
81+
82+ return filtered_dict
83+
84+
85 class TestReuploadPackageTranslations(TestCaseWithFactory):
86 """Test `ReuploadPackageTranslations`."""
87 layer = LaunchpadZopelessLayer
88@@ -91,6 +116,27 @@
89 '-p', sourcepackagename.name,
90 '-qqq'])
91
92+ def _uploadAndProcess(self, files_dict):
93+ """Fake an upload and cause _processPackage to be run on it.
94+
95+ :param files_dict: A dict mapping file paths to file contents.
96+ :return: A dict describing the resulting import queue for the
97+ package.
98+ """
99+ tar_alias = upload_tarball(files_dict)
100+
101+ # Force Librarian update
102+ transaction.commit()
103+
104+ self.script._findPackage = UploadInjector(self.script, tar_alias)
105+ self.script.main()
106+ self.assertEqual([], self.script.uploadless_packages)
107+
108+ # Force Librarian update
109+ transaction.commit()
110+
111+ return summarize_translations_queue(self.sourcepackage)
112+
113 def test_findPackage(self):
114 # _findPackage finds a SourcePackage by name.
115 self.script._setDistroDetails()
116@@ -109,23 +155,20 @@
117 # _processPackage will fetch the package's latest translations
118 # upload from the Librarian and re-import it.
119 translation_files = {
120- 'po/messages.pot': '# pot',
121- 'po/nl.po': '# nl',
122- }
123- tar_alias = upload_tarball(translation_files)
124-
125- # Force Librarian update
126- transaction.commit()
127-
128- self.script._findPackage = UploadInjector(self.script, tar_alias)
129- self.script.main()
130- self.assertEqual([], self.script.uploadless_packages)
131-
132- # Force Librarian update
133- transaction.commit()
134-
135- queue_summary = summarize_translations_queue(self.sourcepackage)
136- self.assertEqual(translation_files, queue_summary)
137+ 'source/po/messages.pot': '# pot',
138+ 'source/po/nl.po': '# nl',
139+ }
140+ queue_summary = self._uploadAndProcess(translation_files)
141+ self.assertEqual(filter_paths(translation_files), queue_summary)
142+
143+ def test_processPackage_filters_paths(self):
144+ # Uploads are filtered just like other Ubuntu tarballs.
145+ translation_files = {
146+ 'source/foo.pot': '# foo',
147+ 'elsewhere/bar.pot': '# bar',
148+ }
149+ queue_summary = self._uploadAndProcess(translation_files)
150+ self.assertEqual({'foo.pot': '# foo'}, queue_summary)
151
152
153 class TestReuploadScript(TestCaseWithFactory):