Rev 4781: (robertc) Overhaul TestResult handling of test outcomes to degrade in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Nov 2 11:05:13 GMT 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4781 [merge]
revision-id: pqm at pqm.ubuntu.com-20091102110512-yw3aimu88nbmvoo9
parent: pqm at pqm.ubuntu.com-20091031042050-avqp46clpe9fabzm
parent: robertc at robertcollins.net-20091101055216-yfyue9tskyprkd0t
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-11-02 11:05:12 +0000
message:
  (robertc) Overhaul TestResult handling of test outcomes to degrade
  	more gracefully and be more compatible with Python2.7 (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
=== modified file 'NEWS'
--- a/NEWS	2009-10-30 16:13:05 +0000
+++ b/NEWS	2009-11-01 05:41:57 +0000
@@ -36,6 +36,27 @@
 Testing
 *******
 
+* KnownFailure is now signalled to ``ExtendedTestResult`` using the same
+  method that Python 2.7 uses - ``addExpectedFailure``. (Robert Collins)
+
+* TestNotApplicable is now handled within the TestCase.run method rather
+  than being looked for within ``ExtendedTestResult.addError``. This
+  provides better handling with other ``TestResult`` objects, degrading to
+  sucess rather than error. (Robert Collins)
+
+* The private method ``_testConcluded`` on ``ExtendedTestResult`` has been
+  removed - it was empty and unused. (Robert Collins)
+
+* UnavailableFeature is now handled within the TestCase.run method rather
+  than being looked for within addError. If the Result object does not
+  have an addNotSupported method, addSkip is attempted instead, and
+  failing that addSuccess. (Robert Collins)
+
+* When a TestResult does not have an addSkip method, skipped tests are now
+  reported as successful tests, rather than as errors. This change is
+  to make it possible to get a clean test run with a less capable
+  TestResult. (Robert Collins)
+
 
 
 bzr 2.0.3 (not released yet)

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-10-29 21:13:16 +0000
+++ b/bzrlib/tests/__init__.py	2009-11-01 05:52:16 +0000
@@ -298,19 +298,13 @@
         Called from the TestCase run() method when the test
         fails with an unexpected error.
         """
-        self._testConcluded(test)
-        if isinstance(err[1], TestNotApplicable):
-            return self._addNotApplicable(test, err)
-        elif isinstance(err[1], UnavailableFeature):
-            return self.addNotSupported(test, err[1].args[0])
-        else:
-            self._post_mortem()
-            unittest.TestResult.addError(self, test, err)
-            self.error_count += 1
-            self.report_error(test, err)
-            if self.stop_early:
-                self.stop()
-            self._cleanupLogFile(test)
+        self._post_mortem()
+        unittest.TestResult.addError(self, test, err)
+        self.error_count += 1
+        self.report_error(test, err)
+        if self.stop_early:
+            self.stop()
+        self._cleanupLogFile(test)
 
     def addFailure(self, test, err):
         """Tell result that test failed.
@@ -318,24 +312,19 @@
         Called from the TestCase run() method when the test
         fails because e.g. an assert() method failed.
         """
-        self._testConcluded(test)
-        if isinstance(err[1], KnownFailure):
-            return self._addKnownFailure(test, err)
-        else:
-            self._post_mortem()
-            unittest.TestResult.addFailure(self, test, err)
-            self.failure_count += 1
-            self.report_failure(test, err)
-            if self.stop_early:
-                self.stop()
-            self._cleanupLogFile(test)
+        self._post_mortem()
+        unittest.TestResult.addFailure(self, test, err)
+        self.failure_count += 1
+        self.report_failure(test, err)
+        if self.stop_early:
+            self.stop()
+        self._cleanupLogFile(test)
 
     def addSuccess(self, test):
         """Tell result that test completed successfully.
 
         Called from the TestCase run()
         """
-        self._testConcluded(test)
         if self._bench_history is not None:
             benchmark_time = self._extractBenchmarkTime(test)
             if benchmark_time is not None:
@@ -347,14 +336,7 @@
         unittest.TestResult.addSuccess(self, test)
         test._log_contents = ''
 
-    def _testConcluded(self, test):
-        """Common code when a test has finished.
-
-        Called regardless of whether it succeded, failed, etc.
-        """
-        pass
-
-    def _addKnownFailure(self, test, err):
+    def addExpectedFailure(self, test, err):
         self.known_failure_count += 1
         self.report_known_failure(test, err)
 
@@ -362,11 +344,11 @@
         """The test will not be run because of a missing feature.
         """
         # this can be called in two different ways: it may be that the
-        # test started running, and then raised (through addError)
+        # test started running, and then raised (through requireFeature)
         # UnavailableFeature.  Alternatively this method can be called
-        # while probing for features before running the tests; in that
-        # case we will see startTest and stopTest, but the test will never
-        # actually run.
+        # while probing for features before running the test code proper; in
+        # that case we will see startTest and stopTest, but the test will
+        # never actually run.
         self.unsupported.setdefault(str(feature), 0)
         self.unsupported[str(feature)] += 1
         self.report_unsupported(test, feature)
@@ -376,21 +358,9 @@
         self.skip_count += 1
         self.report_skip(test, reason)
 
-    def _addNotApplicable(self, test, skip_excinfo):
-        if isinstance(skip_excinfo[1], TestNotApplicable):
-            self.not_applicable_count += 1
-            self.report_not_applicable(test, skip_excinfo)
-        try:
-            test.tearDown()
-        except KeyboardInterrupt:
-            raise
-        except:
-            self.addError(test, test.exc_info())
-        else:
-            # seems best to treat this as success from point-of-view of unittest
-            # -- it actually does nothing so it barely matters :)
-            unittest.TestResult.addSuccess(self, test)
-            test._log_contents = ''
+    def addNotApplicable(self, test, reason):
+        self.not_applicable_count += 1
+        self.report_not_applicable(test, reason)
 
     def printErrorList(self, flavour, errors):
         for test, err in errors:
@@ -535,7 +505,7 @@
     def report_skip(self, test, reason):
         pass
 
-    def report_not_applicable(self, test, skip_excinfo):
+    def report_not_applicable(self, test, reason):
         pass
 
     def report_unsupported(self, test, feature):
@@ -602,10 +572,9 @@
         self.stream.writeln(' SKIP %s\n%s'
                 % (self._testTimeString(test), reason))
 
-    def report_not_applicable(self, test, skip_excinfo):
-        self.stream.writeln('  N/A %s\n%s'
-                % (self._testTimeString(test),
-                   self._error_summary(skip_excinfo)))
+    def report_not_applicable(self, test, reason):
+        self.stream.writeln('  N/A %s\n    %s'
+                % (self._testTimeString(test), reason))
 
     def report_unsupported(self, test, feature):
         """test cannot be run because feature is missing."""
@@ -713,6 +682,8 @@
 class UnavailableFeature(Exception):
     """A feature required for this test was not available.
 
+    This can be considered a specialised form of SkippedTest.
+
     The feature should be used to construct the exception.
     """
 
@@ -1595,89 +1566,127 @@
     def _do_skip(self, result, reason):
         addSkip = getattr(result, 'addSkip', None)
         if not callable(addSkip):
-            result.addError(self, sys.exc_info())
+            result.addSuccess(result)
         else:
             addSkip(self, reason)
 
+    def _do_known_failure(self, result):
+        err = sys.exc_info()
+        addExpectedFailure = getattr(result, 'addExpectedFailure', None)
+        if addExpectedFailure is not None:
+            addExpectedFailure(self, err)
+        else:
+            result.addSuccess(self)
+
+    def _do_not_applicable(self, result, e):
+        if not e.args:
+            reason = 'No reason given'
+        else:
+            reason = e.args[0]
+        addNotApplicable = getattr(result, 'addNotApplicable', None)
+        if addNotApplicable is not None:
+            result.addNotApplicable(self, reason)
+        else:
+            self._do_skip(result, reason)
+
+    def _do_unsupported_or_skip(self, result, reason):
+        addNotSupported = getattr(result, 'addNotSupported', None)
+        if addNotSupported is not None:
+            result.addNotSupported(self, reason)
+        else:
+            self._do_skip(result, reason)
+
     def run(self, result=None):
         if result is None: result = self.defaultTestResult()
+        result.startTest(self)
+        try:
+            self._run(result)
+            return result
+        finally:
+            result.stopTest(self)
+
+    def _run(self, result):
         for feature in getattr(self, '_test_needs_features', []):
             if not feature.available():
-                result.startTest(self)
-                if getattr(result, 'addNotSupported', None):
-                    result.addNotSupported(self, feature)
-                else:
-                    result.addSuccess(self)
-                result.stopTest(self)
-                return result
+                return self._do_unsupported_or_skip(result, feature)
         try:
+            absent_attr = object()
+            # Python 2.5
+            method_name = getattr(self, '_testMethodName', absent_attr)
+            if method_name is absent_attr:
+                # Python 2.4
+                method_name = getattr(self, '_TestCase__testMethodName')
+            testMethod = getattr(self, method_name)
             try:
-                result.startTest(self)
-                absent_attr = object()
-                # Python 2.5
-                method_name = getattr(self, '_testMethodName', absent_attr)
-                if method_name is absent_attr:
-                    # Python 2.4
-                    method_name = getattr(self, '_TestCase__testMethodName')
-                testMethod = getattr(self, method_name)
-                try:
-                    try:
-                        self.setUp()
-                        if not self._bzr_test_setUp_run:
-                            self.fail(
-                                "test setUp did not invoke "
-                                "bzrlib.tests.TestCase's setUp")
-                    except KeyboardInterrupt:
-                        self._runCleanups()
-                        raise
-                    except TestSkipped, e:
-                        self._do_skip(result, e.args[0])
-                        self.tearDown()
-                        return result
-                    except:
-                        result.addError(self, sys.exc_info())
-                        self._runCleanups()
-                        return result
-
+                try:
+                    self.setUp()
+                    if not self._bzr_test_setUp_run:
+                        self.fail(
+                            "test setUp did not invoke "
+                            "bzrlib.tests.TestCase's setUp")
+                except KeyboardInterrupt:
+                    self._runCleanups()
+                    raise
+                except KnownFailure:
+                    self._do_known_failure(result)
+                    self.tearDown()
+                    return
+                except TestNotApplicable, e:
+                    self._do_not_applicable(result, e)
+                    self.tearDown()
+                    return
+                except TestSkipped, e:
+                    self._do_skip(result, e.args[0])
+                    self.tearDown()
+                    return result
+                except UnavailableFeature, e:
+                    self._do_unsupported_or_skip(result, e.args[0])
+                    self.tearDown()
+                    return
+                except:
+                    result.addError(self, sys.exc_info())
+                    self._runCleanups()
+                    return result
+
+                ok = False
+                try:
+                    testMethod()
+                    ok = True
+                except KnownFailure:
+                    self._do_known_failure(result)
+                except self.failureException:
+                    result.addFailure(self, sys.exc_info())
+                except TestNotApplicable, e:
+                    self._do_not_applicable(result, e)
+                except TestSkipped, e:
+                    if not e.args:
+                        reason = "No reason given."
+                    else:
+                        reason = e.args[0]
+                    self._do_skip(result, reason)
+                except UnavailableFeature, e:
+                    self._do_unsupported_or_skip(result, e.args[0])
+                except KeyboardInterrupt:
+                    self._runCleanups()
+                    raise
+                except:
+                    result.addError(self, sys.exc_info())
+
+                try:
+                    self.tearDown()
+                    if not self._bzr_test_tearDown_run:
+                        self.fail(
+                            "test tearDown did not invoke "
+                            "bzrlib.tests.TestCase's tearDown")
+                except KeyboardInterrupt:
+                    self._runCleanups()
+                    raise
+                except:
+                    result.addError(self, sys.exc_info())
+                    self._runCleanups()
                     ok = False
-                    try:
-                        testMethod()
-                        ok = True
-                    except self.failureException:
-                        result.addFailure(self, sys.exc_info())
-                    except TestSkipped, e:
-                        if not e.args:
-                            reason = "No reason given."
-                        else:
-                            reason = e.args[0]
-                        self._do_skip(result, reason)
-                    except KeyboardInterrupt:
-                        self._runCleanups()
-                        raise
-                    except:
-                        result.addError(self, sys.exc_info())
-
-                    try:
-                        self.tearDown()
-                        if not self._bzr_test_tearDown_run:
-                            self.fail(
-                                "test tearDown did not invoke "
-                                "bzrlib.tests.TestCase's tearDown")
-                    except KeyboardInterrupt:
-                        self._runCleanups()
-                        raise
-                    except:
-                        result.addError(self, sys.exc_info())
-                        self._runCleanups()
-                        ok = False
-                    if ok: result.addSuccess(self)
-                finally:
-                    result.stopTest(self)
+                if ok: result.addSuccess(self)
                 return result
-            except TestNotApplicable:
-                # Not moved from the result [yet].
-                self._runCleanups()
-                raise
             except KeyboardInterrupt:
                 self._runCleanups()
                 raise
@@ -3381,8 +3390,8 @@
     def addFailure(self, test, err):
         known = self._error_looks_like('KnownFailure: ', err)
         if known is not None:
-            self.result._addKnownFailure(test, [KnownFailure,
-                                                KnownFailure(known), None])
+            self.result.addExpectedFailure(test,
+                [KnownFailure, KnownFailure(known), None])
         else:
             self.result.addFailure(test, err)
 

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2009-10-29 21:13:16 +0000
+++ b/bzrlib/tests/test_selftest.py	2009-11-01 05:41:57 +0000
@@ -827,9 +827,10 @@
             def report_known_failure(self, test, err):
                 self._call = test, err
         result = InstrumentedTestResult(None, None, None, None)
-        def test_function():
-            raise tests.KnownFailure('failed!')
-        test = unittest.FunctionTestCase(test_function)
+        class Test(tests.TestCase):
+            def test_function(self):
+                raise tests.KnownFailure('failed!')
+        test = Test("test_function")
         test.run(result)
         # it should invoke 'report_known_failure'.
         self.assertEqual(2, len(result._call))
@@ -926,9 +927,10 @@
                 self._call = test, feature
         result = InstrumentedTestResult(None, None, None, None)
         feature = tests.Feature()
-        def test_function():
-            raise tests.UnavailableFeature(feature)
-        test = unittest.FunctionTestCase(test_function)
+        class Test(tests.TestCase):
+            def test_function(self):
+                raise tests.UnavailableFeature(feature)
+        test = Test("test_function")
         test.run(result)
         # it should invoke 'addNotSupported'.
         self.assertEqual(2, len(result._call))
@@ -951,7 +953,7 @@
                                              verbosity=1)
         test = self.get_passing_test()
         err = (tests.KnownFailure, tests.KnownFailure('foo'), None)
-        result._addKnownFailure(test, err)
+        result.addExpectedFailure(test, err)
         self.assertFalse(result.wasStrictlySuccessful())
         self.assertEqual(None, result._extractBenchmarkTime(test))
 
@@ -1014,10 +1016,11 @@
     def test_known_failure_failed_run(self):
         # run a test that generates a known failure which should be printed in
         # the final output when real failures occur.
-        def known_failure_test():
-            raise tests.KnownFailure('failed')
+        class Test(tests.TestCase):
+            def known_failure_test(self):
+                raise tests.KnownFailure('failed')
         test = unittest.TestSuite()
-        test.addTest(unittest.FunctionTestCase(known_failure_test))
+        test.addTest(Test("known_failure_test"))
         def failing_test():
             raise AssertionError('foo')
         test.addTest(unittest.FunctionTestCase(failing_test))
@@ -1041,10 +1044,12 @@
             )
 
     def test_known_failure_ok_run(self):
-        # run a test that generates a known failure which should be printed in the final output.
-        def known_failure_test():
-            raise tests.KnownFailure('failed')
-        test = unittest.FunctionTestCase(known_failure_test)
+        # run a test that generates a known failure which should be printed in
+        # the final output.
+        class Test(tests.TestCase):
+            def known_failure_test(self):
+                raise tests.KnownFailure('failed')
+        test = Test("known_failure_test")
         stream = StringIO()
         runner = tests.TextTestRunner(stream=stream)
         result = self.run_test_runner(runner, test)
@@ -1127,11 +1132,12 @@
 
     def test_not_applicable(self):
         # run a test that is skipped because it's not applicable
-        def not_applicable_test():
-            raise tests.TestNotApplicable('this test never runs')
+        class Test(tests.TestCase):
+            def not_applicable_test(self):
+                raise tests.TestNotApplicable('this test never runs')
         out = StringIO()
         runner = tests.TextTestRunner(stream=out, verbosity=2)
-        test = unittest.FunctionTestCase(not_applicable_test)
+        test = Test("not_applicable_test")
         result = self.run_test_runner(runner, test)
         self._log_file.write(out.getvalue())
         self.assertTrue(result.wasSuccessful())




More information about the bazaar-commits mailing list