Opened 6 days ago
Last modified 6 days ago
#37089 assigned Uncategorized
ignore_warnings() handles message arg differently from other test helpers — at Initial Version
| Reported by: | Mike Edmunds | Owned by: | Mike Edmunds |
|---|---|---|---|
| Component: | Uncategorized | Version: | dev |
| Severity: | Normal | Keywords: | ignore_warnings, deprecation |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
This use of ignore_warnings() does not ignore the warning:
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 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
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)