Merge lp:~yellow/testrepository/bug988481 into lp:~testrepository/testrepository/trunk

[Gary Poster]

This branch depends on lp:~yellow/subunit/real-time and lp:~yellow/subunit/test-count. The following MP text is slightly modified from the MP for the first branch.

testrepository had a class (also called TestResultFilter) that tried to send all time calls through. To implement, it subclassed subunit's TestResultFilter and then reached into the _buffered_calls while in the stopTest method to try and find the time calls. Because of use of the decorator pattern, this was fragile.

The associated change in subunit gives that class the behavior that testrepository wants.

This change in subunit, plus the change in lp:~yellow/subunit/test-count, means that testrepository can use subunit's TestResultFilter directly, thereby getting rid of the fragility associated with the combination of subclassing, decorating, and accessing a protected attribute.

This is work done by Benji York and Brad Crittenden; I'm just helping out with the MP.

[Jonathan Lange (jml)]

Thanks. I'm very much in favour of removing this subclass, and this patch looks like the right way to do it. I've taken issue with https://code.launchpad.net/~yellow/subunit/test-count/+merge/103717, and I suspect this patch will have to change as a result of required changes there.

[Robert Collins (lifeless)]

This would drop compatability with the prior release of subunit; we try moderately hard to make each new release compatible with the prior release of the related libraries; that makes migration a lot easier for users. I'll roll a relevant update into my WIP branch.

review: Disapprove

Unmerged revisions

149. By Gary Poster on 2012-04-26

remove functionality that has been moved into subunit

=== modified file 'testrepository/commands/failing.py'
--- testrepository/commands/failing.py	2010-10-01 15:49:32 +0000
+++ testrepository/commands/failing.py	2012-04-26 14:20:32 +0000
@@ -19,7 +19,7 @@
 from testtools import MultiTestResult, TestResult
 
 from testrepository.commands import Command
-from testrepository.results import TestResultFilter
+from subunit.test_results import TestResultFilter
 
 
 class failing(Command):

=== modified file 'testrepository/commands/last.py'
--- testrepository/commands/last.py	2011-11-03 20:41:47 +0000
+++ testrepository/commands/last.py	2012-04-26 14:20:32 +0000
@@ -15,7 +15,7 @@
 """Show the last run loaded into a repository."""
 
 from testrepository.commands import Command
-from testrepository.results import TestResultFilter
+from subunit.test_results import TestResultFilter
 
 
 class last(Command):

=== modified file 'testrepository/results.py'
--- testrepository/results.py	2011-11-02 20:55:16 +0000
+++ testrepository/results.py	2012-04-26 14:20:32 +0000
@@ -1,47 +1,8 @@
-from subunit import test_results
-
 from testtools import TestResult
 
 from testrepository.utils import timedelta_to_seconds
 
 
-class TestResultFilter(test_results.TestResultFilter):
-    """Test result filter."""
-
-    def _get_concrete_result(self):
-        # XXX: This is really crappy. It assumes that the test result we
-        # actually care about is decorated and that we can find our way to the
-        # one we care about. We want to report counts before filtering, so we
-        # should actually use two result objects - one to count and one to
-        # show. Arguably also the testsRun incrementing facility should be in
-        # testtools / subunit
-        concrete = self
-        while True:
-            next = getattr(concrete, 'decorated', None)
-            if next is None:
-                return concrete
-            concrete = next
-
-    def _filtered(self):
-        super(TestResultFilter, self)._filtered()
-        concrete = self._get_concrete_result()
-        concrete.testsRun += 1
-
-    def stopTest(self, test):
-        # Filter out 'time' calls, because we want to forward those events
-        # regardless of whether the test is filtered.
-        #
-        # XXX: Should this be pushed into subunit?
-        buffered_calls = []
-        for method, args, kwargs in self._buffered_calls:
-            if method == 'time':
-                self.decorated.time(*args, **kwargs)
-            else:
-                buffered_calls.append((method, args, kwargs))
-        self._buffered_calls = buffered_calls
-        super(TestResultFilter, self).stopTest(test)
-
-
 class SummarizingResult(TestResult):
 
     def __init__(self):

=== modified file 'testrepository/tests/commands/test_load.py'
--- testrepository/tests/commands/test_load.py	2012-04-19 13:27:28 +0000
+++ testrepository/tests/commands/test_load.py	2012-04-26 14:20:32 +0000
@@ -27,7 +27,6 @@
 from testrepository.ui.model import UI
 from testrepository.tests import ResourcedTestCase, Wildcard
 from testrepository.tests.test_repository import RecordingRepositoryFactory
-from testrepository.tests.repository.test_file import HomeDirTempDir
 from testrepository.repository import memory, RepositoryNotFound
 
 

=== modified file 'testrepository/tests/test_results.py'
--- testrepository/tests/test_results.py	2011-11-02 20:55:16 +0000
+++ testrepository/tests/test_results.py	2012-04-26 14:20:32 +0000
@@ -25,10 +25,8 @@
     ThreadsafeForwardingResult,
     )
 
-from testrepository.results import (
-    SummarizingResult,
-    TestResultFilter,
-    )
+from subunit.test_results import TestResultFilter
+from testrepository.results import SummarizingResult
 from testrepository.ui import BaseUITestResult
 from testrepository.ui.model import UI
 

=== modified file 'testrepository/ui/cli.py'
--- testrepository/ui/cli.py	2012-04-17 20:05:44 +0000
+++ testrepository/ui/cli.py	2012-04-26 14:20:32 +0000
@@ -23,7 +23,7 @@
 from testtools.compat import unicode_output_stream
 
 from testrepository import ui
-from testrepository.results import TestResultFilter
+from subunit.test_results import TestResultFilter
 
 
 class CLITestResult(ui.BaseUITestResult):