assertWarnsMessage() also ignores entire warning category

[Mike Edmunds]

assertWarnsMessage(category, message) implicitly does the equivalent of ignore_warnings(category). As a result, tests for particular deprecation warnings can overlook other, unrelated deprecation warnings coming from the code being tested.

You would expect the second test case below to cause an error for RemovedInDjango70Warning: Unrelated deprecation, but it actually passes:

class SomeTests(SimpleTestCase):
    def test_this_errors_as_expected(self):
        # ERROR: RemovedInDjango70Warning: Unrelated deprecation
        warnings.warn("Unrelated deprecation", RemovedInDjango70Warning)

    def test_so_this_should_also_error_but_does_not(self):
        with self.assertWarnsMessage(RemovedInDjango70Warning, "Expected deprecation"):
            warnings.warn("Expected deprecation", RemovedInDjango70Warning)
            # This gets ignored.
            warnings.warn("Unrelated deprecation", RemovedInDjango70Warning)

    def test_duplicate_warnings_should_maybe_also_error(self):
        with self.assertWarnsMessage(RemovedInDjango70Warning, "Expected deprecation"):
            warnings.warn("Expected deprecation", RemovedInDjango70Warning)
            # This also gets ignored. Maybe it shouldn't?
            warnings.warn("Expected deprecation", RemovedInDjango70Warning)

The problem is assertWarnsMessage() and assertRaisesMessage() try to share SimpleTestCase._assertFooMessage(), which wraps assertWarns() and assertRaises() respectively. But those methods aren't really parallel: there can be only one error raised, but execution can continue after a warning. assertWarns(category) captures all warnings in the category.

[Mike Edmunds]

Here's a test case that could be added to tests/test_utils/tests.py to verify a fix:

class AssertWarnsMessageTests(SimpleTestCase):
    ...

    def test_does_not_ignore_entire_category(self):
        with (
            self.assertRaisesMessage(RemovedAfterNextVersionWarning, "Unexpected"),
            self.assertWarnsMessage(RemovedAfterNextVersionWarning, "Expected"),
        ):
            warnings.warn("Expected", RemovedAfterNextVersionWarning)
            warnings.warn("Unexpected", RemovedAfterNextVersionWarning)

[Edit: Changed RemovedInNextVersionWarning to RemovedAfterNextVersionWarning. runtests.py always converts the "after next" warning to an error, in all releases. In x.1 releases, the "in next" warning is unused so not treated as an error.]

[Mike Edmunds]

See also #37076.

I'm thinking maybe assertWarnsMessage() should do something like:

• Capture all warnings while running the tested code
• Check that at least one captured warning matches the expected category and message
• Optionally check that exactly one (or exactly a specified expected_count) matches
• Re-emit any non-matching warnings that were captured, to be handled by the original warning filters

[Mike Edmunds]

Python unittest's assertWarnsRegex() has the same (problematic) behavior: ​https://github.com/python/cpython/issues/143231. And a comment in that issue proposes a similar fix to the one I described above for Django's assertWarnsMessage().

