Merge lp:~gz/testtools/avoid_exc_info_cycles into lp:~testtools-committers/testtools/trunk

[Martin Packman]

Adds try/finally/del in a few places that assigned result of sys.exc_info call to a local variable. This creates a reference cycle, which while mostly harmless as will be dealt with by gc, can be annoying when trying to write deterministic code.

There are a couple I've not resolved. The one in testtools.tests.helpers was easy to avoid by not using that (non-public?) module. The expectedFailure method has me stumped for the moment, as the unittest _ExpectedFailure signature isn't as sane as the old Bazaar version (see also bug 607400). On irc lifeless suggested some frame mangling might be able to fix this, any hints on that I'd appreciate.

See <lp:~gz/bzr/cleanup_testcases_by_collection_613247> for branch to bzr that prompted this change.

[Robert Collins (lifeless)]

I've also tweaked this one - you had no test coverage (your change only checked the error path), and I've made the no-reason fallback supply the whole details dict (because something has obviously gone wrong).

review: Approve

[Robert Collins (lifeless)]

bah wrong mp

[Robert Collins (lifeless)]

there was a conflict with my multiple-exception support, I've trivially resolved that, but its possible that that work added more cycles.

review: Approve

[Martin Packman (gz)]

Thanks! The merge looks fine by eyeball, once lp:~gz/bzr/transport_post_connect_hook lands which will unblock me on the test collection front I'll double check that everything still works as expected.

=== modified file 'testtools/run.py'
--- testtools/run.py	2010-07-01 11:33:57 +0000
+++ testtools/run.py	2010-08-15 23:26:39 +0000
@@ -187,9 +187,7 @@
                 self.testNames = (self.defaultTest,)
             self.createTests()
         except getopt.error:
-            exc_info = sys.exc_info()
-            msg = exc_info[1]
-            self.usageExit(msg)
+            self.usageExit(sys.exc_info()[1])
 
     def createTests(self):
         if self.testNames is None:

=== modified file 'testtools/runtest.py'
--- testtools/runtest.py	2010-07-29 18:20:02 +0000
+++ testtools/runtest.py	2010-08-15 23:26:39 +0000
@@ -146,8 +146,11 @@
             raise
         except:
             exc_info = sys.exc_info()
-            e = exc_info[1]
-            self.case.onException(exc_info)
+            try:
+                e = exc_info[1]
+                self.case.onException(exc_info)
+            finally:
+                del exc_info
             for exc_class, handler in self.handlers:
                 if isinstance(e, exc_class):
                     self._exceptions.append(e)

=== modified file 'testtools/testcase.py'
--- testtools/testcase.py	2010-08-04 13:05:20 +0000
+++ testtools/testcase.py	2010-08-15 23:26:39 +0000
@@ -179,8 +179,11 @@
                 raise
             except:
                 exc_info = sys.exc_info()
-                self._report_traceback(exc_info)
-                last_exception = exc_info[1]
+                try:
+                    self._report_traceback(exc_info)
+                    last_exception = exc_info[1]
+                finally:
+                    del exc_info
         return last_exception
 
     def addCleanup(self, function, *arguments, **keywordArguments):
@@ -319,9 +322,14 @@
         try:
             predicate(*args, **kwargs)
         except self.failureException:
+            # GZ 2010-08-12: Don't know how to avoid exc_info cycle as the new
+            #                unittest _ExpectedFailure wants old traceback
             exc_info = sys.exc_info()
-            self._report_traceback(exc_info)
-            raise _ExpectedFailure(exc_info)
+            try:
+                self._report_traceback(exc_info)
+                raise _ExpectedFailure(exc_info)
+            finally:
+                del exc_info
         else:
             raise _UnexpectedSuccess(reason)
 

=== modified file 'testtools/tests/helpers.py'
--- testtools/tests/helpers.py	2009-12-31 03:15:19 +0000
+++ testtools/tests/helpers.py	2010-08-15 23:26:39 +0000
@@ -12,6 +12,7 @@
 from testtools import TestResult
 
 
+# GZ 2010-08-12: Don't do this, pointlessly creates an exc_info cycle
 try:
     raise Exception
 except Exception: