Launchpad itself

Merge lp:~jtv/launchpad/translationtemplatescollection-test into lp:launchpad

  1. translationtemplatescollection-test
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 11150
Proposed branch: lp:~jtv/launchpad/translationtemplatescollection-test
Merge into: lp:launchpad
Diff against target: 362 lines (+244/-22)
5 files modified
lib/lp/services/database/collection.py (+17/-3)
lib/lp/services/database/tests/test_collection.py (+1/-3)
lib/lp/services/doc/collection.txt (+8/-2)
lib/lp/translations/model/potemplate.py (+11/-14)
lib/lp/translations/tests/test_translationtemplatescollection.py (+207/-0)
To merge this branch: bzr merge lp:~jtv/launchpad/translationtemplatescollection-test
Reviewer Review Type Date Requested Status
Данило Шеган (community) Approve
Launchpad code reviewers code Pending
Review via email: mp+30114@code.launchpad.net

Commit message

Tests & simplification for TranslationTemplatesCollection.

Description of the change

= TranslationTemplatesCollection test =

This branch provides intermediate testing for TranslationTemplatesCollection, layered between the existing tests for Collection and for the one class implemented in terms of collections, HasTranslationTemplatesMixin. A silly typo in the TranslationTemplatesCollection revealed a gap in test coverage, which was a good excuse for both fixing the bug and testing the class separately. I should have done it from the start.

This also introduces another piece of convenience for Collection: a concrete collection class based on Collection selects, by default, just the class that it is a collection of. So calling TranslationTemplatesCollection.select() without arguments is now allowed, and returns a sequence of POTemplate.

There's also some drive-by lint cleanup. However I left some pre-existing ones in place because what "make lint" is complaining about looks to me an awful lot like our preferred syntax:

    list_comprehension = [
        frobnicate(item)
        for item in items
        if effable(item)
        ]

Jeroen

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Oops. I forgot to include the preceding branch as a requirement for this one, and the preceding one hasn't landed yet. (It's been reviewed and approved though).

Incremental diff compared to that branch: http://paste.ubuntu.com/464611/

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

1. Split test_restrictCurrent() into two: one test should test one thing, and you are testing for both current=True and current=False.

2. I'd like to see more tests for "simple" conditions, i.e. negative tests: nothing returned, or that stuff in other contexts is not returned. For the second we probably don't care about it since it's tested in the base collection tests.

I mostly care about 1, and do 2 only if you agree.

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

