Change get_test() APIs to return StreamResult rather than TestResult emitting test objects.

10 files changed, 135 insertions, 75 deletions

diff --git a/testrepository/commands/failing.py b/testrepository/commands/failing.py
index 3c77e14..68ab890 100644
--- a/testrepository/commands/failing.py
+++ b/testrepository/commands/failing.py
@@ -16,7 +16,8 @@
 
 import optparse
 
-from testtools import ExtendedToStreamDecorator, MultiTestResult, TestResult
+import testtools
+from testtools import ExtendedToStreamDecorator, MultiTestResult
 
 from testrepository.commands import Command
 from testrepository.results import TestResultFilter
@@ -61,12 +62,12 @@ class failing(Command):
         else:
             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), summary_result
+            return testtools.CopyStreamResult(
+                [list_result, output_result]), summary_result
 
     def run(self):
         repo = self.repository_factory.open(self.ui.here)
@@ -75,7 +76,7 @@ class failing(Command):
             return self._show_subunit(run)
         case = run.get_test()
         failed = False
-        list_result = TestResult()
+        list_result = testtools.StreamSummary()
         result, summary = self._make_result(repo, list_result)
         result.startTestRun()
         try:
diff --git a/testrepository/commands/run.py b/testrepository/commands/run.py
index 524716a..a18c235 100644
--- a/testrepository/commands/run.py
+++ b/testrepository/commands/run.py
@@ -22,8 +22,8 @@ import re
 from extras import try_import
 import subunit
 v2_avail = try_import('subunit.ByteStreamToStreamResult')
+import testtools
 from testtools import (
-    TestResult,
     TestByTestResult,
     )
 from testtools.compat import _b
@@ -48,6 +48,9 @@ class ReturnCodeToSubunit(object):
     synthetic test is added to the output, making the error accessible to
     subunit stream consumers. If the process closes its stdout and then does
     not terminate, reading from the ReturnCodeToSubunit stream will hang.
+
+    This class will be deleted at some point, allowing parsing to read from the
+    actual fd and benefit from select for aggregating non-subunit output.
     """
 
     def __init__(self, process):
@@ -149,14 +152,16 @@ class run(Command):
     def _find_failing(self, repo):
         run = repo.get_failing()
         case = run.get_test()
-        result = TestResult()
+        ids = []
+        def gather_errors(test_dict):
+            if test_dict['status'] == 'fail':
+                ids.append(test_dict['id'])
+        result = testtools.StreamToDict(gather_errors)
         result.startTestRun()
         try:
             case.run(result)
         finally:
             result.stopTestRun()
-        ids = [failure[0].id() for failure in result.failures]
-        ids.extend([error[0].id() for error in result.errors])
         return ids
 
     def run(self):
@@ -235,11 +240,10 @@ class run(Command):
                         # check that the test we're probing still failed - still
                         # awkward.
                         found_fail = []
-                        def find_fail(test, status, start_time, stop_time, tags,
-                            details):
-                            if test.id() == spurious_failure:
+                        def find_fail(test_dict):
+                            if test_dict['id'] == spurious_failure:
                                 found_fail.append(True)
-                        checker = TestByTestResult(find_fail)
+                        checker = testtools.StreamToDict(find_fail)
                         checker.startTestRun()
                         try:
                             repo.get_failing().get_test().run(checker)
@@ -283,6 +287,7 @@ class run(Command):
         Tests that ran in a different worker are not included in the result.
         """
         if not getattr(self, '_worker_to_test', False):
+            # TODO: switch to route codes?
             case = run.get_test()
             # Use None if there is no worker-N tag
             # If there are multiple, map them all.
@@ -290,7 +295,9 @@ class run(Command):
             worker_to_test = {}
             # (testid -> [workerN, ...])
             test_to_worker = {}
