Merge lp:~jml/testrepository/show-failures-incrementally-613152 into lp:~testrepository/testrepository/trunk

[Jonathan Lange]

Show test failures and errors as we get them in testr load.

In the Wikipedia spirit of editing boldly, I've hacked up testrepository to show failures incrementally. Most of the damage occurs in the UI contract.

Here's what I've done:

 * Added UI.make_result to the public interface, requiring a UI
   to be responsible for making its object.

 * Changed 'failing', 'load' and 'last' to use the result
   provided by the UI, rather than making their own staging
   result object to capture the stream.

 * Change Command.output_run to no longer take an output stream,
   as such a thing is no longer required -- the UI's result takes
   care of it now.

 * Dropped UI.output_results, since there's no longer any actual
   use case.

 * Gave the CLI UI a TestResult implementation that prints errors
   and failures as it gets them.

Changing 'last' and 'failing' wasn't really necessary to fix bug 613152, neither was removing UI.output_results or changing Command.output_run, but it seemed better to me to go the whole hog.

I also cleaned up pyflakes warnings where I saw them.

[lp:~jml/testrepository/show-failures-incrementally-613152]

106. By Jonathan Lange on 2010-08-04

Remove output_results, not needed.

[Jelmer Vernooij (jelmer)]

I can't find the "I like this" button.

[Jonathan Lange (jml)]

Still need a review for this.

[Robert Collins (lifeless)]

output_run looks like it doesn't output a run anymore: its intent is sufficiently different, I would like it to actually match.

output_run_summary, perhaps.

or

output_result, which seems to be what it does now.

I think things are a little confused in fact; it looks to me that the summary printing at the end of the result wants to happen in the stopRun method of the result resturned from makeResult, and output_run can be wholly deleted.

You've added a TODO which there is an example of doing that TODO elsewhere in your diff.

The rest looks ok.

Thanks for doing this, it is appreciated, I've just been stupidly busy bootstrapping stuff in lp.

review: Needs Fixing

[lp:~jml/testrepository/show-failures-incrementally-613152]

107. By Jonathan Lange on 2010-09-12

Merge trunk, doing major work to resolve the conflict in the failing command.

108. By Jonathan Lange on 2010-09-12

make_result now takes a callable that returns the id of the test run.
Not actually used yet.

109. By Jonathan Lange on 2010-09-12

Refactor the CLITestResult tests so they don't care so much about how results
are constructed.

110. By Jonathan Lange on 2010-09-19

Wildcard object equal to everything.

111. By Jonathan Lange on 2010-09-19

Use Wildcard to make matching UI output a little nicer.

112. By Jonathan Lange on 2010-09-20

Give the UI's TestResult object full responsibility for summing up the result
of the test,

113. By Jonathan Lange on 2010-09-20

Oops.

114. By Jonathan Lange on 2010-09-20

Delete unused output_run.

115. By Jonathan Lange on 2010-09-20

Tests for results module.

116. By Jonathan Lange on 2010-09-20

Probably not.

[Jonathan Lange (jml)]

As indicated on IRC, I didn't add a TODO, I just moved it.

It took me a while to refactor the code to use stopTestRun rather than output_run. I have had to change some of the behaviour to do so.

Specifically:
 * 'testr failing' now shows run id, total tests and skip count as well failure count
 * 'testr failing' now has return code 1 when there are failing tests
 * 'testr load' will show skips

There's probably other stuff, although I tried to minimize it. Big enough that it definitely needs review.

[Robert Collins (lifeless)]

+ def _make_result(self, repo, evaluator):
+ if self.ui.options.list:
+ return evaluator

this two things seem disconnected; perhaps rather than evaluator you should say list_result or something. I think something originally together has been split out far enough that the parameter needs a better name.

+Wildcard = _Wildcard()

perhaps
wildcard = Wildcard()

would be nicer. Many things (like str, object, etc) are lowercase for instances in Python.

+ def _output_run(self, run_id):

def _output_summary

 - I think.

+ return ''.join([
+ self.sep1,
+ '%s: %s\n' % (label, test.id()),
+ self.sep2,
+ error_text,
+ ])

Looks like a lurking UnicodeDecodeError to me; we either need to make this unicode always or manually encode error. One way to test that would be to throw a mixed encoding, non-ascii test outcome at it.

Would you be kind enough to do these tweaks? then its definitely gtg.

review: Approve

[Jonathan Lange (jml)]

On Sun, Sep 26, 2010 at 9:46 AM, Robert Collins
<email address hidden> wrote:
> Review: Approve
> +    def _make_result(self, repo, evaluator):
> +        if self.ui.options.list:
> +            return evaluator
>
> this two things seem disconnected; perhaps rather than evaluator you should say list_result or something. I think something originally together has been split out far enough that the parameter needs a better name.

