Merge lp:~jml/testtools/forward-current-tags into lp:~testtools-committers/testtools/trunk

[Jonathan Lange]

Revamp testtools's tag support to correctly conform to the subunit standard and to have a rigorous test suite.

I tried to write a subunit tag filter and could not due to bugs in the way that tags behave in testtools. At around the same time, we received reports from the Launchpad developers that testtools TestResult and friends assume that tags are one continuous stream, and does not scope them for tests as described in the subunit README.

To address both of these issues, I wrote a bunch of tests that exercise the "tags" contract of TestResults. I've updated TestResult, MultiTestResult, ExtendedToOriginalDecorator, ExtendedTestResult, and ThreadsafeForwardingResult to implement this contract correctly. I don't think I've missed any.

While doing this I discovered that testtools was under the impression that Python 2.7 has tags support. It does not, and I removed the tests that suggested otherwise: it's a bug if code calls a Python 2.6 or 2.7 TestResult expecting tags to work.

To implement this better I made a TagContext object. I originally implemented this with functions alone (you can see the implementation at r253), but switched to a class as it seemed to be more readable to me.

[lp:~jml/testtools/forward-current-tags]

262. By Jonathan Lange on 2012-04-11

Fix typo. Thanks Gary.

263. By Jonathan Lange on 2012-04-12

Change get_parent() to simple attribute access.

=== added file 'testtools/tags.py'
--- testtools/tags.py	1970-01-01 00:00:00 +0000
+++ testtools/tags.py	2012-04-12 16:29:22 +0000
@@ -0,0 +1,34 @@
+# Copyright (c) 2012 testtools developers. See LICENSE for details.
+
+"""Tag support."""
+
+
+class TagContext(object):
+    """A tag context."""
+
+    def __init__(self, parent=None):
+        """Create a new TagContext.
+
+        :parent: If provided, uses this as the parent context.  Any tags that
+            are current on the parent at the time of construction are current
+            in this context.
+        """
+        self.parent = parent
+        self._tags = set()
+        if parent:
+            self._tags.update(parent.get_current_tags())
+
+    def get_current_tags(self):
+        """Return any current tags."""
+        return set(self._tags)
+
+    def change_tags(self, new_tags, gone_tags):
+        """Change the tags on this context.
+
+        :param new_tags: A set of tags to add to this context.
+        :param gone_tags: A set of tags to remove from this context.
+        :return: The tags now current on this context.
+        """
+        self._tags.update(new_tags)
+        self._tags.difference_update(gone_tags)
+        return self.get_current_tags()

=== modified file 'testtools/testresult/doubles.py'
--- testtools/testresult/doubles.py	2012-01-10 16:16:45 +0000
+++ testtools/testresult/doubles.py	2012-04-12 16:29:22 +0000
@@ -9,6 +9,9 @@
     ]
 
 
+from testtools.tags import TagContext
+
+
 class LoggingBase(object):
     """Basic support for logging of results."""
 
@@ -16,7 +19,6 @@
         self._events = []
         self.shouldStop = False
         self._was_successful = True
-        self.current_tags = set()
 
 
 class Python26TestResult(LoggingBase):
@@ -45,10 +47,6 @@
     def wasSuccessful(self):
         return self._was_successful
 
-    def tags(self, new_tags, gone_tags):
-        self.current_tags.update(new_tags)
-        self.current_tags.difference_update(gone_tags)
-
 
 class Python27TestResult(Python26TestResult):
     """A precisely python 2.7 like test result, that logs."""
@@ -72,6 +70,10 @@
 class ExtendedTestResult(Python27TestResult):
     """A test result like the proposed extended unittest result API."""
 
+    def __init__(self):
+        super(ExtendedTestResult, self).__init__()
+        self._tags = TagContext()
+
     def addError(self, test, err=None, details=None):
         self._was_successful = False
         self._events.append(('addError', test, err or details))
@@ -105,9 +107,22 @@
     def startTestRun(self):
         super(ExtendedTestResult, self).startTestRun()
         self._was_successful = True
+        self._tags = TagContext()
+
+    def startTest(self, test):
+        super(ExtendedTestResult, self).startTest(test)
+        self._tags = TagContext(self._tags)
+
+    def stopTest(self, test):
+        self._tags = self._tags.parent
+        super(ExtendedTestResult, self).stopTest(test)
+
+    @property
+    def current_tags(self):
+        return self._tags.get_current_tags()
 
     def tags(self, new_tags, gone_tags):