In some local experimentation, there are a handful of existing Django tests that unintentionally depend on the current behavior and would need to be updated (e.g., by adding appropriate ignore_warnings() for the non-matching deprecation messages those tests don't care about).

There also seem to be a handful of existing Django tests that intentionally depend on assertWarnsMessage() capturing multiple instances of the same message. (So the answer to my question in test_duplicate_warnings_should_maybe_also_error() above is: duplicate warnings should not error, at least not by default.)

[Artyom Kotovskiy]

Replying to Mike Edmunds:

Python unittest's assertWarnsRegex() has the same (problematic) behavior: ​https://github.com/python/cpython/issues/143231. And a comment in that issue proposes a similar fix to the one I described above for Django's assertWarnsMessage().

In some local experimentation, there are a handful of existing Django tests that unintentionally depend on the current behavior and would need to be updated (e.g., by adding appropriate ignore_warnings() for the non-matching deprecation messages those tests don't care about).

There also seem to be a handful of existing Django tests that intentionally depend on assertWarnsMessage() capturing multiple instances of the same message. (So the answer to my question in test_duplicate_warnings_should_maybe_also_error() above is: duplicate warnings should not error, at least not by default.)

Thanks for the suggestions and analysis. I’m taking my time going through the test case code since I’m new to contributing.

[Artyom Kotovskiy]

Replying to Mike Edmunds:

Python unittest's assertWarnsRegex() has the same (problematic) behavior: ​https://github.com/python/cpython/issues/143231. And a comment in that issue proposes a similar fix to the one I described above for Django's assertWarnsMessage().

In some local experimentation, there are a handful of existing Django tests that unintentionally depend on the current behavior and would need to be updated (e.g., by adding appropriate ignore_warnings() for the non-matching deprecation messages those tests don't care about).

There also seem to be a handful of existing Django tests that intentionally depend on assertWarnsMessage() capturing multiple instances of the same message. (So the answer to my question in test_duplicate_warnings_should_maybe_also_error() above is: duplicate warnings should not error, at least not by default.)

Am I missing something or those #37072 and #37076 are kind of contradicting each other?

We expect this to pass in #37076

def test_deprecated_in_my_app(self):
        with self.assertWarnsMessage(DeprecationWarning, "Deprecated in MyApp"):
            warnings.warn("Deprecated in Django", RemovedInNextVersionWarning)
            warnings.warn("Deprecated in MyApp", DeprecationWarning)

And this to fail in #37072.

def test_so_this_should_also_error_but_does_not(self):
        with self.assertWarnsMessage(RemovedInDjango70Warning, "Expected deprecation"):
            warnings.warn("Expected deprecation", RemovedInDjango70Warning)
            warnings.warn("Unrelated deprecation", RemovedInDjango70Warning)

RemovedInNextVersionWarning is a subclass of DeprecationWarning. So as I see it they both have same category, different message — but one should pass and one should fail.

[Mike Edmunds]

Replying to Artyom Kotovskiy:

Am I missing something or those #37072 and #37076 are kind of contradicting each other?

We expect this to pass in #37076

def test_deprecated_in_my_app(self):
        with self.assertWarnsMessage(DeprecationWarning, "Deprecated in MyApp"):
            warnings.warn("Deprecated in Django", RemovedInNextVersionWarning)
            warnings.warn("Deprecated in MyApp", DeprecationWarning)

And this to fail in #37072.

def test_so_this_should_also_error_but_does_not(self):
        with self.assertWarnsMessage(RemovedInDjango70Warning, "Expected deprecation"):
            warnings.warn("Expected deprecation", RemovedInDjango70Warning)
            warnings.warn("Unrelated deprecation", RemovedInDjango70Warning)

RemovedInNextVersionWarning is a subclass of DeprecationWarning. So as I see it they both have same category, different message — but one should pass and one should fail.

Django's runtests.py ​converts some warnings to errors within the tests. In particular, RemovedInDjango70Warning is currently converted to an error, but the current superclass PendingDeprecationWarning is not, and RemovedInNextVersion warning is not. (There is no RemovedInDjango62Warning due to a quirk of Django's deprecation cycles. If we did have that, it would be equivalent to RemovedInNextVersionWarning, and runtests.py would convert RemovedInDjango62Warning to an error.)

So in Django's own tests, if both issues are fixed we'd expect:

• test_so_this_should_also_error_but_does_not() would error due to the non-matching "Unrelated deprecation" message (converted to an error by the runtests.py warning filters).
• test_deprecated_in_my_app() would pass (because of the matching "Deprecated in MyApp") and might print "RemovedInNextVersionWarning: Deprecated in Django" to the console, because runtests.py doesn't currently convert RemovedInNextVersionWarning to an error. (But that will change after 6.1 is released and runtests.py is updated for 6.2.)

The examples in the bug reports aren't good tests to add to Django itself: they're a little ambiguous depending on how the testing environment is set up, and they make assumptions about which Django warnings are currently defined.

comment:1 suggests a test_does_not_ignore_entire_category() for #37072. It uses an outer assertRaisesMessage() to verify "Unrelated deprecation" makes it out of assertWarnsMessage() and is converted to an error by runtests.py. (It also has a problem that I will fix in an edit.)

We'll need to craft a better test case for #37076 that takes all this into account. You can rely on runtests.py always treating RemovedAfterNextVersionWarning as an error (in all releases), and never treating the base-class DeprecationWarning or PendingDeprecationWarning as errors.

[Artyom Kotovskiy]

Pr -> ​https://github.com/django/django/pull/21228