Thanks. I split up test_restrictCurrent into one test for current templates and another for obsolete ones, as discussed. I also added negative tests: condition False matches nothing, and searching for a productseries/distroseries/sourcepackagename excludes all other templates.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/database/collection.py'
2--- lib/lp/services/database/collection.py 2010-07-15 15:00:52 +0000
3+++ lib/lp/services/database/collection.py 2010-07-17 16:22:44 +0000
4@@ -26,7 +26,7 @@
5 individual columns or other Storm expressions.
6 """
7
8- # Default table for this collection that will always be needed.
9+ # Default table for this collection that will always be included.
10 # Derived collection classes can use this to say what type they are
11 # a collection of.
12 starting_table = None
13@@ -113,13 +113,27 @@
14 return self.refine(tables=[LeftJoin(cls, *conditions)])
15
16 def select(self, *values):
17- """Return a result set containing the requested `values`."""
18+ """Return a result set containing the requested `values`.
19+
20+ If no values are requested, this selects the type of object that
21+ the Collection is a collection of.
22+ """
23 if len(self.tables) == 0:
24 source = self.store
25 else:
26 source = self.store.using(*self.tables)
27
28- if len(values) == 1:
29+ if len(values) > 1:
30+ # Selecting a tuple of values. Pass it to Storm unchanged.
31+ pass
32+ elif len(values) == 1:
33+ # One value requested. Unpack for convenience.
34 values = values[0]
35+ else:
36+ # Select the starting table by default.
37+ assert self.starting_table is not None, (
38+ "Collection %s does not define a starting table." %
39+ self.__class__.__name__)
40+ values = self.starting_table
41
42 return source.find(values, *self.conditions)
43
44=== modified file 'lib/lp/services/database/tests/test_collection.py'
45--- lib/lp/services/database/tests/test_collection.py 2010-07-16 10:05:21 +0000
46+++ lib/lp/services/database/tests/test_collection.py 2010-07-17 16:22:44 +0000
47@@ -5,8 +5,6 @@
48
49 __metaclass__ = type
50
51-import unittest
52-
53 from storm.locals import Int, Storm
54 from zope.component import getUtility
55
56@@ -185,4 +183,4 @@
57 collection_with_store = collection.use(store)
58 self.assertEqual(store, collection_with_store.store)
59
60- self.assertEqual([], collection_with_store.select())
61+ self.assertEqual([], collection_with_store.select(TestTable))
62
63=== modified file 'lib/lp/services/doc/collection.txt'
64--- lib/lp/services/doc/collection.txt 2010-07-15 15:00:52 +0000
65+++ lib/lp/services/doc/collection.txt 2010-07-17 16:22:44 +0000
66@@ -16,7 +16,7 @@
67 >>> from storm.locals import Count, Int, Storm
68 >>> from canonical.launchpad.webapp.interfaces import (
69 ... IStoreSelector, MAIN_STORE, MASTER_FLAVOR)
70-
71+
72 >>> store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
73 >>> ok = store.execute("CREATE TEMP TABLE Kumquat(id integer UNIQUE)")
74 >>> class Kumquat(Storm):
75@@ -30,7 +30,8 @@
76 >>> obj = store.add(Kumquat(1))
77 >>> obj = store.add(Kumquat(2))
78
79-A custom KumquatCollection class derives from Kumquat.
80+A custom KumquatCollection class derives from Collection. The
81+starting_table attribute tells Collection what it is a collection of.
82
83 >>> from lp.services.database.collection import Collection
84 >>> class KumquatCollection(Collection):
85@@ -41,6 +42,11 @@
86 set.
87
88 >>> collection = KumquatCollection()
89+ >>> print list(collection.select().order_by(Kumquat.id))
90+ [Kumquat-1, Kumquat-2]
91+
92+Actually, select() is just shorthand for select(Kumquat).
93+
94 >>> print list(collection.select(Kumquat).order_by(Kumquat.id))
95 [Kumquat-1, Kumquat-2]
96
97
98=== modified file 'lib/lp/translations/model/potemplate.py'
99--- lib/lp/translations/model/potemplate.py 2010-07-16 10:22:00 +0000
100+++ lib/lp/translations/model/potemplate.py 2010-07-17 16:22:44 +0000
101@@ -1,4 +1,4 @@
102-# Copyright 2009 Canonical Ltd. This software is licensed under the
103+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
104 # GNU Affero General Public License version 3 (see the file LICENSE).
105
106 # pylint: disable-msg=E0611,W0212
107@@ -92,16 +92,15 @@
108 '''
109
110 standardTemplateHeader = (
111-"Project-Id-Version: %(origin)s\n"
112-"Report-Msgid-Bugs-To: FULL NAME <EMAIL@ADDRESS>\n"
113-"POT-Creation-Date: %(templatedate)s\n"
114-"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
115-"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
116-"Language-Team: %(languagename)s <%(languagecode)s@li.org>\n"
117-"MIME-Version: 1.0\n"
118-"Content-Type: text/plain; charset=UTF-8\n"
119-"Content-Transfer-Encoding: 8bit\n"
120-)
121+ "Project-Id-Version: %(origin)s\n"
122+ "Report-Msgid-Bugs-To: FULL NAME <EMAIL@ADDRESS>\n"
123+ "POT-Creation-Date: %(templatedate)s\n"
124+ "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
125+ "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
126+ "Language-Team: %(languagename)s <%(languagecode)s@li.org>\n"
127+ "MIME-Version: 1.0\n"
128+ "Content-Type: text/plain; charset=UTF-8\n"
129+ "Content-Transfer-Encoding: 8bit\n")
130
131
132 standardPOFileHeader = (standardTemplateHeader +
133@@ -1380,7 +1379,6 @@
134 self.sourcepackagename = sourcepackagename
135 self.product = product
136
137-
138 def _get_potemplate_equivalence_class(self, template):
139 """Return whatever we group `POTemplate`s by for sharing purposes."""
140 if template.sourcepackagename is None:
141@@ -1588,8 +1586,7 @@
142
143 def getTranslationTemplates(self):
144 """See `IHasTranslationTemplates`."""
145- return self._orderTemplates(
146- self.getTemplatesCollection().select(POTemplate))
147+ return self._orderTemplates(self.getTemplatesCollection().select())
148
149 def getTranslationTemplateFormats(self):
150 """See `IHasTranslationTemplates`."""
151
152=== added file 'lib/lp/translations/tests/test_translationtemplatescollection.py'
153--- lib/lp/translations/tests/test_translationtemplatescollection.py 1970-01-01 00:00:00 +0000
154+++ lib/lp/translations/tests/test_translationtemplatescollection.py 2010-07-17 16:22:44 +0000
155@@ -0,0 +1,207 @@
156+# Copyright 2010 Canonical Ltd. This software is licensed under the
157+# GNU Affero General Public License version 3 (see the file LICENSE).
158+
159+"""Test `TranslationTemplatesCollection`."""
160+
161+__metaclass__ = type
162+
163+from zope.security.proxy import removeSecurityProxy
164+from canonical.testing import DatabaseFunctionalLayer
165+from lp.testing import TestCaseWithFactory
166+
167+from lp.translations.model.pofile import POFile
168+from lp.translations.model.potemplate import (
169+ POTemplate,
170+ TranslationTemplatesCollection)
171+
172+
173+class TestSomething(TestCaseWithFactory):
174+ layer = DatabaseFunctionalLayer
175+
176+ def test_baseline(self):
177+ # A collection constructed with no arguments selects all
178+ # templates.
179+ template = self.factory.makePOTemplate()
180+
181+ collection = TranslationTemplatesCollection()
182+ self.assertIn(template, collection.select())
183+
184+ def test_none_found(self):
185+ trunk = self.factory.makeProduct().getSeries('trunk')
186+ collection = TranslationTemplatesCollection()
187+ by_series = collection.restrictProductSeries(trunk)
188+
189+ self.assertContentEqual([], by_series.select())
190+
191+ def test_restrictProductSeries(self):
192+ trunk = self.factory.makeProduct().getSeries('trunk')
193+ template = self.factory.makePOTemplate(productseries=trunk)
194+
195+ collection = TranslationTemplatesCollection()
196+ by_series = collection.restrictProductSeries(trunk)
197+
198+ self.assertContentEqual([template], by_series.select())
199+
200+ def test_restrictProductSeries_restricts(self):
201+ # restrictProductSeries makes the collection ignore templates
202+ # from other productseries and source packages.
203+ product = self.factory.makeProduct()
204+ trunk = product.getSeries('trunk')
205+
206+ nontrunk = removeSecurityProxy(product).newSeries(
207+ product.owner, 'nontrunk', 'foo')
208+ sourcepackage = self.factory.makeSourcePackage()
209+
210+ self.factory.makePOTemplate(productseries=nontrunk)
211+ self.factory.makePOTemplate(
212+ distroseries=sourcepackage.distroseries,
213+ sourcepackagename=sourcepackage.sourcepackagename)
214+
215+ collection = TranslationTemplatesCollection()
216+ by_series = collection.restrictProductSeries(trunk)
217+
218+ self.assertContentEqual([], by_series.select())
219+
220+ def test_restrictDistroSeries(self):
221+ package = self.factory.makeSourcePackage()
222+ template = self.factory.makePOTemplate(
223+ distroseries=package.distroseries,
224+ sourcepackagename=package.sourcepackagename)
225+
226+ collection = TranslationTemplatesCollection()
227+ by_series = collection.restrictDistroSeries(package.distroseries)
228+
229+ self.assertContentEqual([template], by_series.select())
230+
231+ def test_restrictDistroSeries_restricts(self):
232+ # restrictProductSeries makes the collection ignore templates
233+ # from other productseries and distroseries.
234+ distribution = self.factory.makeDistribution()
235+ series = self.factory.makeDistroSeries(distribution=distribution)
236+ other_series = self.factory.makeDistroSeries(
237+ distribution=distribution)
238+ productseries = self.factory.makeProductSeries()
239+ package = self.factory.makeSourcePackageName()
240+
241+ self.factory.makePOTemplate(
242+ distroseries=other_series, sourcepackagename=package)
243+ self.factory.makePOTemplate(productseries=productseries)
244+
245+ collection = TranslationTemplatesCollection()
246+ by_series = collection.restrictDistroSeries(series)
247+
248+ self.assertContentEqual([], by_series.select())
249+
250+ def test_restrictSourcePackageName(self):
251+ package = self.factory.makeSourcePackage()
252+ template = self.factory.makePOTemplate(
253+ distroseries=package.distroseries,
254+ sourcepackagename=package.sourcepackagename)
255+
256+ assert package.sourcepackagename
257+ collection = TranslationTemplatesCollection()
258+ by_packagename = collection.restrictSourcePackageName(
259+ package.sourcepackagename)
260+
261+ self.assertContentEqual([template], by_packagename.select())
262+
263+ def test_restrictSourcePackageName_restricts(self):
264+ # restrictSourcePackageName makes the collection ignore
265+ # templates from other source package names and productseries.
266+ package = self.factory.makeSourcePackage()
267+ distroseries = package.distroseries
268+ other_package = self.factory.makeSourcePackage(
269+ distroseries=distroseries)
270+ productseries = self.factory.makeProductSeries()
271+
272+ self.factory.makePOTemplate(
273+ distroseries=distroseries,
274+ sourcepackagename=other_package.sourcepackagename)
275+ self.factory.makePOTemplate(productseries=productseries)
276+
277+ collection = TranslationTemplatesCollection()
278+ by_packagename = collection.restrictSourcePackageName(
279+ package.sourcepackagename)
280+
281+ self.assertContentEqual([], by_packagename.select())
282+
283+ def test_restrict_SourcePackage(self):
284+ # You can restrict to a source package by restricting both to a
285+ # DistroSeries and to a SourcePackageName.
286+ package = self.factory.makeSourcePackage()
287+ template = self.factory.makePOTemplate(
288+ distroseries=package.distroseries,
289+ sourcepackagename=package.sourcepackagename)
290+
291+ collection = TranslationTemplatesCollection()
292+ by_series = collection.restrictDistroSeries(package.distroseries)
293+ by_package = by_series.restrictSourcePackageName(
294+ package.sourcepackagename)
295+
296+ self.assertContentEqual([template], by_package.select())
297+
298+ def test_restrictCurrent_current(self):
299+ trunk = self.factory.makeProduct().getSeries('trunk')
300+ template = self.factory.makePOTemplate(productseries=trunk)
301+ collection = TranslationTemplatesCollection()
302+ by_series = collection.restrictProductSeries(trunk)
303+
304+ current_templates = by_series.restrictCurrent(True)
305+
306+ removeSecurityProxy(template).iscurrent = True
307+ self.assertContentEqual(
308+ [template], current_templates.select())
309+
310+ removeSecurityProxy(template).iscurrent = False
311+ self.assertContentEqual([], current_templates.select())
312+
313+ def test_restrictCurrent_obsolete(self):
314+ trunk = self.factory.makeProduct().getSeries('trunk')
315+ template = self.factory.makePOTemplate(productseries=trunk)
316+ collection = TranslationTemplatesCollection()
317+ by_series = collection.restrictProductSeries(trunk)
318+
319+ obsolete_templates = by_series.restrictCurrent(False)
320+
321+ removeSecurityProxy(template).iscurrent = True
322+ self.assertContentEqual([], obsolete_templates.select())
323+
324+ removeSecurityProxy(template).iscurrent = False
325+ self.assertContentEqual(
326+ [template], obsolete_templates.select())
327+
328+ def test_joinPOFile(self):
329+ trunk = self.factory.makeProduct().getSeries('trunk')
330+ translated_template = self.factory.makePOTemplate(productseries=trunk)
331+ untranslated_template = self.factory.makePOTemplate(
332+ productseries=trunk)
333+ nl = translated_template.newPOFile('nl')
334+ de = translated_template.newPOFile('de')
335+
336+ collection = TranslationTemplatesCollection()
337+ by_series = collection.restrictProductSeries(trunk)
338+ joined = by_series.joinPOFile()
339+
340+ self.assertContentEqual(
341+ [(translated_template, nl), (translated_template, de)],
342+ joined.select(POTemplate, POFile))
343+
344+ def test_joinOuterPOFile(self):
345+ trunk = self.factory.makeProduct().getSeries('trunk')
346+ translated_template = self.factory.makePOTemplate(productseries=trunk)
347+ untranslated_template = self.factory.makePOTemplate(
348+ productseries=trunk)
349+ nl = translated_template.newPOFile('nl')
350+ de = translated_template.newPOFile('de')
351+
352+ collection = TranslationTemplatesCollection()
353+ by_series = collection.restrictProductSeries(trunk)
354+ joined = by_series.joinOuterPOFile()
355+
356+ expected_outcome = [
357+ (translated_template, nl),
358+ (translated_template, de),
359+ (untranslated_template, None),
360+ ]
361+ self.assertContentEqual(
362+ expected_outcome, joined.select(POTemplate, POFile))