Done.

>
>
> +Wildcard = _Wildcard()
>
> perhaps
> wildcard = Wildcard()
>
> would be nicer. Many things (like str, object, etc) are lowercase for instances in Python.
>

Instances of 'type', perhaps. I chose the case to reflect None, True
and False: other singleton constants in Python.

>
> +    def _output_run(self, run_id):
>
> def _output_summary
>
>  - I think.
>

Changed.

>
> +        return ''.join([
> +            self.sep1,
> +            '%s: %s\n' % (label, test.id()),
> +            self.sep2,
> +            error_text,
> +            ])
>
> Looks like a lurking UnicodeDecodeError to me; we either need to make this unicode always or manually encode error. One way to test that would be to throw a mixed encoding, non-ascii test outcome at it.

Well, it's not a *new* lurking UnicodeDecodeError. It's equivalent to
what was there earlier.

We are always going to be getting the error_text from the base
TestResult. In this case, we are relying on testtools to store
unicode. I've changed all of the literals to be unicode to at least
communicate this more clearly.

I was going to add a test (below), but it seems to be a case of
"Doctor, it hurts when I do this!".

    def test_format_error_mixed_encoding(self):
        result = self.make_result()
        error_text = 'foo' + u'проба'.encode('utf-16')
        error = result._format_error(u'label', self, error_text)
        expected = u'%s%s: %s\n%s%s' % (
            result.sep1, u'label', self.id(), result.sep2, error_text)
        self.assertEqual(error, expected)

>
> Would you be kind enough to do these tweaks? then its definitely gtg.

Thanks,
jml

[Jelmer Vernooij (jelmer)]

W00t!

=== modified file 'testrepository/commands/__init__.py'
--- testrepository/commands/__init__.py	2010-02-28 23:02:29 +0000
+++ testrepository/commands/__init__.py	2010-09-20 17:47:50 +0000
@@ -150,27 +150,6 @@
     def _init(self):
         """Per command init call, called into by Command.__init__."""
 
-    def output_run(self, run_id, output, evaluator):
-        """Output a test run.
-
-        :param run_id: The run id.
-        :param output: A StringIO containing a subunit stream for some portion of the run to show.
-        :param evaluator: A TestResult evaluating the entire run.
-        """
-        if self.ui.options.quiet:
-            return
-        if output.getvalue():
-            output.seek(0)
-            self.ui.output_results(subunit.ProtocolTestCase(output))
-        values = [('id', run_id), ('tests', evaluator.testsRun)]
-        failures = len(evaluator.failures) + len(evaluator.errors)
-        if failures:
-            values.append(('failures', failures))
-        skips = sum(map(len, evaluator.skip_reasons.itervalues()))
-        if skips:
-            values.append(('skips', skips))
-        self.ui.output_values(values)
-
     def run(self):
         """The core logic for this command to be implemented by subclasses."""
         raise NotImplementedError(self.run)

=== modified file 'testrepository/commands/failing.py'
--- testrepository/commands/failing.py	2010-09-11 19:56:11 +0000
+++ testrepository/commands/failing.py	2010-09-20 17:47:50 +0000
@@ -14,13 +14,13 @@
 
 """Show the current failures in the repository."""
 
-from cStringIO import StringIO
 import optparse
 
-import subunit.test_results
 from testtools import MultiTestResult, TestResult
 
 from testrepository.commands import Command
