Split out event forwarding and summarising roles for make_result.

8 files changed, 34 insertions, 28 deletions

diff --git a/testrepository/commands/failing.py b/testrepository/commands/failing.py
index 267d414..3c77e14 100644
--- a/testrepository/commands/failing.py
+++ b/testrepository/commands/failing.py
@@ -56,15 +56,17 @@ class failing(Command):
     def _make_result(self, repo, list_result):
         testcommand = self.command_factory(self.ui, repo)
         if self.ui.options.list:
-            return testcommand.make_result(list_result)
+            result = testcommand.make_result(list_result)
+            return result, result
         else:
-            output_result = ExtendedToStreamDecorator(
-                self.ui.make_result(repo.latest_id, testcommand))
+            output_result, summary_result = self.ui.make_result(
+                repo.latest_id, testcommand)
+            output_result = ExtendedToStreamDecorator(output_result)
             # This probably wants to be removed or pushed into the CLIResult
             # responsibilities, it attempts to preserve skips, but the ui
             # make_result filters them - a mismatch.
             errors_only = TestResultFilter(output_result, filter_skip=True)
-            return MultiTestResult(list_result, output_result)
+            return MultiTestResult(list_result, output_result), summary_result
 
     def run(self):
         repo = self.repository_factory.open(self.ui.here)
@@ -74,16 +76,13 @@ class failing(Command):
         case = run.get_test()
         failed = False
         list_result = TestResult()
-        result = self._make_result(repo, list_result)
+        result, summary = self._make_result(repo, list_result)
         result.startTestRun()
         try:
             case.run(result)
         finally:
             result.stopTestRun()
-        # XXX: This bypasses the user defined transforms, and also sets a
-        # non-zero return even on --list, which is inappropriate. The UI result
-        # knows about success/failure in more detail.
-        failed = not list_result.wasSuccessful()
+        failed = not summary.wasSuccessful()
         if failed:
             result = 1
         else:
diff --git a/testrepository/commands/last.py b/testrepository/commands/last.py
index a8d7c04..4c73b04 100644
--- a/testrepository/commands/last.py
+++ b/testrepository/commands/last.py
@@ -56,14 +56,14 @@ class last(Command):
         except KeyError:
             previous_run = None
         failed = False
-        result = self.ui.make_result(
+        result, summary = self.ui.make_result(
             latest_run.get_id, testcommand, previous_run=previous_run)
         result.startTestRun()
         try:
             case.run(result)
         finally:
             result.stopTestRun()
-        failed = not result.wasSuccessful()
+        failed = not summary.wasSuccessful()
         if failed:
             return 1
         else:
diff --git a/testrepository/commands/load.py b/testrepository/commands/load.py
index ab8d879..c762ce0 100644
--- a/testrepository/commands/load.py
+++ b/testrepository/commands/load.py
@@ -97,6 +97,7 @@ class load(Command):
                 else:
                     # Calls TestResult API.
                     case = subunit.ProtocolTestCase(stream)
+                    # Now calls StreamResult API :).
                     case = testtools.DecorateTestCaseResult(
                         case,
                         testtools.ExtendedToStreamDecorator,
@@ -107,7 +108,7 @@ class load(Command):
                         [result], add=['worker-%d' % pos]))
                 yield (case, str(pos))
         case = testtools.ConcurrentStreamTestSuite(cases, make_tests)
-        # One copy of the stream to repository storage
+        # One unmodified copy of the stream to repository storage
         inserter = repo.get_inserter(partial=self.ui.options.partial)
         # One copy of the stream to the UI layer after performing global
         # filters.
@@ -115,7 +116,7 @@ class load(Command):
             previous_run = repo.get_latest_run()
         except KeyError:
             previous_run = None
-        output_result = self.ui.make_result(
+        output_result, summary_result = self.ui.make_result(
             inserter.get_id, testcommand, previous_run=previous_run)
         result = testtools.CopyStreamResult([inserter, output_result])
         result.startTestRun()
@@ -123,7 +124,7 @@ class load(Command):
             case.run(result)
         finally:
             result.stopTestRun()
-        if not output_result.wasSuccessful():
+        if not summary_result.wasSuccessful():
             return 1
         else:
             return 0
diff --git a/testrepository/tests/test_ui.py b/testrepository/tests/test_ui.py
index 679332e..6cafba6 100644
--- a/testrepository/tests/test_ui.py
+++ b/testrepository/tests/test_ui.py
@@ -226,21 +226,23 @@ class TestUIContract(ResourcedTestCase):
         out, err = proc.communicate()
 
     def test_make_result(self):
-        # make_result should return a StreamResult.
+        # make_result should return a StreamResult and a summary result.
         ui = self.ui_factory()
         ui.set_command(commands.Command(ui))
-        result = ui.make_result(lambda: None, StubTestCommand())
+        result, summary = ui.make_result(lambda: None, StubTestCommand())
         result.startTestRun()
         result.status()
         result.stopTestRun()
+        summary.wasSuccessful()
 
     def test_make_result_previous_run(self):
         # make_result can take a previous run.
         ui = self.ui_factory()
         ui.set_command(commands.Command(ui))
-        result = ui.make_result(
+        result, summary = ui.make_result(
             lambda: None, StubTestCommand(),
             previous_run=memory.Repository().get_failing())
         result.startTestRun()
         result.status()
         result.stopTestRun()
+        summary.wasSuccessful()
diff --git a/testrepository/tests/ui/test_cli.py b/testrepository/tests/ui/test_cli.py
index ea4170c..eaa4193 100644
--- a/testrepository/tests/ui/test_cli.py
+++ b/testrepository/tests/ui/test_cli.py
@@ -131,7 +131,7 @@ class TestCLIUI(ResourcedTestCase):
         class Case(ResourcedTestCase):
             def method(self):
                 self.fail('quux')
-        result = ui.make_result(lambda: None, StubTestCommand())
+        result, summary = ui.make_result(lambda: None, StubTestCommand())
         result.startTestRun()
         Case('method').run(testtools.ExtendedToStreamDecorator(result))
         result.stopTestRun()
@@ -344,7 +344,7 @@ class TestCLITestResult(TestCase):
         # 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._unwrap(self.make_result())
+        result = self._unwrap(self.make_result()[0])
         error = result._format_error('label', self, 'error text')
         expected = '%s%s: %s\n%s%s' % (
             result.sep1, 'label', self.id(), result.sep2, 'error text')
@@ -352,7 +352,7 @@ class TestCLITestResult(TestCase):
 
     def test_format_error_includes_tags(self):
         result1 = self.make_result()
-        result = self._unwrap(result1)
+        result = self._unwrap(result1[0])
         #result1.startTestRun()
         #result1.status(test_id=self.id(), test_status='fail', eof=True,
         #    test_tags=set(['foo']), file_name='traceback',
@@ -367,7 +367,7 @@ class TestCLITestResult(TestCase):
         # CLITestResult.status test_status='fail' outputs the given error
         # immediately to the stream.
         stream = StringIO()
-        result = self.make_result(stream)
+        result = self.make_result(stream)[0]
         error = self.make_exc_info()
         error_text = 'foo\nbar\n'
         result.startTestRun()
@@ -384,7 +384,7 @@ class TestCLITestResult(TestCase):
         # characters.
         # Lets say we have bytes output, not string for some reason.
         stream = BytesIO()
-        result = self.make_result(stream)
+        result = self.make_result(stream)[0]
         result.startTestRun()
         result.status(test_id='foo', test_status='fail', file_name='traceback',
             mime_type='text/plain;charset=utf8',
diff --git a/testrepository/ui/__init__.py b/testrepository/ui/__init__.py
index 6b4588b..b7c1f82 100644
--- a/testrepository/ui/__init__.py
+++ b/testrepository/ui/__init__.py
@@ -96,6 +96,8 @@ class AbstractUI(object):
         :param test_command: A TestCommand object used to configure user
             transforms.
         :param previous_run: An optional previous test run.
+        :return: A two-tuple with the stream to forward events to, and a
+            StreamSummary for querying success after the stream is finished.
         """
         raise NotImplementedError(self.make_result)
 
diff --git a/testrepository/ui/cli.py b/testrepository/ui/cli.py
index 8353d57..65f7413 100644
--- a/testrepository/ui/cli.py
+++ b/testrepository/ui/cli.py
@@ -81,12 +81,13 @@ class UI(ui.AbstractUI):
     def make_result(self, get_id, test_command, previous_run=None):
         if getattr(self.options, 'subunit', False):
             # By pass user transforms - just forward it all.
-            return ExtendedToStreamDecorator(StreamToExtendedDecorator(
+            result = ExtendedToStreamDecorator(StreamToExtendedDecorator(
                 subunit.TestProtocolClient(self._stdout)))
-        output = CLITestResult(self, get_id, self._stdout, previous_run)
-        # Apply user defined transforms.
-        return ExtendedToStreamDecorator(
-            StreamToExtendedDecorator(test_command.make_result(output)))
+        else:
+            output = CLITestResult(self, get_id, self._stdout, previous_run)
+            # Apply user defined transforms.
+            result = ExtendedToStreamDecorator(StreamToExtendedDecorator(test_command.make_result(output)))
+        return result, result
 
     def output_error(self, error_tuple):
         if 'TESTR_PDB' in os.environ:
diff --git a/testrepository/ui/model.py b/testrepository/ui/model.py
index fd438d4..59717f8 100644
--- a/testrepository/ui/model.py
+++ b/testrepository/ui/model.py
@@ -156,10 +156,11 @@ class UI(ui.AbstractUI):
                 yield BytesIO(stream_value)
 
     def make_result(self, get_id, test_command, previous_run=None):
-        return testtools.ExtendedToStreamDecorator(
+        result = testtools.ExtendedToStreamDecorator(
             testtools.StreamToExtendedDecorator(
             test_command.make_result(
             TestResultModel(self, get_id, previous_run))))
+        return result, result
 
     def output_error(self, error_tuple):
         self.outputs.append(('error', error_tuple))