-            def map_test(test, status, start_time, stop_time, tags, details):
+            def map_test(test_dict):
+                tags = test_dict['tags']
+                id = test_dict['id']
                 workers = []
                 for tag in tags:
                     if tag.startswith('worker-'):
@@ -298,9 +305,9 @@ class run(Command):
                 if not workers:
                     workers = [None]
                 for worker in workers:
-                    worker_to_test.setdefault(worker, []).append(test.id())
-                test_to_worker.setdefault(test.id(), []).extend(workers)
-            mapper = TestByTestResult(map_test)
+                    worker_to_test.setdefault(worker, []).append(id)
+                test_to_worker.setdefault(id, []).extend(workers)
+            mapper = testtools.StreamToDict(map_test)
             mapper.startTestRun()
             try:
                 case.run(mapper)
diff --git a/testrepository/repository/__init__.py b/testrepository/repository/__init__.py
index be9feec..b58c0a2 100644
--- a/testrepository/repository/__init__.py
+++ b/testrepository/repository/__init__.py
@@ -27,7 +27,7 @@ Repositories are identified by their URL, and new ones are made by calling
 the initialize function in the appropriate repository module.
 """
 
-from testtools import TestResult
+from testtools import StreamToDict, TestResult
 
 
 class AbstractRepositoryFactory(object):
@@ -142,9 +142,16 @@ class AbstractRepository(object):
             were part of the specified test run.
         """
         run = self.get_test_run(run_id)
-        result = TestIDCapturer()
-        run.get_test().run(result)
-        return result.ids
+        ids = []
+        def gather(test_dict):
+            ids.append(test_dict['id'])
+        result = StreamToDict(gather)
+        result.startTestRun()
+        try:
+            run.get_test().run(result)
+        finally:
+            result.stopTestRun()
+        return ids
 
 
 class AbstractTestRun(object):
@@ -184,20 +191,3 @@ class RepositoryNotFound(Exception):
         self.url = url
         msg = 'No repository found in %s. Create one by running "testr init".'
         Exception.__init__(self, msg % url)
-
-
-class TestIDCapturer(TestResult):
-    """Capture the test ids from a test run.
-
-    After using the result with a test run, the ids of
-    the tests that were run are available in the ids
-    attribute.
-    """
-
-    def __init__(self):
-        super(TestIDCapturer, self).__init__()
-        self.ids = []
-
-    def startTest(self, test):
-        super(TestIDCapturer, self).startTest(test)
-        self.ids.append(test.id())
diff --git a/testrepository/repository/file.py b/testrepository/repository/file.py
index 30911ea..85ee1a3 100644
--- a/testrepository/repository/file.py
+++ b/testrepository/repository/file.py
@@ -20,6 +20,7 @@ try:
 except ImportError:
     import dbm
 import errno
+from operator import methodcaller
 import os.path
 import sys
 import tempfile
@@ -190,7 +191,17 @@ class _DiskRun(AbstractTestRun):
         return BytesIO(self._content)
 
     def get_test(self):
-        return subunit.ProtocolTestCase(self.get_subunit_stream())
+        case = subunit.ProtocolTestCase(self.get_subunit_stream())
+        def wrap_result(result):
+            # Wrap in a router to mask out startTestRun/stopTestRun from the
+            # ExtendedToStreamDecorator.
+            result = testtools.StreamResultRouter(result, do_start_stop_run=False)
+            # Wrap that in ExtendedToStreamDecorator to convert v1 calls to
+            # StreamResult.
+            return testtools.ExtendedToStreamDecorator(result)
+        return testtools.DecorateTestCaseResult(
+            case, wrap_result, methodcaller('startTestRun'),
+            methodcaller('stopTestRun'))
 
 
 class _SafeInserter(object):
diff --git a/testrepository/repository/memory.py b/testrepository/repository/memory.py
index a332d6f..8aee71f 100644
--- a/testrepository/repository/memory.py
+++ b/testrepository/repository/memory.py
@@ -15,6 +15,7 @@
 """In memory storage of test results."""
 
 from io import BytesIO
