Update skip support in python to be in line with testtools.

4 files changed, 93 insertions, 25 deletions

diff --git a/python/subunit/__init__.py b/python/subunit/__init__.py
index 84426ac..9f7dd64 100644
--- a/python/subunit/__init__.py
+++ b/python/subunit/__init__.py
@@ -133,7 +133,7 @@ class TestProtocolServer(object):
             self.current_test_description == line[offset:-1]):
             self.state = TestProtocolServer.OUTSIDE_TEST
             self.current_test_description = None
-            self.client.addSuccess(self._current_test)
+            self._skip_or_error()
             self.client.stopTest(self._current_test)
         elif (self.state == TestProtocolServer.TEST_STARTED and
             self.current_test_description + " [" == line[offset:-1]):
@@ -142,6 +142,16 @@ class TestProtocolServer(object):
         else:
             self.stdOutLineReceived(line)
 
+    def _skip_or_error(self, message=None):
+        """Report the current test as a skip if possible, or else an error."""
+        addSkip = getattr(self.client, 'addSkip', None)
+        if not callable(addSkip):
+            self.client.addError(self._current_test, RemoteError(message))
+        else:
+            if not message:
+                message = "No reason given"
+            addSkip(self._current_test, message)
+
     def _addSuccess(self, offset, line):
         if (self.state == TestProtocolServer.TEST_STARTED and
             self.current_test_description == line[offset:-1]):
@@ -173,8 +183,12 @@ class TestProtocolServer(object):
             self.client.addError(self._current_test,
                                  RemoteError(self._message))
             self.client.stopTest(self._current_test)
+        elif self.state == TestProtocolServer.READING_SKIP:
+            self.state = TestProtocolServer.OUTSIDE_TEST
+            self.current_test_description = None
+            self._skip_or_error(self._message)
+            self.client.stopTest(self._current_test)
         elif self.state in (
-            TestProtocolServer.READING_SKIP,
             TestProtocolServer.READING_SUCCESS,
             TestProtocolServer.READING_XFAIL,
             ):
@@ -314,6 +328,12 @@ class TestProtocolClient(unittest.TestResult):
             self._stream.write("%s\n" % line)
         self._stream.write("]\n")
 
+    def addSkip(self, test, reason):
+        """Report a skipped test."""
+        self._stream.write("skip: %s [\n" % test.id())
+        self._stream.write("%s\n" % reason)
+        self._stream.write("]\n")
+
     def addSuccess(self, test):
         """Report a success in a test."""
         self._stream.write("successful: %s\n" % test.id())
@@ -651,6 +671,7 @@ class TestResultStats(unittest.TestResult):
         unittest.TestResult.__init__(self)
         self._stream = stream
         self.failed_tests = 0
+        self.skipped_tests = 0
         self.tags = set()
 
     @property
@@ -663,16 +684,20 @@ class TestResultStats(unittest.TestResult):
     def addFailure(self, test, err):
         self.failed_tests += 1
 
+    def addSkip(self, test, reason):
+        self.skipped_tests += 1
+
     def formatStats(self):
-        self._stream.write("Total tests:  %5d\n" % self.total_tests)
-        self._stream.write("Passed tests: %5d\n" % self.passed_tests)
-        self._stream.write("Failed tests: %5d\n" % self.failed_tests)
+        self._stream.write("Total tests:   %5d\n" % self.total_tests)
+        self._stream.write("Passed tests:  %5d\n" % self.passed_tests)
+        self._stream.write("Failed tests:  %5d\n" % self.failed_tests)
+        self._stream.write("Skipped tests: %5d\n" % self.skipped_tests)
         tags = sorted(self.tags)
         self._stream.write("Tags: %s\n" % (", ".join(tags)))
 
     @property
     def passed_tests(self):
-        return self.total_tests - self.failed_tests
+        return self.total_tests - self.failed_tests - self.skipped_tests
 
     def stopTest(self, test):
         unittest.TestResult.stopTest(self, test)
@@ -694,18 +719,20 @@ class TestResultFilter(unittest.TestResult):
     """
 
     def __init__(self, result, filter_error=False, filter_failure=False,
-        filter_success=True):
+        filter_success=True, filter_skip=False):
         """Create a FilterResult object filtering to result.
         
         :param filter_error: Filter out errors.
         :param filter_failure: Filter out failures.
         :param filter_success: Filter out successful tests.
