Closed seberg closed 4 years ago
pv
commented
4 years ago Run on what project? I'm not seeing problems with Scipy or Numpy (for numpy see gh-34, but that was problem also before). I'm also using pytest-5.3.1. I'd need a repro to say something.
seberg
commented
4 years ago Ah, sorry, yes numpy. Must be gh-34...
seberg
commented
4 years ago I will investigate a bit. Thanks, sorry for the noise.
seberg
commented
4 years ago Bisected it to this: (commit 96c788d4cfd0fa051b58b16605e5bfc3c175ceb6)
diff --git a/pytest_leaks/plugin.py b/pytest_leaks/plugin.py
index e4cae38..95fc5b8 100644
--- a/pytest_leaks/plugin.py
+++ b/pytest_leaks/plugin.py
@@ -120,12 +120,22 @@ class LeakChecker(object):
doctest_original_globs = None
def run_test():
+ hasrequest = hasattr(item, "_request")
+ if hasrequest and not item._request:
+ item._initrequest()
+
when[0] = "setup"
hook.pytest_runtest_setup(item=item)
when[0] = "call"
hook.pytest_runtest_call(item=item)
when[0] = "teardown"
hook.pytest_runtest_teardown(item=item, nextitem=nextitem)
+
+ if hasrequest:
+ # Ensure fixtures etc are reset properly
+ item._request = False
+ item.funcargs = None
+
if doctest_original_globs is not None:
item.dtest.globs.update(doctest_original_globs)After the commit I have general leaks with NumPy, Before that commit things seem fine.
pv
commented
4 years ago Without it, fixtures don't run on each repeat. Indeed not running fixtures stops them from leaking, but that's not the correct thing to do.
So what you're seeing is gh-34. Numpy can disable the problematic doctest namespace fixture if running pytest-leaks for now, and some effort could be spent on tracking down why some types of fixtures leak.
On December 2, 2019 11:41:57 PM UTC, Sebastian Berg notifications@github.com wrote:
Bisected it to this:
diff --git a/pytest_leaks/plugin.py b/pytest_leaks/plugin.py index e4cae38..95fc5b8 100644 --- a/pytest_leaks/plugin.py +++ b/pytest_leaks/plugin.py @@ -120,12 +120,22 @@ class LeakChecker(object): doctest_original_globs = None def run_test(): + hasrequest = hasattr(item, "_request") + if hasrequest and not item._request: + item._initrequest() + when[0] = "setup" hook.pytest_runtest_setup(item=item) when[0] = "call" hook.pytest_runtest_call(item=item) when[0] = "teardown" hook.pytest_runtest_teardown(item=item, nextitem=nextitem) + + if hasrequest: + # Ensure fixtures etc are reset properly + item._request = False + item.funcargs = None + if doctest_original_globs is not None: item.dtest.globs.update(doctest_original_globs)After the commit I have general leaks with NumPy, Before that commit things seem fine.
pv
commented
4 years ago Pytest perhaps has some global registry to keep track of fixtures that have non-test scope, or the (autouse and module-scope only?) fixtures themselves accumulate something. Anyway, would need to read pytest sources again to understand what it does here
On December 2, 2019 11:41:57 PM UTC, Sebastian Berg notifications@github.com wrote:
Bisected it to this:
diff --git a/pytest_leaks/plugin.py b/pytest_leaks/plugin.py index e4cae38..95fc5b8 100644 --- a/pytest_leaks/plugin.py +++ b/pytest_leaks/plugin.py @@ -120,12 +120,22 @@ class LeakChecker(object): doctest_original_globs = None def run_test(): + hasrequest = hasattr(item, "_request") + if hasrequest and not item._request: + item._initrequest() + when[0] = "setup" hook.pytest_runtest_setup(item=item) when[0] = "call" hook.pytest_runtest_call(item=item) when[0] = "teardown" hook.pytest_runtest_teardown(item=item, nextitem=nextitem) + + if hasrequest: + # Ensure fixtures etc are reset properly + item._request = False + item.funcargs = None + if doctest_original_globs is not None: item.dtest.globs.update(doctest_original_globs)After the commit I have general leaks with NumPy, Before that commit things seem fine.
Using an older version I can run pytest-leaks fine. But the newer master version seems to generally create Leaks, I tried with pytest 3.10.1 and at least pytest 5.3.1 (possibly another one I had installed before). Going back to the version I used previously (may have been a weird state), things seem to run fine.
I may be missing a point about the pytest compatibility here maybe. Not sure if 5.3.1 should have worked to begin with, but thoguht you may notice/know something quickly.
(reduced) Diff from master to old version:
```diff diff --git a/pytest_leaks/__init__.py b/pytest_leaks/__init__.py index 0a2bb53..b484438 100644 --- a/pytest_leaks/__init__.py +++ b/pytest_leaks/__init__.py @@ -1 +1,5 @@ -__version__ = "0.3.dev" +try: + # _version.py is written by setuptools-scm + from ._version import version as __version__ +except ImportError: + __version__ = "unknown" diff --git a/pytest_leaks/plugin.py b/pytest_leaks/plugin.py index e4cae38..7c4b627 100644 --- a/pytest_leaks/plugin.py +++ b/pytest_leaks/plugin.py @@ -6,6 +6,7 @@ from __future__ import print_function import sys import re +import json from collections import OrderedDict @@ -70,6 +71,12 @@ def pytest_configure(config): checker = LeakChecker(config) config.pluginmanager.register(checker, 'leaks_checker') + config.addinivalue_line( + "markers", + "no_leak_check(fail=False, reason=""): don't run pytest-leaks on " + "this test, optionally failing the leak test without checking with " + "some reason given.") + @pytest.fixture def leaks_checker(request): @@ -102,13 +109,23 @@ class LeakChecker(object): # Get access to the builtin "runner" plugin. self.runner = config.pluginmanager.get_plugin('runner') - self.leaks = {} # nodeid -> leaks + + # Temporary storage for leak data + self._leaks = {} # item.nodeid -> result def hunt_leaks(self, func): return hunt_leaks(func, self.stab, self.run) @pytest.hookimpl(tryfirst=True) def pytest_runtest_protocol(self, item, nextitem): + marker = item.get_closest_marker('no_leak_check') + if marker: + # Don't run leak check + if marker.kwargs.get('fail'): + reason = marker.kwargs.get('reason', "") + self._leaks[item.nodeid] = {'(not checked)': reason} + return + when = ["setup"] hook = item.ihook @@ -120,15 +137,29 @@ class LeakChecker(object): doctest_original_globs = None def run_test(): + hasrequest = hasattr(item, "_request") + if hasrequest and not item._request: + item._initrequest() + when[0] = "setup" hook.pytest_runtest_setup(item=item) when[0] = "call" hook.pytest_runtest_call(item=item) when[0] = "teardown" hook.pytest_runtest_teardown(item=item, nextitem=nextitem) + + if hasrequest: + # Ensure fixtures etc are reset properly + item._request = False + item.funcargs = None + if doctest_original_globs is not None: + # Restore doctest environment item.dtest.globs.update(doctest_original_globs) + # Clear pytest captured output etc., if any + item._report_sections = [] + if hasattr(self.runner.CallInfo, 'from_call'): # pytest >= 4 from _pytest.outcomes import Exit @@ -154,16 +185,40 @@ class LeakChecker(object): location=item.location) return True # skip pytest implementation else: - self.leaks[item.nodeid] = call.result + self._leaks[item.nodeid] = call.result return # proceed to pytest implementation + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_makereport(self, item, call): + outcome = yield + + # Append leak report in 'call' phase + if call.when != 'call': + return + + report = outcome.get_result() + leaks = self._leaks.pop(item.nodeid, None) + if leaks: + report.sections.append(('pytest-leaks', json.dumps(leaks))) + outcome.force_result(report) + + def _leaks_from_report(self, report): + if report.when != "call": + return None + + leaks = [data for key, data in report.get_sections('pytest-leaks') + if key == 'pytest-leaks'] + if leaks: + return Leaks(json.loads(leaks[0])) + + return None + @pytest.hookimpl(hookwrapper=True, trylast=True) def pytest_report_teststatus(self, report): outcome = yield if report.when == 'call' and report.outcome == 'passed': - leaks = self.leaks.get(report.nodeid) - if leaks: + if self._leaks_from_report(report): # cat, letter, word outcome.force_result(('leaked', 'L', 'LEAKED')) @@ -171,19 +226,19 @@ class LeakChecker(object): def pytest_terminal_summary(self, terminalreporter, exitstatus): tr = terminalreporter - leaked = list(tr.getreports('leaked')) - if 'pytest_sugar' in type(tr).__module__: # pytest-sugar doesn't run pytest_report_teststatus: ensure # leak summary gets shown - for rep in tr.getreports('passed'): - if self.leaks.get(rep.nodeid) and rep.when == "call": - leaked.append(rep) + leaked = list(tr.getreports('passed')) + else: + leaked = list(tr.getreports('leaked')) if leaked: tr.write_sep("=", 'leaks summary', cyan=True) for rep in leaked: - tr.line("%s: %s" % (rep.nodeid, Leaks(self.leaks[rep.nodeid]))) + leaks = self._leaks_from_report(rep) + if leaks: + tr.line("%s: %s" % (rep.nodeid, leaks)) class Namespace(object): diff --git a/pytest_leaks/support.py b/pytest_leaks/support.py index f794996..6dddf89 100644 --- a/pytest_leaks/support.py +++ b/pytest_leaks/support.py @@ -24,7 +24,7 @@ def fd_count(): # Substract one because listdir() opens internally a file # descriptor to list the content of the /proc/self/fd/ directory. return len(names) - 1 - except FileNotFoundError: + except OSError: pass MAXFD = 256 diff --git a/setup.py b/setup.py index 73cae91..1f26835 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ AUTHORS = [ setup( name='pytest-leaks', - version='0.3.0', + use_scm_version={"write_to": "pytest_leaks/_version.py"}, author=", ".join(AUTHORS), author_email='alexander.belopolsky@gmail.com', maintainer=", ".join(AUTHORS), @@ -28,9 +28,11 @@ setup( license='MIT and Python-2.0', url='https://github.com/abalkin/pytest-leaks', description='A pytest plugin to trace resource leaks.', - long_description=read('README.rst'), + long_description=read('README.md'), + long_description_content_type="text/markdown", packages=['pytest_leaks'], install_requires=['pytest>=3'], + setup_requires=["setuptools-scm", "setuptools>=40.0"], python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*', classifiers=[ 'Development Status :: 1 - Planning', @@ -46,7 +48,6 @@ setup( 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: Implementation :: CPython', - 'Programming Language :: Python :: Implementation :: PyPy', 'Operating System :: OS Independent', 'License :: OSI Approved :: Python Software Foundation License', 'License :: OSI Approved :: MIT License', diff --git a/tests/test_leaks.py b/tests/test_leaks.py index 9681cde..7edaa18 100644 --- a/tests/test_leaks.py +++ b/tests/test_leaks.py @@ -3,8 +3,9 @@ import sys import pytest -if not hasattr(sys, 'gettotalrefcount'): - pytest.fail('python debug build compiled with --with-pydebug is required') +pytestmark = pytest.mark.skipif( + not hasattr(sys, 'gettotalrefcount'), + reason='python debug build compiled with --with-pydebug is required') def test_config_options_fixture(testdir): @@ -101,6 +102,21 @@ def test_leaks_checker(testdir): assert result.ret == 0 +def test_xdist_compatibility(testdir, request): + if not request.config.pluginmanager.hasplugin('xdist'): + pytest.skip('test requires pytest-xdist') + + testdir.makepyfile(test_leaks_code) + testdir.plugins = ['xdist', 'leaks'] + result = testdir.runpytest_subprocess('-R', ':', '-v', '-n2') + result.stdout.fnmatch_lines([ + '*LEAKED*::test_refleaks*', + '*leaks summary*', + '*:test_refleaks: leaked references*', + ]) + assert result.ret == 0 + + leaking = None test_leaks_code = """ garbage = [] @@ -177,3 +193,130 @@ def test_doctest_failure(testdir): ]) assert result.ret == 1 + + +def test_doctest_pass(testdir): + test_code = """ + class SomeClass(object): + ''' + >>> SomeClass() + <... + ''' + """ + + testdir.makepyfile(test_code) + + # XXX: fails without subprocess (gh-12) + result = testdir.runpytest_subprocess( + '-R', ':', '--doctest-modules', '-v' + ) + + result.stdout.fnmatch_lines([ + "*::test_doctest_pass.SomeClass PASSED*" + ]) + + +def test_fixture_setup_teardown(testdir): + test_code = """ + import pytest + + global_state = False + global_count = 0 + prev_global_count = -1 + + @pytest.fixture + def myfixture(): + global global_state, global_count + global_state = True + try: + yield + finally: + global_state = False + global_count += 1 + + def test_sth(myfixture): + global prev_global_count + assert global_state + assert global_count > prev_global_count + prev_global_count = global_count + """ + + testdir.makepyfile(test_code) + + result = testdir.runpytest_subprocess( + '-R', ':', '-v' + ) + + result.stdout.fnmatch_lines([ + "*::test_sth PASSED*" + ]) + + +def test_output_capture(testdir): + test_code = """ + def test_output_capture_default(): + print('Hello there.') + + def test_output_capture_capsys(capsys): + print('Hello here.') + out, err = capsys.readouterr() + assert out.strip() == 'Hello here.' + print('Hello where.') + + def test_output_capture_capfd(capfd): + print('Hello their.') + out, err = capfd.readouterr() + assert out.strip() == 'Hello their.' + print('Hello these.') + """ + + testdir.makepyfile(test_code) + + result = testdir.runpytest_subprocess( + '-R', ':', '-v' + ) + + result.stdout.fnmatch_lines([ + "*::test_output_capture_default PASSED*", + "*::test_output_capture_capsys PASSED*", + "*::test_output_capture_capfd PASSED*", + ]) + + +def test_marker_skip_leaks(testdir): + test_code = """ + import pytest + + garbage = [] + + @pytest.mark.no_leak_check + def test_leaking_skip(): + garbage.append(None) + + def test_leaking_noskip(): + garbage.append(None) + + @pytest.mark.no_leak_check(fail=True, reason="something") + def test_leaking_skip_fail(): + pass + + @pytest.mark.no_leak_check(fail=False, reason="something") + def test_leaking_skip_success(): + garbage.append(None) + """ + + testdir.makepyfile(test_code) + + result = testdir.runpytest_subprocess( + '-R', ':', '-v' + ) + + result.stdout.fnmatch_lines([ + "*::test_leaking_skip PASSED*", + "*::test_leaking_noskip LEAKED*", + "*::test_leaking_skip_fail LEAKED*", + "*::test_leaking_skip_success PASSED*", + "*leaks summary*", + "*::test_leaking_noskip: leaked references*", + "*::test_leaking_skip_fail: leaked*something*", + ]) diff --git a/tox.ini b/tox.ini index 6156be3..41dffb1 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,9 @@ envlist = flake8,rstlint,python2-debug,python3-debug,python2.7-dbg,python3-dbg,p skip_missing_interpreters = true [testenv] -deps = pytest +deps = + pytest + pytest-xdist commands = pytest {posargs:tests} [testenv:flake8] @@ -58,6 +60,7 @@ basepython = {envname} [testenv:python3.5-dbg] # Debian +deps = pytest<5.1.0 basepython = {envname} [testenv:python3.6-dbg] ```