+from operator import methodcaller
 
 import subunit
 import testtools
@@ -104,7 +105,16 @@ class _Failures(AbstractTestRun):
         return result
 
     def get_test(self):
-        return self
+        def wrap_result(result):
+            # Wrap in a router to mask out startTestRun/stopTestRun from the
+            # ExtendedToStreamDecorator.
+            result = testtools.StreamResultRouter(result, do_start_stop_run=False)
+            # Wrap that in ExtendedToStreamDecorator to convert v1 calls to
+            # StreamResult.
+            return testtools.ExtendedToStreamDecorator(result)
+        return testtools.DecorateTestCaseResult(
+            self, wrap_result, methodcaller('startTestRun'),
+            methodcaller('stopTestRun'))
 
     def run(self, result):
         # Speaks original.
@@ -168,7 +178,16 @@ class _Inserter(AbstractTestRun):
         return self._subunit
 
     def get_test(self):
-        return self
+        def wrap_result(result):
+            # Wrap in a router to mask out startTestRun/stopTestRun from the
+            # ExtendedToStreamDecorator.
+            result = testtools.StreamResultRouter(result, do_start_stop_run=False)
+            # Wrap that in ExtendedToStreamDecorator to convert v1 calls to
+            # StreamResult.
+            return testtools.ExtendedToStreamDecorator(result)
+        return testtools.DecorateTestCaseResult(
+            self, wrap_result, methodcaller('startTestRun'),
+            methodcaller('stopTestRun'))
 
     def run(self, result):
         # Speaks original.
diff --git a/testrepository/results.py b/testrepository/results.py
index 47f427c..32a5fa8 100644
--- a/testrepository/results.py
+++ b/testrepository/results.py
@@ -1,6 +1,6 @@
 from subunit import test_results
 
-from testtools import TestResult
+from testtools import StreamSummary
 
 from testrepository.utils import timedelta_to_seconds
 
@@ -46,26 +46,32 @@ else:
     TestResultFilter = test_results.TestResultFilter
 
 
-class SummarizingResult(TestResult):
+class SummarizingResult(StreamSummary):
 
     def __init__(self):
         super(SummarizingResult, self).__init__()
-        self._first_time = None
 
     def startTestRun(self):
         super(SummarizingResult, self).startTestRun()
         self._first_time = None
+        self._last_time = None
+
+    def status(self, *args, **kwargs):
+        if 'timestamp' in kwargs:
+            timestamp = kwargs['timestamp']
+            if self._last_time is None:
+                self._first_time = timestamp
+                self._last_time = timestamp
+            if timestamp < self._first_time:
+                self._first_time = timestamp
+            if timestamp > self._last_time:
+                self._last_time = timestamp
+        super(SummarizingResult, self).status(*args, **kwargs)
 
     def get_num_failures(self):
         return len(self.failures) + len(self.errors)
 
-    def time(self, a_time):
-        if self._first_time is None:
-            self._first_time = a_time
-        super(SummarizingResult, self).time(a_time)
-
     def get_time_taken(self):
-        now = self._now()
-        if None in (self._first_time, now):
+        if None in (self._last_time, self._first_time):
             return None
-        return timedelta_to_seconds(now - self._first_time)
+        return timedelta_to_seconds(self._last_time - self._first_time)
diff --git a/testrepository/tests/commands/test_run.py b/testrepository/tests/commands/test_run.py
index a2d2a3e..41a05ad 100644
--- a/testrepository/tests/commands/test_run.py
+++ b/testrepository/tests/commands/test_run.py
@@ -422,7 +422,7 @@ class TestCommand(ResourcedTestCase):
             ('popen', (expected_cmd,),
              {'shell': True, 'stdin': PIPE, 'stdout': PIPE}),
             ('results', Wildcard),
