Merge lp:~henninge/launchpad/bug-523810-needs-information-age into lp:launchpad
- bug-523810-needs-information-age
- 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-523810-needs-information-age | ||||
| Merge into: | lp:launchpad | ||||
| Diff against target: |
548 lines (+122/-89) 3 files modified
lib/lp/translations/interfaces/translationimportqueue.py (+19/-0) lib/lp/translations/model/translationimportqueue.py (+3/-15) lib/lp/translations/tests/test_autoapproval.py (+100/-74) |
||||
| To merge this branch: | bzr merge lp:~henninge/launchpad/bug-523810-needs-information-age | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Abel Deuring (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
Added a maximum age for translation import queue entries in the new 'Needs Information' state.
Description of the change
To post a comment you must log in.
Revision history for this message
| Henning Eggers (henninge) wrote : | # |
Revision history for this message
| Abel Deuring (adeuring) wrote : | # |
Download full text (10.6 KiB)
Hi Henning,
overall a nice branc, but I have a couple of questions, see
below.
Abel
> = Bug 523810 =
>
> The import queue gardener removes entries from the import
> queue that have reached a certain age in certain states.
> The age is configurable through a look-up table and the new
> "Needs Information" status needs an entry in that table.
> The maximum age should be identical to "Needs Review" for
> starters.
>
> == Implementation details ==
>
> Driven by the want to extend test coverage to all entry
> states that are being cleaned out I moved the look-up
> table to the interface module where it can be reached
> from the test.
>
> I also found a nice little use of the with statement again. ;-)
>
> == Tests ==
>
> Run all autoapproval test to verify that the introduction of
> the GardenerDbUserMixin did not mess up existing tests.
>
> bin/test -vvt autoapproval
>
> == Demo/QA ==
>
> In order to not have to wait half a year for an entry to
> expire on staging, some SQL magic will be needed to make an
> entry in the "Needs Information" stage that old. Then wait
> for a day or so to see it disappear. Staging updates might
> be a nuissance here.
>
> = 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/
> lib/lp/
> lib/lp/
>
>
> == Pylint notices ==
>
> lib/lp/
> 12: [F0401] Unable to import 'lazr.enum' (No module named enum)
> 22: [F0401] Unable to import 'lazr.restful.
> 23: [F0401] Unable to import 'lazr.restful.
> 24: [F0401] Unable to import 'lazr.restful.
>
>
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -3,6 +3,8 @@
>
> # pylint: disable-
>
> +from datetime import timedelta
> +
> from zope.interface import Interface, Attribute
> from zope.schema import (
> Bool, Choice, Datetime, Field, Int, Object, Text, TextLine)
> @@ -39,6 +41,7 @@
> 'RosettaImportS
> 'SpecialTransla
> 'TranslationFil
> + 'TranslationImp
> 'UserCannotSetT
> ]
>
> @@ -111,6 +114,22 @@
> """)
>
>
> +# Some time spans in days.
> +_month = 30
> +_half_year = 366 / 2
This being constants, they should be defined as MONTH and HALF_YEAR,
I think, but...
> +
> +
> +# Period after which entries with certain statuses are culled from the
> +# queue.
> +TranslationImp
> + RosettaImportSt
> + RosettaImportSt
...seeing this, I think the constants are probably better called
DAYS_OF_
review:
Needs Information
(code)
Revision history for this message
| Henning Eggers (henninge) wrote : | # |
Download full text (8.3 KiB)
> Hi Henning,
Hallo Abel!
> overall a nice branc, but I have a couple of questions, see
> below.
Thanks for the review, though. ;)
> > === modified file
'lib/lp/
> > --- lib/lp/
> 10:39:45 +0000
> > +++ lib/lp/
> 09:03:16 +0000
> > @@ -3,6 +3,8 @@
> >
> > # pylint: disable-
> >
> > +from datetime import timedelta
> > +
> > from zope.interface import Interface, Attribute
> > from zope.schema import (
> > Bool, Choice, Datetime, Field, Int, Object, Text, TextLine)
> > @@ -39,6 +41,7 @@
> > 'RosettaImportS
> > 'SpecialTransla
> > 'TranslationFil
> > + 'TranslationImp
> > 'UserCannotSetT
> > ]
> >
> > @@ -111,6 +114,22 @@
> > """)
> >
> >
> > +# Some time spans in days.
> > +_month = 30
> > +_half_year = 366 / 2
>
> This being constants, they should be defined as MONTH and HALF_YEAR,
> I think, but...
>
> > +
> > +
> > +# Period after which entries with certain statuses are culled from the
> > +# queue.
> > +TranslationImp
> > + RosettaImportSt
> > + RosettaImportSt
>
> ...seeing this, I think the constants are probably better called
> DAYS_OF_
Yes, you are right about the capitalization. And the names are good
suggestions, too. Fixed that.
>
> > + RosettaImportSt
> > + RosettaImportSt
> > + RosettaImportSt
> > +}
>
> Our coding style convetions say that this should called
> translation_
> need to define it in the interfaces module, you could also
> add it to the __all__ list of the model module.
I renamed it but left it in place here, as per our discussion on IRC. We
agreed that the general rule is that tests should not import from model
code. I also noted that I consider these expiration times as
implementation-
have this here in the interface module. You agreed to the latter.
>
> > +
> > +
> > class SpecialTranslat
> > """Special "meta-targets" to filter the queue view by."""
> >
> >
> > === modified file 'lib/lp/
[...]
> > === modified file 'lib/lp/
> > --- lib/lp/
15:25:53
> +0000
> > +++ lib/lp/
09:03:16
> +0000
> > @@ -8,6 +8,9 @@
> > through the possibilities should go here.
> > """
> >
> > +from __future__ import with_statement
> > +
> > +from contextlib import contextmanager
> > from datetime import datetime, timedelta
> > from pytz import UTC
> > import transaction
> > @@ -31,17 +34,28 @@
>...
| 1 | === modified file 'lib/lp/translations/interfaces/translationimportqueue.py' |
| 2 | --- lib/lp/translations/interfaces/translationimportqueue.py 2010-02-19 08:37:27 +0000 |
| 3 | +++ lib/lp/translations/interfaces/translationimportqueue.py 2010-02-19 10:36:42 +0000 |
| 4 | @@ -41,7 +41,7 @@ |
| 5 | 'RosettaImportStatus', |
| 6 | 'SpecialTranslationImportTargetFilter', |
| 7 | 'TranslationFileType', |
| 8 | - 'TranslationImportQueueEntryAge', |
| 9 | + 'translation_import_queue_entry_age', |
| 10 | 'UserCannotSetTranslationImportStatus', |
| 11 | ] |
| 12 | |
| 13 | @@ -115,18 +115,18 @@ |
| 14 | |
| 15 | |
| 16 | # Some time spans in days. |
| 17 | -_month = 30 |
| 18 | -_half_year = 366 / 2 |
| 19 | +DAYS_IN_MONTH = 30 |
| 20 | +DAYS_IN_HALF_YEAR = 366 / 2 |
| 21 | |
| 22 | |
| 23 | # Period after which entries with certain statuses are culled from the |
| 24 | # queue. |
| 25 | -TranslationImportQueueEntryAge = { |
| 26 | +translation_import_queue_entry_age = { |
| 27 | RosettaImportStatus.DELETED: timedelta(days=3), |
| 28 | - RosettaImportStatus.FAILED: timedelta(days=_month), |
| 29 | + RosettaImportStatus.FAILED: timedelta(days=DAYS_IN_MONTH), |
| 30 | RosettaImportStatus.IMPORTED: timedelta(days=3), |
| 31 | - RosettaImportStatus.NEEDS_INFORMATION: timedelta(days=_half_year), |
| 32 | - RosettaImportStatus.NEEDS_REVIEW: timedelta(days=_half_year), |
| 33 | + RosettaImportStatus.NEEDS_INFORMATION: timedelta(days=DAYS_IN_HALF_YEAR), |
| 34 | + RosettaImportStatus.NEEDS_REVIEW: timedelta(days=DAYS_IN_HALF_YEAR), |
| 35 | } |
| 36 | |
| 37 | |
| 38 | |
| 39 | === modified file 'lib/lp/translations/model/translationimportqueue.py' |
| 40 | --- lib/lp/translations/model/translationimportqueue.py 2010-02-19 07:46:41 +0000 |
| 41 | +++ lib/lp/translations/model/translationimportqueue.py 2010-02-19 10:56:20 +0000 |
| 42 | @@ -58,7 +58,7 @@ |
| 43 | RosettaImportStatus, |
| 44 | SpecialTranslationImportTargetFilter, |
| 45 | TranslationImportQueueConflictError, |
| 46 | - TranslationImportQueueEntryAge, |
| 47 | + translation_import_queue_entry_age, |
| 48 | UserCannotSetTranslationImportStatus) |
| 49 | from lp.translations.interfaces.potemplate import IPOTemplate |
| 50 | from lp.translations.interfaces.translations import TranslationConstants |
| 51 | @@ -1244,7 +1244,7 @@ |
| 52 | """ |
| 53 | now = datetime.datetime.now(pytz.UTC) |
| 54 | deletion_clauses = [] |
| 55 | - for status, max_age in TranslationImportQueueEntryAge.iteritems(): |
| 56 | + for status, max_age in translation_import_queue_entry_age.iteritems(): |
| 57 | cutoff = now - max_age |
| 58 | deletion_clauses.append(And( |
| 59 | TranslationImportQueueEntry.status == status, |
| 60 | |
| 61 | === modified file 'lib/lp/translations/tests/test_autoapproval.py' |
| 62 | --- lib/lp/translations/tests/test_autoapproval.py 2010-02-19 09:01:21 +0000 |
| 63 | +++ lib/lp/translations/tests/test_autoapproval.py 2010-02-19 10:57:42 +0000 |
| 64 | @@ -34,7 +34,7 @@ |
| 65 | TranslationImportQueue, TranslationImportQueueEntry) |
| 66 | from lp.translations.interfaces.customlanguagecode import ICustomLanguageCode |
| 67 | from lp.translations.interfaces.translationimportqueue import ( |
| 68 | - RosettaImportStatus, TranslationImportQueueEntryAge) |
| 69 | + RosettaImportStatus, translation_import_queue_entry_age) |
| 70 | from lp.testing import TestCaseWithFactory |
| 71 | from lp.testing.factory import LaunchpadObjectFactory |
| 72 | from canonical.launchpad.webapp.testing import verifyObject |
| 73 | @@ -42,20 +42,26 @@ |
| 74 | |
| 75 | |
| 76 | class GardenerDbUserMixin(object): |
| 77 | - """Switch to the translations import queue gardener database role.""" |
| 78 | - |
| 79 | - def become_the_gardener(self): |
| 80 | - """Switch the user once.""" |
| 81 | + """Switch to the translations import queue gardener database role. |
| 82 | + |
| 83 | + Admittedly, this might be a little over-engineered but it looks good. ;) |
| 84 | + """ |
| 85 | + |
| 86 | + def _become(self, dbuser): |
| 87 | + """Switch to a different db user.""" |
| 88 | transaction.commit() |
| 89 | - self.layer.switchDbUser('translations_import_queue_gardener') |
| 90 | + self.layer.switchDbUser(dbuser) |
| 91 | + |
| 92 | + def becomeTheGardener(self): |
| 93 | + """One-way method to avoid unnecessary switch back.""" |
| 94 | + self._become('translations_import_queue_gardener') |
| 95 | |
| 96 | @contextmanager |
| 97 | - def being_the_gardener(self): |
| 98 | + def beingTheGardener(self): |
| 99 | """Context manager to restore the launchpad user.""" |
| 100 | - self.become_the_gardener() |
| 101 | + self._become('translations_import_queue_gardener') |
| 102 | yield |
| 103 | - transaction.commit() |
| 104 | - self.layer.switchDbUser('launchpad') |
| 105 | + self._become('launchpad') |
| 106 | |
| 107 | |
| 108 | class TestCustomLanguageCode(unittest.TestCase): |
| 109 | @@ -191,7 +197,7 @@ |
| 110 | # Of course matching will work without custom language codes. |
| 111 | tr_file = self._makePOFile('tr') |
| 112 | entry = self._makeQueueEntry('tr') |
| 113 | - self.become_the_gardener() |
| 114 | + self.becomeTheGardener() |
| 115 | self.assertEqual(entry.getGuessedPOFile(), tr_file) |
| 116 | |
| 117 | def test_CustomLanguageCodeEnablesMatch(self): |
| 118 | @@ -203,7 +209,7 @@ |
| 119 | |
| 120 | self._setCustomLanguageCode('fy_NL', 'fy') |
| 121 | |
| 122 | - self.become_the_gardener() |
| 123 | + self.becomeTheGardener() |
| 124 | self.assertEqual(entry.getGuessedPOFile(), fy_file) |
| 125 | |
| 126 | def test_CustomLanguageCodeParsesBogusLanguage(self): |
| 127 | @@ -214,7 +220,7 @@ |
| 128 | |
| 129 | self._setCustomLanguageCode('flemish', 'nl') |
| 130 | |
| 131 | - self.become_the_gardener() |
| 132 | + self.becomeTheGardener() |
| 133 | nl_file = entry.getGuessedPOFile() |
| 134 | self.assertEqual(nl_file.language.code, 'nl') |
| 135 | |
| 136 | @@ -227,7 +233,7 @@ |
| 137 | |
| 138 | self._setCustomLanguageCode('sv', None) |
| 139 | |
| 140 | - self.become_the_gardener() |
| 141 | + self.becomeTheGardener() |
| 142 | self.assertEqual(entry.getGuessedPOFile(), None) |
| 143 | self.assertEqual(entry.status, RosettaImportStatus.DELETED) |
| 144 | |
| 145 | @@ -240,7 +246,7 @@ |
| 146 | |
| 147 | self._setCustomLanguageCode('elx', 'el') |
| 148 | |
| 149 | - self.become_the_gardener() |
| 150 | + self.becomeTheGardener() |
| 151 | el_file = entry.getGuessedPOFile() |
| 152 | self.failIfEqual(el_file, elx_file) |
| 153 | self.assertEqual(el_file.language.code, 'el') |
| 154 | @@ -255,7 +261,7 @@ |
| 155 | |
| 156 | self._setCustomLanguageCode('nb', 'nn') |
| 157 | |
| 158 | - self.become_the_gardener() |
| 159 | + self.becomeTheGardener() |
| 160 | self.assertEqual(entry.getGuessedPOFile(), nn_file) |
| 161 | |
| 162 | def test_CustomLanguageCodeReplacesMatch(self): |
| 163 | @@ -268,7 +274,7 @@ |
| 164 | self._setCustomLanguageCode('pt', None) |
| 165 | self._setCustomLanguageCode('pt_PT', 'pt') |
| 166 | |
| 167 | - self.become_the_gardener() |
| 168 | + self.becomeTheGardener() |
| 169 | self.assertEqual(pt_entry.getGuessedPOFile(), None) |
| 170 | self.assertEqual(pt_PT_entry.getGuessedPOFile(), pt_file) |
| 171 | |
| 172 | @@ -282,7 +288,7 @@ |
| 173 | self._setCustomLanguageCode('zh_CN', 'zh_TW') |
| 174 | self._setCustomLanguageCode('zh_TW', 'zh_CN') |
| 175 | |
| 176 | - self.become_the_gardener() |
| 177 | + self.becomeTheGardener() |
| 178 | self.assertEqual(zh_CN_entry.getGuessedPOFile(), zh_TW_file) |
| 179 | self.assertEqual(zh_TW_entry.getGuessedPOFile(), zh_CN_file) |
| 180 | |
| 181 | @@ -327,7 +333,7 @@ |
| 182 | # When multiple templates match for a product series, |
| 183 | # getPOTemplateByPathAndOrigin returns none. |
| 184 | self._setUpProduct() |
| 185 | - self.become_the_gardener() |
| 186 | + self.becomeTheGardener() |
| 187 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 188 | 'test.pot', productseries=self.productseries) |
| 189 | self.assertEqual(None, guessed_template) |
| 190 | @@ -336,7 +342,7 @@ |
| 191 | # When multiple templates match on sourcepackagename, |
| 192 | # getPOTemplateByPathAndOrigin returns none. |
| 193 | self._setUpDistro() |
| 194 | - self.become_the_gardener() |
| 195 | + self.becomeTheGardener() |
| 196 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 197 | 'test.pot', sourcepackagename=self.packagename) |
| 198 | self.assertEqual(None, guessed_template) |
| 199 | @@ -345,7 +351,7 @@ |
| 200 | # When multiple templates match on from_sourcepackagename, |
| 201 | # getPOTemplateByPathAndOrigin returns none. |
| 202 | self._setUpDistro() |
| 203 | - self.become_the_gardener() |
| 204 | + self.becomeTheGardener() |
| 205 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 206 | 'test.pot', sourcepackagename=self.from_packagename) |
| 207 | self.assertEqual(None, guessed_template) |
| 208 | @@ -379,7 +385,7 @@ |
| 209 | self.distrotemplate1.sourcepackagename = match_package |
| 210 | self.distrotemplate2.from_sourcepackagename = match_package |
| 211 | |
| 212 | - self.become_the_gardener() |
| 213 | + self.becomeTheGardener() |
| 214 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 215 | 'test.pot', sourcepackagename=match_package) |
| 216 | self.assertEqual(self.distrotemplate2, guessed_template) |
| 217 | @@ -390,7 +396,7 @@ |
| 218 | self._setUpProduct() |
| 219 | self.producttemplate1.iscurrent = False |
| 220 | self.producttemplate2.iscurrent = True |
| 221 | - self.become_the_gardener() |
| 222 | + self.becomeTheGardener() |
| 223 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 224 | 'test.pot', productseries=self.productseries) |
| 225 | self.assertEqual(guessed_template, self.producttemplate2) |
| 226 | @@ -400,7 +406,7 @@ |
| 227 | self._setUpProduct() |
| 228 | self.producttemplate1.iscurrent = False |
| 229 | self.producttemplate2.iscurrent = False |
| 230 | - self.become_the_gardener() |
| 231 | + self.becomeTheGardener() |
| 232 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 233 | 'test.pot', productseries=self.productseries) |
| 234 | self.assertEqual(guessed_template, None) |
| 235 | @@ -414,7 +420,7 @@ |
| 236 | self.distrotemplate2.iscurrent = True |
| 237 | self.distrotemplate1.from_sourcepackagename = None |
| 238 | self.distrotemplate2.from_sourcepackagename = None |
| 239 | - self.become_the_gardener() |
| 240 | + self.becomeTheGardener() |
| 241 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 242 | 'test.pot', distroseries=self.distroseries, |
| 243 | sourcepackagename=self.packagename) |
| 244 | @@ -427,7 +433,7 @@ |
| 245 | self.distrotemplate2.iscurrent = False |
| 246 | self.distrotemplate1.from_sourcepackagename = None |
| 247 | self.distrotemplate2.from_sourcepackagename = None |
| 248 | - self.become_the_gardener() |
| 249 | + self.becomeTheGardener() |
| 250 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 251 | 'test.pot', distroseries=self.distroseries, |
| 252 | sourcepackagename=self.packagename) |
| 253 | @@ -442,7 +448,7 @@ |
| 254 | self.distrotemplate2.iscurrent = True |
| 255 | self.distrotemplate1.from_sourcepackagename = self.from_packagename |
| 256 | self.distrotemplate2.from_sourcepackagename = self.from_packagename |
| 257 | - self.become_the_gardener() |
| 258 | + self.becomeTheGardener() |
| 259 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 260 | 'test.pot', distroseries=self.distroseries, |
| 261 | sourcepackagename=self.from_packagename) |
| 262 | @@ -456,7 +462,7 @@ |
| 263 | self.distrotemplate2.iscurrent = False |
| 264 | self.distrotemplate1.from_sourcepackagename = self.from_packagename |
| 265 | self.distrotemplate2.from_sourcepackagename = self.from_packagename |
| 266 | - self.become_the_gardener() |
| 267 | + self.becomeTheGardener() |
| 268 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 269 | 'test.pot', distroseries=self.distroseries, |
| 270 | sourcepackagename=self.from_packagename) |
| 271 | @@ -467,7 +473,7 @@ |
| 272 | # translation domain. |
| 273 | self._setUpDistro() |
| 274 | subset = POTemplateSubset(distroseries=self.distroseries) |
| 275 | - self.become_the_gardener() |
| 276 | + self.becomeTheGardener() |
| 277 | potemplate = subset.getPOTemplateByTranslationDomain('test1') |
| 278 | self.assertEqual(potemplate, self.distrotemplate1) |
| 279 | |
| 280 | @@ -475,7 +481,7 @@ |
| 281 | # Test getPOTemplateByTranslationDomain for the zero-match case. |
| 282 | self._setUpDistro() |
| 283 | subset = POTemplateSubset(distroseries=self.distroseries) |
| 284 | - self.become_the_gardener() |
| 285 | + self.becomeTheGardener() |
| 286 | potemplate = subset.getPOTemplateByTranslationDomain('notesthere') |
| 287 | self.assertEqual(potemplate, None) |
| 288 | |
| 289 | @@ -490,7 +496,7 @@ |
| 290 | clashing_template = other_subset.new( |
| 291 | 'test3', 'test1', 'test3.pot', self.distro.owner) |
| 292 | distro_subset = POTemplateSubset(distroseries=self.distroseries) |
| 293 | - self.become_the_gardener() |
| 294 | + self.becomeTheGardener() |
| 295 | potemplate = distro_subset.getPOTemplateByTranslationDomain('test1') |
| 296 | self.assertEqual(potemplate, None) |
| 297 | |
| 298 | @@ -520,7 +526,7 @@ |
| 299 | 'program/nl.po', 'other contents', False, template.owner, |
| 300 | productseries=template.productseries, potemplate=template) |
| 301 | |
| 302 | - self.become_the_gardener() |
| 303 | + self.becomeTheGardener() |
| 304 | entry1.getGuessedPOFile() |
| 305 | |
| 306 | self.assertEqual(entry1.potemplate, None) |
| 307 | @@ -642,7 +648,7 @@ |
| 308 | poname, self.pocontents, False, self.distroseries.owner, |
| 309 | sourcepackagename=self.kde_i18n_ca, |
| 310 | distroseries=self.distroseries) |
| 311 | - self.become_the_gardener() |
| 312 | + self.becomeTheGardener() |
| 313 | pofile = entry.getGuessedPOFile() |
| 314 | self.assertEqual(pofile, self.pofile_ca) |
| 315 | |
| 316 | @@ -654,7 +660,7 @@ |
| 317 | poname, self.pocontents, False, self.distroseries.owner, |
| 318 | sourcepackagename=self.kde_l10n_nl, |
| 319 | distroseries=self.distroseries) |
| 320 | - self.become_the_gardener() |
| 321 | + self.becomeTheGardener() |
| 322 | pofile = entry.getGuessedPOFile() |
| 323 | self.assertEqual(pofile, self.pofile_nl) |
| 324 | |
| 325 | @@ -682,7 +688,7 @@ |
| 326 | entry = self.queue.addOrUpdateEntry( |
| 327 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 328 | |
| 329 | - self.become_the_gardener() |
| 330 | + self.becomeTheGardener() |
| 331 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 332 | self.assertNotEqual(None, pofile) |
| 333 | |
| 334 | @@ -700,7 +706,7 @@ |
| 335 | entry = self.queue.addOrUpdateEntry( |
| 336 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 337 | |
| 338 | - self.become_the_gardener() |
| 339 | + self.becomeTheGardener() |
| 340 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 341 | self.assertEqual(None, pofile) |
| 342 | |
| 343 | @@ -720,7 +726,7 @@ |
| 344 | entry = self.queue.addOrUpdateEntry( |
| 345 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 346 | |
| 347 | - self.become_the_gardener() |
| 348 | + self.becomeTheGardener() |
| 349 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 350 | self.assertNotEqual(None, pofile) |
| 351 | |
| 352 | @@ -778,10 +784,14 @@ |
| 353 | entry_id = entry.id |
| 354 | |
| 355 | self._setStatus(entry, RosettaImportStatus.APPROVED, one_year_ago) |
| 356 | + # No write or delete action expected, so no reason to switch the |
| 357 | + # database user. If it writes or deletes, the test has failed anyway. |
| 358 | self.queue._cleanUpObsoleteEntries(self.store) |
| 359 | self.assertTrue(self._exists(entry_id)) |
| 360 | |
| 361 | self._setStatus(entry, RosettaImportStatus.BLOCKED, one_year_ago) |
| 362 | + # No write or delete action expected, so no reason to switch the |
| 363 | + # database user. If it writes or deletes, the test has failed anyway. |
| 364 | self.queue._cleanUpObsoleteEntries(self.store) |
| 365 | self.assertTrue(self._exists(entry_id)) |
| 366 | |
| 367 | @@ -799,7 +809,7 @@ |
| 368 | for status in affected_statuses: |
| 369 | entry = self._makeProductEntry() |
| 370 | entry.potemplate = self.factory.makePOTemplate() |
| 371 | - maximum_age = TranslationImportQueueEntryAge[status] |
| 372 | + maximum_age = translation_import_queue_entry_age[status] |
| 373 | # Avoid the exact date here by a day because that could introduce |
| 374 | # spurious test failures. |
| 375 | almost_oldest_possible_date = ( |
| 376 | @@ -807,14 +817,17 @@ |
| 377 | self._setStatus(entry, status, almost_oldest_possible_date) |
| 378 | entry_id = entry.id |
| 379 | |
| 380 | + # No write or delete action expected, so no reason to switch the |
| 381 | + # database user. If it writes or deletes, the test has failed |
| 382 | + # anyway. |
| 383 | self.queue._cleanUpObsoleteEntries(self.store) |
| 384 | self.assertTrue(self._exists(entry_id)) |
| 385 | |
| 386 | - # Now cross the border, again cushoning it by a day. |
| 387 | + # Now cross the border, again cushioning it by a day. |
| 388 | entry.date_status_changed -= timedelta(days=2) |
| 389 | entry.syncUpdate() |
| 390 | |
| 391 | - with self.being_the_gardener(): |
| 392 | + with self.beingTheGardener(): |
| 393 | self.queue._cleanUpObsoleteEntries(self.store) |
| 394 | self.assertFalse( |
| 395 | self._exists(entry_id), |
| 396 | @@ -833,7 +846,7 @@ |
| 397 | entry.productseries.product.active = False |
| 398 | entry.productseries.product.syncUpdate() |
| 399 | |
| 400 | - self.become_the_gardener() |
| 401 | + self.becomeTheGardener() |
| 402 | self.queue._cleanUpInactiveProductEntries(self.store) |
| 403 | self.assertFalse(self._exists(entry_id)) |
| 404 | |
| 405 | @@ -849,7 +862,7 @@ |
| 406 | entry.distroseries.status = SeriesStatus.OBSOLETE |
| 407 | entry.distroseries.syncUpdate() |
| 408 | |
| 409 | - self.become_the_gardener() |
| 410 | + self.becomeTheGardener() |
| 411 | self.queue._cleanUpObsoleteDistroEntries(self.store) |
| 412 | self.assertFalse(self._exists(entry_id)) |
| 413 | |
| 414 | @@ -885,7 +898,7 @@ |
| 415 | entry = self._makeQueueEntry(trunk) |
| 416 | rosetta_experts = getUtility(ILaunchpadCelebrities).rosetta_experts |
| 417 | |
| 418 | - self.become_the_gardener() |
| 419 | + self.becomeTheGardener() |
| 420 | |
| 421 | pofile = entry.getGuessedPOFile() |
| 422 | |
| 423 | @@ -903,7 +916,7 @@ |
| 424 | |
| 425 | entry = self._makeQueueEntry(trunk) |
| 426 | |
| 427 | - self.become_the_gardener() |
| 428 | + self.becomeTheGardener() |
| 429 | |
| 430 | pofile = entry.getGuessedPOFile() |
| 431 |
Revision history for this message
| Abel Deuring (adeuring) wrote : | # |
Thanks for your changes!
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/translations/interfaces/translationimportqueue.py' |
| 2 | --- lib/lp/translations/interfaces/translationimportqueue.py 2010-02-05 10:39:45 +0000 |
| 3 | +++ lib/lp/translations/interfaces/translationimportqueue.py 2010-02-19 11:18:22 +0000 |
| 4 | @@ -3,6 +3,8 @@ |
| 5 | |
| 6 | # pylint: disable-msg=E0211,E0213 |
| 7 | |
| 8 | +from datetime import timedelta |
| 9 | + |
| 10 | from zope.interface import Interface, Attribute |
| 11 | from zope.schema import ( |
| 12 | Bool, Choice, Datetime, Field, Int, Object, Text, TextLine) |
| 13 | @@ -39,6 +41,7 @@ |
| 14 | 'RosettaImportStatus', |
| 15 | 'SpecialTranslationImportTargetFilter', |
| 16 | 'TranslationFileType', |
| 17 | + 'translation_import_queue_entry_age', |
| 18 | 'UserCannotSetTranslationImportStatus', |
| 19 | ] |
| 20 | |
| 21 | @@ -111,6 +114,22 @@ |
| 22 | """) |
| 23 | |
| 24 | |
| 25 | +# Some time spans in days. |
| 26 | +DAYS_IN_MONTH = 30 |
| 27 | +DAYS_IN_HALF_YEAR = 366 / 2 |
| 28 | + |
| 29 | + |
| 30 | +# Period after which entries with certain statuses are culled from the |
| 31 | +# queue. |
| 32 | +translation_import_queue_entry_age = { |
| 33 | + RosettaImportStatus.DELETED: timedelta(days=3), |
| 34 | + RosettaImportStatus.FAILED: timedelta(days=DAYS_IN_MONTH), |
| 35 | + RosettaImportStatus.IMPORTED: timedelta(days=3), |
| 36 | + RosettaImportStatus.NEEDS_INFORMATION: timedelta(days=DAYS_IN_HALF_YEAR), |
| 37 | + RosettaImportStatus.NEEDS_REVIEW: timedelta(days=DAYS_IN_HALF_YEAR), |
| 38 | +} |
| 39 | + |
| 40 | + |
| 41 | class SpecialTranslationImportTargetFilter(DBEnumeratedType): |
| 42 | """Special "meta-targets" to filter the queue view by.""" |
| 43 | |
| 44 | |
| 45 | === modified file 'lib/lp/translations/model/translationimportqueue.py' |
| 46 | --- lib/lp/translations/model/translationimportqueue.py 2010-02-05 09:23:07 +0000 |
| 47 | +++ lib/lp/translations/model/translationimportqueue.py 2010-02-19 11:18:22 +0000 |
| 48 | @@ -58,6 +58,7 @@ |
| 49 | RosettaImportStatus, |
| 50 | SpecialTranslationImportTargetFilter, |
| 51 | TranslationImportQueueConflictError, |
| 52 | + translation_import_queue_entry_age, |
| 53 | UserCannotSetTranslationImportStatus) |
| 54 | from lp.translations.interfaces.potemplate import IPOTemplate |
| 55 | from lp.translations.interfaces.translations import TranslationConstants |
| 56 | @@ -66,19 +67,6 @@ |
| 57 | from canonical.librarian.interfaces import ILibrarianClient |
| 58 | |
| 59 | |
| 60 | -# Approximate number of days in a 6-month period. |
| 61 | -half_year = 366 / 2 |
| 62 | - |
| 63 | -# Period after which entries with certain statuses are culled from the |
| 64 | -# queue. |
| 65 | -entry_gc_age = { |
| 66 | - RosettaImportStatus.DELETED: datetime.timedelta(days=3), |
| 67 | - RosettaImportStatus.IMPORTED: datetime.timedelta(days=3), |
| 68 | - RosettaImportStatus.FAILED: datetime.timedelta(days=30), |
| 69 | - RosettaImportStatus.NEEDS_REVIEW: datetime.timedelta(days=half_year), |
| 70 | -} |
| 71 | - |
| 72 | - |
| 73 | def is_gettext_name(path): |
| 74 | """Does given file name indicate it's in gettext (PO or POT) format?""" |
| 75 | base_name, extension = os.path.splitext(path) |
| 76 | @@ -1256,8 +1244,8 @@ |
| 77 | """ |
| 78 | now = datetime.datetime.now(pytz.UTC) |
| 79 | deletion_clauses = [] |
| 80 | - for status, gc_age in entry_gc_age.iteritems(): |
| 81 | - cutoff = now - gc_age |
| 82 | + for status, max_age in translation_import_queue_entry_age.iteritems(): |
| 83 | + cutoff = now - max_age |
| 84 | deletion_clauses.append(And( |
| 85 | TranslationImportQueueEntry.status == status, |
| 86 | TranslationImportQueueEntry.date_status_changed < cutoff)) |
| 87 | |
| 88 | === modified file 'lib/lp/translations/tests/test_autoapproval.py' |
| 89 | --- lib/lp/translations/tests/test_autoapproval.py 2010-01-26 15:25:53 +0000 |
| 90 | +++ lib/lp/translations/tests/test_autoapproval.py 2010-02-19 11:18:22 +0000 |
| 91 | @@ -8,6 +8,9 @@ |
| 92 | through the possibilities should go here. |
| 93 | """ |
| 94 | |
| 95 | +from __future__ import with_statement |
| 96 | + |
| 97 | +from contextlib import contextmanager |
| 98 | from datetime import datetime, timedelta |
| 99 | from pytz import UTC |
| 100 | import transaction |
| 101 | @@ -31,17 +34,34 @@ |
| 102 | TranslationImportQueue, TranslationImportQueueEntry) |
| 103 | from lp.translations.interfaces.customlanguagecode import ICustomLanguageCode |
| 104 | from lp.translations.interfaces.translationimportqueue import ( |
| 105 | - RosettaImportStatus) |
| 106 | + RosettaImportStatus, translation_import_queue_entry_age) |
| 107 | from lp.testing import TestCaseWithFactory |
| 108 | from lp.testing.factory import LaunchpadObjectFactory |
| 109 | from canonical.launchpad.webapp.testing import verifyObject |
| 110 | from canonical.testing import LaunchpadZopelessLayer |
| 111 | |
| 112 | |
| 113 | -def become_the_gardener(layer): |
| 114 | - """Switch to the translations import queue gardener database role.""" |
| 115 | - transaction.commit() |
| 116 | - layer.switchDbUser('translations_import_queue_gardener') |
| 117 | +class GardenerDbUserMixin(object): |
| 118 | + """Switch to the translations import queue gardener database role. |
| 119 | + |
| 120 | + Admittedly, this might be a little over-engineered but it looks good. ;) |
| 121 | + """ |
| 122 | + |
| 123 | + def _become(self, dbuser): |
| 124 | + """Switch to a different db user.""" |
| 125 | + transaction.commit() |
| 126 | + self.layer.switchDbUser(dbuser) |
| 127 | + |
| 128 | + def becomeTheGardener(self): |
| 129 | + """One-way method to avoid unnecessary switch back.""" |
| 130 | + self._become('translations_import_queue_gardener') |
| 131 | + |
| 132 | + @contextmanager |
| 133 | + def beingTheGardener(self): |
| 134 | + """Context manager to restore the launchpad user.""" |
| 135 | + self._become('translations_import_queue_gardener') |
| 136 | + yield |
| 137 | + self._become('launchpad') |
| 138 | |
| 139 | |
| 140 | class TestCustomLanguageCode(unittest.TestCase): |
| 141 | @@ -129,7 +149,8 @@ |
| 142 | self.assertEqual(Brazilian_code.language, Language.byCode('pt_BR')) |
| 143 | |
| 144 | |
| 145 | -class TestGuessPOFileCustomLanguageCode(unittest.TestCase): |
| 146 | +class TestGuessPOFileCustomLanguageCode( |
| 147 | + unittest.TestCase, GardenerDbUserMixin): |
| 148 | """Test interaction with `TranslationImportQueueEntry.getGuessedPOFile`. |
| 149 | |
| 150 | Auto-approval of translation files, i.e. figuring out which existing |
| 151 | @@ -176,7 +197,7 @@ |
| 152 | # Of course matching will work without custom language codes. |
| 153 | tr_file = self._makePOFile('tr') |
| 154 | entry = self._makeQueueEntry('tr') |
| 155 | - become_the_gardener(self.layer) |
| 156 | + self.becomeTheGardener() |
| 157 | self.assertEqual(entry.getGuessedPOFile(), tr_file) |
| 158 | |
| 159 | def test_CustomLanguageCodeEnablesMatch(self): |
| 160 | @@ -188,7 +209,7 @@ |
| 161 | |
| 162 | self._setCustomLanguageCode('fy_NL', 'fy') |
| 163 | |
| 164 | - become_the_gardener(self.layer) |
| 165 | + self.becomeTheGardener() |
| 166 | self.assertEqual(entry.getGuessedPOFile(), fy_file) |
| 167 | |
| 168 | def test_CustomLanguageCodeParsesBogusLanguage(self): |
| 169 | @@ -199,7 +220,7 @@ |
| 170 | |
| 171 | self._setCustomLanguageCode('flemish', 'nl') |
| 172 | |
| 173 | - become_the_gardener(self.layer) |
| 174 | + self.becomeTheGardener() |
| 175 | nl_file = entry.getGuessedPOFile() |
| 176 | self.assertEqual(nl_file.language.code, 'nl') |
| 177 | |
| 178 | @@ -212,7 +233,7 @@ |
| 179 | |
| 180 | self._setCustomLanguageCode('sv', None) |
| 181 | |
| 182 | - become_the_gardener(self.layer) |
| 183 | + self.becomeTheGardener() |
| 184 | self.assertEqual(entry.getGuessedPOFile(), None) |
| 185 | self.assertEqual(entry.status, RosettaImportStatus.DELETED) |
| 186 | |
| 187 | @@ -225,7 +246,7 @@ |
| 188 | |
| 189 | self._setCustomLanguageCode('elx', 'el') |
| 190 | |
| 191 | - become_the_gardener(self.layer) |
| 192 | + self.becomeTheGardener() |
| 193 | el_file = entry.getGuessedPOFile() |
| 194 | self.failIfEqual(el_file, elx_file) |
| 195 | self.assertEqual(el_file.language.code, 'el') |
| 196 | @@ -240,7 +261,7 @@ |
| 197 | |
| 198 | self._setCustomLanguageCode('nb', 'nn') |
| 199 | |
| 200 | - become_the_gardener(self.layer) |
| 201 | + self.becomeTheGardener() |
| 202 | self.assertEqual(entry.getGuessedPOFile(), nn_file) |
| 203 | |
| 204 | def test_CustomLanguageCodeReplacesMatch(self): |
| 205 | @@ -253,7 +274,7 @@ |
| 206 | self._setCustomLanguageCode('pt', None) |
| 207 | self._setCustomLanguageCode('pt_PT', 'pt') |
| 208 | |
| 209 | - become_the_gardener(self.layer) |
| 210 | + self.becomeTheGardener() |
| 211 | self.assertEqual(pt_entry.getGuessedPOFile(), None) |
| 212 | self.assertEqual(pt_PT_entry.getGuessedPOFile(), pt_file) |
| 213 | |
| 214 | @@ -267,12 +288,12 @@ |
| 215 | self._setCustomLanguageCode('zh_CN', 'zh_TW') |
| 216 | self._setCustomLanguageCode('zh_TW', 'zh_CN') |
| 217 | |
| 218 | - become_the_gardener(self.layer) |
| 219 | + self.becomeTheGardener() |
| 220 | self.assertEqual(zh_CN_entry.getGuessedPOFile(), zh_TW_file) |
| 221 | self.assertEqual(zh_TW_entry.getGuessedPOFile(), zh_CN_file) |
| 222 | |
| 223 | |
| 224 | -class TestTemplateGuess(unittest.TestCase): |
| 225 | +class TestTemplateGuess(unittest.TestCase, GardenerDbUserMixin): |
| 226 | """Test auto-approval's attempts to find the right template.""" |
| 227 | layer = LaunchpadZopelessLayer |
| 228 | |
| 229 | @@ -312,7 +333,7 @@ |
| 230 | # When multiple templates match for a product series, |
| 231 | # getPOTemplateByPathAndOrigin returns none. |
| 232 | self._setUpProduct() |
| 233 | - become_the_gardener(self.layer) |
| 234 | + self.becomeTheGardener() |
| 235 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 236 | 'test.pot', productseries=self.productseries) |
| 237 | self.assertEqual(None, guessed_template) |
| 238 | @@ -321,7 +342,7 @@ |
| 239 | # When multiple templates match on sourcepackagename, |
| 240 | # getPOTemplateByPathAndOrigin returns none. |
| 241 | self._setUpDistro() |
| 242 | - become_the_gardener(self.layer) |
| 243 | + self.becomeTheGardener() |
| 244 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 245 | 'test.pot', sourcepackagename=self.packagename) |
| 246 | self.assertEqual(None, guessed_template) |
| 247 | @@ -330,7 +351,7 @@ |
| 248 | # When multiple templates match on from_sourcepackagename, |
| 249 | # getPOTemplateByPathAndOrigin returns none. |
| 250 | self._setUpDistro() |
| 251 | - become_the_gardener(self.layer) |
| 252 | + self.becomeTheGardener() |
| 253 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 254 | 'test.pot', sourcepackagename=self.from_packagename) |
| 255 | self.assertEqual(None, guessed_template) |
| 256 | @@ -364,7 +385,7 @@ |
| 257 | self.distrotemplate1.sourcepackagename = match_package |
| 258 | self.distrotemplate2.from_sourcepackagename = match_package |
| 259 | |
| 260 | - become_the_gardener(self.layer) |
| 261 | + self.becomeTheGardener() |
| 262 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 263 | 'test.pot', sourcepackagename=match_package) |
| 264 | self.assertEqual(self.distrotemplate2, guessed_template) |
| 265 | @@ -375,7 +396,7 @@ |
| 266 | self._setUpProduct() |
| 267 | self.producttemplate1.iscurrent = False |
| 268 | self.producttemplate2.iscurrent = True |
| 269 | - become_the_gardener(self.layer) |
| 270 | + self.becomeTheGardener() |
| 271 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 272 | 'test.pot', productseries=self.productseries) |
| 273 | self.assertEqual(guessed_template, self.producttemplate2) |
| 274 | @@ -385,7 +406,7 @@ |
| 275 | self._setUpProduct() |
| 276 | self.producttemplate1.iscurrent = False |
| 277 | self.producttemplate2.iscurrent = False |
| 278 | - become_the_gardener(self.layer) |
| 279 | + self.becomeTheGardener() |
| 280 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 281 | 'test.pot', productseries=self.productseries) |
| 282 | self.assertEqual(guessed_template, None) |
| 283 | @@ -399,7 +420,7 @@ |
| 284 | self.distrotemplate2.iscurrent = True |
| 285 | self.distrotemplate1.from_sourcepackagename = None |
| 286 | self.distrotemplate2.from_sourcepackagename = None |
| 287 | - become_the_gardener(self.layer) |
| 288 | + self.becomeTheGardener() |
| 289 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 290 | 'test.pot', distroseries=self.distroseries, |
| 291 | sourcepackagename=self.packagename) |
| 292 | @@ -412,7 +433,7 @@ |
| 293 | self.distrotemplate2.iscurrent = False |
| 294 | self.distrotemplate1.from_sourcepackagename = None |
| 295 | self.distrotemplate2.from_sourcepackagename = None |
| 296 | - become_the_gardener(self.layer) |
| 297 | + self.becomeTheGardener() |
| 298 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 299 | 'test.pot', distroseries=self.distroseries, |
| 300 | sourcepackagename=self.packagename) |
| 301 | @@ -427,7 +448,7 @@ |
| 302 | self.distrotemplate2.iscurrent = True |
| 303 | self.distrotemplate1.from_sourcepackagename = self.from_packagename |
| 304 | self.distrotemplate2.from_sourcepackagename = self.from_packagename |
| 305 | - become_the_gardener(self.layer) |
| 306 | + self.becomeTheGardener() |
| 307 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 308 | 'test.pot', distroseries=self.distroseries, |
| 309 | sourcepackagename=self.from_packagename) |
| 310 | @@ -441,7 +462,7 @@ |
| 311 | self.distrotemplate2.iscurrent = False |
| 312 | self.distrotemplate1.from_sourcepackagename = self.from_packagename |
| 313 | self.distrotemplate2.from_sourcepackagename = self.from_packagename |
| 314 | - become_the_gardener(self.layer) |
| 315 | + self.becomeTheGardener() |
| 316 | guessed_template = self.templateset.getPOTemplateByPathAndOrigin( |
| 317 | 'test.pot', distroseries=self.distroseries, |
| 318 | sourcepackagename=self.from_packagename) |
| 319 | @@ -452,7 +473,7 @@ |
| 320 | # translation domain. |
| 321 | self._setUpDistro() |
| 322 | subset = POTemplateSubset(distroseries=self.distroseries) |
| 323 | - become_the_gardener(self.layer) |
| 324 | + self.becomeTheGardener() |
| 325 | potemplate = subset.getPOTemplateByTranslationDomain('test1') |
| 326 | self.assertEqual(potemplate, self.distrotemplate1) |
| 327 | |
| 328 | @@ -460,7 +481,7 @@ |
| 329 | # Test getPOTemplateByTranslationDomain for the zero-match case. |
| 330 | self._setUpDistro() |
| 331 | subset = POTemplateSubset(distroseries=self.distroseries) |
| 332 | - become_the_gardener(self.layer) |
| 333 | + self.becomeTheGardener() |
| 334 | potemplate = subset.getPOTemplateByTranslationDomain('notesthere') |
| 335 | self.assertEqual(potemplate, None) |
| 336 | |
| 337 | @@ -475,7 +496,7 @@ |
| 338 | clashing_template = other_subset.new( |
| 339 | 'test3', 'test1', 'test3.pot', self.distro.owner) |
| 340 | distro_subset = POTemplateSubset(distroseries=self.distroseries) |
| 341 | - become_the_gardener(self.layer) |
| 342 | + self.becomeTheGardener() |
| 343 | potemplate = distro_subset.getPOTemplateByTranslationDomain('test1') |
| 344 | self.assertEqual(potemplate, None) |
| 345 | |
| 346 | @@ -505,7 +526,7 @@ |
| 347 | 'program/nl.po', 'other contents', False, template.owner, |
| 348 | productseries=template.productseries, potemplate=template) |
| 349 | |
| 350 | - become_the_gardener(self.layer) |
| 351 | + self.becomeTheGardener() |
| 352 | entry1.getGuessedPOFile() |
| 353 | |
| 354 | self.assertEqual(entry1.potemplate, None) |
| 355 | @@ -575,7 +596,7 @@ |
| 356 | self.assertEqual(template, entry.guessed_potemplate) |
| 357 | |
| 358 | |
| 359 | -class TestKdePOFileGuess(unittest.TestCase): |
| 360 | +class TestKdePOFileGuess(unittest.TestCase, GardenerDbUserMixin): |
| 361 | """Test auto-approval's `POFile` guessing for KDE uploads. |
| 362 | |
| 363 | KDE has an unusual setup that the approver recognizes as a special |
| 364 | @@ -627,7 +648,7 @@ |
| 365 | poname, self.pocontents, False, self.distroseries.owner, |
| 366 | sourcepackagename=self.kde_i18n_ca, |
| 367 | distroseries=self.distroseries) |
| 368 | - become_the_gardener(self.layer) |
| 369 | + self.becomeTheGardener() |
| 370 | pofile = entry.getGuessedPOFile() |
| 371 | self.assertEqual(pofile, self.pofile_ca) |
| 372 | |
| 373 | @@ -639,12 +660,12 @@ |
| 374 | poname, self.pocontents, False, self.distroseries.owner, |
| 375 | sourcepackagename=self.kde_l10n_nl, |
| 376 | distroseries=self.distroseries) |
| 377 | - become_the_gardener(self.layer) |
| 378 | + self.becomeTheGardener() |
| 379 | pofile = entry.getGuessedPOFile() |
| 380 | self.assertEqual(pofile, self.pofile_nl) |
| 381 | |
| 382 | |
| 383 | -class TestGetPOFileFromLanguage(TestCaseWithFactory): |
| 384 | +class TestGetPOFileFromLanguage(TestCaseWithFactory, GardenerDbUserMixin): |
| 385 | """Test `TranslationImportQueueEntry._get_pofile_from_language`.""" |
| 386 | |
| 387 | layer = LaunchpadZopelessLayer |
| 388 | @@ -667,7 +688,7 @@ |
| 389 | entry = self.queue.addOrUpdateEntry( |
| 390 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 391 | |
| 392 | - become_the_gardener(self.layer) |
| 393 | + self.becomeTheGardener() |
| 394 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 395 | self.assertNotEqual(None, pofile) |
| 396 | |
| 397 | @@ -685,7 +706,7 @@ |
| 398 | entry = self.queue.addOrUpdateEntry( |
| 399 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 400 | |
| 401 | - become_the_gardener(self.layer) |
| 402 | + self.becomeTheGardener() |
| 403 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 404 | self.assertEqual(None, pofile) |
| 405 | |
| 406 | @@ -705,12 +726,12 @@ |
| 407 | entry = self.queue.addOrUpdateEntry( |
| 408 | 'nl.po', '# ...', False, template.owner, productseries=trunk) |
| 409 | |
| 410 | - become_the_gardener(self.layer) |
| 411 | + self.becomeTheGardener() |
| 412 | pofile = entry._get_pofile_from_language('nl', 'domain') |
| 413 | self.assertNotEqual(None, pofile) |
| 414 | |
| 415 | |
| 416 | -class TestCleanup(TestCaseWithFactory): |
| 417 | +class TestCleanup(TestCaseWithFactory, GardenerDbUserMixin): |
| 418 | """Test `TranslationImportQueueEntry` garbage collection.""" |
| 419 | |
| 420 | layer = LaunchpadZopelessLayer |
| 421 | @@ -763,10 +784,14 @@ |
| 422 | entry_id = entry.id |
| 423 | |
| 424 | self._setStatus(entry, RosettaImportStatus.APPROVED, one_year_ago) |
| 425 | + # No write or delete action expected, so no reason to switch the |
| 426 | + # database user. If it writes or deletes, the test has failed anyway. |
| 427 | self.queue._cleanUpObsoleteEntries(self.store) |
| 428 | self.assertTrue(self._exists(entry_id)) |
| 429 | |
| 430 | self._setStatus(entry, RosettaImportStatus.BLOCKED, one_year_ago) |
| 431 | + # No write or delete action expected, so no reason to switch the |
| 432 | + # database user. If it writes or deletes, the test has failed anyway. |
| 433 | self.queue._cleanUpObsoleteEntries(self.store) |
| 434 | self.assertTrue(self._exists(entry_id)) |
| 435 | |
| 436 | @@ -774,38 +799,39 @@ |
| 437 | # _cleanUpObsoleteEntries deletes entries in terminal states |
| 438 | # (Imported, Failed, Deleted) after a few days. The exact |
| 439 | # period depends on the state. |
| 440 | - entry = self._makeProductEntry() |
| 441 | - entry.potemplate = self.factory.makePOTemplate() |
| 442 | - self._setStatus(entry, RosettaImportStatus.IMPORTED, None) |
| 443 | - entry_id = entry.id |
| 444 | - |
| 445 | - self.queue._cleanUpObsoleteEntries(self.store) |
| 446 | - self.assertTrue(self._exists(entry_id)) |
| 447 | - |
| 448 | - entry.date_status_changed -= timedelta(days=7) |
| 449 | - entry.syncUpdate() |
| 450 | - |
| 451 | - become_the_gardener(self.layer) |
| 452 | - self.queue._cleanUpObsoleteEntries(self.store) |
| 453 | - self.assertFalse(self._exists(entry_id)) |
| 454 | - |
| 455 | - def test_cleanUpObsoleteEntries_needs_review(self): |
| 456 | - # _cleanUpObsoleteEntries cleans up entries in Needs Review |
| 457 | - # state after a very long wait. |
| 458 | - entry = self._makeProductEntry() |
| 459 | - entry.potemplate = self.factory.makePOTemplate() |
| 460 | - self._setStatus(entry, RosettaImportStatus.NEEDS_REVIEW, None) |
| 461 | - entry_id = entry.id |
| 462 | - |
| 463 | - self.queue._cleanUpObsoleteEntries(self.store) |
| 464 | - self.assertTrue(self._exists(entry_id)) |
| 465 | - |
| 466 | - entry.date_status_changed -= timedelta(days=200) |
| 467 | - entry.syncUpdate() |
| 468 | - |
| 469 | - become_the_gardener(self.layer) |
| 470 | - self.queue._cleanUpObsoleteEntries(self.store) |
| 471 | - self.assertFalse(self._exists(entry_id)) |
| 472 | + affected_statuses = [ |
| 473 | + RosettaImportStatus.DELETED, |
| 474 | + RosettaImportStatus.FAILED, |
| 475 | + RosettaImportStatus.IMPORTED, |
| 476 | + RosettaImportStatus.NEEDS_INFORMATION, |
| 477 | + RosettaImportStatus.NEEDS_REVIEW, |
| 478 | + ] |
| 479 | + for status in affected_statuses: |
| 480 | + entry = self._makeProductEntry() |
| 481 | + entry.potemplate = self.factory.makePOTemplate() |
| 482 | + maximum_age = translation_import_queue_entry_age[status] |
| 483 | + # Avoid the exact date here by a day because that could introduce |
| 484 | + # spurious test failures. |
| 485 | + almost_oldest_possible_date = ( |
| 486 | + datetime.now(UTC) - maximum_age + timedelta(days=1)) |
| 487 | + self._setStatus(entry, status, almost_oldest_possible_date) |
| 488 | + entry_id = entry.id |
| 489 | + |
| 490 | + # No write or delete action expected, so no reason to switch the |
| 491 | + # database user. If it writes or deletes, the test has failed |
| 492 | + # anyway. |
| 493 | + self.queue._cleanUpObsoleteEntries(self.store) |
| 494 | + self.assertTrue(self._exists(entry_id)) |
| 495 | + |
| 496 | + # Now cross the border, again cushioning it by a day. |
| 497 | + entry.date_status_changed -= timedelta(days=2) |
| 498 | + entry.syncUpdate() |
| 499 | + |
| 500 | + with self.beingTheGardener(): |
| 501 | + self.queue._cleanUpObsoleteEntries(self.store) |
| 502 | + self.assertFalse( |
| 503 | + self._exists(entry_id), |
| 504 | + "Queue entry in state '%s' was not removed." % status) |
| 505 | |
| 506 | |
| 507 | def test_cleanUpInactiveProductEntries(self): |
| 508 | @@ -820,7 +846,7 @@ |
| 509 | entry.productseries.product.active = False |
| 510 | entry.productseries.product.syncUpdate() |
| 511 | |
| 512 | - become_the_gardener(self.layer) |
| 513 | + self.becomeTheGardener() |
| 514 | self.queue._cleanUpInactiveProductEntries(self.store) |
| 515 | self.assertFalse(self._exists(entry_id)) |
| 516 | |
| 517 | @@ -836,12 +862,12 @@ |
| 518 | entry.distroseries.status = SeriesStatus.OBSOLETE |
| 519 | entry.distroseries.syncUpdate() |
| 520 | |
| 521 | - become_the_gardener(self.layer) |
| 522 | + self.becomeTheGardener() |
| 523 | self.queue._cleanUpObsoleteDistroEntries(self.store) |
| 524 | self.assertFalse(self._exists(entry_id)) |
| 525 | |
| 526 | |
| 527 | -class TestAutoApprovalNewPOFile(TestCaseWithFactory): |
| 528 | +class TestAutoApprovalNewPOFile(TestCaseWithFactory, GardenerDbUserMixin): |
| 529 | """Test creation of new `POFile`s in approval.""" |
| 530 | |
| 531 | layer = LaunchpadZopelessLayer |
| 532 | @@ -872,7 +898,7 @@ |
| 533 | entry = self._makeQueueEntry(trunk) |
| 534 | rosetta_experts = getUtility(ILaunchpadCelebrities).rosetta_experts |
| 535 | |
| 536 | - become_the_gardener(self.layer) |
| 537 | + self.becomeTheGardener() |
| 538 | |
| 539 | pofile = entry.getGuessedPOFile() |
| 540 | |
| 541 | @@ -890,7 +916,7 @@ |
| 542 | |
| 543 | entry = self._makeQueueEntry(trunk) |
| 544 | |
| 545 | - become_the_gardener(self.layer) |
| 546 | + self.becomeTheGardener() |
| 547 | |
| 548 | pofile = entry.getGuessedPOFile() |
| 549 |
= Bug 523810 =
The import queue gardener removes entries from the import queue that have reached a certain age in certain states. The age is configurable through a look-up table and the new "Needs Information" status needs an entry in that table. The maximum age should be identical to "Needs Review" for starters.
== Implementation details ==
Driven by the want to extend test coverage to all entry states that are being cleaned out I moved the look-up table to the interface module where it can be reached from the test.
I also found a nice little use of the with statement again. ;-)
== Tests ==
Run all autoapproval test to verify that the introduction of the GardenerDbUserMixin did not mess up existing tests.
bin/test -vvt autoapproval
== Demo/QA ==
In order to not have to wait half a year for an entry to expire on staging, some SQL magic will be needed to make an entry in the "Needs Information" stage that old. Then wait for a day or so to see it disappear. Staging updates might be a nuissance here.
= 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/
lib/lp/
lib/lp/
== Pylint notices ==
lib/lp/
12: [F0401] Unable to import 'lazr.enum' (No module named enum)
22: [F0401] Unable to import 'lazr.restful.
23: [F0401] Unable to import 'lazr.restful.
24: [F0401] Unable to import 'lazr.restful.