pytest: Warning integration breaks warnings.filterwarnings

SQLAlchemy CI is now broken due to the unconditional inclusion of pytest-warnings in py.test 3.1.0. I need at least to know how to disable this plugin entirely.

@zzzeek you can disable the warnings plugin by passing -p no:warnings; this will disable the plugin completely and you should get green builds again. You can choose to add this to your pytest.ini file:

[pytest]
addopts = -p no:warnings

As for the problem itself, I will take a more careful look later.

@zzzeek thanks for the comprehensive and detailed response! I really appreciate your time to write it, it is invaluable to us to get the whole picture. 👍

We (the core team) feel that most warnings go unnoticed, specially deprecation warnings where action about the deprecation is usually taken only when things actualy brake, long after the initial deprecation warnings started to be emited. We made good strides towards this by always showing that some warnings were emitted in the summary (“X pytest-warnings”), and later on by finally deciding to always show warnings regardless if the user opt-in to see them (although it was easy to opt-out). Given this experience, our general conclusion is that pytest should display warnings more proemiently because otherwise very few users will bother to opt-in to see them.

So we decided to introduce the pytest-warnings plugin to the core: improve communication on deprecated pytest features and to give users ample time to deal with them at their own pace, but also important for warnings from libraries in general.

Now, your test suite has an advanced treatment of warnings and that unfortunately conflicts with pytest’s own warning treatment. That’s unfortunate but also understandable, and I can see where the same problem could happen with other internal plugins: the terminal plugin and pytest-xdist comes to mind and I also can imagine a test suite which does its own stdin/stdout capture can conflict with the standard pytest capture for example.

Given that pytest’s warning handling is a problem only in the few advanced and rare cases, having it on by default and in consequence benefit the larger user base is the best long term decision, specially if it is easily opted out.

As @The-Compiler mentioned, this is also apparently the approach taken by the standard library:

# content of pytest_warnings.py
import unittest
import warnings

class MyWarning(Warning):
    pass

warnings.filterwarnings("error", category=MyWarning)

class TestWarnings(unittest.TestCase):
    def test_my_warning(self):
        try:
            warnings.warn(MyWarning("warn!"))
            assert False
        except MyWarning:
            assert True

$ python -m unittest test_warnings
test_warnings:13: MyWarning: warn!
  warnings.warn(MyWarning("warn!"))
F
======================================================================
FAIL: test_my_warning (foo.TestWarnings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_warnings.py", line 14, in test_my_warning
    assert False
AssertionError

----------------------------------------------------------------------
Ran 1 test in 0.002s

FAILED (failures=1)

(This is the original example posted, only adapted to standard unittest)

This further makes me believe that we are following the correct course of action: the most value out of the box for most users, with an easy way to opt-out when it is problematic. Of course we should improve the documentation on how to op-out if this is the instance we will ultimately go with.

Well that’s the full picture as I see it. Would love to hear other’s opinions on this.

Having a warning system is hugely useful. Defaulting to all warnings on is not.

Even if everyone decides to leave this feature in, it’s broken. Only reasonable course of action is to back it out until it’s more thoroughly tested.ASAP

1. This code shows a warning that should not be on by default:

test_something.py

class TestHelper():
  def __init__(self):
    pass

  def someNonTestStuff(self):
    pass

(venv) $ pytest test_something.py 
========================== test session starts ==========================
platform darwin -- Python 3.6.1, pytest-3.1.0, py-1.4.33, pluggy-0.4.0
rootdir: /Users/okken/foo, inifile:
collected 0 items 

=========================== warnings summary ============================
test_something.py::TestHelper
  cannot collect test class 'TestHelper' because it has a __init__ constructor

-- Docs: http://doc.pytest.org/en/latest/warnings.html
====================== 1 warnings in 0.00 seconds =======================

2. --strict doesn’t work

(venv) $ pytest --help
...
  --strict              run pytest in strict mode, warnings become errors
...

(venv) $ pytest --strict test_something.py 
========================== test session starts ==========================
platform darwin -- Python 3.6.1, pytest-3.1.0, py-1.4.33, pluggy-0.4.0
rootdir: /Users/okken/projects/book/bopytest/Book/code/foo, inifile:
collected 0 items 

=========================== warnings summary ============================
test_something.py::TestHelper
  cannot collect test class 'TestHelper' because it has a __init__ constructor

-- Docs: http://doc.pytest.org/en/latest/warnings.html
====================== 1 warnings in 0.01 seconds =======================

@zzzeek

a filter that wants to ensure a certain class of warnings is errored is still IMO a little better expressed in code rather than config since it refers to a class that’s part of the library being tested.

This is a very valid point, thanks. This seems like a feature request though, would you like to create a new issue for it so we can track it?

Python does hide DeprecationWarnings (and some others?) by default, because it doesn’t want to confront users with those when they’re simply using a library.

Test runners should turn them on (like unittest.py does too), so they aren’t simply ignored.

@The-Compiler this is not about showing vs hiding, but about not altering the warnings subsystem unless requested

python warnings are inherently state-full, and people register different actions for different warnings any test-runner that changes anything by default is broken

note that warnings are not hidden by default, but library authors may choose to hide/error on certain errors, or decide to show some errors only once

changing those behaviours inwarranted breaks code

any choice we make breaks some peoples expectations, we should jsut let the people decide

we simply should not do anying per default