Skip to content

Establish best practices for errors on warnings - of which some get ignored #3800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RonnyPfannschmidt opened this issue Aug 10, 2018 · 8 comments
Labels
plugin: warnings related to the warnings builtin plugin type: enhancement new feature or API change, should be merged into features branch

Comments

@RonnyPfannschmidt
Copy link
Member

follow-up to pypa/setuptools-scm#305

it i blatantly missed an ignored exception from a resource warning due to green test results while setting an error for all warnings

pytest should either establish a well known best practice for configuring that, or go yellow on those errors that get ignored by the stdlib for a good reason

@nicoddemus
Copy link
Member

I believe this is a duplicate of #2908? If so I propose to close this in favor of #2908 as it contains more information.

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus its unrelated - the problem here is that i did set a global error on all - but the resoruce warning happens in a place where the exception is ignored by the stdlib, so i missed the problem since i expected a hard failure, instead i got an "exception ignored"

@nicoddemus
Copy link
Member

I see, thanks. I think we might catch that once #3251 lands, but more investigation would be needed.

@nicoddemus nicoddemus added type: enhancement new feature or API change, should be merged into features branch plugin: warnings related to the warnings builtin plugin labels Aug 10, 2018
@nicoddemus nicoddemus changed the title etablish best practices for errors on warnings - of which some get ignored Establish best practices for errors on warnings - of which some get ignored Aug 10, 2018
@SnarkBoojum
Copy link

Hmmm... this isn't about a deprecation warning, is it?

@RonnyPfannschmidt
Copy link
Member Author

@SnarkBoojum no, its about a RessourceWarning triggering in a place where python does a catchall

to catch that pytest would need to track those exceptions as well and return a error exit state

@Lekensteyn
Copy link

The error is written to stderr when Python cannot handle an exception (in __del__ or during GC). Not sure how to properly check for it except explicitly capturing and checking stderr with capsys.

Here is a minimal reproducer:

test_resleak.py:

def test_resleak():
    f = open(__file__)

Output of pytest -sv -Werror:

============================= test session starts ==============================
platform darwin -- Python 3.7.3, pytest-4.4.1, py-1.8.0, pluggy-0.9.0 -- /usr/local/opt/python/bin/python3.7
cachedir: .pytest_cache
rootdir: /Users/me/tmp/pyex
plugins: xdist-1.28.0, forked-1.0.2
collecting ... collected 1 item

test_resleak.py::test_resleak PASSED

=========================== 1 passed in 0.01 seconds ===========================
Exception ignored in: <_io.FileIO name='/Users/me/tmp/pyex/test_resleak.py' mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.TextIOWrapper name='/Users/me/tmp/pyex/test_resleak.py' mode='r' encoding='UTF-8'>

@nicoddemus
Copy link
Member

nicoddemus commented Apr 26, 2019

I think by that point we are already outside pytest.main() so, I don't see what pytest can do.

@nicoddemus
Copy link
Member

I'm closing this for now: the exceptions/warnings mentioned here happen after pytest.main() has exited, so I don't see how we can implement this (if somebody does, please feel free to reopen with a suggestion). 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: warnings related to the warnings builtin plugin type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

4 participants