Skip to content

Conversation

@vstinner
Copy link
Member

bpo-23890: unittest.TestCase.assertRaises() now manually breaks a
reference cycle to not keep objects alive longer than expected.
(cherry picked from commit bbd3cf8)

@vstinner vstinner changed the title [3.6] Fix ref cycles in TestCase.assertRaises() (#193) [3.6] bpo-23890: Fix ref cycles in TestCase.assertRaises() (#193) Mar 27, 2017
@vstinner vstinner force-pushed the backport-bbd3cf8-3.6 branch from b866adf to 655f38e Compare March 27, 2017 23:01
@vstinner vstinner changed the title [3.6] bpo-23890: Fix ref cycles in TestCase.assertRaises() (#193) [3.6] bpo-23890: Fix ref cycles in TestCase.assertRaises() Mar 27, 2017
with self.assertRaises(TypeError):
self.assertRaises((ValueError, object))

def testAssertRaisesRefcount(self):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the new try/finally handling and the potential for unexpected behavior, I think the context manager mode needs to be checked as well. I don't see a need for a separate test function, just add something like:

with self.assertRaises(ValueError):
    func()

self.assertEqual(refcount, sys.getrefcount(func))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reference cycle when using the context manager protocol.

It seems like bpo-23890 was very specific to the function API: assertRaises(ValueError, func) since you pass func to assertRaises().

with self.assertRaises(ValueError):
    func()

Here func variable may also leak in the traceback objects linked to the raisen exception. But again, I'm unable to see any leak here without my fix.

If you still consider that an unit test is needed, please comment http://bugs.python.org/issue23890

unittest.TestCase.assertRaises() now manually breaks a
reference cycle to not keep objects alive longer than expected.
(cherry picked from commit bbd3cf8)
@vstinner vstinner force-pushed the backport-bbd3cf8-3.6 branch from 655f38e to b41dbb3 Compare June 15, 2017 22:04
@vstinner vstinner merged commit 50dbf57 into 3.6 Jun 15, 2017
@vstinner vstinner deleted the backport-bbd3cf8-3.6 branch June 15, 2017 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants