Skip to content

Make assertion rewrite warning messages filterable #13429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ikappaki
Copy link
Contributor

@ikappaki ikappaki commented May 17, 2025

Hi,

can you please consider patch to mare assertion rewrite warning messages filterable using the standard -W option. It addresses #5473.

The issue arises when a Python package defines a pytest plugin, and also calls pytest.main() programmatically, after importing one of its own submodules. Because the module is already imported, pytest.main() cannot rewrite its assertions and emits a warning like:

Module already imported so cannot be rewritten: basilisp

Unfortunately, the : in the message conflicts with Python’s warning filter syntax, where : is used as a delimiter. This prevents users from filtering out the warning via -W.

This patch replaces the : in the warning message with a ;, making it possible to write a valid warning filter like:

-W ignore:Module already imported so cannot be rewritten; mymodule:pytest.PytestAssertRewriteWarning

We believe there's no need to force assertion rewriting simply because a module defines a plugin, and users should be able to suppress this warning when needed.

For background and alternative solutions considered, see #5473 (comment).

Happy to discuss further or consider alternatives.

  • Include new tests or update existing tests when applicable.
  • Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.
  • Create a new changelog file in the changelog folder

Thanks

so it can be filtered using standard Python warning filters.
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, simple and to the point! Thanks @ikappaki!

@nicoddemus nicoddemus added this to the 8.4 milestone May 21, 2025
@RonnyPfannschmidt
Copy link
Member

Arent other regex matches avaliable to match?

@ikappaki
Copy link
Contributor Author

HI @RonnyPfannschmidt,

The issue is that the string passed to the -W command line option is treated as a literal, so regex characters have no effect:
https://docs.pytest.org/en/stable/how-to/capture-warnings.html#controlling-warnings:

image

i.e., in CPython, the message portion is escaped using re.escape:
https://github.com/python/cpython/blob/e1f891414b2329414a6160ed246f5f869a218bfd/Lib/_py_warnings.py#L359-L384

@RonnyPfannschmidt
Copy link
Member

So we should aim for warning filters in pytest settings instead of using slightly off characters in the long run

@nicoddemus
Copy link
Member

So we should aim for warning filters in pytest settings instead of using slightly off characters in the long run

You OK to merge the PR or would you like to hold it off to discuss it further?

@RonnyPfannschmidt
Copy link
Member

Let's merge it for now

But eventually id like to be able to block the warnings properly

@nicoddemus nicoddemus merged commit c77995b into pytest-dev:main May 22, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants