Context manager to ensure no warnings are issued

[nicoddemus]

Follow up to #9386, starting this thread to discuss how to support the use case of "inside the given with block, no warning should be generated".

Seems most users assumed this idiom:

with pytest.warns(None):
    ...

Would ensure that the code inside the with block would not issue any warning, raising an error if it did.

However that was never the case, and pytest.warns(None) would do nothing (warning or no warning generated). To warn users about this common mistake, we added a deprecation warning when None was passed to pytest.warns (#8677).

When 7.0.0rc1 was released, some issues were reported about this: #9402, #9386.

Users would like a way to ensure a block of code does not raise any warning, something which we never really supported.

Our current suggestion is to use warnings.catch_warnings(), followed by simplefilter("error"):

with warnings.catch_warnings():
    warnings.simplefilter("error")
    ...

However as mentioned in #9402, this does not apply directly if you have a dynamic check:

warn_typ = FutureWarning if input == "nc" else None
with pytest.warns(warn_typ):
    ...

So far we have the following proposals:

1. Keep things as is, improving the docs and suggesting catch_warnings (Improve pytest.warns() docs to clarify difference with catch_warnings() #9002).
2. Introduce a new context manager, pytest.does_not_warn() ([prerelease] pytest.warns() doesn't seem to work properly for coverage.py #9386 (comment)).
3. Change pytest.warns(None) to mean that no warning should be raised (FR: Revert pytest.warns(None) changes #9402 (comment)).

I think this is worth reaching a consensus before the 7.0.0 release is out.

[WarrenWeckesser]

Seems most users assumed this idiom [...] Would ensure that the code inside the with block would not issue any warning, raising an error if it did.

I mentioned over in #9002 that SciPy had three uses of pytest.warns(None). Since you are discussing a design issue, I want to clarify exactly what those tests were doing.

The tests were of the form:

with pytest.warns(None) as record:
    <do some stuff>
    <check record for warnings, and possibly raise an error>

The tests were not assuming that the pytest context code would raise an error if a warning occurred. Instead, these tests were doing their own error checking based on record. So they were really using pytest.warns(None) exactly like warnings.catch_warnings(record=True). (There is a pull request to make the change to catch_warnings.)

[bashtage]

Thanks for opening this. It definitely feels helpful to have a pytest-based method to assert that code produces no (visible) warnings. We (statsmodels) use the dynamic pattern in parameterized tests and sometimes with upstream dependencies (e.g., NumPy 1.23 warns but <=1.22 does not). I can imagine a couple of APIs that would be friendly.

1. Make None mean no warnings. Would change the intent of the deprecation but could raise warnings when used in a block with pytest.raises(None) and a warning is raised that this needs to be fixed by pytest 8.
2. Accept a sequence in pytest.raises so that an empty sequence would mean no warnings are raised. If more than 1 warning is in the seeuence, assert that all warnings listed are raised,

# pass
with pytes.warn([FutureWarning, RuntimeWarning]):
    warnings.warn("",FutureWarning)
    warnings.warn("",RuntimeWarning)

# error
with pytes.warn([FutureWarning, RuntimeWarning]):
    warnings.warn("",FutureWarning)

# error
with pytes.warn([]):
    warnings.warn("",FutureWarning)

This approach would replace the often indented use of None with and empty sequence.

FWIW NumPy has numpy.testing.assert_no_warnings (undocumented IIRC, but something like this)

[nicoddemus]

Gentle ping for the maintainers. 😁

I think it is worthwhile to discuss this, even if we decide to go with option 1 (do nothing).

[Zac-HD]

1. Make None mean no warnings. Would change the intent of the deprecation but could raise warnings when used in a block with pytest.raises(None) and a warning is raised that this needs to be fixed by pytest 8.

I'd be OK with either this, or with an explicit with pytest.does_not_warn(): helper. I prefer the latter but recognise that convenience actually does matter and the risk of misunderstandings and consequent test bugs isn't that bad.

I am strongly opposed to the old "allow any or no warnings" though; there's the stdlib warnings.catch_warnings(record) for that!

2. Accept a sequence in pytest.raises so that an empty sequence would mean no warnings are raised. If more than 1 warning is in the seeuence, assert that all warnings listed are raised, ...

pytest.warns() already accept a sequence of Warning classes, with the meaning "instance of any of these" - just like isinstance(..., (int, float)) for example. Same for pytest.raises(), though I think that's a typo?

[bashtage]

pytest.warns() already accept a sequence of Warning classes, with the meaning "instance of any of these" - just like isinstance(..., (int, float)) for example. Same for pytest.raises(), though I think that's a typo?

I checked the docs after I wrote that comment and saw that a sequence is accepted. This would then be the more forward-compatible way IMO.

exp_warn = FutureWarning if COND else [] # or (,)

with pytest.warns(exp_warn):
    code_with_possible_changes()

An empty sequence seems very consistent with the sequence interpretation that all warnings in the list must be raised and no others are allowed).

What happens now with an empty sequence? Is pytest.warns([]) the same as pytest.warns()?

The different context managers is a bit painful to use in runtime scenarios. It would need to be something like

cm = pytest.raises(FutureWarning) if COND else pytest.does_not_raise()
with cm:
    code_with_possible_changes()

[bashtage]

I am strongly opposed to the old "allow any or no warnings" though; there's the stdlib warnings.catch_warnings(record) for that!

I also think this is the right approach. Any or None is not a test, and so doesn't belong in a test runner.

[Zac-HD]

An empty sequence seems very consistent with the sequence interpretation that all warnings in the list must be raised and no others are allowed).

That's not quite right; the block must emit a warning such that isinstance(w, expected_warning) is True. For sequences, that's equivalent to writing any(isinstance(w, ex) for ex in expected_warning).

What happens now with an empty sequence? Is pytest.warns([]) the same as pytest.warns()?

any([]) == False, so this always fails: there are no possible warnings which are an instance of (). See #8646 for discussion of raising an earlier error to explain what's going on with that.

[bashtage]

Sorry - mental tick earlier when I wrote raises instead of warns.

I guess either way is a somewhat breaking change. The empty sequence feels low consequence since it cannot pass now. The None is probably cleaner in terms of reading code.

with pytest.warns(None):
    does_not_warn()

with pytest.warns([]):
    does_not_warn()

# Cleanest, but harder to use at runtime
with pytest.does_not_warn():
    does_not_warn()

[Zac-HD]

The empty sequence feels low consequence since it cannot pass now.

But "Special cases aren't special enough to break the rules." - the empty sequence already has a meaning, and I'd much rather use it to support user learning (with a specific error that explains that meaning) than give it a special-case alternative meaning. None is at least clearly different.

Cleanest, but harder to use at runtime

Yep, that's the tradeoff 😢

[olgarithms]

This issue is about introducing a feature that didn't exist before in pytest, therefore I am in favour of adding a new method to introduce a new functionality. pytest.does_not_warn() would have a very clear scope and wound't leave a lot of room for misinterpretations :)

It is true that the deprecation of pytest.warns(None) is breaking some projects, but I find that changing pytest.warns(None) to mean something else might be confusing for people who were using it correctly (or weren't using it because they understood it wasn't supported).