Merge lp:~gz/bzr/unbreak_BZR_TEST_PDB_504070 into lp:bzr

[Martin Packman]

Fix BZR_TEST_PDB. (#504070)

Fix regression from introduction of testtools which made BZR_TEST_PDB useless as it didn't get you the actual traceback from the test.

As explained in the bug, because of the different way testtools runs test cases, we need to jump through some extra hoops to get the real traceback from the test, stash it somewhere, and then pass it on in addError and addFailure. The traceback is cleared in stopTest to break the cycle.

There's one case this branch doesn't handle, where the test passes but an exception is raised from a testtools cleanup. There the onException handler doesn't get given the traceback, so pdb will raise a ValueError as there's nothing for it to look at.

[John A Meinel (jameinel)]

seems good to me.

review: Approve

[Andrew Bennetts (spiv)]

sent to pqm by email

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2010-09-27 19:31:45 +0000
+++ bzrlib/tests/__init__.py	2010-10-04 07:45:58 +0000
@@ -195,6 +195,7 @@
         self._strict = strict
         self._first_thread_leaker_id = None
         self._tests_leaking_threads_count = 0
+        self._traceback_from_test = None
 
     def stopTestRun(self):
         run = self.testsRun
@@ -280,6 +281,14 @@
         what = re.sub(r'^bzrlib\.tests\.', '', what)
         return what
 
+    # GZ 2010-10-04: Cloned tests may end up harmlessly calling this method
+    #                multiple times in a row, because the handler is added for
+    #                each test but the container list is shared between cases.
+    #                See lp:498869 lp:625574 and lp:637725 for background.
+    def _record_traceback_from_test(self, exc_info):
+        """Store the traceback from passed exc_info tuple till"""
+        self._traceback_from_test = exc_info[2]
+
     def startTest(self, test):
         super(ExtendedTestResult, self).startTest(test)
         if self.count == 0:
@@ -288,6 +297,10 @@
         self.report_test_start(test)
         test.number = self.count
         self._recordTestStartTime()
+        # Make testtools cases give us the real traceback on failure
+        addOnException = getattr(test, "addOnException", None)
+        if addOnException is not None:
+            addOnException(self._record_traceback_from_test)
         # Only check for thread leaks if the test case supports cleanups
         addCleanup = getattr(test, "addCleanup", None)
         if addCleanup is not None:
@@ -297,6 +310,9 @@
         self.report_tests_starting()
         self._active_threads = threading.enumerate()
 
+    def stopTest(self, test):
+        self._traceback_from_test = None
+
     def _check_leaked_threads(self, test):
         """See if any threads have leaked since last call
 
@@ -323,7 +339,7 @@
         Called from the TestCase run() method when the test
         fails with an unexpected error.
         """
-        self._post_mortem()
+        self._post_mortem(self._traceback_from_test)
         super(ExtendedTestResult, self).addError(test, err)
         self.error_count += 1
         self.report_error(test, err)
@@ -336,7 +352,7 @@
         Called from the TestCase run() method when the test
         fails because e.g. an assert() method failed.
         """
-        self._post_mortem()
+        self._post_mortem(self._traceback_from_test)
         super(ExtendedTestResult, self).addFailure(test, err)
         self.failure_count += 1
         self.report_failure(test, err)
@@ -384,10 +400,11 @@
         self.not_applicable_count += 1
         self.report_not_applicable(test, reason)
 
-    def _post_mortem(self):
+    def _post_mortem(self, tb=None):
         """Start a PDB post mortem session."""
         if os.environ.get('BZR_TEST_PDB', None):
-            import pdb;pdb.post_mortem()
+            import pdb
+            pdb.post_mortem(tb)
 
     def progress(self, offset, whence):
         """The test is adjusting the count of tests to run."""

=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2010-09-27 19:31:45 +0000
+++ bzrlib/tests/test_selftest.py	2010-10-04 07:45:58 +0000
@@ -3301,6 +3301,77 @@
         self.assertContainsString(result.stream.getvalue(), "leaking threads")
 
 
+class TestPostMortemDebugging(tests.TestCase):
+    """Check post mortem debugging works when tests fail or error"""
+
+    class TracebackRecordingResult(tests.ExtendedTestResult):
+        def __init__(self):
+            tests.ExtendedTestResult.__init__(self, StringIO(), 0, 1)
+            self.postcode = None
+        def _post_mortem(self, tb=None):
+            """Record the code object at the end of the current traceback"""
+            tb = tb or sys.exc_info()[2]
+            if tb is not None:
+                next = tb.tb_next
+                while next is not None:
+                    tb = next
+                    next = next.tb_next
+                self.postcode = tb.tb_frame.f_code
+        def report_error(self, test, err):
+            pass
+        def report_failure(self, test, err):
+            pass
+
+    def test_location_unittest_error(self):
+        """Needs right post mortem traceback with erroring unittest case"""
+        class Test(unittest.TestCase):
+            def runTest(self):
+                raise RuntimeError
+        result = self.TracebackRecordingResult()
+        Test().run(result)
+        self.assertEqual(result.postcode, Test.runTest.func_code)
+
+    def test_location_unittest_failure(self):
+        """Needs right post mortem traceback with failing unittest case"""
+        class Test(unittest.TestCase):
+            def runTest(self):
+                raise self.failureException
+        result = self.TracebackRecordingResult()
+        Test().run(result)
+        self.assertEqual(result.postcode, Test.runTest.func_code)
+
+    def test_location_bt_error(self):
+        """Needs right post mortem traceback with erroring bzrlib.tests case"""
+        class Test(tests.TestCase):
+            def test_error(self):
+                raise RuntimeError
+        result = self.TracebackRecordingResult()
+        Test("test_error").run(result)
+        self.assertEqual(result.postcode, Test.test_error.func_code)
+
+    def test_location_bt_failure(self):
+        """Needs right post mortem traceback with failing bzrlib.tests case"""
+        class Test(tests.TestCase):
+            def test_failure(self):
+                raise self.failureException
+        result = self.TracebackRecordingResult()
+        Test("test_failure").run(result)
+        self.assertEqual(result.postcode, Test.test_failure.func_code)
+
+    def test_env_var_triggers_post_mortem(self):
+        """Check pdb.post_mortem is called iff BZR_TEST_PDB is set"""
+        import pdb
+        result = tests.ExtendedTestResult(StringIO(), 0, 1)
+        post_mortem_calls = []
+        self.overrideAttr(pdb, "post_mortem", post_mortem_calls.append)
+        self.addCleanup(osutils.set_or_unset_env, "BZR_TEST_PDB",
+            osutils.set_or_unset_env("BZR_TEST_PDB", None))
+        result._post_mortem(1)
+        os.environ["BZR_TEST_PDB"] = "on"
+        result._post_mortem(2)
+        self.assertEqual([2], post_mortem_calls)
+
+
 class TestRunSuite(tests.TestCase):
 
     def test_runner_class(self):