+from testrepository.results import TestResultFilter
+
 
 class failing(Command):
     """Show the current failures known by the repository.
@@ -41,17 +41,31 @@
             default=False, help="Show only a list of failing tests."),
         ]
 
+    def _list_subunit(self, run):
+        # TODO only failing tests.
+        stream = run.get_subunit_stream()
+        self.ui.output_stream(stream)
+        if stream:
+            return 1
+        else:
+            return 0
+
+    def _make_result(self, repo, evaluator):
+        if self.ui.options.list:
+            return evaluator
+        output_result = self.ui.make_result(repo.latest_id)
+        filtered = TestResultFilter(output_result, filter_skip=True)
+        return MultiTestResult(evaluator, filtered)
+
     def run(self):
         repo = self.repository_factory.open(self.ui.here)
         run = repo.get_failing()
+        if self.ui.options.subunit:
+            return self._list_subunit(run)
         case = run.get_test()
         failed = False
         evaluator = TestResult()
-        output = StringIO()
-        output_stream = subunit.TestProtocolClient(output)
-        filtered = subunit.test_results.TestResultFilter(output_stream,
-            filter_skip=True)
-        result = MultiTestResult(evaluator, filtered)
+        result = self._make_result(repo, evaluator)
         result.startTestRun()
         try:
             case.run(result)
@@ -66,19 +80,4 @@
             failing_tests = [
                 test for test, _ in evaluator.errors + evaluator.failures]
             self.ui.output_tests(failing_tests)
-            return result
-        if self.ui.options.subunit:
-            # TODO only failing tests.
-            self.ui.output_stream(run.get_subunit_stream())
-            return result
-        if self.ui.options.quiet:
-            return result
-        if output.getvalue():
-            output.seek(0)
-            self.ui.output_results(subunit.ProtocolTestCase(output))
-        values = []
-        failures = len(evaluator.failures) + len(evaluator.errors)
-        if failures:
-            values.append(('failures', failures))
-        self.ui.output_values(values)
         return result

=== modified file 'testrepository/commands/last.py'
--- testrepository/commands/last.py	2010-01-10 08:52:00 +0000
+++ testrepository/commands/last.py	2010-09-20 17:47:50 +0000
@@ -5,7 +5,7 @@
 # license at the users choice. A copy of both licenses are available in the
 # project source as Apache-2.0 and BSD. You may not use this file except in
 # compliance with one of these two licences.
-# 
+#
 # Unless required by applicable law or agreed to in writing, software
 # distributed under these licenses is distributed on an "AS IS" BASIS, WITHOUT
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
@@ -14,16 +14,13 @@
 
 """Show the last run loaded into a repository."""
 
-from cStringIO import StringIO
-
-import subunit.test_results
-from testtools import MultiTestResult, TestResult
-
 from testrepository.commands import Command
+from testrepository.results import TestResultFilter
+
 
 class last(Command):
     """Show the last run loaded into a repository.
-    
+
     Failing tests are shown on the console and a summary of the run is printed
     at the end.
     """
@@ -33,19 +30,14 @@
         run_id = repo.latest_id()
         case = repo.get_test_run(run_id).get_test()
         failed = False
-        evaluator = TestResult()
-        output = StringIO()
-        output_stream = subunit.TestProtocolClient(output)
-        filtered = subunit.test_results.TestResultFilter(output_stream,
-            filter_skip=True)
-        result = MultiTestResult(evaluator, filtered)
+        output_result = self.ui.make_result(lambda: run_id)
+        result = TestResultFilter(output_result, filter_skip=True)
         result.startTestRun()
         try:
             case.run(result)
         finally:
             result.stopTestRun()
-        failed = not evaluator.wasSuccessful()
-        self.output_run(run_id, output, evaluator)
+        failed = not result.wasSuccessful()
         if failed:
             return 1
         else:

=== modified file 'testrepository/commands/load.py'
--- testrepository/commands/load.py	2010-01-10 08:52:00 +0000
+++ testrepository/commands/load.py	2010-09-20 17:47:50 +0000
@@ -1,11 +1,11 @@
 #
 # Copyright (c) 2009 Testrepository Contributors
-# 
+#
 # Licensed under either the Apache License, Version 2.0 or the BSD 3-clause
 # license at the users choice. A copy of both licenses are available in the
 # project source as Apache-2.0 and BSD. You may not use this file except in
 # compliance with one of these two licences.
-# 
+#
 # Unless required by applicable law or agreed to in writing, software
 # distributed under these licenses is distributed on an "AS IS" BASIS, WITHOUT
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
@@ -14,16 +14,16 @@
 
 """Load data into a repository."""
 
-from cStringIO import StringIO
-
-import subunit.test_results
-from testtools import MultiTestResult, TestResult
+import subunit
+from testtools import MultiTestResult
 
 from testrepository.commands import Command
+from testrepository.results import TestResultFilter
+
 
 class load(Command):
     """Load a subunit stream into a repository.
