Diff
Modified: trunk/Tools/ChangeLog (87595 => 87596)
--- trunk/Tools/ChangeLog 2011-05-28 02:24:20 UTC (rev 87595)
+++ trunk/Tools/ChangeLog 2011-05-28 02:28:00 UTC (rev 87596)
@@ -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 <dpra...@chromium.org>
+
+ Reviewed by Eric Seidel.
+
NRWT: minor cleanup in printing module
https://bugs.webkit.org/show_bug.cgi?id=60329
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream.py (87595 => 87596)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream.py 2011-05-28 02:24:20 UTC (rev 87595)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream.py 2011-05-28 02:28:00 UTC (rev 87596)
@@ -37,110 +37,47 @@
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().
+ It can be used directly as a stream, by calling write(), but also provides
+ a method called update() that will overwite prior updates().
+ """
- 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.
-
- 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()
+ # 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()
- def progress(self, str):
- """
- Write a message to the stream that will get overwritten.
+ def update(self, txt):
+ """Write a message that will be overwritten by subsequent update() or write() calls."""
+ self._overwrite(txt)
- 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 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 _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 = ''
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py (87595 => 87596)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py 2011-05-28 02:24:20 UTC (rev 87595)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/metered_stream_unittest.py 2011-05-28 02:28:00 UTC (rev 87596)
@@ -29,10 +29,6 @@
"""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 @@
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 @@
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 @@
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()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py (87595 => 87596)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py 2011-05-28 02:24:20 UTC (rev 87595)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py 2011-05-28 02:28:00 UTC (rev 87596)
@@ -130,6 +130,7 @@
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 @@
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 @@
action = ""
if retrying:
action = ""
- 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 @@
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 @@
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)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py (87595 => 87596)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py 2011-05-28 02:24:20 UTC (rev 87595)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py 2011-05-28 02:28:00 UTC (rev 87596)
@@ -91,7 +91,7 @@
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)
