Merge lp:~lifeless/testtools/bug-1090582 into lp:~testtools-committers/testtools/trunk

[Robert Collins]

Our CLI help says we support -f / --failfast. We don't. This makes us.

[Jonathan Lange (jml)]

Thanks for the patch. All looks pretty straight-forward & well-tested.

review: Approve

[Jonathan Lange (jml)]

Oops. Tried to merge this, ran the tests and got errors.

=====================================================================
ERROR: testtools.tests.test_distutilscmd.TestCommandTest.test_test_module
----------------------------------------------------------------------
Traceback (most recent call last):
  File "testtools/tests/test_distutilscmd.py", line 55, in test_test_module
    runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
AttributeError: 'module' object has no attribute 'DetailStream'
======================================================================
ERROR: testtools.tests.test_distutilscmd.TestCommandTest.test_test_suite
----------------------------------------------------------------------
Traceback (most recent call last):
  File "testtools/tests/test_distutilscmd.py", line 75, in test_test_suite
    runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
AttributeError: 'module' object has no attribute 'DetailStream'
======================================================================
ERROR: testtools.tests.test_run.TestRun.test_run_failfast
----------------------------------------------------------------------
Traceback (most recent call last):
  File "testtools/tests/test_run.py", line 82, in test_run_failfast
    runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
AttributeError: 'module' object has no attribute 'DetailStream'

Appears this branch refers to ``fixtures.DetailStream``, but that doesn't seem to be available in any version of python-fixtures, including trunk.

review: Needs Fixing

[Robert Collins (lifeless)]

Its in fixtures on pypi.

[lp:~lifeless/testtools/bug-1090582]

296. By Robert Collins on 2012-12-16

Adjust trunk for TestToolsTestRunner change.

=== modified file 'NEWS'
--- NEWS	2012-12-15 14:11:14 +0000
+++ NEWS	2012-12-16 12:29:26 +0000
@@ -6,9 +6,20 @@
 NEXT
 ~~~~
 
+Changes
+-------
+
+* ``run.TestToolsTestRunner`` now accepts the verbosity, buffer and failfast
+  arguments the upstream python TestProgram code wants to give it, making it
+  possible to support them in a compatible fashion. (Robert Collins)
+
 Improvements
 ------------
 
+* ``testtools.run`` now supports the ``-f`` or ``--failfast`` parameter.
+  Previously it was advertised in the help but ignored.
+  (Robert Collins, #1090582)
+
 * ``AnyMatch`` added, a new matcher that matches when any item in a collection
   matches the given matcher.  (Jonathan Lange)
 

=== modified file 'testtools/run.py'
--- testtools/run.py	2012-11-29 11:40:17 +0000
+++ testtools/run.py	2012-12-16 12:29:26 +0000
@@ -35,12 +35,19 @@
 class TestToolsTestRunner(object):
     """ A thunk object to support unittest.TestProgram."""
 
-    def __init__(self, stdout):
-        self.stdout = stdout
+    def __init__(self, verbosity=None, failfast=None, buffer=None):
+        """Create a TestToolsTestRunner.
+
+        :param verbosity: Ignored.
+        :param failfast: Stop running tests at the first failure.
+        :param buffer: Ignored.
+        """
+        self.failfast = failfast
 
     def run(self, test):
         "Run the given test case or test suite."
-        result = TextTestResult(unicode_output_stream(self.stdout))
+        result = TextTestResult(
+            unicode_output_stream(sys.stdout), failfast=self.failfast)
         result.startTestRun()
         try:
             return test.run(result)
@@ -307,7 +314,7 @@
             and getattr(unittest, 'installHandler', None) is not None):
             unittest.installHandler()
         if self.testRunner is None:
