Skip to content

Docs on warnings filters should note that ResourceWarning is often delayed #9825

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

Open
dchevell opened this issue Mar 28, 2022 · 6 comments
Open
Labels
type: docs documentation improvement, missing or needing clarification

Comments

@dchevell
Copy link

Some of my tests need to ensure open files are closed. Let's use the simplest example:

# test_example.py
import pytest

@pytest.mark.filterwarnings('error::ResourceWarning')
def test_resourcewarning():
    open('/dev/null')

When I run pytest, the warning is thrown and even printed in the test output, but the test still passes:

$ pytest
======================================== test session starts =========================================
platform darwin -- Python 3.10.2, pytest-7.1.1, pluggy-1.0.0
rootdir: /path/to/project
collected 1 item

test_example.py .                                                                              [100%]

========================================== warnings summary ==========================================
test_example.py::test_resourcewarning
  /path/to/python/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: 
Exception ignored in: <_io.FileIO [closed]>

  Traceback (most recent call last):
    File "/path/to/project/test_example.py", line 5, in test_resourcewarning
      open('/dev/null')
  ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='r' encoding='UTF-8'>

    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
==================================== 1 passed, 1 warning in 0.01s ====================================

Removing @pytest.mark.filterwarnings or changing the warning type to something unrelated (like DeprecationWarning) results in the test passing without printing any warnings at all. That tells me pytest is picking up the warning, but it's being subsequently caught by pytest.PytestUnraisableExceptionWarning, and my tests still pass because I wasn't filtering for that. If I filter for pytest.PytestUnraisableExceptionWarning instead the test also passes, because it isn't looking for the original ResourceWarning.

The only solution I can think of is to filter for both:

@pytest.mark.filterwarnings('error::ResourceWarning')
@pytest.mark.filterwarnings('error::pytest.PytestUnraisableExceptionWarning')

Unless I'm missing something this seems to be a bug for ResourceWarning in particular, since I can't reproduce this with other warning types. I think failing the test case without requiring the 2nd generic warning filter is the reasonable expected behaviour here.

Note: this test example was run on a vanilla virtual env:

$ pip freeze
attrs==21.4.0
iniconfig==1.1.1
packaging==21.3
pluggy==1.0.0
py==1.11.0
pyparsing==3.0.7
pytest==7.1.1
tomli==2.0.1
@nicoddemus
Copy link
Member

I think ResourceWarning is special in the sense that it is not triggered immediately by what causes it (the open call in your case), but raised later by the interpreter when it finally notices the resource leak.

@dchevell
Copy link
Author

That would explain the difference in behaviour, at least.

On the one hand it's easy to make an argument not to special case this sort of thing - workarounds are easy enough to find - on the other, warnings are meant to be about abnormal behaviour so perhaps special cases are warranted. If nothing else, could be worth a note in the docs that some categories of warnings need additional filters.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 19, 2022

Where specifically in the docs would you have seen such a warning? I'd be happy for someone to add one (perhaps in multiple places), but to be of any use it has to be somewhere that people would see...

@Zac-HD Zac-HD added the type: docs documentation improvement, missing or needing clarification label Apr 19, 2022
@Zac-HD Zac-HD changed the title pytest.mark.filterwarnings doesn't correctly catch ResourceWarning Docs on warnings filters should note that ResourceWarning is often delayed Apr 19, 2022
@dchevell
Copy link
Author

@Zac-HD I see you've relabelled this as docs - is that a confirmation that filtering for warnings not working as expected is … expected?

My docs comment is more of an "if this is simply too hard to fix" (which it might be) but I'd still consider this a bug, in that a feature designed to catch warnings fails to catch a significant class of warning. It seems worth keeping this open as a bug to at least explore a fix rather than just documenting places where a feature doesn't work.

In any case, I had been scouring https://docs.pytest.org/en/6.2.x/warnings.html when looking at this, so a "here are cases where this feature doesn't work" note for ResourceWarning would make the most sense there.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 19, 2022

It's not behaving how you expect, but it is behaving how I expect it to - and IMO that means it's a docs issue. And yes, unfortunately it's not feasible to change this, because the ResourceWarning literally doesn't exist in the scope that we want to handle it; the warning is only raised much later during garbage collection. In fact the language doesn't guarantee that you'll ever get a ResourceWarning, and so sometimes (often, on PyPy) you simply won't!

So I think an explanatory section much like the "DeprecationWarning and PendingDeprecationWarning" section, and located just below it", would be useful here 🙂

@dchevell
Copy link
Author

Your explanation of expected behaviour is precisely what I was looking for, without that the change to docs wasn't very clear.

Agreed on the explanatory section, perhaps the solution of filtering in combination with 'error::pytest.PytestUnraisableExceptionWarning' would fit there as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

No branches or pull requests

3 participants