-    
+
     Failing tests are shown on the console and a summary of the stream is
     printed at the end.
     """
@@ -34,21 +34,21 @@
         path = self.ui.here
         repo = self.repository_factory.open(path)
         failed = False
+        run_id = None
         for stream in self.ui.iter_streams('subunit'):
             inserter = repo.get_inserter()
-            evaluator = TestResult()
-            output = StringIO()
-            output_stream = subunit.TestProtocolClient(output)
-            filtered = subunit.test_results.TestResultFilter(output_stream,
-                filter_skip=True)
+            output_result = self.ui.make_result(lambda: run_id)
+            # XXX: We want to *count* skips, but not show them.
+            filtered = TestResultFilter(output_result, filter_skip=False)
             case = subunit.ProtocolTestCase(stream)
+            filtered.startTestRun()
             inserter.startTestRun()
             try:
-                case.run(MultiTestResult(inserter, evaluator, filtered))
+                case.run(MultiTestResult(inserter, filtered))
             finally:
                 run_id = inserter.stopTestRun()
-            failed = failed or not evaluator.wasSuccessful()
-            self.output_run(run_id, output, evaluator)
+                filtered.stopTestRun()
+            failed = failed or not filtered.wasSuccessful()
         if failed:
             return 1
         else:

=== added file 'testrepository/results.py'
--- testrepository/results.py	1970-01-01 00:00:00 +0000
+++ testrepository/results.py	2010-09-20 17:47:50 +0000
@@ -0,0 +1,13 @@
+from subunit import test_results
+
+
+class TestResultFilter(test_results.TestResultFilter):
+    """Test result filter."""
+
+    def _filtered(self):
+        super(TestResultFilter, self)._filtered()
+        # XXX: This is really crappy. It assumes that the test result we
+        # actually care about is decorated twice. Probably the more correct
+        # thing to do is fix subunit so that incrementing 'testsRun' on a test
+        # result increments them on the decorated test result.
+        self.decorated.decorated.testsRun += 1

=== modified file 'testrepository/tests/__init__.py'
--- testrepository/tests/__init__.py	2010-01-16 00:01:45 +0000
+++ testrepository/tests/__init__.py	2010-09-20 17:47:50 +0000
@@ -30,6 +30,22 @@
             self)
 
 
+class _Wildcard(object):
+    """Object that is equal to everything."""
+
+    def __repr__(self):
+        return '*'
+
+    def __eq__(self, other):
+        return True
+
+    def __ne__(self, other):
+        return False
+
+
+Wildcard = _Wildcard()
+
+
 def test_suite():
     packages = [
         'arguments',
@@ -43,6 +59,7 @@
         'matchers',
         'monkeypatch',
         'repository',
+        'results',
         'setup',
         'stubpackage',
         'testr',

=== modified file 'testrepository/tests/commands/test_failing.py'
--- testrepository/tests/commands/test_failing.py	2010-09-07 12:37:17 +0000
+++ testrepository/tests/commands/test_failing.py	2010-09-20 17:47:50 +0000
@@ -22,7 +22,7 @@
 from testrepository.commands import failing
 from testrepository.ui.model import UI
 from testrepository.repository import memory
-from testrepository.tests import ResourcedTestCase
+from testrepository.tests import ResourcedTestCase, Wildcard
 
 
 class TestCommand(ResourcedTestCase):
@@ -48,14 +48,12 @@
         Cases('ok').run(inserter)
         inserter.stopTestRun()
         self.assertEqual(1, cmd.execute())
-        self.assertEqual('results', ui.outputs[0][0])
-        suite = ui.outputs[0][1]
-        ui.outputs[0] = ('results', None)
         # We should have seen test outputs (of the failure) and summary data.
         self.assertEqual([
-            ('results', None),
-            ('values', [('failures', 1)])],
+            ('results', Wildcard),
+            ('values', [('id', 0), ('tests', 1), ('failures', 1)])],
             ui.outputs)
+        suite = ui.outputs[0][1]
         result = testtools.TestResult()
         result.startTestRun()
         try:
@@ -116,6 +114,16 @@
         open = cmd.repository_factory.open
         def decorate_open_with_get_failing(url):
             repo = open(url)
+            inserter = repo.get_inserter()
+            inserter.startTestRun()
+            class Cases(ResourcedTestCase):
+                def failing(self):
+                    self.fail('foo')
+                def ok(self):
+                    pass
+            Cases('failing').run(inserter)
+            Cases('ok').run(inserter)
+            inserter.stopTestRun()
             orig = repo.get_failing
             def get_failing():
                 calls.append(True)
@@ -124,5 +132,6 @@
             return repo
         cmd.repository_factory.open = decorate_open_with_get_failing
         cmd.repository_factory.initialise(ui.here)
-        self.assertEqual(0, cmd.execute())
+        self.assertEqual(1, cmd.execute())
         self.assertEqual([True], calls)
+

=== modified file 'testrepository/tests/commands/test_last.py'
--- testrepository/tests/commands/test_last.py	2010-01-10 08:52:00 +0000
+++ testrepository/tests/commands/test_last.py	2010-09-20 17:47:50 +0000
@@ -19,7 +19,7 @@
 from testrepository.commands import last
 from testrepository.ui.model import UI
 from testrepository.repository import memory
-from testrepository.tests import ResourcedTestCase
+from testrepository.tests import ResourcedTestCase, Wildcard
 
 
 class TestCommand(ResourcedTestCase):
@@ -45,14 +45,12 @@
         Cases('ok').run(inserter)
         id = inserter.stopTestRun()
         self.assertEqual(1, cmd.execute())
-        self.assertEqual('results', ui.outputs[0][0])
-        suite = ui.outputs[0][1]
-        ui.outputs[0] = ('results', None)
         # We should have seen test outputs (of the failure) and summary data.
         self.assertEqual([
-            ('results', None),
+            ('results', Wildcard),
             ('values', [('id', id), ('tests', 2), ('failures', 1)])],
             ui.outputs)
+        suite = ui.outputs[0][1]
         result = testtools.TestResult()
         result.startTestRun()
         try:

=== modified file 'testrepository/tests/commands/test_load.py'
--- testrepository/tests/commands/test_load.py	2010-01-08 12:08:41 +0000
+++ testrepository/tests/commands/test_load.py	2010-09-20 17:47:50 +0000
@@ -18,7 +18,7 @@
 
 from testrepository.commands import load
 from testrepository.ui.model import UI
-from testrepository.tests import ResourcedTestCase
+from testrepository.tests import ResourcedTestCase, Wildcard
 from testrepository.tests.test_repository import RecordingRepositoryFactory
 from testrepository.repository import memory
 
@@ -77,10 +77,8 @@
         cmd.repository_factory.initialise(ui.here)
         self.assertEqual(1, cmd.execute())
         suite = ui.outputs[0][1]
-        self.assertEqual('results', ui.outputs[0][0])
-        ui.outputs[0] = ('results', None)
         self.assertEqual([
-            ('results', None),
+            ('results', Wildcard),
             ('values', [('id', 0), ('tests', 1), ('failures', 1)])],
             ui.outputs)
         result = testtools.TestResult()
@@ -100,7 +98,8 @@
         cmd.repository_factory.initialise(ui.here)
         self.assertEqual(0, cmd.execute())
         self.assertEqual(
-            [('values', [('id', 0), ('tests', 1), ('skips', 1)])],
+            [('results', Wildcard),
+             ('values', [('id', 0), ('tests', 1), ('skips', 1)])],
             ui.outputs)
 
     def test_load_new_shows_test_summary_no_tests(self):
@@ -110,7 +109,9 @@
         cmd.repository_factory = memory.RepositoryFactory()
         cmd.repository_factory.initialise(ui.here)
         self.assertEqual(0, cmd.execute())
-        self.assertEqual([('values', [('id', 0), ('tests', 0)])], ui.outputs)
+        self.assertEqual(
+            [('results', Wildcard), ('values', [('id', 0), ('tests', 0)])],
+            ui.outputs)
 
     def test_load_new_shows_test_summary_per_stream(self):
         # This may not be the final layout, but for now per-stream stats are
@@ -122,7 +123,9 @@
         cmd.repository_factory.initialise(ui.here)
         self.assertEqual(0, cmd.execute())
         self.assertEqual([
+            ('results', Wildcard),
             ('values', [('id', 0), ('tests', 0)]),
+            ('results', Wildcard),
             ('values', [('id', 1), ('tests', 0)])],
             ui.outputs)
 

=== modified file 'testrepository/tests/test_matchers.py'
--- testrepository/tests/test_matchers.py	2010-01-16 00:01:45 +0000
+++ testrepository/tests/test_matchers.py	2010-09-20 17:47:50 +0000
@@ -15,6 +15,7 @@
 """Tests for matchers used by or for testing testrepository."""
 
 import sys
+from testtools import TestCase
 
 from testrepository.tests import ResourcedTestCase
 from testrepository.tests.matchers import MatchesException
@@ -55,3 +56,19 @@
             error = sys.exc_info()
         mismatch = matcher.match(error)
         self.assertEqual(None, mismatch)
+
+
+class TestWildcard(TestCase):
+
+    def test_wildcard_equals_everything(self):
+        from testrepository.tests import Wildcard
+        self.assertTrue(Wildcard == 5)
+        self.assertTrue(Wildcard == 'orange')
+        self.assertTrue('orange' == Wildcard)
+        self.assertTrue(5 == Wildcard)
+
+    def test_wildcard_not_equals_nothing(self):
+        from testrepository.tests import Wildcard
+        self.assertFalse(Wildcard != 5)
+        self.assertFalse(Wildcard != 'orange')
+

=== added file 'testrepository/tests/test_results.py'
--- testrepository/tests/test_results.py	1970-01-01 00:00:00 +0000
+++ testrepository/tests/test_results.py	2010-09-20 17:47:50 +0000
@@ -0,0 +1,28 @@
+#
+# Copyright (c) 2010 Testrepository Contributors
+#
+# Licensed under either the Apache License, Version 2.0 or the BSD 3-clause
+# license at the users choice. A copy of both licenses are available in the
+# project source as Apache-2.0 and BSD. You may not use this file except in
+# compliance with one of these two licences.
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under these licenses is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# license you chose for the specific language governing permissions and
+# limitations under that license.
+
+from testtools import TestCase, TestResult
+
+from testrepository.results import TestResultFilter
+
+
+class ResultFilter(TestCase):
+
+    def test_addSuccess_increases_count(self):
+        result = TestResult()
+        filtered = TestResultFilter(result)
+        filtered.startTest(self)
+        filtered.addSuccess(self)
+        filtered.stopTest(self)
+        self.assertEqual(1, result.testsRun)

=== modified file 'testrepository/tests/test_ui.py'
--- testrepository/tests/test_ui.py	2010-09-07 12:37:17 +0000
+++ testrepository/tests/test_ui.py	2010-09-20 17:47:50 +0000
@@ -102,14 +102,6 @@
         ui = self.get_test_ui()
         ui.output_rest('')
 
-    def test_output_results(self):
-        # output_results can be called and takes a thing that can be 'run'.
-        ui = self.get_test_ui()
-        class Case(ResourcedTestCase):
-            def method(self):
-                pass
-        ui.output_results(Case('method'))
-
     def test_output_stream(self):
         # a stream of bytes can be output.
         ui = self.get_test_ui()
@@ -192,3 +184,12 @@
             stderr=subprocess.PIPE)
         out, err = proc.communicate()
         proc.returncode
+
+    def test_make_result(self):
+        # make_result should return a TestResult.
+        ui = self.ui_factory()
+        ui.set_command(commands.Command(ui))
+        result = ui.make_result(lambda: None)
+        result.startTestRun()
+        result.stopTestRun()
+        self.assertEqual(0, result.testsRun)

=== modified file 'testrepository/tests/ui/test_cli.py'
--- testrepository/tests/ui/test_cli.py	2010-09-11 19:56:11 +0000
+++ testrepository/tests/ui/test_cli.py	2010-09-20 17:47:50 +0000
@@ -1,11 +1,11 @@
 #
 # Copyright (c) 2009, 2010 Testrepository Contributors
-# 
+#
 # Licensed under either the Apache License, Version 2.0 or the BSD 3-clause
 # license at the users choice. A copy of both licenses are available in the
 # project source as Apache-2.0 and BSD. You may not use this file except in
 # compliance with one of these two licences.
-# 
+#
 # Unless required by applicable law or agreed to in writing, software
 # distributed under these licenses is distributed on an "AS IS" BASIS, WITHOUT
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
@@ -18,10 +18,10 @@
 from cStringIO import StringIO
 import sys
 
+from testtools import TestCase
 from testtools.matchers import DocTestMatches
 
 from testrepository import arguments
-import testrepository.arguments.command
 from testrepository import commands
 from testrepository.ui import cli
 from testrepository.tests import ResourcedTestCase
@@ -42,7 +42,7 @@
         stdout = StringIO()
         stdin = StringIO()
         stderr = StringIO()
-        ui = cli.UI([], stdin, stdout, stderr)
+        cli.UI([], stdin, stdout, stderr)
 
     def test_stream_comes_from_stdin(self):
         stdout = StringIO()
@@ -89,7 +89,8 @@
         class Case(ResourcedTestCase):
             def method(self):
                 self.fail('quux')
-        ui.output_results(Case('method'))
+        result = ui.make_result(lambda: None)
+        Case('method').run(result)
         self.assertThat(ui._stdout.getvalue(),DocTestMatches(
             """======================================================================
 FAIL: testrepository.tests.ui.test_cli.Case.method