-            self.testRunner = TestToolsTestRunner(sys.stdout)
+            self.testRunner = TestToolsTestRunner
         if isinstance(self.testRunner, classtypes()):
             try:
                 testRunner = self.testRunner(verbosity=self.verbosity,
@@ -325,8 +332,8 @@
 ################
 
 def main(argv, stdout):
-    runner = TestToolsTestRunner(stdout)
-    program = TestProgram(argv=argv, testRunner=runner, stdout=stdout)
+    program = TestProgram(argv=argv, testRunner=TestToolsTestRunner,
+        stdout=stdout)
 
 if __name__ == '__main__':
     main(sys.argv, sys.stdout)

=== modified file 'testtools/testresult/doubles.py'
--- testtools/testresult/doubles.py	2012-04-12 16:26:02 +0000
+++ testtools/testresult/doubles.py	2012-12-16 12:29:26 +0000
@@ -19,6 +19,7 @@
         self._events = []
         self.shouldStop = False
         self._was_successful = True
+        self.testsRun = 0
 
 
 class Python26TestResult(LoggingBase):
@@ -37,6 +38,7 @@
 
     def startTest(self, test):
         self._events.append(('startTest', test))
+        self.testsRun += 1
 
     def stop(self):
         self.shouldStop = True
@@ -51,6 +53,20 @@
 class Python27TestResult(Python26TestResult):
     """A precisely python 2.7 like test result, that logs."""
 
+    def __init__(self):
+        super(Python27TestResult, self).__init__()
+        self.failfast = False
+
+    def addError(self, test, err):
+        super(Python27TestResult, self).addError(test, err)
+        if self.failfast:
+            self.stop()
+
+    def addFailure(self, test, err):
+        super(Python27TestResult, self).addFailure(test, err)
+        if self.failfast:
+            self.stop()
+
     def addExpectedFailure(self, test, err):
         self._events.append(('addExpectedFailure', test, err))
 
@@ -59,6 +75,8 @@
 
     def addUnexpectedSuccess(self, test):
         self._events.append(('addUnexpectedSuccess', test))
+        if self.failfast:
+            self.stop()
 
     def startTestRun(self):
         self._events.append(('startTestRun',))

=== modified file 'testtools/testresult/real.py'
--- testtools/testresult/real.py	2012-09-21 15:15:24 +0000
+++ testtools/testresult/real.py	2012-12-16 12:29:26 +0000
@@ -21,6 +21,7 @@
     text_content,
     TracebackContent,
     )
+from testtools.helpers import safe_hasattr
 from testtools.tags import TagContext
 
 # From http://docs.python.org/library/datetime.html
@@ -60,11 +61,12 @@
     :ivar skip_reasons: A dict of skip-reasons -> list of tests. See addSkip.
     """
 
-    def __init__(self):
+    def __init__(self, failfast=False):
         # startTestRun resets all attributes, and older clients don't know to
         # call startTestRun, so it is called once here.
         # Because subclasses may reasonably not expect this, we call the
         # specific version we want to run.
+        self.failfast = failfast
         TestResult.startTestRun(self)
 
     def addExpectedFailure(self, test, err=None, details=None):
@@ -89,6 +91,8 @@
         """
         self.errors.append((test,
             self._err_details_to_string(test, err, details)))
+        if self.failfast:
+            self.stop()
 
     def addFailure(self, test, err=None, details=None):
         """Called when an error has occurred. 'err' is a tuple of values as
@@ -99,6 +103,8 @@
         """
         self.failures.append((test,
             self._err_details_to_string(test, err, details)))
+        if self.failfast:
+            self.stop()
 
     def addSkip(self, test, reason=None, details=None):
         """Called when a test has been skipped rather than running.
@@ -131,6 +137,8 @@
     def addUnexpectedSuccess(self, test, details=None):
         """Called when a test was expected to fail, but succeed."""
         self.unexpectedSuccesses.append(test)
+        if self.failfast:
+            self.stop()
 
     def wasSuccessful(self):
         """Has this result been successful so far?
