testtools-dev team mailing list archive
-
testtools-dev team
-
Mailing list archive
-
Message #00045
[Merge] lp:~gz/testtools/avoid_exc_info_cycles into lp:testtools
Martin [gz] has proposed merging lp:~gz/testtools/avoid_exc_info_cycles into lp:testtools.
Requested reviews:
testtools developers (testtools-dev)
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.
--
https://code.launchpad.net/~gz/testtools/avoid_exc_info_cycles/+merge/32726
Your team testtools developers is requested to review the proposed merge of lp:~gz/testtools/avoid_exc_info_cycles into lp:testtools.
=== 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:
Follow ups