@@ -158,3 +159,60 @@
         cmd.args = [arguments.string.StringArgument('args', max=None)]
         ui.set_command(cmd)
         self.assertEqual({'args':['one', '--two', 'three']}, ui.arguments)
+
+
+class TestCLITestResult(TestCase):
+
+    def make_exc_info(self):
+        # Make an exc_info tuple for use in testing.
+        try:
+            1/0
+        except ZeroDivisionError:
+            return sys.exc_info()
+
+    def make_result(self, stream=None):
+        if stream is None:
+            stream = StringIO()
+        ui = cli.UI([], None, stream, None)
+        return ui.make_result(lambda: None)
+
+    def test_initial_stream(self):
+        # CLITestResult.__init__ does not do anything to the stream it is
+        # given.
+        stream = StringIO()
+        cli.CLITestResult(cli.UI(None, None, None, None), stream, lambda: None)
+        self.assertEqual('', stream.getvalue())
+
+    def test_format_error(self):
+        # CLITestResult formats errors by giving them a big fat line, a title
+        # made up of their 'label' and the name of the test, another different
+        # big fat line, and then the actual error itself.
+        result = self.make_result()
+        error = result._format_error('label', self, 'error text')
+        expected = '%s%s: %s\n%s%s' % (
+            result.sep1, 'label', self.id(), result.sep2, 'error text')
+        self.assertThat(error, DocTestMatches(expected))
+
+    def test_addError_outputs_error(self):
+        # CLITestResult.addError outputs the given error immediately to the
+        # stream.
+        stream = StringIO()
+        result = self.make_result(stream)
+        error = self.make_exc_info()
+        error_text = result._err_details_to_string(self, error)
+        result.addError(self, error)
+        self.assertThat(
+            stream.getvalue(),
+            DocTestMatches(result._format_error('ERROR', self, error_text)))
+
+    def test_addFailure_outputs_failure(self):
+        # CLITestResult.addError outputs the given error immediately to the
+        # stream.
+        stream = StringIO()
+        result = self.make_result(stream)
+        error = self.make_exc_info()
+        error_text = result._err_details_to_string(self, error)
+        result.addFailure(self, error)
+        self.assertThat(
+            stream.getvalue(),
+            DocTestMatches(result._format_error('FAIL', self, error_text)))

=== modified file 'testrepository/ui/__init__.py'
--- testrepository/ui/__init__.py	2010-02-28 07:33:06 +0000
+++ testrepository/ui/__init__.py	2010-09-20 17:47:50 +0000
@@ -22,6 +22,9 @@
 for.
 """
 
+from testtools import TestResult
+
+
 class AbstractUI(object):
     """The base class for UI objects, this providers helpers and the interface.
 
@@ -82,6 +85,14 @@
         """Helper for iter_streams which subclasses should implement."""
         raise NotImplementedError(self._iter_streams)
 
+    def make_result(self, get_id):
+        """Make a `TestResult` that can be used to display test results.
+
+        :param get_id: A nullary callable that returns the id of the test run
+            when called.
+        """
+        raise NotImplementedError(self.make_result)
+
     def output_error(self, error_tuple):
         """Show an error to the user.
 
@@ -102,18 +113,6 @@
         """
         raise NotImplementedError(self.output_rest)
 
-    def output_results(self, suite_or_test):
-        """Show suite_or_test to the user by 'running' it.
-
-        This expects the run to be fast/cheap.
-
-        :param suite_or_test: A suite or test to show to the user. This should
-            obey the 'TestCase' protocol - it should have a method run(result)
-            that causes all the tests contained in the object to be handed to
-            the result object.
-        """
-        raise NotImplementedError(self.output_results)
-
     def output_stream(self, stream):
         """Show a byte stream to the user.
 
@@ -163,3 +162,41 @@
         """
         # This might not be the right place.
         raise NotImplementedError(self.subprocess_Popen)