-        super(ExtendedTestResult, self).tags(new_tags, gone_tags)
+        self._tags.change_tags(new_tags, gone_tags)
         self._events.append(('tags', new_tags, gone_tags))
 
     def time(self, time):

=== modified file 'testtools/testresult/real.py'
--- testtools/testresult/real.py	2012-02-09 17:52:15 +0000
+++ testtools/testresult/real.py	2012-04-12 16:29:22 +0000
@@ -16,6 +16,7 @@
 
 from testtools.compat import all, str_is_unicode, _u
 from testtools.content import TracebackContent
+from testtools.tags import TagContext
 
 # From http://docs.python.org/library/datetime.html
 _ZERO = datetime.timedelta(0)
@@ -171,10 +172,10 @@
         super(TestResult, self).__init__()
         self.skip_reasons = {}
         self.__now = None
+        self._tags = TagContext()
         # -- Start: As per python 2.7 --
         self.expectedFailures = []
         self.unexpectedSuccesses = []
-        self.current_tags = set()
         # -- End:   As per python 2.7 --
 
     def stopTestRun(self):
@@ -183,6 +184,27 @@
         New in python 2.7
         """
 
+    def startTest(self, test):
+        super(TestResult, self).startTest(test)
+        self._tags = TagContext(self._tags)
+
+    def stopTest(self, test):
+        self._tags = self._tags.parent
+        super(TestResult, self).stopTest(test)
+
+    @property
+    def current_tags(self):
+        """The currently set tags."""
+        return self._tags.get_current_tags()
+
+    def tags(self, new_tags, gone_tags):
+        """Add and remove tags from the test.
+
+        :param new_tags: A set of tags to be added to the stream.
+        :param gone_tags: A set of tags to be removed from the stream.
+        """
+        self._tags.change_tags(new_tags, gone_tags)
+
     def time(self, a_datetime):
         """Provide a timestamp to represent the current time.
 
