Inconsistent behavior of pytest.deprecated_call()

[netme]

We are currently upgrading py.test from 2.7.1 to 2.8.3 . We have found a small inconsistency in pytest.deprecated_call() behaviour.

We have few tests which are calling deprecated methods in our codebase. And afterwards we have few tests which are checking that the methods are deprecated. Running that tests on 2.7.1 produce no issues. After switching to 2.8.3, we have found out that the tests which are using pytest.deprecated_call() started failing. Here is an example:

import warnings

import pytest


def deprecated_function():
    warnings.warn("deprecated", DeprecationWarning)


def test_one():
    deprecated_function()


def test_two():
    pytest.deprecated_call(deprecated_function)

When running it on 2.8.3, the following AssertionError is raised:

    def test_two():
>       pytest.deprecated_call(deprecated_function)
E       AssertionError: <function deprecated_function at 0x10ac51a28> did not produce DeprecationWarning

On 2.7.1 it works fine.

Seems to be adding warnings.simplefilter('always') call before deprecated_function() call fixes the issue:

import warnings

import pytest


def deprecated_function():
    warnings.warn("deprecated", DeprecationWarning)


def test_one():
    warnings.simplefilter('always')
    deprecated_function()


def test_two():
    pytest.deprecated_call(deprecated_function)

Could you please check this small issue?

[netme]

Also wanted to share an extended example:

import warnings

import pytest


def deprecated_function():
    warnings.warn("deprecated", DeprecationWarning)


def test_one():
    pass


def test_two():
    pytest.deprecated_call(deprecated_function)


def test_three():
    pytest.deprecated_call(deprecated_function)


def test_four():
    deprecated_function()


def test_five():
    pytest.deprecated_call(deprecated_function)

This example fails with the same error around test_five . test_two and test_three are passing.
Hope that can help.

[nicoddemus]

Thanks for the report @netme!

[nicoddemus]

Note: this fails only on Python 2.7.

[netme]

@nicoddemus Thanks for checking it. Yes, forgot to mention. Our product is using Python 2.7.8 .

[netme]

Temporary fixed it using

warnings.simplefilter('always', DeprecationWarning)

call close to our fixtures definition.

[nicoddemus]

Hmm that's strange. In #897 deprecated_call was refactored to use an internal RecordWarnings object which is very similar to the builtin warnings.catch_warnings, but which seems to have some special treatment regarding DeprecationWarning.

The following fails (note that pytest is not even involved):

import warnings

def deprecated_function():
    warnings.warn("deprecated", DeprecationWarning)

with warnings.catch_warnings(record=True) as ws:    
    deprecated_function()

assert len(ws) == 1

If you change deprecated_function to issue a UserWarning instead, then it works. I see this behavior both for Python 2.7 and 3.5. I would expect catch_warnings to capture all warnings, independently from any filters.

deprecated_call in 2.7 monkey-patched both warn and warn_explicit instead, that's why it worked on that version.

Anybody with more knowledge of the warnings module care to comment? @hunse perhaps?

[hunse]

DeprecationWarnings are not raised by Python with the default warning filter set (i.e. the default warning filter suppresses them). As @nicoddemus mentioned, the old deprecated_call monkey patched warn, so that's how it got around the filters.

I think the new behaviour makes sense, because ultimately, you want to test what the user will be seeing, right? If a user looks at your test, and sees that deprecated_function in the test is supposed to be raising a DeprecationWarning, and then runs deprecated_function herself and sees that there's no warning, she's going to wonder why there's a discrepancy. So I think the best thing to do is make sure the warning filters are set at the top of any test function or file testing DeprecationWarnings. One drawback to setting them in the test function is that it will leave them set for subsequent test function calls as well, which may be unexpected.

If we do decide to keep the new behaviour, we should definitely make sure it's documented in the changelog.

Alternatively, we can revert to the old behaviour. The way I would do this is by removing filters for DeprecationWarning at the beginning of deprecated_call, and then set the filters back to the old state at the end of the function.

[hunse]

For interest, here's the reasoning on why DeprecationWarnings are suppressed by default. It looks like unittest re-enables them; maybe pytest should do the same?

[RonnyPfannschmidt]

in my oppinion py.test should always reenable all warnings

@hpk42, @nicoddemus what do you think?

[nicoddemus]

Thanks a lot @hunse for the clear explanation. 😄

[nicoddemus]

@hunse

If a user looks at your test, and sees that deprecated_function in the test is supposed to be raising a DeprecationWarning...
Alternatively, we can revert to the old behaviour.

I think we should revert it: pytest.deprecated_call does not convey that a function is actually raising a warning, but that it has been deprecated (the fact that usually this is done by a warning is secondary I think). If it were named pytest.raises_deprecation_warning then I would agree with you. 😉

IMO keeping the current behavior would just lead to confusion, as users would see it fail all the time, unless they explicitly reset the filters themselves; seems like a poor user experience to me.

@RonnyPfannschmidt

in my oppinion py.test should always reenable all warnings

Personally I don't use warnings much so I don't have a strong opinion on this, but following what the unittest module does it makes sense to me for pytest to do the same, besides making porting unittest code more straightforward.

If we agree to this, I think we should re-enable all warnings by default in 2.9 only.

[nicoddemus]

Created a separate issue to discuss re-enabling all warnings. 😄