@@ -174,6 +182,8 @@
         pristine condition ready for use in another test run.  Note that this
         is different from Python 2.7's startTestRun, which does nothing.
         """
+        # failfast is reset by the super __init__, so stash it.
+        failfast = self.failfast
         super(TestResult, self).__init__()
         self.skip_reasons = {}
         self.__now = None
@@ -181,6 +191,7 @@
         # -- Start: As per python 2.7 --
         self.expectedFailures = []
         self.unexpectedSuccesses = []
+        self.failfast = failfast
         # -- End:   As per python 2.7 --
 
     def stopTestRun(self):
@@ -236,8 +247,9 @@
     """A test result that dispatches to many test results."""
 
     def __init__(self, *results):
+        # Setup _results first, as the base class __init__ assigns to failfast.
+        self._results = list(map(ExtendedToOriginalDecorator, results))
         super(MultiTestResult, self).__init__()
-        self._results = list(map(ExtendedToOriginalDecorator, results))
 
     def __repr__(self):
         return '<%s (%s)>' % (
@@ -248,10 +260,26 @@
             getattr(result, message)(*args, **kwargs)
             for result in self._results)
 
+    def _get_failfast(self):
+        return getattr(self._results[0], 'failfast', False)
+    def _set_failfast(self, value):
+        self._dispatch('__setattr__', 'failfast', value)
+    failfast = property(_get_failfast, _set_failfast)
+
+    def _get_shouldStop(self):
+        return any(self._dispatch('__getattr__', 'shouldStop'))
+    def _set_shouldStop(self, value):
+        # Called because we subclass TestResult. Probably should not do that.
+        pass
+    shouldStop = property(_get_shouldStop, _set_shouldStop)
+
     def startTest(self, test):
         super(MultiTestResult, self).startTest(test)
         return self._dispatch('startTest', test)
 
+    def stop(self):
+        return self._dispatch('stop')
+
     def stopTest(self, test):
         super(MultiTestResult, self).stopTest(test)
         return self._dispatch('stopTest', test)
@@ -303,9 +331,9 @@
 class TextTestResult(TestResult):
     """A TestResult which outputs activity to a text stream."""
 
-    def __init__(self, stream):
+    def __init__(self, stream, failfast=False):
         """Construct a TextTestResult writing to stream."""
-        super(TextTestResult, self).__init__()
+        super(TextTestResult, self).__init__(failfast=failfast)
         self.stream = stream
         self.sep1 = '=' * 70 + '\n'
         self.sep2 = '-' * 70 + '\n'
@@ -451,6 +479,24 @@
         finally:
             self.semaphore.release()
 
+    def _get_shouldStop(self):
+        self.semaphore.acquire()
+        try:
+            return self.result.shouldStop
+        finally:
+            self.semaphore.release()
+    def _set_shouldStop(self, value):
+        # Another case where we should not subclass TestResult
+        pass
+    shouldStop = property(_get_shouldStop, _set_shouldStop)
+
+    def stop(self):
+        self.semaphore.acquire()
+        try:
+            self.result.stop()
+        finally:
+            self.semaphore.release()
+
     def stopTestRun(self):
         self.semaphore.acquire()
         try:
@@ -507,6 +553,8 @@
     def __init__(self, decorated):
         self.decorated = decorated
         self._tags = TagContext()
+        # Only used for old TestResults that do not have failfast.
+        self._failfast = False
 
     def __repr__(self):
         return '<%s %r>' % (self.__class__.__name__, self.decorated)
@@ -515,14 +563,18 @@
         return getattr(self.decorated, name)
 
     def addError(self, test, err=None, details=None):
-        self._check_args(err, details)
-        if details is not None:
-            try:
-                return self.decorated.addError(test, details=details)
-            except TypeError:
-                # have to convert
-                err = self._details_to_exc_info(details)
-        return self.decorated.addError(test, err)
+        try:
+            self._check_args(err, details)
+            if details is not None:
+                try:
+                    return self.decorated.addError(test, details=details)
+                except TypeError:
+                    # have to convert
+                    err = self._details_to_exc_info(details)
+            return self.decorated.addError(test, err)
+        finally:
+            if self.failfast:
+                self.stop()
 
     def addExpectedFailure(self, test, err=None, details=None):
         self._check_args(err, details)
@@ -539,14 +591,18 @@
         return addExpectedFailure(test, err)
 
     def addFailure(self, test, err=None, details=None):
-        self._check_args(err, details)
-        if details is not None:
-            try:
-                return self.decorated.addFailure(test, details=details)
-            except TypeError:
-                # have to convert
-                err = self._details_to_exc_info(details)
-        return self.decorated.addFailure(test, err)
+        try:
+            self._check_args(err, details)
+            if details is not None:
+                try:
+                    return self.decorated.addFailure(test, details=details)
+                except TypeError:
+                    # have to convert
+                    err = self._details_to_exc_info(details)
+            return self.decorated.addFailure(test, err)
+        finally:
+            if self.failfast:
+                self.stop()
 
     def addSkip(self, test, reason=None, details=None):
         self._check_args(reason, details)
@@ -565,18 +621,22 @@
         return addSkip(test, reason)
 
     def addUnexpectedSuccess(self, test, details=None):
-        outcome = getattr(self.decorated, 'addUnexpectedSuccess', None)
-        if outcome is None:
-            try:
-                test.fail("")
-            except test.failureException:
-                return self.addFailure(test, sys.exc_info())
-        if details is not None:
-            try:
-                return outcome(test, details=details)
-            except TypeError:
-                pass
-        return outcome(test)
+        try:
+            outcome = getattr(self.decorated, 'addUnexpectedSuccess', None)
+            if outcome is None:
+                try:
+                    test.fail("")
+                except test.failureException:
+                    return self.addFailure(test, sys.exc_info())
+            if details is not None:
+                try:
+                    return outcome(test, details=details)
+                except TypeError:
+                    pass
+            return outcome(test)
+        finally:
+            if self.failfast:
+                self.stop()
 
     def addSuccess(self, test, details=None):
         if details is not None:
@@ -614,6 +674,15 @@
         except AttributeError:
             return
 
+    def _get_failfast(self):
+        return getattr(self.decorated, 'failfast', self._failfast)
+    def _set_failfast(self, value):
+        if safe_hasattr(self.decorated, 'failfast'):
+            self.decorated.failfast = value
+        else:
+            self._failfast = value
+    failfast = property(_get_failfast, _set_failfast)
+
     def progress(self, offset, whence):
         method = getattr(self.decorated, 'progress', None)
         if method is None:

=== modified file 'testtools/tests/helpers.py'
--- testtools/tests/helpers.py	2012-02-04 16:47:09 +0000
+++ testtools/tests/helpers.py	2012-12-16 12:29:26 +0000
@@ -38,6 +38,10 @@
         self._events.append(('startTest', test))
         super(LoggingResult, self).startTest(test)
 
+    def stop(self):
+        self._events.append('stop')
+        super(LoggingResult, self).stop()
+
     def stopTest(self, test):
         self._events.append(('stopTest', test))
         super(LoggingResult, self).stopTest(test)

=== modified file 'testtools/tests/test_distutilscmd.py'
--- testtools/tests/test_distutilscmd.py	2011-07-26 23:56:06 +0000
+++ testtools/tests/test_distutilscmd.py	2012-12-16 12:29:26 +0000
@@ -52,7 +52,7 @@
 
     def test_test_module(self):
         self.useFixture(SampleTestFixture())
-        stream = BytesIO()
+        runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
         dist = Distribution()
         dist.script_name = 'setup.py'
         dist.script_args = ['test']
@@ -60,10 +60,10 @@
         dist.command_options = {
             'test': {'test_module': ('command line', 'testtools.runexample')}}
         cmd = dist.reinitialize_command('test')
-        cmd.runner.stdout = stream
-        dist.run_command('test')
+        with fixtures.MonkeyPatch('sys.stdout', runner_stdout):
+            dist.run_command('test')
         self.assertThat(
-            stream.getvalue(),
+            runner_stdout.getvalue(),
             MatchesRegex(_b("""Tests running...
 
 Ran 2 tests in \\d.\\d\\d\\ds
