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

[Gary Poster]

This branch depends on the subunit change in https://code.launchpad.net/~yellow/subunit/on_filter/+merge/103968 . It replaces https://code.launchpad.net/~yellow/testrepository/bug988481/+merge/103704 .

Because of the proposed subunit change, we can significantly simplify testrepository's TestResultFilter subclass.

Moreover, it will support https://code.launchpad.net/~jml/subunit/filter-tags/+merge/102840 in the future: that branch breaks testrepository because _filtered is no longer called on subunit's TestResultFilter in those changes.

[Jonathan Lange (jml)]

Is this still needed?

[Gary Poster (gary)]

I…don't know. It is one way to get your subunit tag filter branch to not cause test failures with testrepository. The only reason this branch, of one like it would not be necessary is if Robert's branch fixed it *and* it landed before yours.

I personally suggest that you all land the two on_filter branches of something like them that replaces dependency on _filtered, and then land your subunit-filter with tags branch, which has been proven to work as we'd like, at least. Robert's branch isn't ready to land, and these are, afaik.

[Robert Collins (lifeless)]

This comment in the removed lines - 'so we
12 - # should actually use two result objects - one to count and one to
13 - # show. ' - is key: rather than preserving the hack it should be straight forward enough to fix directly and drop the hack entirely. I'll look into doing that. As this stands it would need fixing to preserv compat with the current released testtools/subunit. I'll put it into WIP for now.

review: Needs Fixing

[Gary Poster (gary)]

Cool Robert, thanks. I'd put this into a withdrawn state if I could; maybe I'll delete later, but I'll keep it around for history for now. If anyone else would like to delete it, feel free.

[Robert Collins (lifeless)]

So I think the consensus was to not land this. Marking rejected so its not shown as pending.

Unmerged revisions

149. By Gary Poster on 2012-04-28

simplify TestResultFilter to take advantage of changes in lp:~yellow/subunit/on_filter; this also prepares the way for lp:~jml/subunit/filter-tags

=== modified file 'testrepository/results.py'
--- testrepository/results.py	2011-11-02 20:55:16 +0000
+++ testrepository/results.py	2012-04-28 02:36:19 +0000
@@ -8,38 +8,14 @@
 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)
+    def __init__(self, result, *args, **kwargs):
+        # This test count should be equal to all of the tests run, not
+        # just the tests that pass through the filter.
+        def on_filter(test):
+            result.testsRun += 1
+        kwargs['on_filter'] = on_filter
+        super(TestResultFilter, self).__init__(
+            result, *args, **kwargs)
 
 
 class SummarizingResult(TestResult):