+        :param filter_skip: Filter out skipped tests.
         """
         unittest.TestResult.__init__(self)
         self.result = result
         self._filter_error = filter_error
         self._filter_failure = filter_failure
         self._filter_success = filter_success
+        self._filter_skip = filter_skip
         
     def addError(self, test, err):
         if not self._filter_error:
@@ -719,6 +746,18 @@ class TestResultFilter(unittest.TestResult):
             self.result.addFailure(test, err)
             self.result.stopTest(test)
 
+    def addSkip(self, test, reason):
+        if not self._filter_skip:
+            self.result.startTest(test)
+            # This is duplicated, it would be nice to have on a 'calls
+            # TestResults' mixin perhaps.
+            addSkip = getattr(self.result, 'addSkip', None)
+            if not callable(addSkip):
+                self.result.addError(test, RemoteError(reason))
+            else:
+                self.result.addSkip(test, reason)
+            self.result.stopTest(test)
+
     def addSuccess(self, test):
         if not self._filter_success:
             self.result.startTest(test)
diff --git a/python/subunit/tests/test_subunit_filter.py b/python/subunit/tests/test_subunit_filter.py
index 73ec1ed..e9ff4d1 100644
--- a/python/subunit/tests/test_subunit_filter.py
+++ b/python/subunit/tests/test_subunit_filter.py
@@ -36,36 +36,47 @@ class TestTestResultFilter(unittest.TestCase):
         self.filtered_result = unittest.TestResult()
         self.filter = subunit.TestResultFilter(self.filtered_result)
         self.run_tests()
-        self.assertEqual(['error'],
+        # skips are seen as errors by default python TestResult.
+        self.assertEqual(['error', 'skipped'],
             [error[0].id() for error in self.filtered_result.errors])
         self.assertEqual(['failed'],
             [failure[0].id() for failure in
             self.filtered_result.failures])
-        self.assertEqual(2, self.filtered_result.testsRun)
+        self.assertEqual(3, self.filtered_result.testsRun)
 
     def test_exclude_errors(self):
         self.filtered_result = unittest.TestResult()
         self.filter = subunit.TestResultFilter(self.filtered_result,
             filter_error=True)
         self.run_tests()
-        self.assertEqual([],
+        # skips are seen as errors by default python TestResult.
+        self.assertEqual(['skipped'],
             [error[0].id() for error in self.filtered_result.errors])
         self.assertEqual(['failed'],
             [failure[0].id() for failure in
             self.filtered_result.failures])
-        self.assertEqual(1, self.filtered_result.testsRun)
+        self.assertEqual(2, self.filtered_result.testsRun)
 
     def test_exclude_failure(self):
         self.filtered_result = unittest.TestResult()
         self.filter = subunit.TestResultFilter(self.filtered_result,
             filter_failure=True)
         self.run_tests()
-        self.assertEqual(['error'],
+        self.assertEqual(['error', 'skipped'],
             [error[0].id() for error in self.filtered_result.errors])
         self.assertEqual([],
             [failure[0].id() for failure in
             self.filtered_result.failures])
-        self.assertEqual(1, self.filtered_result.testsRun)
+        self.assertEqual(2, self.filtered_result.testsRun)
+
+    def test_exclude_skips(self):
+        self.filtered_result = subunit.TestResultStats(None)
+        self.filter = subunit.TestResultFilter(self.filtered_result,
+            filter_skip=True)
+        self.run_tests()
+        self.assertEqual(0, self.filtered_result.skipped_tests)
+        self.assertEqual(2, self.filtered_result.failed_tests)
+        self.assertEqual(2, self.filtered_result.testsRun)
 
     def test_include_success(self):
         """Success's can be included if requested."""
@@ -73,14 +84,13 @@ class TestTestResultFilter(unittest.TestCase):
         self.filter = subunit.TestResultFilter(self.filtered_result,
             filter_success=False)
         self.run_tests()
