launchpad-reviewers team mailing list archive

[Colin Watson <mp+429651@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:reset-logger-propagation into launchpad:master.

Commit message:
Reset logger propagation between tests

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/429651

In a few places (`BuilddManager._setupLogger`, `TestGarbo.setUp`, and `NumberCruncher._setupLogger`) we set up a logger in such a way that it doesn't propagate unhandled messages to its parent logger.  We need to reset such loggers between tests so that they don't cause isolation failures with later tests.

I specifically excluded the "amqp", "brz", and "bzr" loggers from this because `lp_sitecustomize` sets those to non-propagating, and we should leave that alone.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:reset-logger-propagation into launchpad:master.

diff --git a/lib/lp/testing/__init__.py b/lib/lp/testing/__init__.py
index b250fb0..4133a2a 100644
--- a/lib/lp/testing/__init__.py
+++ b/lib/lp/testing/__init__.py
@@ -186,7 +186,8 @@ def reset_logging():
     Currently, defaults means 'the way the Z3 testrunner sets it up'
     plus customizations made in lp_sitecustomize
     """
-    # Remove all handlers from non-root loggers.
+    # Remove all handlers from non-root loggers, and make sure they're all
+    # set to propagate messages.
     loggerDict = logging.Logger.manager.loggerDict
     for name, logger in list(loggerDict.items()):
         if name == "pagetests-access":
@@ -195,6 +196,8 @@ def reset_logging():
         if not isinstance(logger, logging.PlaceHolder):
             for handler in list(logger.handlers):
                 logger.removeHandler(handler)
+        if name not in {"amqp", "brz", "bzr"}:
+            logger.propagate = True
 
     # Remove all handlers from the root logger
     root = logging.getLogger("")
diff --git a/lib/lp_sitecustomize.py b/lib/lp_sitecustomize.py
index 4624d29..3fe64df 100644
--- a/lib/lp_sitecustomize.py
+++ b/lib/lp_sitecustomize.py
@@ -63,6 +63,7 @@ def silence_amqp_logger():
     """Install the NullHandler on the amqp logger to silence logs."""
     amqp_logger = logging.getLogger("amqp")
     amqp_logger.addHandler(logging.NullHandler())
+    # See also lp.testing.reset_logging.
     amqp_logger.propagate = False
 
 
@@ -71,6 +72,7 @@ def silence_bzr_loggers():
     for logger_name in ("bzr", "brz"):
         logger = logging.getLogger(logger_name)
         logger.addHandler(logging.NullHandler())
+        # See also lp.testing.reset_logging.
         logger.propagate = False