Skip to content

Commit

Permalink
Fix the stubborn test about cyclic references left by pytest.raises
Browse files Browse the repository at this point in the history
In Python 2, a context manager's __exit__() leaves sys.exc_info with the exception values even when it was supposed
to suppress the exception, so we explicitly call sys.exc_clear() which removes the traceback and allow the object
to be released.

Also updated the test to not depend on the immediate destruction of the object but instead to ensure it is not being
tracked as a cyclic reference.

Fix #1965
  • Loading branch information
nicoddemus committed Nov 9, 2016
1 parent 552c7d4 commit 1130b9f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
* When loading plugins, import errors which contain non-ascii messages are now properly handled in Python 2 (`#1998`_).
Thanks `@nicoddemus`_ for the PR.

* Fixed memory leak in the RaisesContext (pytest.raises) (`#1965`_).
* Fixed cyclic reference when ``pytest.raises`` is used in context-manager form (`#1965`_). Also as a
result of this fix, ``sys.exc_info()`` is left empty in both context-manager and function call usages.
Previously, ``sys.exc_info`` would contain the exception caught by the context manager,
even when the expected exception occurred.
Thanks `@MSeifert04`_ for the report and the PR.

* Fixed false-positives warnings from assertion rewrite hook for modules that were rewritten but
Expand Down
6 changes: 5 additions & 1 deletion _pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,11 @@ def __exit__(self, *tp):
exc_type, value, traceback = tp
tp = exc_type, exc_type(value), traceback
self.excinfo.__init__(tp)
return issubclass(self.excinfo.type, self.expected_exception)
suppress_exception = issubclass(self.excinfo.type, self.expected_exception)
if sys.version_info[0] == 2 and suppress_exception:
sys.exc_clear()
return suppress_exception


# builtin pytest.approx helper

Expand Down
22 changes: 17 additions & 5 deletions testing/python/raises.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import pytest
import sys


class TestRaises:
def test_raises(self):
Expand Down Expand Up @@ -98,10 +100,13 @@ def test_custom_raise_message(self):
assert False, "Expected pytest.raises.Exception"

@pytest.mark.parametrize('method', ['function', 'with'])
def test_raises_memoryleak(self, method):
import weakref
def test_raises_cyclic_reference(self, method):
"""
Ensure pytest.raises does not leave a reference cycle (#1965).
"""
import gc

class T:
class T(object):
def __call__(self):
raise ValueError

Expand All @@ -112,5 +117,12 @@ def __call__(self):
with pytest.raises(ValueError):
t()

t = weakref.ref(t)
assert t() is None
# ensure both forms of pytest.raises don't leave exceptions in sys.exc_info()
assert sys.exc_info() == (None, None, None)

del t

# ensure the t instance is not stuck in a cyclic reference
for o in gc.get_objects():
assert type(o) is not T

0 comments on commit 1130b9f

Please sign in to comment.