2011-05-27 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 May 2011 02:28:00 +0000 (02:28 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 May 2011 02:28:00 +0000 (02:28 +0000)
        Reviewed by Eric Seidel.

        NRWT: clean up metered_stream code in preparation for 'nooverwriting' patch
        https://bugs.webkit.org/show_bug.cgi?id=60326

        This patch removes a lot of the complexity from the
        metered_stream implementation that was unnecessary since there
        was only one caller and the logic could be coordinated better.

        There should be no functional changes in this patch, just code
        getting deleted and cleaned up.

        * Scripts/webkitpy/layout_tests/layout_package/metered_stream.py:
        * Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/printing.py:
        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@87596 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream.py
Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py
Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py
Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py

index 23a97d7..947cb8f 100644 (file)
@@ -2,6 +2,25 @@
 
         Reviewed by Eric Seidel.
 
+        NRWT: clean up metered_stream code in preparation for 'nooverwriting' patch
+        https://bugs.webkit.org/show_bug.cgi?id=60326
+
+        This patch removes a lot of the complexity from the
+        metered_stream implementation that was unnecessary since there
+        was only one caller and the logic could be coordinated better.
+
+        There should be no functional changes in this patch, just code
+        getting deleted and cleaned up.
+
+        * Scripts/webkitpy/layout_tests/layout_package/metered_stream.py:
+        * Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/printing.py:
+        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
+
+2011-05-27  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Seidel.
+
         NRWT: minor cleanup in printing module
         https://bugs.webkit.org/show_bug.cgi?id=60329
 
index 20646a1..d45ab09 100644 (file)
@@ -37,110 +37,47 @@ This package should only be called by the printing module in the layout_tests
 package.
 """
 
-import logging
-
-_log = logging.getLogger("webkitpy.layout_tests.metered_stream")
-
-
 class MeteredStream:
     """This class is a wrapper around a stream that allows you to implement
     meters (progress bars, etc.).
 
-    It can be used directly as a stream, by calling write(), but provides
-    two other methods for output, update(), and progress().
-
-    In normal usage, update() will overwrite the output of the immediately
-    preceding update() (write() also will overwrite update()). So, calling
-    multiple update()s in a row can provide an updating status bar (note that
-    if an update string contains newlines, only the text following the last
-    newline will be overwritten/erased).
-
-    If the MeteredStream is constructed in "verbose" mode (i.e., by passing
-    verbose=true), then update() no longer overwrite a previous update(), and
-    instead the call is equivalent to write(), although the text is
-    actually sent to the logger rather than to the stream passed
-    to the constructor.
-
-    progress() is just like update(), except that if you are in verbose mode,
-    progress messages are not output at all (they are dropped). This is
-    used for things like progress bars which are presumed to be unwanted in
-    verbose mode.
+    It can be used directly as a stream, by calling write(), but also provides
+    a method called update() that will overwite prior updates().
+   """
 
-    Note that the usual usage for this class is as a destination for
-    a logger that can also be written to directly (i.e., some messages go
-    through the logger, some don't). We thus have to dance around a
-    layering inversion in update() for things to work correctly.
-    """
-
-    def __init__(self, verbose, stream):
+    def __init__(self, stream):
         """
         Args:
-          verbose: whether progress is a no-op and updates() aren't overwritten
           stream: output stream to write to
         """
-        self._dirty = False
-        self._verbose = verbose
         self._stream = stream
+        self._dirty = False
         self._last_update = ""
 
     def write(self, txt):
         """Write to the stream, overwriting and resetting the meter."""
-        if self._dirty:
-            self._write(txt)
-            self._dirty = False
-            self._last_update = ''
-        else:
-            self._stream.write(txt)
-
-    def flush(self):
-        """Flush any buffered output."""
-        self._stream.flush()
-
-    def progress(self, str):
-        """
-        Write a message to the stream that will get overwritten.
-
-        This is used for progress updates that don't need to be preserved in
-        the log. If the MeteredStream was initialized with verbose==True,
-        then this output is discarded. We have this in case we are logging
-        lots of output and the update()s will get lost or won't work
-        properly (typically because verbose streams are redirected to files).
-
-        """
-        if self._verbose:
-            return
-        self._write(str)
 
-    def update(self, str):
-        """
-        Write a message that is also included when logging verbosely.
+        # This routine is called by the logging infrastructure, and
+        # must not call back into logging. It is not a public function.
+        self._overwrite(txt)
+        self._reset()
 
-        This routine preserves the same console logging behavior as progress(),
-        but will also log the message if verbose() was true.
-
-        """
-        # Note this is a separate routine that calls either into the logger
-        # or the metering stream. We have to be careful to avoid a layering
-        # inversion (stream calling back into the logger).
-        if self._verbose:
-            _log.info(str)
-        else:
-            self._write(str)
-
-    def _write(self, str):
-        """Actually write the message to the stream."""
-
-        # FIXME: Figure out if there is a way to detect if we're writing
-        # to a stream that handles CRs correctly (e.g., terminals). That might
-        # be a cleaner way of handling this.
+    def update(self, txt):
+        """Write a message that will be overwritten by subsequent update() or write() calls."""
+        self._overwrite(txt)
 
+    def _overwrite(self, txt):
         # Print the necessary number of backspaces to erase the previous
         # message.
         if len(self._last_update):
             self._stream.write("\b" * len(self._last_update) +
                                " " * len(self._last_update) +
                                "\b" * len(self._last_update))
-        self._stream.write(str)
-        last_newline = str.rfind("\n")
-        self._last_update = str[(last_newline + 1):]
+        self._stream.write(txt)
+        last_newline = txt.rfind("\n")
+        self._last_update = txt[(last_newline + 1):]
         self._dirty = True
+
+    def _reset(self):
+        self._dirty = False
+        self._last_update = ''
index 9421ff8..ffb5b0a 100644 (file)
 
 """Unit tests for metered_stream.py."""
 
-import os
-import optparse
-import pdb
-import sys
 import unittest
 
 from webkitpy.common.array_stream import ArrayStream
@@ -42,13 +38,11 @@ from webkitpy.layout_tests.layout_package import metered_stream
 class TestMeteredStream(unittest.TestCase):
     def test_regular(self):
         a = ArrayStream()
-        m = metered_stream.MeteredStream(verbose=False, stream=a)
+        m = metered_stream.MeteredStream(stream=a)
         self.assertTrue(a.empty())
 
-        # basic test - note that the flush() is a no-op, but we include it
-        # for coverage.
+        # basic test
         m.write("foo")
-        m.flush()
         exp = ['foo']
         self.assertEquals(a.get(), exp)
 
@@ -69,14 +63,9 @@ class TestMeteredStream(unittest.TestCase):
         exp.append('foo')
         self.assertEquals(a.get(), exp)
 
-        m.progress("progress")
-        exp.append('\b\b\b   \b\b\b')
-        exp.append('progress')
-        self.assertEquals(a.get(), exp)
-
-        # now check that a write() does overwrite the progress bar
+        # now check that a write() does overwrite the update
         m.write("foo")
-        exp.append('\b\b\b\b\b\b\b\b        \b\b\b\b\b\b\b\b')
+        exp.append('\b\b\b   \b\b\b')
         exp.append('foo')
         self.assertEquals(a.get(), exp)
 
@@ -89,27 +78,6 @@ class TestMeteredStream(unittest.TestCase):
         m.update("baz")
         self.assertEquals(a.get(), ['foo\nbar', '\b\b\b   \b\b\b', 'baz'])
 
-    def test_verbose(self):
-        a = ArrayStream()
-        m = metered_stream.MeteredStream(verbose=True, stream=a)
-        self.assertTrue(a.empty())
-        m.write("foo")
-        self.assertEquals(a.get(), ['foo'])
-
-        import logging
-        b = ArrayStream()
-        logger = logging.getLogger()
-        handler = logging.StreamHandler(b)
-        logger.addHandler(handler)
-        m.update("bar")
-        logger.handlers.remove(handler)
-        self.assertEquals(a.get(), ['foo'])
-        self.assertEquals(b.get(), ['bar\n'])
-
-        m.progress("dropped")
-        self.assertEquals(a.get(), ['foo'])
-        self.assertEquals(b.get(), ['bar\n'])
-
 
 if __name__ == '__main__':
     unittest.main()
index 502e29f..53cf150 100644 (file)
@@ -130,6 +130,7 @@ def parse_print_options(print_options, verbose):
         switches = set(print_options.split(','))
     elif verbose:
         switches = set(PRINT_EVERYTHING.split(','))
+        switches.discard('one-line-progress')
     else:
         switches = set(PRINT_DEFAULT.split(','))
 
@@ -207,10 +208,11 @@ class Printer(object):
         self._port = port
         self._stream = regular_output
 
-        self._meter = metered_stream.MeteredStream(options.verbose,
-                                                   regular_output)
-        self._logging_handler = None
-        if configure_logging:
+        self._meter = None
+        if options.verbose:
+            self._logging_handler = _configure_logging(regular_output, options.verbose)
+        else:
+            self._meter = metered_stream.MeteredStream(regular_output)
             self._logging_handler = _configure_logging(self._meter, options.verbose)
 
         self.switches = parse_print_options(options.print_options,
@@ -346,7 +348,7 @@ class Printer(object):
         action = "Testing"
         if retrying:
             action = "Retrying"
-        self._meter.progress("%s (%d%%): %d ran as expected, %d didn't,"
+        self._meter.update("%s (%d%%): %d ran as expected, %d didn't,"
             " %d left" % (action, percent_complete, result_summary.expected,
              result_summary.unexpected, result_summary.remaining))
 
@@ -439,7 +441,7 @@ class Printer(object):
     def print_update(self, msg):
         if self.disabled('updates'):
             return
-        self._meter.update(msg)
+        self._update(msg)
 
     def write(self, msg, option="misc"):
         if self.disabled(option):
@@ -447,10 +449,10 @@ class Printer(object):
         self._write(msg)
 
     def _write(self, msg):
-        # FIXME: we could probably get away with calling _log.info() all of
-        # the time, but there doesn't seem to be a good way to test the output
-        # from the logger :(.
-        if self._options.verbose:
-            _log.info(msg)
+        _log.info(msg)
+
+    def _update(self, msg):
+        if self._meter:
+            self._meter.update(msg)
         else:
-            self._meter.write("%s\n" % msg)
+            self._write(msg)
index f3c43fb..0d674b0 100644 (file)
@@ -91,7 +91,7 @@ class TestUtilityFunctions(unittest.TestCase):
         test_switches([], printing.PRINT_DEFAULT)
 
         # test that verbose defaults to everything
-        test_switches([], printing.PRINT_EVERYTHING, verbose=True)
+        test_switches([], printing.PRINT_EVERYTHING.replace(',one-line-progress',''), verbose=True)
 
         # test that --print default does what it's supposed to
         test_switches(['--print', 'default'], printing.PRINT_DEFAULT)