Make warnings plugin opt-in #2443
Conversation
There was a problem hiding this comment.
if we just disable it that way, we would have to do a 4.0 release and i would like to get feedback from @hpk42
my understanding is that the main cause is in
https://github.com/pytest-dev/pytest/blob/master/_pytest/warnings.py#L53-L56
the most apparent blunder being the addition of the once filter
personally i'd prefer to just strike out that filter, so that we don't change any behavior unless requested by options
| @@ -99,7 +99,7 @@ def directory_arg(path, optname): | |||
| "mark main terminal runner python fixtures debugging unittest capture skipping " | |||
| "tmpdir monkeypatch recwarn pastebin helpconfig nose assertion " | |||
| "junitxml resultlog doctest cacheprovider freeze_support " | |||
| "setuponly setupplan warnings").split() | |||
| "setuponly setupplan").split() | |||
There was a problem hiding this comment.
if done like that, we will have to do a 4.0 release
There was a problem hiding this comment.
the most apparent blunder being the addition of the once filter
Not really, unfortunately catch_warnings will remove all current filters in order to be able to catch all warnings. That filter is there just to prevent the same warning to be triggered more than once by the same test.
How would you like to make this change without making a 4.0 release? Or do you want to make a 4.0 release regardless?
|
I'm not sure what to think about this... I can see it's problematic how it broke some stuff, but with this we turn loud failures into silent failures. Now that everyone removed the |
|
@The-Compiler we shouldn't take responsibility for crude workarounds instead of version pins if at all we should in future warn if people disable core bits |
|
What crude workarounds? I'm not talking about people disabling anything. I'm talking about us disabling something which was enabled before, and that enabling causing people to get rid of custom solutions. |
|
@The-Compiler then we need a correct default filter - because the one we use is broken with absolute certainty |
|
As @nicoddemus mentioned in #2443 (comment) it's probably not as easy as that. |
|
@The-Compiler the assessment that catch_warnings removes all filters is incorrect, as per code and doc-string it does a copy, and restores that backup |
|
...which still has the same effect: All existing filters are disabled while a test runs. |
|
@The-Compiler incorrect, filters add to it, and one too simple filter added to the filter list simply overrides everything because it matches first |
|
Closing in favor of #2445 |
Fix #2430