-        self.assertEqual(['error'],
+        self.assertEqual(['error', 'skipped'],
             [error[0].id() for error in self.filtered_result.errors])
         self.assertEqual(['failed'],
             [failure[0].id() for failure in
             self.filtered_result.failures])
         self.assertEqual(5, self.filtered_result.testsRun)
 
-
     def run_tests(self):
         self.setUpTestStream()
         self.test = subunit.ProtocolTestCase(self.input_stream)
diff --git a/python/subunit/tests/test_subunit_stats.py b/python/subunit/tests/test_subunit_stats.py
index b789089..e2f30fc 100644
--- a/python/subunit/tests/test_subunit_stats.py
+++ b/python/subunit/tests/test_subunit_stats.py
@@ -62,15 +62,17 @@ xfail todo
         # Statistics are calculated usefully.
         self.setUpUsedStream()
         self.assertEqual(5, self.result.total_tests)
-        self.assertEqual(3, self.result.passed_tests)
+        self.assertEqual(2, self.result.passed_tests)
         self.assertEqual(2, self.result.failed_tests)
+        self.assertEqual(1, self.result.skipped_tests)
         self.assertEqual(set(["global", "local"]), self.result.tags)
 
     def test_stat_formatting(self):
         expected = ("""
-Total tests:      5
-Passed tests:     3
-Failed tests:     2
+Total tests:       5
+Passed tests:      2
+Failed tests:      2
+Skipped tests:     1
 Tags: global, local
 """)[1:]
         self.setUpUsedStream()
diff --git a/python/subunit/tests/test_test_protocol.py b/python/subunit/tests/test_test_protocol.py
index 73f0821..80f91e1 100644
--- a/python/subunit/tests/test_test_protocol.py
+++ b/python/subunit/tests/test_test_protocol.py
@@ -32,6 +32,7 @@ try:
             self.end_calls = []
             self.error_calls = []
             self.failure_calls = []
+            self.skip_calls = []
             self.start_calls = []
             self.success_calls = []
             super(MockTestProtocolServerClient, self).__init__()
@@ -42,6 +43,9 @@ try:
         def addFailure(self, test, error):
             self.failure_calls.append((test, error))
 
+        def addSkip(self, test, reason):
+            self.skip_calls.append((test, reason))
+
         def addSuccess(self, test):
             self.success_calls.append(test)
 
@@ -589,8 +593,8 @@ class TestTestProtocolServerAddxFail(unittest.TestCase):
 class TestTestProtocolServerAddSkip(unittest.TestCase):
     """Tests for the skip keyword.
 
-    In Python this thunks through to Success due to stdlib limitations. (See
-    README).
+    In python this meets the testtools extended TestResult contract.
+    (See https://launchpad.net/testtools).
     """
 
     def setUp(self):
@@ -606,7 +610,9 @@ class TestTestProtocolServerAddSkip(unittest.TestCase):
         self.assertEqual(self.client.end_calls, [self.test])
         self.assertEqual(self.client.error_calls, [])
         self.assertEqual(self.client.failure_calls, [])
-        self.assertEqual(self.client.success_calls, [self.test])
+        self.assertEqual(self.client.success_calls, [])
+        self.assertEqual(self.client.skip_calls,
+            [(self.test, 'No reason given')])
 
     def test_simple_skip(self):
         self.simple_skip_keyword("skip")
@@ -621,7 +627,9 @@ class TestTestProtocolServerAddSkip(unittest.TestCase):
         self.assertEqual(self.client.end_calls, [self.test])
         self.assertEqual(self.client.error_calls, [])
         self.assertEqual(self.client.failure_calls, [])
-        self.assertEqual(self.client.success_calls, [self.test])
+        self.assertEqual(self.client.success_calls, [])
+        self.assertEqual(self.client.skip_calls,
+            [(self.test, "No reason given")])
 
     def skip_quoted_bracket(self, keyword):
         # This tests it is accepted, but cannot test it is used today, because
@@ -633,7 +641,9 @@ class TestTestProtocolServerAddSkip(unittest.TestCase):
         self.assertEqual(self.client.end_calls, [self.test])
         self.assertEqual(self.client.error_calls, [])
         self.assertEqual(self.client.failure_calls, [])
-        self.assertEqual(self.client.success_calls, [self.test])
+        self.assertEqual(self.client.success_calls, [])
+        self.assertEqual(self.client.skip_calls,
+            [(self.test, "]\n")])
 
     def test_skip_quoted_bracket(self):
         self.skip_quoted_bracket("skip")
@@ -933,7 +943,6 @@ class TestTestProtocolClient(unittest.TestCase):
         self.protocol = subunit.TestProtocolClient(self.io)
         self.test = TestTestProtocolClient("test_start_test")
 
-
     def test_start_test(self):
         """Test startTest on a TestProtocolClient."""
         self.protocol.startTest(self.test)
@@ -968,6 +977,14 @@ class TestTestProtocolClient(unittest.TestCase):
             "RemoteException: phwoar crikey\n"
             "]\n" % self.test.id())
 
+    def test_add_skip(self):
+        """Test addSkip on a TestProtocolClient."""
+        self.protocol.addSkip(
+            self.test, "Has it really?")
+        self.assertEqual(
+            self.io.getvalue(),
+            'skip: %s [\nHas it really?\n]\n' % self.test.id())
+
 
 def test_suite():
     loader = subunit.tests.TestUtil.TestLoader()