launchpad-reviewers team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/launchpad/bug-987441-externalbugtracker-debbugs into lp:launchpad.

Requested reviews:
  Launchpad Yellow Squad (yellow)
Related bugs:
  Bug #987441 in Launchpad itself: "lib/lp/bugs/doc/externalbugtracker-debbugs.txt fails intermittently in parallel tests"
  https://bugs.launchpad.net/launchpad/+bug/987441

For more details, see:
https://code.launchpad.net/~frankban/launchpad/bug-987441-externalbugtracker-debbugs/+merge/103903

== Changes ==

I've narrowed down all the tests included in the subunit output attached to the bug, and found the conflicting tests.
The offending ones are those in lp.translations.tests.test_translationmerger inheriting from TranslatableProductMixin.
The mixin changed the log level of the global logger.
Those tests now use a fake logger in place of the real one.

No QA.
-- 
https://code.launchpad.net/~frankban/launchpad/bug-987441-externalbugtracker-debbugs/+merge/103903
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/bugs/doc/externalbugtracker-debbugs.txt'
--- lib/lp/bugs/doc/externalbugtracker-debbugs.txt	2012-04-12 11:38:44 +0000
+++ lib/lp/bugs/doc/externalbugtracker-debbugs.txt	2012-04-27 15:26:26 +0000
@@ -682,13 +682,13 @@
 won't be imported again. A warning is logged so that the person runnning
 the script gets notified about it.
 
-    >>> from lp.bugs.scripts.importdebianbugs import (
-    ...     import_debian_bugs)
+    >>> from lp.bugs.scripts.importdebianbugs import import_debian_bugs
+    >>> from lp.services.log.logger import FakeLogger
     >>> [bug.id for bug in debbugs.getBugsWatching('304014')]
     [1]
-    >>> import_debian_bugs(['304014'])
-    WARNING:...:Not importing debbugs #304014, since it's already
-                linked from LP bug(s) #1.
+    >>> import_debian_bugs(['304014'], logger=FakeLogger())
+    WARNING Not importing debbugs #304014, since it's already
+            linked from LP bug(s) #1.
     >>> [bug.id for bug in debbugs.getBugsWatching('304014')]
     [1]
 

=== modified file 'lib/lp/bugs/scripts/importdebianbugs.py'
--- lib/lp/bugs/scripts/importdebianbugs.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/scripts/importdebianbugs.py	2012-04-27 15:26:26 +0000
@@ -15,17 +15,19 @@
 from lp.services.scripts.logger import log
 
 
-def import_debian_bugs(bugs_to_import):
+def import_debian_bugs(bugs_to_import, logger=None):
     """Import the specified Debian bugs into Launchpad."""
+    if logger is None:
+        logger = log
     debbugs = getUtility(ILaunchpadCelebrities).debbugs
     external_debbugs = get_external_bugtracker(debbugs)
-    bug_watch_updater = CheckwatchesMaster(transaction, log)
+    bug_watch_updater = CheckwatchesMaster(transaction, logger)
     debian = getUtility(ILaunchpadCelebrities).debian
     for debian_bug in bugs_to_import:
         existing_bug_ids = [
             str(bug.id) for bug in debbugs.getBugsWatching(debian_bug)]
         if len(existing_bug_ids) > 0:
-            log.warning(
+            logger.warning(
                 "Not importing debbugs #%s, since it's already linked"
                 " from LP bug(s) #%s." % (
                     debian_bug, ', '.join(existing_bug_ids)))
@@ -41,6 +43,6 @@
             target = target.getSourcePackage(debian_task.sourcepackagename)
         getUtility(IBugTaskSet).createTask(
             bug, getUtility(ILaunchpadCelebrities).bug_watch_updater, target)
-        log.info(
+        logger.info(
             "Imported debbugs #%s as Launchpad bug #%s." % (
                 debian_bug, bug.id))

=== modified file 'lib/lp/translations/tests/test_translationmerger.py'
--- lib/lp/translations/tests/test_translationmerger.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/tests/test_translationmerger.py	2012-04-27 15:26:26 +0000
@@ -11,6 +11,7 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.services.log.logger import FakeLogger
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.testing import (
     person_logged_in,
@@ -53,7 +54,8 @@
         self.stable_template.iscurrent = False
         self.templates = [self.trunk_template, self.stable_template]
 
-        self.script = MessageSharingMerge('tms-merging-test', test_args=[])
+        self.script = MessageSharingMerge(
+            'tms-merging-test', test_args=[], logger=FakeLogger())
         self.script.logger.setLevel(ERROR)
         tm = TransactionManager(self.script.txn, self.script.options.dry_run)
         self.merger = TranslationMerger(self.templates, tm)