@@ -204,21 +226,12 @@
         deprecated in favour of stopTestRun.
         """
 
-    def tags(self, new_tags, gone_tags):
-        """Add and remove tags from the test.
-
-        :param new_tags: A set of tags to be added to the stream.
-        :param gone_tags: A set of tags to be removed from the stream.
-        """
-        self.current_tags.update(new_tags)
-        self.current_tags.difference_update(gone_tags)
-
 
 class MultiTestResult(TestResult):
     """A test result that dispatches to many test results."""
 
     def __init__(self, *results):
-        TestResult.__init__(self)
+        super(MultiTestResult, self).__init__()
         self._results = list(map(ExtendedToOriginalDecorator, results))
 
     def __repr__(self):
@@ -231,9 +244,11 @@
             for result in self._results)
 
     def startTest(self, test):
+        super(MultiTestResult, self).startTest(test)
         return self._dispatch('startTest', test)
 
     def stopTest(self, test):
+        super(MultiTestResult, self).stopTest(test)
         return self._dispatch('stopTest', test)
 
     def addError(self, test, error=None, details=None):
@@ -256,12 +271,14 @@
         return self._dispatch('addUnexpectedSuccess', test, details=details)
 
     def startTestRun(self):
+        super(MultiTestResult, self).startTestRun()
         return self._dispatch('startTestRun')
 
     def stopTestRun(self):
         return self._dispatch('stopTestRun')
 
     def tags(self, new_tags, gone_tags):
+        super(MultiTestResult, self).tags(new_tags, gone_tags)
         return self._dispatch('tags', new_tags, gone_tags)
 
     def time(self, a_datetime):
@@ -417,6 +434,7 @@
             test, details=details)
 
     def startTestRun(self):
+        super(ThreadsafeForwardingResult, self).startTestRun()
         self.semaphore.acquire()
         try:
             self.result.startTestRun()
@@ -446,6 +464,7 @@
 
     def tags(self, new_tags, gone_tags):
         """See `TestResult`."""
+        super(ThreadsafeForwardingResult, self).tags(new_tags, gone_tags)
         if self._test_start is not None:
             self._test_tags = self._merge_tags(
                 self._test_tags, new_tags, gone_tags)
@@ -475,6 +494,7 @@
 
     def __init__(self, decorated):
         self.decorated = decorated
+        self._tags = TagContext()
 
     def __repr__(self):
         return '<%s %r>' % (self.__class__.__name__, self.decorated)
@@ -571,6 +591,11 @@
             _StringException(_details_to_str(details, special='traceback')),
             None)
 
+    @property
+    def current_tags(self):
+        return getattr(
+            self.decorated, 'current_tags', self._tags.get_current_tags())
+
     def done(self):
         try:
             return self.decorated.done()
@@ -588,9 +613,11 @@
         return self.decorated.shouldStop
 
     def startTest(self, test):
+        self._tags = TagContext(self._tags)
         return self.decorated.startTest(test)
 
     def startTestRun(self):
+        self._tags = TagContext()
         try:
             return self.decorated.startTestRun()
         except AttributeError:
@@ -600,6 +627,7 @@
         return self.decorated.stop()
 
     def stopTest(self, test):
+        self._tags = self._tags.parent
         return self.decorated.stopTest(test)
 
     def stopTestRun(self):
@@ -610,9 +638,10 @@
 
     def tags(self, new_tags, gone_tags):
         method = getattr(self.decorated, 'tags', None)
-        if method is None:
-            return
-        return method(new_tags, gone_tags)
+        if method is not None:
+            return method(new_tags, gone_tags)
+        else:
+            self._tags.change_tags(new_tags, gone_tags)
 
     def time(self, a_datetime):
         method = getattr(self.decorated, 'time', None)

=== modified file 'testtools/tests/__init__.py'
--- testtools/tests/__init__.py	2011-08-15 13:48:10 +0000
+++ testtools/tests/__init__.py	2012-04-12 16:29:22 +0000
@@ -19,6 +19,7 @@
         test_run,
         test_runtest,
         test_spinner,
+        test_tags,
         test_testcase,
         test_testresult,
         test_testsuite,
@@ -36,6 +37,7 @@
         test_run,
         test_runtest,
         test_spinner,
+        test_tags,
         test_testcase,
         test_testresult,
         test_testsuite,

=== added file 'testtools/tests/test_tags.py'
--- testtools/tests/test_tags.py	1970-01-01 00:00:00 +0000
+++ testtools/tests/test_tags.py	2012-04-12 16:29:22 +0000
@@ -0,0 +1,84 @@
+# Copyright (c) 2012 testtools developers. See LICENSE for details.
+
+"""Test tag support."""
+
+
+from testtools import TestCase
+from testtools.tags import TagContext
+
+
+class TestTags(TestCase):
+
+    def test_no_tags(self):
+        # A tag context has no tags initially.
+        tag_context = TagContext()
+        self.assertEqual(set(), tag_context.get_current_tags())
+
+    def test_add_tag(self):
+        # A tag added with change_tags appears in get_current_tags.
+        tag_context = TagContext()
+        tag_context.change_tags(set(['foo']), set())
+        self.assertEqual(set(['foo']), tag_context.get_current_tags())
+
+    def test_add_tag_twice(self):
+        # Calling change_tags twice to add tags adds both tags to the current
+        # tags.
+        tag_context = TagContext()
+        tag_context.change_tags(set(['foo']), set())
+        tag_context.change_tags(set(['bar']), set())
+        self.assertEqual(
+            set(['foo', 'bar']), tag_context.get_current_tags())
+
+    def test_change_tags_returns_tags(self):
+        # change_tags returns the current tags.  This is a convenience.
+        tag_context = TagContext()
+        tags = tag_context.change_tags(set(['foo']), set())
+        self.assertEqual(set(['foo']), tags)
+
+    def test_remove_tag(self):
+        # change_tags can remove tags from the context.
+        tag_context = TagContext()
+        tag_context.change_tags(set(['foo']), set())
+        tag_context.change_tags(set(), set(['foo']))
+        self.assertEqual(set(), tag_context.get_current_tags())
+
+    def test_child_context(self):
+        # A TagContext can have a parent.  If so, its tags are the tags of the
+        # parent at the moment of construction.
+        parent = TagContext()
+        parent.change_tags(set(['foo']), set())
+        child = TagContext(parent)
+        self.assertEqual(
+            parent.get_current_tags(), child.get_current_tags())
+
+    def test_add_to_child(self):
+        # Adding a tag to the child context doesn't affect the parent.
+        parent = TagContext()
+        parent.change_tags(set(['foo']), set())
+        child = TagContext(parent)
+        child.change_tags(set(['bar']), set())
+        self.assertEqual(set(['foo', 'bar']), child.get_current_tags())
+        self.assertEqual(set(['foo']), parent.get_current_tags())
+
+    def test_remove_in_child(self):
+        # A tag that was in the parent context can be removed from the child
+        # context without affect the parent.
+        parent = TagContext()
+        parent.change_tags(set(['foo']), set())
+        child = TagContext(parent)
+        child.change_tags(set(), set(['foo']))
+        self.assertEqual(set(), child.get_current_tags())
+        self.assertEqual(set(['foo']), parent.get_current_tags())
+
+    def test_parent(self):
+        # The parent can be retrieved from a child context.
+        parent = TagContext()
+        parent.change_tags(set(['foo']), set())
+        child = TagContext(parent)
+        child.change_tags(set(), set(['foo']))
+        self.assertEqual(parent, child.parent)
+
+
+def test_suite():
+    from unittest import TestLoader
+    return TestLoader().loadTestsFromName(__name__)

=== modified file 'testtools/tests/test_testresult.py'
--- testtools/tests/test_testresult.py	2012-02-09 17:52:15 +0000
+++ testtools/tests/test_testresult.py	2012-04-12 16:29:22 +0000
@@ -134,12 +134,6 @@
         result.stopTest(self)
         self.assertTrue(result.wasSuccessful())
 
-    def test_tags(self):
-        # tags() does not fail the test run.
-        result = self.makeResult()
-        result.startTest(self)
-        result.tags(set([]), set([]))
-
 
 class Python27Contract(Python26Contract):
 
@@ -192,7 +186,81 @@
         result.stopTestRun()
 
 
-class DetailsContract(Python27Contract):
+class TagsContract(Python27Contract):
+    """Tests to ensure correct tagging behaviour.
+
+    See the subunit docs for guidelines on how this is supposed to work.
+    """
+
+    def test_no_tags_by_default(self):
+        # Results initially have no tags.
+        result = self.makeResult()
+        self.assertEqual(frozenset(), result.current_tags)
+
+    def test_adding_tags(self):
+        # Tags are added using 'tags' and thus become visible in
+        # 'current_tags'.
+        result = self.makeResult()
+        result.tags(set(['foo']), set())
+        self.assertEqual(set(['foo']), result.current_tags)
+
+    def test_removing_tags(self):
+        # Tags are removed using 'tags'.
+        result = self.makeResult()
+        result.tags(set(['foo']), set())
+        result.tags(set(), set(['foo']))
+        self.assertEqual(set(), result.current_tags)
+
+    def test_startTestRun_resets_tags(self):
+        # startTestRun makes a new test run, and thus clears all the tags.
+        result = self.makeResult()
+        result.tags(set(['foo']), set())
+        result.startTestRun()
+        self.assertEqual(set(), result.current_tags)
+
+    def test_add_tags_within_test(self):
+        # Tags can be added after a test has run.
+        result = self.makeResult()
+        result.startTestRun()
+        result.tags(set(['foo']), set())
+        result.startTest(self)
+        result.tags(set(['bar']), set())
+        self.assertEqual(set(['foo', 'bar']), result.current_tags)
+
+    def test_tags_added_in_test_are_reverted(self):
+        # Tags added during a test run are then reverted once that test has
+        # finished.
+        result = self.makeResult()
+        result.startTestRun()
+        result.tags(set(['foo']), set())
+        result.startTest(self)
+        result.tags(set(['bar']), set())
+        result.addSuccess(self)
+        result.stopTest(self)
+        self.assertEqual(set(['foo']), result.current_tags)
+
+    def test_tags_removed_in_test(self):
+        # Tags can be removed during tests.
+        result = self.makeResult()
+        result.startTestRun()
+        result.tags(set(['foo']), set())
+        result.startTest(self)
+        result.tags(set(), set(['foo']))
+        self.assertEqual(set(), result.current_tags)
+
+    def test_tags_removed_in_test_are_restored(self):
+        # Tags removed during tests are restored once that test has finished.
+        result = self.makeResult()
+        result.startTestRun()
+        result.tags(set(['foo']), set())
+        result.startTest(self)
+        result.tags(set(), set(['foo']))
+        result.addSuccess(self)
+        result.stopTest(self)
+        self.assertEqual(set(['foo']), result.current_tags)
+
+
+class DetailsContract(TagsContract):
     """Tests for the details API of TestResults."""
 
     def test_addExpectedFailure_details(self):