@@ -72,7 +72,7 @@
 
     def test_test_suite(self):
         self.useFixture(SampleTestFixture())
-        stream = BytesIO()
+        runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
         dist = Distribution()
         dist.script_name = 'setup.py'
         dist.script_args = ['test']
@@ -82,10 +82,10 @@
                 'test_suite': (
                     'command line', 'testtools.runexample.test_suite')}}
         cmd = dist.reinitialize_command('test')
-        cmd.runner.stdout = stream
-        dist.run_command('test')
+        with fixtures.MonkeyPatch('sys.stdout', runner_stdout):
+            dist.run_command('test')
         self.assertThat(
-            stream.getvalue(),
+            runner_stdout.getvalue(),
             MatchesRegex(_b("""Tests running...
 
 Ran 2 tests in \\d.\\d\\d\\ds

=== modified file 'testtools/tests/test_run.py'
--- testtools/tests/test_run.py	2011-07-26 23:27:18 +0000
+++ testtools/tests/test_run.py	2012-12-16 12:29:26 +0000
@@ -2,6 +2,8 @@
 
 """Tests for the test runner logic."""
 
+from unittest import TestSuite
+
 from testtools.compat import (
     _b,
     StringIO,
@@ -11,6 +13,7 @@
 
 import testtools
 from testtools import TestCase, run
+from testtools.matchers import Contains
 
 
 if fixtures:
@@ -41,9 +44,12 @@
 
 class TestRun(TestCase):
 
+    def setUp(self):
+        super(TestRun, self).setUp()
+        if fixtures is None:
+            self.skipTest("Need fixtures")
+
     def test_run_list(self):
-        if fixtures is None:
-            self.skipTest("Need fixtures")
         self.useFixture(SampleTestFixture())
         out = StringIO()
         run.main(['prog', '-l', 'testtools.runexample.test_suite'], out)
@@ -52,8 +58,6 @@
 """, out.getvalue())
 
     def test_run_load_list(self):
-        if fixtures is None:
-            self.skipTest("Need fixtures")
         self.useFixture(SampleTestFixture())
         out = StringIO()
         # We load two tests - one that exists and one that doesn't, and we
@@ -74,6 +78,19 @@
         self.assertEqual("""testtools.runexample.TestFoo.test_bar
 """, out.getvalue())
 
+    def test_run_failfast(self):
+        runner_stdout = self.useFixture(fixtures.DetailStream('stdout')).stream
+        class Failing(TestCase):
+            def test_a(self):
+                self.fail('a')
+            def test_b(self):
+                self.fail('b')
+        runner = run.TestToolsTestRunner(failfast=True)
+        with fixtures.MonkeyPatch('sys.stdout', runner_stdout):
+            runner.run(TestSuite([Failing('test_a'), Failing('test_b')]))
+        self.assertThat(runner_stdout.getvalue(), Contains('Ran 1 test'))
+
+
 
 def test_suite():
     from unittest import TestLoader

=== modified file 'testtools/tests/test_testresult.py'
--- testtools/tests/test_testresult.py	2012-10-19 14:29:59 +0000
+++ testtools/tests/test_testresult.py	2012-12-16 12:29:26 +0000
@@ -12,6 +12,7 @@
 import sys
 import tempfile
 import threading
+from unittest import TestSuite
 import warnings
 
 from testtools import (
@@ -43,6 +44,7 @@
     TracebackContent,
     )
 from testtools.content_type import ContentType, UTF8_TEXT
+from testtools.helpers import safe_hasattr
 from testtools.matchers import (
     Contains,
     DocTestMatches,
@@ -142,6 +144,11 @@
         result.stopTest(self)
         self.assertTrue(result.wasSuccessful())
 
+    def test_stop_sets_shouldStop(self):
+        result = self.makeResult()
+        result.stop()
+        self.assertTrue(result.shouldStop)
+
 
 class Python27Contract(Python26Contract):
 
@@ -193,6 +200,17 @@
         result.startTestRun()
         result.stopTestRun()
 
+    def test_failfast(self):
+        result = self.makeResult()
+        result.failfast = True
+        class Failing(TestCase):
+            def test_a(self):
+                self.fail('a')
+            def test_b(self):
+                self.fail('b')
+        TestSuite([Failing('test_a'), Failing('test_b')]).run(result)
+        self.assertEqual(1, result.testsRun)
+
 
 class TagsContract(Python27Contract):
     """Tests to ensure correct tagging behaviour.
@@ -566,12 +584,36 @@
         # `TestResult`s.
         self.assertResultLogsEqual([])
 
+    def test_failfast_get(self):
+        # Reading reads from the first one - arbitrary choice.
+        self.assertEqual(False, self.multiResult.failfast)
+        self.result1.failfast = True
+        self.assertEqual(True, self.multiResult.failfast)
+
+    def test_failfast_set(self):
+        # Writing writes to all.
+        self.multiResult.failfast = True
+        self.assertEqual(True, self.result1.failfast)
+        self.assertEqual(True, self.result2.failfast)
+
+    def test_shouldStop(self):
+        self.assertFalse(self.multiResult.shouldStop)
+        self.result2.stop()
+        # NB: result1 is not stopped: MultiTestResult has to combine the
+        # values.
+        self.assertTrue(self.multiResult.shouldStop)
+
     def test_startTest(self):
         # Calling `startTest` on a `MultiTestResult` calls `startTest` on all
         # its `TestResult`s.
         self.multiResult.startTest(self)
         self.assertResultLogsEqual([('startTest', self)])
 
+    def test_stop(self):
+        self.assertFalse(self.multiResult.shouldStop)
+        self.multiResult.stop()
+        self.assertResultLogsEqual(['stop'])
+
     def test_stopTest(self):
         # Calling `stopTest` on a `MultiTestResult` calls `stopTest` on all
         # its `TestResult`s.
@@ -1176,6 +1218,19 @@
 class TestExtendedToOriginalResultDecorator(
     TestExtendedToOriginalResultDecoratorBase):
 
+    def test_failfast_py26(self):
+        self.make_26_result()
+        self.assertEqual(False, self.converter.failfast)
+        self.converter.failfast = True
+        self.assertFalse(safe_hasattr(self.converter.decorated, 'failfast'))
+
+    def test_failfast_py27(self):
+        self.make_27_result()
+        self.assertEqual(False, self.converter.failfast)
+        # setting it should write it to the backing result
+        self.converter.failfast = True
+        self.assertEqual(True, self.converter.decorated.failfast)
+
     def test_progress_py26(self):
         self.make_26_result()
         self.converter.progress(1, 2)