+
+
+class BaseUITestResult(TestResult):
+    """An abstract test result used with the UI.
+
+    AbstractUI.make_result probably wants to return an object like this.
+    """
+
+    def __init__(self, ui, get_id):
+        """Construct an `AbstractUITestResult`.
+
+        :param ui: The UI this result is associated with.
+        :param get_id: A nullary callable that returns the id of the test run.
+        """
+        super(BaseUITestResult, self).__init__()
+        self.ui = ui
+        self.get_id = get_id
+
+    def _output_run(self, run_id):
+        """Output a test run.
+
+        :param run_id: The run id.
+        """
+        if self.ui.options.quiet:
+            return
+        values = [('id', run_id), ('tests', self.testsRun)]
+        failures = len(self.failures) + len(self.errors)
+        if failures:
+            values.append(('failures', failures))
+        skips = sum(map(len, self.skip_reasons.itervalues()))
+        if skips:
+            values.append(('skips', skips))
+        self.ui.output_values(values)
+
+    def stopTestRun(self):
+        super(BaseUITestResult, self).stopTestRun()
+        run_id = self.get_id()
+        self._output_run(run_id)

=== modified file 'testrepository/ui/cli.py'
--- testrepository/ui/cli.py	2010-09-11 19:56:11 +0000
+++ testrepository/ui/cli.py	2010-09-20 17:47:50 +0000
@@ -18,31 +18,34 @@
 import os
 import sys
 
-import testtools
-
 from testrepository import ui
 
