Fix handling of unexpected success results

This commit addresses a bug when unexpected success results are used
by a test suite. stestr was relying on wasSuccessful() method in the
StreamResult class from testtools, which doesn't properly account for
unexpected success. So when stestr was run on a test suite it wouldn't
properly handle tests with unexpected success result and thus treat
the run as successful. This addresses the issue by adjusting our
testtools api usage to manually query the results object for
unxsuccess results instead of relying on the exisiting wasSuccessful()
method from the StreamResult class. This is basically the same as the
wasSuccessful() method from the testtools.TestResult class, which exhibits
the correct behavior, but isn't something we're using.

An issue has been opened in testtools testing-cabal/testtools#273
regarding the accounting error with StreamResult.wasSuccessful(),
if/when a fix is introduced into a released testtools we can investigate
switching these calls back to use that method and deleting our local
helper function. But, for the time being this mitigates the issue.

Fixes #189

stestr/commands/failing.py

@@ -113,7 +113,7 @@ def failing(repo_type='file', repo_url=None, list_tests=False, subunit=False,
         case.run(result)
     finally:
         result.stopTestRun()
-    failed = not summary.wasSuccessful()
+    failed = not results.wasSuccessful(summary)
     if failed:
         result = 1
     else:

stestr/commands/load.py

@@ -219,7 +219,7 @@ def make_tests():
     if pretty_out and not subunit_out:
         subunit_trace.print_fails(stdout)
         subunit_trace.print_summary(stdout, elapsed_time)
-    if not summary_result.wasSuccessful():
+    if not results.wasSuccessful(summary_result):
         return 1
     else:
         return 0

stestr/commands/run.py

@@ -29,6 +29,7 @@
 from stestr import output
 from stestr.repository import abstract as repository
 from stestr.repository import util
+from stestr import results
 from stestr.testlist import parse_list
 from stestr import user_config
 
@@ -346,7 +347,7 @@ def run_tests():
                         stream.run(summary)
                     finally:
                         summary.stopTestRun()
-                    if not summary.wasSuccessful():
+                    if not results.wasSuccessful(summary):
                         result = 1
                 if result:
                     return result
@@ -495,7 +496,7 @@ def run_tests():
                         stream.run(summary)
                     finally:
                         summary.stopTestRun()
-                    if not summary.wasSuccessful():
+                    if not results.wasSuccessful(summary):
                         result = 1
                 if result:
                     return result

stestr/results.py

@@ -17,6 +17,11 @@
 from stestr import output
 
 
+def wasSuccessful(summary):
+    return not (summary.errors or summary.failures or
+                summary.unexpectedSuccesses)
+
+
 class SummarizingResult(testtools.StreamSummary):
 
     def __init__(self):

stestr/subunit_trace.py

@@ -29,6 +29,7 @@
 import testtools
 
 from stestr import colorizer
+from stestr import results
 
 # NOTE(mtreinish) on python3 anydbm was renamed dbm and the python2 dbm module
 # was renamed to dbm.ndbm, this block takes that into account
@@ -388,7 +389,7 @@ def trace(stdin, stdout, print_failures=False, failonly=False,
     if count_tests('status', '^success$') == 0:
         print("\nNo tests were successful during the run")
         return 1
-    return 0 if summary.wasSuccessful() else 1
+    return 0 if results.wasSuccessful(summary) else 1
 
 
 def main():

stestr/tests/files/failing-tests

@@ -21,3 +21,7 @@ class FakeTestClass(testtools.TestCase):
     def test_pass_list(self):
         test_list = ['test', 'a', 'b']
         self.assertIn('fail', test_list)
+
+    def test_unexpected_pass(self):
+        self.expectFailure("we are sad",
+                           self.assertEqual, 1, 1)

stestr/tests/files/passing-tests

@@ -21,3 +21,7 @@ class FakeTestClass(testtools.TestCase):
     def test_pass_list(self):
         test_list = ['test', 'a', 'b']
         self.assertIn('test', test_list)
+
+    def test_xfail(self):
+        self.expectFailure("we are sad",
+                           self.assertEqual, 1, 0)

stestr/tests/test_return_codes.py

@@ -127,6 +127,12 @@ def test_parallel_passing_bad_regex(self):
     def test_parallel_fails(self):
         self.assertRunExit('stestr run', 1)
 
+    def test_parallel_passing_xfail(self):
+        self.assertRunExit('stestr run xfail', 0)
+
+    def test_parallel_fails_unxsuccess(self):
+        self.assertRunExit('stestr run unexpected', 1)
+
     def test_parallel_blacklist(self):
         fd, path = tempfile.mkstemp()
         self.addCleanup(os.remove, path)