testtools-dev team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/testtools/bug-683332 into lp:testtools.

Requested reviews:
  testtools developers (testtools-dev)
Related bugs:
  #683332 ExtendedToOriginalDecorator no longer decorates wasSuccessful
  https://bugs.launchpad.net/bugs/683332


Fix bug. Yay.
-- 
https://code.launchpad.net/~lifeless/testtools/bug-683332/+merge/42751
Your team testtools developers is requested to review the proposed merge of lp:~lifeless/testtools/bug-683332 into lp:testtools.

=== modified file 'NEWS'
--- NEWS	2010-11-29 23:46:10 +0000
+++ NEWS	2010-12-04 21:08:52 +0000
@@ -10,10 +10,18 @@
 * addUnexpectedSuccess is translated to addFailure for test results that don't
   know about addUnexpectedSuccess.  Further, it fails the entire result for
   all testtools TestResults (i.e. wasSuccessful() returns False after
-  addUnexpectedSuccess has been called).  (Jonathan Lange, #654474)
+  addUnexpectedSuccess has been called). Note that when using a delegating
+  result such as ThreadsafeForwardingResult, MultiTestResult or
+  ExtendedToOriginalDecorator then the behaviour of addUnexpectedSuccess is
+  determined by the delegated to result(s).
+  (Jonathan Lange, Robert Collins, #654474, #683332)
 
 * startTestRun will reset any errors on the result.  That is, wasSuccessful()
-  will always return True immediately after startTestRun() is called.
+  will always return True immediately after startTestRun() is called. This
+  only applies to delegated test results (ThreadsafeForwardingResult,
+  MultiTestResult and ExtendedToOriginalDecorator) if the delegated to result
+  is a testtools test result - we cannot reliably reset the state of unknown
+  test result class instances. (Jonathan Lange, Robert Collins, #683332)
 
 * Responsibility for running test cleanups has been moved to ``RunTest``.
   This change does not affect public APIs and can be safely ignored by test

=== modified file 'testtools/testresult/real.py'
--- testtools/testresult/real.py	2010-11-30 12:54:20 +0000
+++ testtools/testresult/real.py	2010-12-04 21:08:52 +0000
@@ -35,13 +35,11 @@
     """
 
     def __init__(self):
-        super(TestResult, self).__init__()
-        self.skip_reasons = {}
-        self.__now = None
-        # -- Start: As per python 2.7 --
-        self.expectedFailures = []
-        self.unexpectedSuccesses = []
-        # -- End:   As per python 2.7 --
+        # 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.
+        TestResult.startTestRun(self)
 
     def addExpectedFailure(self, test, err=None, details=None):
         """Called when a test has failed in an expected manner.
@@ -160,9 +158,13 @@
 
         New in python 2.7
         """
+        super(TestResult, self).__init__()
+        self.skip_reasons = {}
+        self.__now = None
+        # -- Start: As per python 2.7 --
+        self.expectedFailures = []
         self.unexpectedSuccesses = []
-        self.errors = []
-        self.failures = []
+        # -- End:   As per python 2.7 --
 
     def stopTestRun(self):
         """Called after a test run completes
@@ -406,13 +408,11 @@
 
     def __init__(self, decorated):
         self.decorated = decorated
-        self._was_successful = True
 
     def __getattr__(self, name):
         return getattr(self.decorated, name)
 
     def addError(self, test, err=None, details=None):
-        self._was_successful = False
         self._check_args(err, details)
         if details is not None:
             try:
@@ -437,7 +437,6 @@
         return addExpectedFailure(test, err)
 
     def addFailure(self, test, err=None, details=None):
-        self._was_successful = False
         self._check_args(err, details)
         if details is not None:
             try:
@@ -464,7 +463,6 @@
         return addSkip(test, reason)
 
     def addUnexpectedSuccess(self, test, details=None):
-        self._was_successful = False
         outcome = getattr(self.decorated, 'addUnexpectedSuccess', None)
         if outcome is None:
             try:
@@ -521,7 +519,6 @@
         return self.decorated.startTest(test)
 
     def startTestRun(self):
-        self._was_successful = True
         try:
             return self.decorated.startTestRun()
         except AttributeError:
@@ -552,7 +549,7 @@
         return method(a_datetime)
 
     def wasSuccessful(self):
-        return self._was_successful
+        return self.decorated.wasSuccessful()
 
 
 class _StringException(Exception):

=== modified file 'testtools/tests/test_testresult.py'
--- testtools/tests/test_testresult.py	2010-11-29 00:32:53 +0000
+++ testtools/tests/test_testresult.py	2010-12-04 21:08:52 +0000
@@ -132,7 +132,7 @@
         result.stopTestRun()
 
 
-class TestResultContract(Python27Contract):
+class DetailsContract(Python27Contract):
     """Tests for the contract of TestResults."""
 
     def test_addExpectedFailure_details(self):
@@ -171,6 +171,13 @@
         result.startTest(self)
         result.addSuccess(self, details={})
 
+
+class FallbackContract(DetailsContract):
+    """When we fallback we take our policy choice to map calls.
+
+    For instance, we map unexpectedSuccess to an error code, not to success.
+    """
+
     def test_addUnexpectedSuccess_was_successful(self):
         # addUnexpectedSuccess fails test run in testtools.
         result = self.makeResult()
@@ -179,6 +186,14 @@
         result.stopTest(self)
         self.assertFalse(result.wasSuccessful())
 
+
+class StartTestRunContract(FallbackContract):
+    """Defines the contract for testtools policy choices.
+    
+    That is things which are not simply extensions to unittest but choices we
+    have made differently.
+    """
+
     def test_startTestRun_resets_unexpected_success(self):
         result = self.makeResult()
         result.startTest(self)
@@ -203,25 +218,26 @@
         result.startTestRun()
         self.assertTrue(result.wasSuccessful())
 
-class TestTestResultContract(TestCase, TestResultContract):
+
+class TestTestResultContract(TestCase, StartTestRunContract):
 
     def makeResult(self):
         return TestResult()
 
 
-class TestMultiTestResultContract(TestCase, TestResultContract):
+class TestMultiTestResultContract(TestCase, StartTestRunContract):
 
     def makeResult(self):
         return MultiTestResult(TestResult(), TestResult())
 
 
-class TestTextTestResultContract(TestCase, TestResultContract):
+class TestTextTestResultContract(TestCase, StartTestRunContract):
 
     def makeResult(self):
         return TextTestResult(StringIO())
 
 
-class TestThreadSafeForwardingResultContract(TestCase, TestResultContract):
+class TestThreadSafeForwardingResultContract(TestCase, StartTestRunContract):
 
     def makeResult(self):
         result_semaphore = threading.Semaphore(1)
@@ -229,7 +245,7 @@
         return ThreadsafeForwardingResult(target, result_semaphore)
 
 
-class TestExtendedTestResultContract(TestCase, TestResultContract):
+class TestExtendedTestResultContract(TestCase, StartTestRunContract):
 
     def makeResult(self):
         return ExtendedTestResult()
@@ -241,7 +257,7 @@
         return Python26TestResult()
 
 
-class TestAdaptedPython26TestResultContract(TestCase, TestResultContract):
+class TestAdaptedPython26TestResultContract(TestCase, FallbackContract):
 
     def makeResult(self):
         return ExtendedToOriginalDecorator(Python26TestResult())
@@ -253,7 +269,7 @@
         return Python27TestResult()
 
 
-class TestAdaptedPython27TestResultContract(TestCase, TestResultContract):
+class TestAdaptedPython27TestResultContract(TestCase, DetailsContract):
 
     def makeResult(self):
         return ExtendedToOriginalDecorator(Python27TestResult())