-class CLITestResult(testtools.TestResult):
+
+class CLITestResult(ui.BaseUITestResult):
     """A TestResult for the CLI."""
 
-    def __init__(self, stream):
+    def __init__(self, ui, get_id, stream):
         """Construct a CLITestResult writing to stream."""
-        super(CLITestResult, self).__init__()
+        super(CLITestResult, self).__init__(ui, get_id)
         self.stream = stream
         self.sep1 = '=' * 70 + '\n'
         self.sep2 = '-' * 70 + '\n'
 
-    def _show_list(self, label, error_list):
-        for test, output in error_list:
-            self.stream.write(self.sep1)
-            self.stream.write("%s: %s\n" % (label, test.id()))
-            self.stream.write(self.sep2)
-            self.stream.write(output)
-
-    def stopTestRun(self):
-        self._show_list('ERROR', self.errors)
-        self._show_list('FAIL', self.failures)
-        super(CLITestResult, self).stopTestRun()
+    def _format_error(self, label, test, error_text):
+        return ''.join([
+            self.sep1,
+            '%s: %s\n' % (label, test.id()),
+            self.sep2,
+            error_text,
+            ])
+
+    def addError(self, test, err=None, details=None):
+        super(CLITestResult, self).addError(test, err=err, details=details)
+        self.stream.write(self._format_error('ERROR', *(self.errors[-1])))
+
+    def addFailure(self, test, err=None, details=None):
+        super(CLITestResult, self).addFailure(test, err=err, details=details)
+        self.stream.write(self._format_error('FAIL', *(self.failures[-1])))
 
 
 class UI(ui.AbstractUI):
@@ -64,6 +67,9 @@
     def _iter_streams(self, stream_type):
         yield self._stdin
 
+    def make_result(self, get_id):
+        return CLITestResult(self, get_id, self._stdout)
+
     def output_error(self, error_tuple):
         self._stderr.write(str(error_tuple[1]) + '\n')
 
@@ -72,14 +78,6 @@
         if not rest_string.endswith('\n'):
             self._stdout.write('\n')
 
-    def output_results(self, suite_or_test):
-        result = CLITestResult(self._stdout)
-        result.startTestRun()
-        try:
-            suite_or_test.run(result)
-        finally:
-            result.stopTestRun()
-
     def output_stream(self, stream):
         contents = stream.read(65536)
         while contents:

=== modified file 'testrepository/ui/model.py'
--- testrepository/ui/model.py	2010-09-07 12:37:17 +0000
+++ testrepository/ui/model.py	2010-09-20 17:47:50 +0000
@@ -19,6 +19,7 @@
 
 from testrepository import ui
 
+
 class ProcessModel(object):
     """A subprocess.Popen test double."""
 
@@ -31,6 +32,47 @@
         return '', ''
 
 
+class TestSuiteModel(object):
+
+    def __init__(self):
+        self._results = []
+
+    def recordResult(self, method, *args):
+        self._results.append((method, args))
+
+    def run(self, result):
+        for method, args in self._results:
+            getattr(result, method)(*args)
+
+
+class TestResultModel(ui.BaseUITestResult):
+
+    def __init__(self, ui, get_id):
+        super(TestResultModel, self).__init__(ui, get_id)
+        self._suite = TestSuiteModel()
+
+    def startTest(self, test):
+        super(TestResultModel, self).startTest(test)
+        self._suite.recordResult('startTest', test)
+
+    def stopTest(self, test):
+        self._suite.recordResult('stopTest', test)
+
+    def addError(self, test, *args):
+        super(TestResultModel, self).addError(test, *args)
+        self._suite.recordResult('addError', test, *args)
+
+    def addFailure(self, test, *args):
+        super(TestResultModel, self).addFailure(test, *args)
+        self._suite.recordResult('addFailure', test, *args)
+
+    def stopTestRun(self):
+        if self.ui.options.quiet:
+            return
+        self.ui.outputs.append(('results', self._suite))
+        return super(TestResultModel, self).stopTestRun()
+
+
 class UI(ui.AbstractUI):
     """A object based UI.
     
@@ -87,15 +129,15 @@
         for stream_bytes in streams:
             yield StringIO(stream_bytes)
 
+    def make_result(self, get_id):
+        return TestResultModel(self, get_id)
+
     def output_error(self, error_tuple):
         self.outputs.append(('error', error_tuple))
 
     def output_rest(self, rest_string):
         self.outputs.append(('rest', rest_string))
 
-    def output_results(self, suite_or_test):
-        self.outputs.append(('results', suite_or_test))
-
     def output_stream(self, stream):
         self.outputs.append(('stream', stream.read()))