pytest icon indicating copy to clipboard operation
pytest copied to clipboard

adds `keep_ignores` kwarg to `pytest.warns`

Open aaronzo opened this issue 1 year ago • 1 comments

workaround for #11933, but also an improved functionality in its own right. The user may set the keyword argument keep_ignores for pytest.warns to avoid catching warnings which were filtered out, in pytest configuration or otherwise. Relevant issue comment

  • [X] Include documentation when adding new features.
  • [X] Include new tests or update existing tests when applicable.
  • [X] Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.
  • [X] Add text like closes #XYZW to the PR description and/or commits (where XYZW is the issue number). See the github docs for more information.
  • [X] Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • [X] Add yourself to AUTHORS in alphabetical order.

aaronzo avatar Oct 16 '24 21:10 aaronzo

Not a maintainer, but thank you for taking this up!

I think the intuition of the method would be that it already respects the ignore filters, and don't see how a user would not want it that way already, so I don't think an additional argument is needed?

This does address your backward compatibility point thought. I think if this hasn't been as big of an issue to this point then I think it's fine to just make the respect-ignore-filters change without adding an additional argument. I could be wrong though, so open to thoughts from others as well!

reaganjlee avatar Oct 18 '24 01:10 reaganjlee

@reaganjlee I'd be nervous about introducing a breaking change without at least a deprecation cycle - it's going to be hard to assess how often the current behaviour is specifically used. Test suite code is often hacky, so I wouldn't be surprised if changing the default behaviour broke a load of code

aaronzo avatar Oct 21 '24 11:10 aaronzo

Hi folks

Thanks @aaronzo for the PR!

I'm for now -0 on the functionality -- not sure the extra complexity is worth it for pytest.warns, but if others feel different I won't oppose merging this.

I'd be nervous about introducing a breaking change without at least a deprecation cycle - it's going to be hard to assess how often the current behaviour is specifically used. Test suite code is often hacky, so I wouldn't be surprised if changing the default behaviour broke a load of code

100% agree.

nicoddemus avatar Nov 06 '24 10:11 nicoddemus