launchpad-reviewers team mailing list archive

[Colin Watson <mp+415820@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:avoid-python-logging-observer into launchpad-buildd:master.

Commit message:
Avoid buggy PythonLoggingObserver on focal

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

The packaged version of Twisted in focal has a broken `twisted.python.log.PythonLoggingObserver` (https://bugs.launchpad.net/bugs/1961455).  While this should probably be fixed in Ubuntu, it only affects one test suite, so it's easy to avoid it by restructuring that suite using `FileLogObserver` from the more modern `twisted.logger` package.

(Unfortunately we can't yet use the simpler `twisted.logger.capturedLogs`, since the packaged version of Twisted in focal is a little too old for that; but this construction is a similar level of complexity to what we had before.)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:avoid-python-logging-observer into launchpad-buildd:master.

diff --git a/lpbuildd/tests/test_builder.py b/lpbuildd/tests/test_builder.py
index 2048bc6..177f644 100644
--- a/lpbuildd/tests/test_builder.py
+++ b/lpbuildd/tests/test_builder.py
@@ -9,15 +9,16 @@ Most tests are done on subclasses instead.
 import io
 import re
 
-from fixtures import (
-    FakeLogger,
-    TempDir,
-    )
+from fixtures import TempDir
 import six
 from testtools import TestCase
 from testtools.deferredruntest import AsynchronousDeferredRunTest
 from twisted.internet import defer
-from twisted.python import log
+from twisted.logger import (
+    FileLogObserver,
+    formatEvent,
+    globalLogPublisher,
+    )
 
 from lpbuildd.builder import (
     Builder,
@@ -32,13 +33,14 @@ class TestBuildManager(TestCase):
 
     def setUp(self):
         super(TestBuildManager, self).setUp()
-        observer = log.PythonLoggingObserver()
-        observer.start()
-        self.addCleanup(observer.stop)
+        self.log_file = io.StringIO()
+        observer = FileLogObserver(
+            self.log_file, lambda event: formatEvent(event) + "\n")
+        globalLogPublisher.addObserver(observer)
+        self.addCleanup(globalLogPublisher.removeObserver, observer)
 
     @defer.inlineCallbacks
     def test_runSubProcess(self):
-        logger = self.useFixture(FakeLogger())
         config = FakeConfig()
         config.set("builder", "filecache", self.useFixture(TempDir()).path)
         builder = Builder(config)
@@ -56,11 +58,10 @@ class TestBuildManager(TestCase):
         self.assertEqual(
             "Build log: RUN: echo 'hello world'\n"
             "Build log: hello world\n",
-            logger.output)
+            self.log_file.getvalue())
 
     @defer.inlineCallbacks
     def test_runSubProcess_bytes(self):
-        logger = self.useFixture(FakeLogger())
         config = FakeConfig()
         config.set("builder", "filecache", self.useFixture(TempDir()).path)
         builder = Builder(config)
@@ -80,4 +81,4 @@ class TestBuildManager(TestCase):
             ["Build log: RUN: echo '%s'" % logged_snowman,
              "Build log: %s" % logged_snowman],
             [re.sub(r".*? \[-\] (.*)", r"\1", line)
-             for line in logger.output.splitlines()])
+             for line in self.log_file.getvalue().splitlines()])