pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
12.1k stars 2.68k forks source link

test_exception_handling_no_traceback leaking FDs (LsofFdLeakChecker plugin) #2366

Open nicoddemus opened 7 years ago

nicoddemus commented 7 years ago

While implementing #2292, it was discovered that LsofFdLeakChecker plugin declares a hook wrapper incorrectly as pytest_runtest_item instead of pytest_runtest_protocol.

AFAICT, the hook was introduced in db5649ec6a8fcd9ef148b9f77797a22cb9f3cda1 already using the incorrect name, so it seems it never checked for leaked file descriptors. @hpk42 could you comment here?

Fixing the hook name enabled the plugin and it is now crashing the test suite because of some leaked fds (example log).

In order to move #2292 forward, the plugin was changed to emit a warning instead of failing the suite.

We need to decide if we want to keep the plugin and fix the tests, or just remove it altogether.

nicoddemus commented 7 years ago

Update: after turning the failure into a warning, it seems only test_exception_handling_no_traceback is leaking fds, and on this Python versions:

hpk42 commented 7 years ago

FWIW and just for quick comment (i leave tomorrow for a couple of days) the leak checker did work some time ago as i used it to fix several leaks. The warnings do sound legit so probably should be fixed and the plugin kept ... unless i am missing something.

nicoddemus commented 7 years ago

Thanks @hpk42! Now that it is clear that a single test is failing, I agree it should be fixed.

The-Compiler commented 7 years ago

Two things I noticed:

blueyed commented 6 years ago

JFI: it was introduced in d4fe273b initially (not db5649e like stated initially).

blueyed commented 5 years ago

Tried it with https://github.com/pytest-dev/pytest/pull/5006, but there are still issues. Just wondered how it should be enabled / used to fail single test only - using pytest.fail in pytest_runtest_protocol makes pytest throw an internal error. But using pytest_runtest_setup / pytest_runtest_teardown is not early enough I assume.