launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04464
[Merge] lp:~jelmer/launchpad/logginguifactory-fixes into lp:launchpad
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/logginguifactory-fixes into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #819751 in Launchpad itself: "LoggingUIFactory has some broken methods"
https://bugs.launchpad.net/launchpad/+bug/819751
For more details, see:
https://code.launchpad.net/~jelmer/launchpad/logginguifactory-fixes/+merge/70144
Fix the implementation of several methods in the custom LoggingUIFactory for Bazaar that is used on the code import workers, with appropriate test coverage.
Also, derive the class from NoninteractiveUIFactory rather than TextUIFactory, which is really more aimed at (interactive) command-line use.
--
https://code.launchpad.net/~jelmer/launchpad/logginguifactory-fixes/+merge/70144
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/logginguifactory-fixes into lp:launchpad.
=== modified file 'lib/lp/codehosting/codeimport/tests/test_uifactory.py'
--- lib/lp/codehosting/codeimport/tests/test_uifactory.py 2011-07-19 18:09:01 +0000
+++ lib/lp/codehosting/codeimport/tests/test_uifactory.py 2011-08-02 11:35:56 +0000
@@ -10,6 +10,7 @@
import unittest
from lp.codehosting.codeimport.uifactory import LoggingUIFactory
+from lp.services.log.logger import BufferLogger
from lp.testing import (
FakeTime,
TestCase,
@@ -22,19 +23,19 @@
def setUp(self):
TestCase.setUp(self)
self.fake_time = FakeTime(12345)
- self.messages = []
+ self.logger = BufferLogger()
def makeLoggingUIFactory(self):
"""Make a `LoggingUIFactory` with fake time and contained output."""
return LoggingUIFactory(
- time_source=self.fake_time.now, writer=self.messages.append)
+ time_source=self.fake_time.now, logger=self.logger)
def test_first_progress_updates(self):
# The first call to progress generates some output.
factory = self.makeLoggingUIFactory()
bar = factory.nested_progress_bar()
bar.update("hi")
- self.assertEqual(['hi'], self.messages)
+ self.assertEqual('INFO hi\n', self.logger.getLogBuffer())
def test_second_rapid_progress_doesnt_update(self):
# The second of two progress calls that are less than the factory's
@@ -44,7 +45,7 @@
bar.update("hi")
self.fake_time.advance(factory.interval / 2)
bar.update("there")
- self.assertEqual(['hi'], self.messages)
+ self.assertEqual('INFO hi\n', self.logger.getLogBuffer())
def test_second_slow_progress_updates(self):
# The second of two progress calls that are more than the factory's
@@ -54,7 +55,10 @@
bar.update("hi")
self.fake_time.advance(factory.interval * 2)
bar.update("there")
- self.assertEqual(['hi', 'there'], self.messages)
+ self.assertEqual(
+ 'INFO hi\n'
+ 'INFO there\n',
+ self.logger.getLogBuffer())
def test_first_progress_on_new_bar_updates(self):
# The first progress on a new progress task always generates output.
@@ -64,14 +68,15 @@
self.fake_time.advance(factory.interval / 2)
bar2 = factory.nested_progress_bar()
bar2.update("there")
- self.assertEqual(['hi', 'hi:there'], self.messages)
+ self.assertEqual(
+ 'INFO hi\nINFO hi:there\n', self.logger.getLogBuffer())
def test_update_with_count_formats_nicely(self):
# When more details are passed to update, they are formatted nicely.
factory = self.makeLoggingUIFactory()
bar = factory.nested_progress_bar()
bar.update("hi", 1, 8)
- self.assertEqual(['hi 1/8'], self.messages)
+ self.assertEqual('INFO hi 1/8\n', self.logger.getLogBuffer())
def test_report_transport_activity_reports_bytes_since_last_update(self):
# If there is no call to _progress_updated for 'interval' seconds, the
@@ -94,9 +99,54 @@
# activity info.
bar.update("hi", 3, 10)
self.assertEqual(
- ['hi 1/10', 'hi 2/10', '110 bytes transferred | hi 2/10',
- 'hi 3/10'],
- self.messages)
+ 'INFO hi 1/10\n'
+ 'INFO hi 2/10\n'
+ 'INFO 110 bytes transferred | hi 2/10\n'
+ 'INFO hi 3/10\n',
+ self.logger.getLogBuffer())
+
+ def test_note(self):
+ factory = self.makeLoggingUIFactory()
+ factory.note("Banja Luka")
+ self.assertEqual('INFO Banja Luka\n', self.logger.getLogBuffer())
+
+ def test_show_error(self):
+ factory = self.makeLoggingUIFactory()
+ factory.show_error("Exploding Peaches")
+ self.assertEqual(
+ "ERROR Exploding Peaches\n", self.logger.getLogBuffer())
+
+ def test_confirm_action(self):
+ factory = self.makeLoggingUIFactory()
+ self.assertTrue(factory.confirm_action(
+ "How are you %(when)s?", "wellness", {"when": "today"}))
+
+ def test_show_message(self):
+ factory = self.makeLoggingUIFactory()
+ factory.show_message("Peaches")
+ self.assertEqual("INFO Peaches\n", self.logger.getLogBuffer())
+
+ def test_get_username(self):
+ factory = self.makeLoggingUIFactory()
+ self.assertIs(
+ None, factory.get_username("Who are you %(when)s?", when="today"))
+
+ def test_get_password(self):
+ factory = self.makeLoggingUIFactory()
+ self.assertIs(
+ None,
+ factory.get_password("How is your %(drink)s", drink="coffee"))
+
+ def test_show_warning(self):
+ factory = self.makeLoggingUIFactory()
+ factory.show_warning("Peaches")
+ self.assertEqual("WARNING Peaches\n", self.logger.getLogBuffer())
+
+ def test_show_warning_unicode(self):
+ factory = self.makeLoggingUIFactory()
+ factory.show_warning(u"Peach\xeas")
+ self.assertEqual(
+ "WARNING Peach\xc3\xaas\n", self.logger.getLogBuffer())
def test_user_warning(self):
factory = self.makeLoggingUIFactory()
@@ -106,9 +156,13 @@
"from_format": "athing",
"to_format": "anotherthing",
}
- self.assertEqual([message], self.messages)
+ self.assertEqual("WARNING %s\n" % message, self.logger.getLogBuffer())
+
+ def test_clear_term(self):
+ factory = self.makeLoggingUIFactory()
+ factory.clear_term()
+ self.assertEqual("", self.logger.getLogBuffer())
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
-
=== modified file 'lib/lp/codehosting/codeimport/uifactory.py'
--- lib/lp/codehosting/codeimport/uifactory.py 2011-07-19 18:09:01 +0000
+++ lib/lp/codehosting/codeimport/uifactory.py 2011-08-02 11:35:56 +0000
@@ -10,38 +10,79 @@
import sys
import time
+from bzrlib.ui import NoninteractiveUIFactory
from bzrlib.ui.text import (
TextProgressView,
- TextUIFactory,
)
-class LoggingUIFactory(TextUIFactory):
+class LoggingUIFactory(NoninteractiveUIFactory):
"""A UI Factory that produces reasonably sparse logging style output.
The goal is to produce a line of output no more often than once a minute
(by default).
"""
- def __init__(self, time_source=time.time, writer=None, interval=60.0):
+ # TODO JRV 2011-08-02: This seems generic enough to live in bzrlib.ui
+
+ def __init__(self, time_source=time.time, logger=None, interval=60.0):
"""Construct a `LoggingUIFactory`.
:param time_source: A callable that returns time in seconds since the
epoch. Defaults to ``time.time`` and should be replaced with
something deterministic in tests.
- :param writer: A callable that takes a string and displays it. It is
- not called with newline terminated strings.
+ :param logger: The logger object to write to
:param interval: Don't produce output more often than once every this
many seconds. Defaults to 60 seconds.
"""
- TextUIFactory.__init__(self)
+ NoninteractiveUIFactory.__init__(self)
self.interval = interval
- self.writer = writer
+ self.logger = logger
self._progress_view = LoggingTextProgressView(
- time_source, writer, interval)
+ time_source, lambda m: logger.info("%s", m), interval)
def show_user_warning(self, warning_id, **message_args):
- self.writer(self.format_user_warning(warning_id, message_args))
+ self.logger.warning(
+ "%s", self.format_user_warning(warning_id, message_args))
+
+ def show_warning(self, msg):
+ if isinstance(msg, unicode):
+ msg = msg.encode("utf-8")
+ self.logger.warning("%s", msg)
+
+ def get_username(self, prompt, **kwargs):
+ return None
+
+ def get_password(self, prompt, **kwargs):
+ return None
+
+ def show_message(self, msg):
+ self.logger.info("%s", msg)
+
+ def note(self, msg):
+ self.logger.info("%s", msg)
+
+ def show_error(self, msg):
+ self.logger.error("%s", msg)
+
+ def _progress_updated(self, task):
+ """A task has been updated and wants to be displayed.
+ """
+ if not self._task_stack:
+ self.logger.warning("%r updated but no tasks are active", task)
+ self._progress_view.show_progress(task)
+
+ def _progress_all_finished(self):
+ self._progress_view.clear()
+
+ def report_transport_activity(self, transport, byte_count, direction):
+ """Called by transports as they do IO.
+
+ This may update a progress bar, spinner, or similar display.
+ By default it does nothing.
+ """
+ self._progress_view.show_transport_activity(transport,
+ direction, byte_count)
class LoggingTextProgressView(TextProgressView):
=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py 2011-07-20 17:20:03 +0000
+++ lib/lp/codehosting/codeimport/worker.py 2011-08-02 11:35:56 +0000
@@ -601,8 +601,7 @@
def _doImport(self):
self._logger.info("Starting job.")
saved_factory = bzrlib.ui.ui_factory
- bzrlib.ui.ui_factory = LoggingUIFactory(
- writer=lambda m: self._logger.info('%s', m))
+ bzrlib.ui.ui_factory = LoggingUIFactory(logger=self._logger)
try:
self._logger.info(
"Getting exising bzr branch from central store.")
@@ -635,9 +634,11 @@
except Exception, e:
if e.__class__ in self.unsupported_feature_exceptions:
self._logger.info(
- "Unable to import branch because of limitations in Bazaar.")
+ "Unable to import branch because of limitations in "
+ "Bazaar.")
self._logger.info(str(e))
- return CodeImportWorkerExitCode.FAILURE_UNSUPPORTED_FEATURE
+ return (
+ CodeImportWorkerExitCode.FAILURE_UNSUPPORTED_FEATURE)
elif e.__class__ in self.invalid_branch_exceptions:
self._logger.info("Branch invalid: %s", e(str))
return CodeImportWorkerExitCode.FAILURE_INVALID
