#37089: ignore_warnings() handles message arg differently from other test
helpers
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
| Edmunds
Type: Uncategorized | Status: assigned
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: ignore_warnings, | Triage Stage:
deprecation | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> This use of `ignore_warnings()` does ''not'' ignore the warning:
>
> {{{#!python
> with ignore_warnings(
> category=RemovedInDjango70Warning,
> message="get_connection() is deprecated.",
> ):
> warnings.warn(
> "get_connection() is deprecated.",
> RemovedInDjango70Warning,
> )
> }}}
>
> That's inconsistent with how `assertWarnsMessage()` works, and is likely
> to be unexpected.
>
> Internally, `ignore_warnings()` uses Python's
> `warnings.catch_warnings()`, which passes the `message` arg directly to
> `re.match()`. (So `ignore_warnings()` is also inconsistent with
> `assertWarnsRegex()`, which uses `re.search()`.) See the reproduction
> example below.
>
> The `message` arg is not documented, but has some specific support in
> `ignore_warnings()`. (Same thing for the `module` arg, though that's less
> of an issue since the only regex pattern char commonly in module names is
> ".", and the pattern `r"."` matches the string `"."`.)
>
> == Proposal ==
>
> We should change `ignore_warnings()` to `re.escape()` the `message` arg
> and prepend `r".*"` to it, making it consistent with
> `assertWarnsMessage()` by default.
>
> For test cases that do want ignore messages by regex, we would introduce
> a `message_re` arg. Only one of `message` and `message_re` would be
> allowed at a time. (The PR for #35514 includes one test case that
> ''does'' use a in regex this way.)
>
> We should maybe also `re.escape()` the `module` arg (but without the
> `r".*"`) and add a `module_re` arg.
>
> == Compatibility ==
>
> `ignore_warnings()` seems to be documented only as an internal API for
> [https://docs.djangoproject.com/en/6.0/internals/contributing/writing-
> code/submitting-
> patches/#deprecating-a-feature:~:text=ignore_warnings%20decorator
> deprecating a feature]. But it's exposed in `django.test`. I'm not clear
> how to apply the deprecation policy to this.
>
> As of right now, none of Django's own tests in the main branch are using
> the `message` arg. (That will change when the PR for #35514 is merged.) A
> handful use `module`, none expecting regex behavior.
>
> If we ''do'' want to retain compatibility with a deprecation period, we
> could add a temporary `message_is_str` arg defaulting to `False`, and
> only escape the `message` arg if it's set True. If `False`, issue a
> deprecation warning and continue treating `message` as a regex during
> deprecation. After the deprecation period, switch to the new behavior and
> start a ''second'' deprecation period to remove the `message_is_str` arg.
>
> == Reproduction ==
>
> {{{#!python
> import re
> from django.test import SimpleTestCase, ignore_warnings
>
> class MessageArgsConsistencyTests(SimpleTestCase):
> def setUp(self):
> # Convert UserWarning to error within these tests.
> self.enterClassContext(
> warnings.catch_warnings(action="error", category=UserWarning)
> )
>
> def test_assert_warns_message(self):
> # assertWarnsMessage() uses `expected in actual` substring check.
> with self.assertWarnsMessage(UserWarning, "foo() will change"):
> warnings.warn("Soon, foo() will change.",
> category=UserWarning)
>
> def test_assert_warns_regex(self):
> # assertWarnsRegex() uses `re.search(expected, actual)`.
> with self.assertWarnsRegex(UserWarning, re.escape("foo() will
> change")):
> warnings.warn("Soon, foo() will change.",
> category=UserWarning)
>
> def test_does_ignore_warnings_work_like_assert_warns_message(self):
> # No. This does not ignore the warning and fails with an error.
> with ignore_warnings(category=UserWarning, message="foo() will
> change"):
> warnings.warn("Soon, foo() will change.",
> category=UserWarning)
>
> def test_does_ignore_warnings_work_like_assert_warns_regex(self):
> # Is the problem that ignore_warnings() expects a regex?
> # Not entirely. This also fails with an error.
> with ignore_warnings(
> category=UserWarning, message=re.escape("foo() will change")
> ):
> warnings.warn("Soon, foo() will change.",
> category=UserWarning)
>
> def test_so_how_does_ignore_warnings_work(self):
> # ignore_warnings() actually uses `re.match(expected, actual)`.
> # This ignores the warning correctly.
> with ignore_warnings(
> category=UserWarning, message=r".*" + re.escape("foo() will
> change")
> ):
> warnings.warn("Soon, foo() will change.",
> category=UserWarning)
> }}}
New description:
This use of `ignore_warnings()` does ''not'' ignore the warning:
{{{#!python
with ignore_warnings(
category=RemovedInDjango70Warning,
message="get_connection() is deprecated.",
):
warnings.warn(
"get_connection() is deprecated.",
RemovedInDjango70Warning,
)
}}}
That's inconsistent with how `assertWarnsMessage()` works, and is likely
to be unexpected.
Internally, `ignore_warnings()` uses Python's `warnings.catch_warnings()`,
which passes the `message` arg directly to `re.match()`. (So
`ignore_warnings()` is also inconsistent with `assertWarnsRegex()`, which
uses `re.search()`.) See the reproduction example below.
The `message` arg is not documented, but has some specific support in
`ignore_warnings()`. (Same thing for the `module` arg, though that's less
of an issue since the only regex pattern char commonly in module names is
".", and the pattern `r"."` matches the string `"."`.)
== Proposal ==
We should change `ignore_warnings()` to `re.escape()` the `message` arg
and prepend `r".*"` to it, making it consistent with
`assertWarnsMessage()` by default.
For test cases that do want ignore messages by regex, we would introduce a
`message_re` arg. Only one of `message` and `message_re` would be allowed
at a time. (The PR for #35514 includes one test case that ''does'' use a
regex in this way.)
We should maybe also `re.escape()` the `module` arg (but without the
`r".*"`) and add a `module_re` arg.
== Compatibility ==
`ignore_warnings()` seems to be documented only as an internal API for
[https://docs.djangoproject.com/en/6.0/internals/contributing/writing-code
/submitting-
patches/#deprecating-a-feature:~:text=ignore_warnings%20decorator
deprecating a feature]. But it's exposed in `django.test`. I'm not clear
how to apply the deprecation policy to this.
As of right now, none of Django's own tests in the main branch are using
the `message` arg. (That will change when the PR for #35514 is merged.) A
handful use `module`, none expecting regex behavior.
If we ''do'' want to retain compatibility with a deprecation period, we
could add a temporary `message_is_str` arg defaulting to `False`, and only
escape the `message` arg if it's set True. If `False`, issue a deprecation
warning and continue treating `message` as a regex during deprecation.
After the deprecation period, switch to the new behavior and start a
''second'' deprecation period to remove the `message_is_str` arg.
== Reproduction ==
{{{#!python
import re
from django.test import SimpleTestCase, ignore_warnings
class MessageArgsConsistencyTests(SimpleTestCase):
def setUp(self):
# Convert UserWarning to error within these tests.
self.enterClassContext(
warnings.catch_warnings(action="error", category=UserWarning)
)
def test_assert_warns_message(self):
# assertWarnsMessage() uses `expected in actual` substring check.
with self.assertWarnsMessage(UserWarning, "foo() will change"):
warnings.warn("Soon, foo() will change.",
category=UserWarning)
def test_assert_warns_regex(self):
# assertWarnsRegex() uses `re.search(expected, actual)`.
with self.assertWarnsRegex(UserWarning, re.escape("foo() will
change")):
warnings.warn("Soon, foo() will change.",
category=UserWarning)
def test_does_ignore_warnings_work_like_assert_warns_message(self):
# No. This does not ignore the warning and fails with an error.
with ignore_warnings(category=UserWarning, message="foo() will
change"):
warnings.warn("Soon, foo() will change.",
category=UserWarning)
def test_does_ignore_warnings_work_like_assert_warns_regex(self):
# Is the problem that ignore_warnings() expects a regex?
# Not entirely. This also fails with an error.
with ignore_warnings(
category=UserWarning, message=re.escape("foo() will change")
):
warnings.warn("Soon, foo() will change.",
category=UserWarning)
def test_so_how_does_ignore_warnings_work(self):
# ignore_warnings() actually uses `re.match(expected, actual)`.
# This ignores the warning correctly.
with ignore_warnings(
category=UserWarning, message=r".*" + re.escape("foo() will
change")
):
warnings.warn("Soon, foo() will change.",
category=UserWarning)
}}}
--
Comment (by Mike Edmunds):
(I'm not sure if this is a bug or feature request, and whether the
component should be "Testing framework" or internal "Utilities".)
--
Ticket URL: <https://code.djangoproject.com/ticket/37089#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019e0464461e-f68957ce-05e1-4f94-a29d-dcdaaba91510-000000%40eu-central-1.amazonses.com.