-            ('summary', True, 1, -2, Wildcard, None, [('id', 1, None)]),
+            ('summary', True, 1, -2, Wildcard, Wildcard, [('id', 1, None)]),
             ('values', [('running', expected_cmd)]),
             ('popen', (expected_cmd,),
              {'shell': True, 'stdin': PIPE, 'stdout': PIPE}),
diff --git a/testrepository/tests/test_repository.py b/testrepository/tests/test_repository.py
index 47b27ec..f3e68d9 100644
--- a/testrepository/tests/test_repository.py
+++ b/testrepository/tests/test_repository.py
@@ -26,7 +26,6 @@ from testresources import TestResource
 from testtools import (
     clone_test_with_new_id,
     PlaceHolder,
-    TestResult,
     )
 import testtools
 from testtools.compat import _b
@@ -139,15 +138,23 @@ class TestRepositoryContract(ResourcedTestCase):
     def get_failing(self, repo):
         """Analyze a failing stream from repo and return it."""
         run = repo.get_failing()
-        analyzer = TestResult()
-        run.get_test().run(analyzer)
+        analyzer = testtools.StreamSummary()
+        analyzer.startTestRun()
+        try:
+            run.get_test().run(analyzer)
+        finally:
+            analyzer.stopTestRun()
         return analyzer
 
     def get_last_run(self, repo):
         """Return the results from a stream."""
         run = repo.get_test_run(repo.latest_id())
-        analyzer = TestResult()
-        run.get_test().run(analyzer)
+        analyzer = testtools.StreamSummary()
+        analyzer.startTestRun()
+        try:
+            run.get_test().run(analyzer)
+        finally:
+            analyzer.stopTestRun()
         return analyzer
 
     def test_can_initialise_with_param(self):
@@ -233,8 +240,8 @@ class TestRepositoryContract(ResourcedTestCase):
         legacy_result.stopTestRun()
         analyzed = self.get_failing(repo)
         self.assertEqual(1, analyzed.testsRun)
-        self.assertEqual(1, len(analyzed.failures))
-        self.assertEqual('failing', analyzed.failures[0][0].id())
+        self.assertEqual(1, len(analyzed.errors))
+        self.assertEqual('failing', analyzed.errors[0][0].id())
 
     def test_unexpected_success(self):
         # Unexpected successes get forwarded too. (Test added because of a
@@ -269,8 +276,8 @@ class TestRepositoryContract(ResourcedTestCase):
         legacy_result.stopTestRun()
         analyzed = self.get_failing(repo)
         self.assertEqual(1, analyzed.testsRun)
-        self.assertEqual(1, len(analyzed.failures))
-        self.assertEqual('passing', analyzed.failures[0][0].id())
+        self.assertEqual(1, len(analyzed.errors))
+        self.assertEqual('passing', analyzed.errors[0][0].id())
 
     def test_get_failing_partial_runs_preserve_missing_failures(self):
         # failures from two runs add to existing failures, and successes remove
@@ -291,9 +298,9 @@ class TestRepositoryContract(ResourcedTestCase):
         legacy_result.stopTestRun()
         analyzed = self.get_failing(repo)
         self.assertEqual(2, analyzed.testsRun)
-        self.assertEqual(2, len(analyzed.failures))
+        self.assertEqual(2, len(analyzed.errors))
         self.assertEqual(set(['passing', 'missing']),
-            set([test[0].id() for test in analyzed.failures]))
+            set([test[0].id() for test in analyzed.errors]))
 
     def test_get_test_run_missing_keyerror(self):
         repo = self.repo_impl.initialise(self.sample_url)
@@ -392,7 +399,7 @@ successful: testrepository.tests.test_repository.Case.method...
         inserted = result.get_id()
         run = repo.get_test_run(inserted)
         test = run.get_test()
-        result = TestResult()
+        result = testtools.StreamSummary()
         result.startTestRun()
         try:
             test.run(result)
