Rev 4584: (mbp) fix selftest progress and error display problems in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Aug 4 06:20:57 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4584 [merge]
revision-id: pqm at pqm.ubuntu.com-20090804052054-yd271soudff1dbgt
parent: pqm at pqm.ubuntu.com-20090804005507-h1qergam3ijkmvci
parent: mbp at sourcefrog.net-20090804040713-3n34fu7bfk8q41uo
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2009-08-04 06:20:54 +0100
message:
  (mbp) fix selftest progress and error display problems
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/progress.py             progress.py-20050610070202-df9faaab791964c0
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
  bzrlib/ui/text.py              text.py-20051130153916-2e438cffc8afc478
=== modified file 'NEWS'
--- a/NEWS	2009-08-04 00:55:07 +0000
+++ b/NEWS	2009-08-04 04:07:13 +0000
@@ -30,6 +30,11 @@
   There are still some complex scenarios where it will fail (bug #399884)
   (John Arbash Meinel, #393366)
 
+* A progress bar is no longer left dangling when ``bzr selftest``
+  completes, and the progress bar updates with zero latency so the
+  displayed test name is always the one that's actually running.
+  (Martin Pool, #123688)
+
 * Authenticating against an ssh server now uses ``auth_none`` to determine
   if password authentication is even supported. This fixes a bug where
   users would be prompted for a launchpad password, even though launchpad
@@ -74,6 +79,9 @@
 * Fixed spurious "Source branch does not support stacking" warning when
   pushing. (Andrew Bennetts, #388908)
 
+* Fixed spurious transport activity indicator appearing while tests are
+  running.  (Martin Pool, #343532)
+
 * Merge now correctly handles empty right-hand revision specs.
   (Aaron Bentley, #333961)
 
@@ -143,6 +151,9 @@
 * New TransformPreview.commit() allows committing without a working tree.
   (Aaron Bentley)
 
+* ``pb`` parameter to ``TextTestResult`` is deprecated and ignored.
+  (Martin Pool)
+
 * ProgressTasks now prefer to talk direct to their ProgressView not to the
   UIFactory. 
   (Martin Pool)

=== modified file 'bzrlib/progress.py'
--- a/bzrlib/progress.py	2009-07-17 10:38:41 +0000
+++ b/bzrlib/progress.py	2009-08-03 05:31:01 +0000
@@ -69,6 +69,15 @@
     Code updating the task may also set fields as hints about how to display
     it: show_pct, show_spinner, show_eta, show_count, show_bar.  UIs
     will not necessarily respect all these fields.
+    
+    :ivar update_latency: The interval (in seconds) at which the PB should be
+        updated.  Setting this to zero suggests every update should be shown
+        synchronously.
+
+    :ivar show_transport_activity: If true (default), transport activity
+        will be shown when this task is drawn.  Disable it if you're sure 
+        that only irrelevant or uninteresting transport activity can occur
+        during this task.
     """
 
     def __init__(self, parent_task=None, ui_factory=None, progress_view=None):
@@ -97,6 +106,8 @@
         self.show_eta = False,
         self.show_count = True
         self.show_bar = True
+        self.update_latency = 0.1
+        self.show_transport_activity = True
 
     def __repr__(self):
         return '%s(%r/%r, msg=%r)' % (

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-08-04 00:55:07 +0000
+++ b/bzrlib/tests/__init__.py	2009-08-04 04:07:13 +0000
@@ -175,6 +175,8 @@
         self._strict = strict
 
     def done(self):
+        # nb: called stopTestRun in the version of this that Python merged
+        # upstream, according to lifeless 20090803
         if self._strict:
             ok = self.wasStrictlySuccessful()
         else:
@@ -397,21 +399,35 @@
                  ):
         ExtendedTestResult.__init__(self, stream, descriptions, verbosity,
             bench_history, strict)
-        if pb is None:
-            self.pb = self.ui.nested_progress_bar()
-            self._supplied_pb = False
-        else:
-            self.pb = pb
-            self._supplied_pb = True
+        # We no longer pass them around, but just rely on the UIFactory stack
+        # for state
+        if pb is not None:
+            warnings.warn("Passing pb to TextTestResult is deprecated")
+        self.pb = self.ui.nested_progress_bar()
         self.pb.show_pct = False
         self.pb.show_spinner = False
         self.pb.show_eta = False,
         self.pb.show_count = False
         self.pb.show_bar = False
+        self.pb.update_latency = 0
+        self.pb.show_transport_activity = False
+
+    def done(self):
+        # called when the tests that are going to run have run
+        self.pb.clear()
+        super(TextTestResult, self).done()
+
+    def finished(self):
+        self.pb.finished()
 
     def report_starting(self):
         self.pb.update('[test 0/%d] Starting' % (self.num_tests))
 
+    def printErrors(self):
+        # clear the pb to make room for the error listing
+        self.pb.clear()
+        super(TextTestResult, self).printErrors()
+
     def _progress_prefix_text(self):
         # the longer this text, the less space we have to show the test
         # name...
@@ -477,10 +493,6 @@
     def report_cleaning_up(self):
         self.pb.update('Cleaning up')
 
-    def finished(self):
-        if not self._supplied_pb:
-            self.pb.finished()
-
 
 class VerboseTestResult(ExtendedTestResult):
     """Produce long output, with one line per test run plus times"""
@@ -724,6 +736,9 @@
     See also CannedInputUIFactory which lets you provide programmatic input in
     a structured way.
     """
+    # TODO: Capture progress events at the model level and allow them to be
+    # observed by tests that care.
+    #
     # XXX: Should probably unify more with CannedInputUIFactory or a
     # particular configuration of TextUIFactory, or otherwise have a clearer
     # idea of how they're supposed to be different.

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2009-08-03 01:57:07 +0000
+++ b/bzrlib/tests/test_selftest.py	2009-08-04 02:09:19 +0000
@@ -681,29 +681,6 @@
         self.assertEqual(url, t.clone('..').base)
 
 
-class MockProgress(progress._BaseProgressBar):
-    """Progress-bar standin that records calls.
-
-    Useful for testing pb using code.
-    """
-
-    def __init__(self):
-        progress._BaseProgressBar.__init__(self)
-        self.calls = []
-
-    def tick(self):
-        self.calls.append(('tick',))
-
-    def update(self, msg=None, current=None, total=None):
-        self.calls.append(('update', msg, current, total))
-
-    def clear(self):
-        self.calls.append(('clear',))
-
-    def note(self, msg, *args):
-        self.calls.append(('note', msg, args))
-
-
 class TestTestResult(tests.TestCase):
 
     def check_timing(self, test_case, expected_re):
@@ -862,41 +839,6 @@
         self.assertEqual(lines[1], '    foo')
         self.assertEqual(2, len(lines))
 
-    def test_text_report_known_failure(self):
-        # text test output formatting
-        pb = MockProgress()
-        result = bzrlib.tests.TextTestResult(
-            StringIO(),
-            descriptions=0,
-            verbosity=1,
-            pb=pb,
-            )
-        test = self.get_passing_test()
-        # this seeds the state to handle reporting the test.
-        result.startTest(test)
-        # the err parameter has the shape:
-        # (class, exception object, traceback)
-        # KnownFailures dont get their tracebacks shown though, so we
-        # can skip that.
-        err = (tests.KnownFailure, tests.KnownFailure('foo'), None)
-        result.report_known_failure(test, err)
-        self.assertEqual(
-            [
-            ('update', '[1 in 0s] passing_test', None, None),
-            ('note', 'XFAIL: %s\n%s\n', ('passing_test', err[1]))
-            ],
-            pb.calls)
-        # known_failures should be printed in the summary, so if we run a test
-        # after there are some known failures, the update prefix should match
-        # this.
-        result.known_failure_count = 3
-        test.run(result)
-        self.assertEqual(
-            [
-            ('update', '[2 in 0s] passing_test', None, None),
-            ],
-            pb.calls[2:])
-
     def get_passing_test(self):
         """Return a test object that can't be run usefully."""
         def passing_test():
@@ -947,35 +889,6 @@
         self.assertEqual(lines, ['NODEP        0ms',
                                  "    The feature 'Feature' is not available."])
 
-    def test_text_report_unsupported(self):
-        # text test output formatting
-        pb = MockProgress()
-        result = bzrlib.tests.TextTestResult(
-            StringIO(),
-            descriptions=0,
-            verbosity=1,
-            pb=pb,
-            )
-        test = self.get_passing_test()
-        feature = tests.Feature()
-        # this seeds the state to handle reporting the test.
-        result.startTest(test)
-        result.report_unsupported(test, feature)
-        # no output on unsupported features
-        self.assertEqual(
-            [('update', '[1 in 0s] passing_test', None, None)
-            ],
-            pb.calls)
-        # the number of missing features should be printed in the progress
-        # summary, so check for that.
-        result.unsupported = {'foo':0, 'bar':0}
-        test.run(result)
-        self.assertEqual(
-            [
-            ('update', '[2 in 0s, 2 missing] passing_test', None, None),
-            ],
-            pb.calls[1:])
-
     def test_unavailable_exception(self):
         """An UnavailableFeature being raised should invoke addNotSupported."""
         class InstrumentedTestResult(tests.ExtendedTestResult):

=== modified file 'bzrlib/ui/text.py'
--- a/bzrlib/ui/text.py	2009-07-24 16:36:51 +0000
+++ b/bzrlib/ui/text.py	2009-08-03 05:31:01 +0000
@@ -224,6 +224,7 @@
         self._bytes_since_update = 0
 
     def _show_line(self, s):
+        # sys.stderr.write("progress %r\n" % s)
         n = self._width - 1
         self._term_file.write('\r%-*.*s\r' % (n, n, s))
 
@@ -283,9 +284,12 @@
             task_msg = self._format_task(self._last_task)
         else:
             task_msg = ''
-        trans = self._last_transport_msg
-        if trans:
-            trans += ' | '
+        if self._last_task and not self._last_task.show_transport_activity:
+            trans = ''
+        else:
+            trans = self._last_transport_msg
+            if trans:
+                trans += ' | '
         return (bar_string + trans + task_msg)
 
     def _repaint(self):
@@ -302,7 +306,7 @@
         must_update = task is not self._last_task
         self._last_task = task
         now = time.time()
-        if (not must_update) and (now < self._last_repaint + 0.1):
+        if (not must_update) and (now < self._last_repaint + task.update_latency):
             return
         if now > self._transport_update_time + 10:
             # no recent activity; expire it
@@ -330,6 +334,10 @@
         self._total_byte_count += byte_count
         self._bytes_since_update += byte_count
         now = time.time()
+        if self._total_byte_count < 2000:
+            # a little resistance at first, so it doesn't stay stuck at 0
+            # while connecting...
+            return
         if self._transport_update_time is None:
             self._transport_update_time = now
         elif now >= (self._transport_update_time + 0.5):




More information about the bazaar-commits mailing list