diff --git a/testrepository/tests/test_results.py b/testrepository/tests/test_results.py
index e9e2fa5..e46d742 100644
--- a/testrepository/tests/test_results.py
+++ b/testrepository/tests/test_results.py
@@ -91,6 +91,8 @@ class TestSummarizingResult(TestCase):
 
     def test_empty(self):
         result = SummarizingResult()
+        result.startTestRun()
+        result.stopTestRun()
         self.assertEqual(0, result.testsRun)
         self.assertEqual(0, result.get_num_failures())
         self.assertIs(None, result.get_time_taken())
@@ -99,8 +101,8 @@ class TestSummarizingResult(TestCase):
         result = SummarizingResult()
         now = datetime.now()
         result.startTestRun()
-        result.time(now)
-        result.time(now + timedelta(seconds=5))
+        result.status(timestamp=now)
+        result.status(timestamp=now + timedelta(seconds=5))
         result.stopTestRun()
         self.assertEqual(5.0, result.get_time_taken())
 
@@ -111,10 +113,8 @@ class TestSummarizingResult(TestCase):
             1/0
         except ZeroDivisionError:
             error = sys.exc_info()
-        for method in ('addError', 'addFailure'):
-            result.startTest(self)
-            getattr(result, method)(self, error)
-            result.stopTest(self)
+        result.status(test_id='foo', test_status='fail')
+        result.status(test_id='foo', test_status='fail')
         result.stopTestRun()
         self.assertEqual(2, result.get_num_failures())
 
@@ -122,8 +122,6 @@ class TestSummarizingResult(TestCase):
         result = SummarizingResult()
         result.startTestRun()
         for i in range(5):
-            result.startTest(self)
-            result.addSuccess(self)
-            result.stopTest(self)
+            result.status(test_id='foo', test_status='success')
         result.stopTestRun()
         self.assertEqual(5, result.testsRun)
diff --git a/testrepository/ui/__init__.py b/testrepository/ui/__init__.py
index b7c1f82..2da1f98 100644
--- a/testrepository/ui/__init__.py
+++ b/testrepository/ui/__init__.py
@@ -22,8 +22,10 @@ See AbstractUI for details on what UI classes should do and are responsible
 for.
 """
 
-from testrepository.results import SummarizingResult
+from testtools import TestResult
 
+from testrepository.results import SummarizingResult
+from testrepository.utils import timedelta_to_seconds
 
 class AbstractUI(object):
     """The base class for UI objects, this providers helpers and the interface.
@@ -187,7 +189,7 @@ class AbstractUI(object):
         raise NotImplementedError(self.subprocess_Popen)
 
 
-class BaseUITestResult(SummarizingResult):
+class BaseUITestResult(TestResult):
     """An abstract test result used with the UI.
 
     AbstractUI.make_result probably wants to return an object like this.
@@ -203,6 +205,7 @@ class BaseUITestResult(SummarizingResult):
         self.ui = ui
         self.get_id = get_id
         self._previous_run = previous_run
+        self._first_time = None
 
     def _get_previous_summary(self):
         if self._previous_run is None:
@@ -245,7 +248,25 @@ class BaseUITestResult(SummarizingResult):
             not bool(failures), self.testsRun, num_tests_run_delta,
             time, time_delta, values)
 
+    def startTestRun(self):
+        super(BaseUITestResult, self).startTestRun()
+        self._first_time = None
+
     def stopTestRun(self):
         super(BaseUITestResult, self).stopTestRun()
         run_id = self.get_id()
         self._output_summary(run_id)
+
+    def get_num_failures(self):
+        return len(self.failures) + len(self.errors)
+
+    def time(self, a_time):
+        if self._first_time is None:
+            self._first_time = a_time
+        super(BaseUITestResult, self).time(a_time)
+
+    def get_time_taken(self):
+        now = self._now()
+        if None in (self._first_time, now):
+            return None
+        return timedelta_to_seconds(now - self._first_time)