From 57307bbc7d88927989cf5b314f16d6e13ade04e6 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 9 Aug 2024 13:03:24 -0400 Subject: [PATCH] Fixed #35666 -- Documented stacklevel usage and testing, and adjusted test suite accordingly. Over the years we've had multiple instances of hit and misses when emitting warnings: either setting the wrong stacklevel or not setting it at all. This work adds assertions for the existing warnings that were declaring the correct stacklevel, but were lacking tests for it. --- .../writing-code/submitting-patches.txt | 4 +- tests/admin_utils/test_logentry.py | 12 ++++-- tests/constraints/tests.py | 12 ++++-- tests/contenttypes_tests/test_fields.py | 6 ++- tests/deprecation/tests.py | 41 +++++++++++++------ .../forms_tests/field_tests/test_urlfield.py | 3 +- tests/gis_tests/gdal_tests/test_geom.py | 3 +- tests/gis_tests/test_geoip2.py | 6 ++- tests/many_to_many/tests.py | 3 +- tests/many_to_one/tests.py | 6 ++- tests/model_enums/tests.py | 3 +- tests/modeladmin/tests.py | 6 ++- tests/one_to_one/tests.py | 3 +- tests/prefetch_related/tests.py | 6 ++- tests/urlpatterns/tests.py | 6 ++- tests/utils_tests/test_itercompat.py | 3 +- 16 files changed, 84 insertions(+), 39 deletions(-) diff --git a/docs/internals/contributing/writing-code/submitting-patches.txt b/docs/internals/contributing/writing-code/submitting-patches.txt index 6a5c68f573..cac6848d04 100644 --- a/docs/internals/contributing/writing-code/submitting-patches.txt +++ b/docs/internals/contributing/writing-code/submitting-patches.txt @@ -209,9 +209,10 @@ You should also add a test for the deprecation warning:: def test_foo_deprecation_warning(self): msg = "Expected deprecation message" - with self.assertWarnsMessage(RemovedInDjangoXXWarning, msg): + with self.assertWarnsMessage(RemovedInDjangoXXWarning, msg) as ctx: # invoke deprecated behavior ... + self.assertEqual(ctx.filename, __file__) It's important to include a ``RemovedInDjangoXXWarning`` comment above code which has no warning reference, but will need to be changed or removed when the @@ -233,6 +234,7 @@ deprecation ends. For example:: warnings.warn( "foo() is deprecated.", category=RemovedInDjangoXXWarning, + stacklevel=2, ) old_private_helper() ... diff --git a/tests/admin_utils/test_logentry.py b/tests/admin_utils/test_logentry.py index 20bbcccb1c..e97441eb2e 100644 --- a/tests/admin_utils/test_logentry.py +++ b/tests/admin_utils/test_logentry.py @@ -240,7 +240,7 @@ class LogEntryTests(TestCase): def test_log_action(self): msg = "LogEntryManager.log_action() is deprecated. Use log_actions() instead." content_type_val = ContentType.objects.get_for_model(Article).pk - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: log_entry = LogEntry.objects.log_action( self.user.pk, content_type_val, @@ -250,6 +250,7 @@ class LogEntryTests(TestCase): change_message="Changed something else", ) self.assertEqual(log_entry, LogEntry.objects.latest("id")) + self.assertEqual(ctx.filename, __file__) def test_log_actions(self): queryset = Article.objects.all().order_by("-id") @@ -297,9 +298,12 @@ class LogEntryTests(TestCase): msg = ( "The usage of log_action() is deprecated. Implement log_actions() instead." ) - with self.assertNumQueries(3): - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): - LogEntry.objects2.log_actions(self.user.pk, queryset, DELETION) + with ( + self.assertNumQueries(3), + self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx, + ): + LogEntry.objects2.log_actions(self.user.pk, queryset, DELETION) + self.assertEqual(ctx.filename, __file__) log_values = ( LogEntry.objects.filter(action_flag=DELETION) .order_by("id") diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index 9ca889ca6d..e1c431956f 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -412,15 +412,19 @@ class CheckConstraintTests(TestCase): def test_check_deprecation(self): msg = "CheckConstraint.check is deprecated in favor of `.condition`." condition = models.Q(foo="bar") - with self.assertWarnsRegex(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: constraint = models.CheckConstraint(name="constraint", check=condition) - with self.assertWarnsRegex(RemovedInDjango60Warning, msg): + self.assertEqual(ctx.filename, __file__) + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: self.assertIs(constraint.check, condition) + self.assertEqual(ctx.filename, __file__) other_condition = models.Q(something="else") - with self.assertWarnsRegex(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: constraint.check = other_condition - with self.assertWarnsRegex(RemovedInDjango60Warning, msg): + self.assertEqual(ctx.filename, __file__) + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: self.assertIs(constraint.check, other_condition) + self.assertEqual(ctx.filename, __file__) def test_database_default(self): models.CheckConstraint( diff --git a/tests/contenttypes_tests/test_fields.py b/tests/contenttypes_tests/test_fields.py index 15f1dafd63..ab16324fb6 100644 --- a/tests/contenttypes_tests/test_fields.py +++ b/tests/contenttypes_tests/test_fields.py @@ -98,8 +98,9 @@ class GetPrefetchQuerySetDeprecation(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: questions[0].answer_set.get_prefetch_queryset(questions) + self.assertEqual(ctx.filename, __file__) def test_generic_foreign_key_warning(self): answers = Answer.objects.all() @@ -107,8 +108,9 @@ class GetPrefetchQuerySetDeprecation(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: Answer.question.get_prefetch_queryset(answers) + self.assertEqual(ctx.filename, __file__) class GetPrefetchQuerySetsTests(TestCase): diff --git a/tests/deprecation/tests.py b/tests/deprecation/tests.py index 5548e90285..66f6a4d922 100644 --- a/tests/deprecation/tests.py +++ b/tests/deprecation/tests.py @@ -20,12 +20,14 @@ class RenameMethodsTests(SimpleTestCase): the faulty method. """ msg = "`Manager.old` method should be renamed `new`." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: class Manager(metaclass=RenameManagerMethods): def old(self): pass + self.assertEqual(ctx.filename, __file__) + def test_get_new_defined(self): """ Ensure `old` complains and not `new` when only `new` is defined. @@ -43,20 +45,23 @@ class RenameMethodsTests(SimpleTestCase): self.assertEqual(len(recorded), 0) msg = "`Manager.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: manager.old() + self.assertEqual(ctx.filename, __file__) def test_get_old_defined(self): """ Ensure `old` complains when only `old` is defined. """ msg = "`Manager.old` method should be renamed `new`." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: class Manager(metaclass=RenameManagerMethods): def old(self): pass + self.assertEqual(ctx.filename, __file__) + manager = Manager() with warnings.catch_warnings(record=True) as recorded: @@ -65,8 +70,9 @@ class RenameMethodsTests(SimpleTestCase): self.assertEqual(len(recorded), 0) msg = "`Manager.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: manager.old() + self.assertEqual(ctx.filename, __file__) def test_deprecated_subclass_renamed(self): """ @@ -79,21 +85,25 @@ class RenameMethodsTests(SimpleTestCase): pass msg = "`Deprecated.old` method should be renamed `new`." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: class Deprecated(Renamed): def old(self): super().old() + self.assertEqual(ctx.filename, __file__) + deprecated = Deprecated() msg = "`Renamed.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: deprecated.new() + self.assertEqual(ctx.filename, __file__) msg = "`Deprecated.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: deprecated.old() + self.assertEqual(ctx.filename, __file__) def test_renamed_subclass_deprecated(self): """ @@ -101,12 +111,14 @@ class RenameMethodsTests(SimpleTestCase): `old` subclass one that didn't. """ msg = "`Deprecated.old` method should be renamed `new`." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: class Deprecated(metaclass=RenameManagerMethods): def old(self): pass + self.assertEqual(ctx.filename, __file__) + class Renamed(Deprecated): def new(self): super().new() @@ -119,8 +131,9 @@ class RenameMethodsTests(SimpleTestCase): self.assertEqual(len(recorded), 0) msg = "`Renamed.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: renamed.old() + self.assertEqual(ctx.filename, __file__) def test_deprecated_subclass_renamed_and_mixins(self): """ @@ -142,20 +155,24 @@ class RenameMethodsTests(SimpleTestCase): super().old() msg = "`DeprecatedMixin.old` method should be renamed `new`." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: class Deprecated(DeprecatedMixin, RenamedMixin, Renamed): pass + self.assertEqual(ctx.filename, __file__) + deprecated = Deprecated() msg = "`RenamedMixin.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: deprecated.new() + self.assertEqual(ctx.filename, __file__) msg = "`DeprecatedMixin.old` is deprecated, use `new` instead." - with self.assertWarnsMessage(DeprecationWarning, msg): + with self.assertWarnsMessage(DeprecationWarning, msg) as ctx: deprecated.old() + self.assertEqual(ctx.filename, __file__) def test_removedafternextversionwarning_pending(self): self.assertTrue( diff --git a/tests/forms_tests/field_tests/test_urlfield.py b/tests/forms_tests/field_tests/test_urlfield.py index 8ba7842064..54bc7c5e46 100644 --- a/tests/forms_tests/field_tests/test_urlfield.py +++ b/tests/forms_tests/field_tests/test_urlfield.py @@ -163,9 +163,10 @@ class URLFieldAssumeSchemeDeprecationTest(FormFieldAssertionsMixin, SimpleTestCa "or set the FORMS_URLFIELD_ASSUME_HTTPS transitional setting to True to " "opt into using 'https' as the new default scheme." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: f = URLField() self.assertEqual(f.clean("example.com"), "http://example.com") + self.assertEqual(ctx.filename, __file__) @ignore_warnings(category=RemovedInDjango60Warning) def test_urlfield_forms_urlfield_assume_https(self): diff --git a/tests/gis_tests/gdal_tests/test_geom.py b/tests/gis_tests/gdal_tests/test_geom.py index 3967b945a4..5c23a6f2cf 100644 --- a/tests/gis_tests/gdal_tests/test_geom.py +++ b/tests/gis_tests/gdal_tests/test_geom.py @@ -972,6 +972,7 @@ class DeprecationTests(SimpleTestCase): def test_coord_setter_deprecation(self): geom = OGRGeometry("POINT (1 2)") msg = "coord_dim setter is deprecated. Use set_3d() instead." - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: geom.coord_dim = 3 self.assertEqual(geom.coord_dim, 3) + self.assertEqual(ctx.filename, __file__) diff --git a/tests/gis_tests/test_geoip2.py b/tests/gis_tests/test_geoip2.py index 12837725c9..11c73bec0c 100644 --- a/tests/gis_tests/test_geoip2.py +++ b/tests/gis_tests/test_geoip2.py @@ -207,16 +207,18 @@ class GeoLite2Test(SimpleTestCase): def test_coords_deprecation_warning(self): g = GeoIP2() msg = "GeoIP2.coords() is deprecated. Use GeoIP2.lon_lat() instead." - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: e1, e2 = g.coords(self.ipv4_str) self.assertIsInstance(e1, float) self.assertIsInstance(e2, float) + self.assertEqual(ctx.filename, __file__) def test_open_deprecation_warning(self): msg = "GeoIP2.open() is deprecated. Use GeoIP2() instead." - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: g = GeoIP2.open(settings.GEOIP_PATH, GeoIP2.MODE_AUTO) self.assertTrue(g._reader) + self.assertEqual(ctx.filename, __file__) @skipUnless(HAS_GEOIP2, "GeoIP2 is required.") diff --git a/tests/many_to_many/tests.py b/tests/many_to_many/tests.py index 351e4eb8cc..c17ed0258c 100644 --- a/tests/many_to_many/tests.py +++ b/tests/many_to_many/tests.py @@ -583,8 +583,9 @@ class ManyToManyTests(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: self.a1.publications.get_prefetch_queryset(articles) + self.assertEqual(ctx.filename, __file__) def test_get_prefetch_querysets_invalid_querysets_length(self): articles = Article.objects.all() diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index ac5b35e055..e7dd0f229f 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -894,8 +894,9 @@ class ManyToOneTests(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: City.country.get_prefetch_queryset(cities) + self.assertEqual(ctx.filename, __file__) def test_get_prefetch_queryset_reverse_warning(self): usa = Country.objects.create(name="United States") @@ -905,8 +906,9 @@ class ManyToOneTests(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: usa.cities.get_prefetch_queryset(countries) + self.assertEqual(ctx.filename, __file__) def test_get_prefetch_querysets_invalid_querysets_length(self): City.objects.create(name="Chicago") diff --git a/tests/model_enums/tests.py b/tests/model_enums/tests.py index 306bfc8d67..ee9dc369f6 100644 --- a/tests/model_enums/tests.py +++ b/tests/model_enums/tests.py @@ -328,5 +328,6 @@ class ChoicesMetaDeprecationTests(SimpleTestCase): from django.db.models import enums msg = "ChoicesMeta is deprecated in favor of ChoicesType." - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: enums.ChoicesMeta + self.assertEqual(ctx.filename, __file__) diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index e8b59ed0bf..062368d94e 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -928,8 +928,9 @@ class ModelAdminTests(TestCase): mock_request.user = User.objects.create(username="bill") content_type = get_content_type_for_model(self.band) msg = "ModelAdmin.log_deletion() is deprecated. Use log_deletions() instead." - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: created = ma.log_deletion(mock_request, self.band, str(self.band)) + self.assertEqual(ctx.filename, __file__) fetched = LogEntry.objects.filter(action_flag=DELETION).latest("id") self.assertEqual(created, fetched) self.assertEqual(fetched.action_flag, DELETION) @@ -966,8 +967,9 @@ class ModelAdminTests(TestCase): "instead." ) with self.assertNumQueries(3): - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: ima.log_deletions(mock_request, queryset) + self.assertEqual(ctx.filename, __file__) logs = ( LogEntry.objects.filter(action_flag=DELETION) .order_by("id") diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py index 280a8273fb..0d30dd8f27 100644 --- a/tests/one_to_one/tests.py +++ b/tests/one_to_one/tests.py @@ -612,8 +612,9 @@ class OneToOneTests(TestCase): "get_prefetch_queryset() is deprecated. Use get_prefetch_querysets() " "instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: Place.bar.get_prefetch_queryset(places) + self.assertEqual(ctx.filename, __file__) def test_get_prefetch_querysets_invalid_querysets_length(self): places = Place.objects.all() diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index 9b1dfdd0d1..856f766d30 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -2022,13 +2022,15 @@ class DeprecationTests(TestCase): "get_current_querysets() instead." ) authors = Author.objects.all() - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: self.assertEqual( Prefetch("authors", authors).get_current_queryset(1), authors, ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + self.assertEqual(ctx.filename, __file__) + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: self.assertIsNone(Prefetch("authors").get_current_queryset(1)) + self.assertEqual(ctx.filename, __file__) @ignore_warnings(category=RemovedInDjango60Warning) def test_prefetch_one_level_fallback(self): diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py index 78b71fe325..6c8d6470c0 100644 --- a/tests/urlpatterns/tests.py +++ b/tests/urlpatterns/tests.py @@ -212,10 +212,11 @@ class SimplifiedURLTests(SimpleTestCase): "converters is deprecated and will be removed in Django 6.0." ) try: - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: register_converter(IntConverter, "int") finally: REGISTERED_CONVERTERS.pop("int", None) + self.assertEqual(ctx.filename, __file__) def test_warning_override_converter(self): # RemovedInDjango60Warning: when the deprecation ends, replace with @@ -226,11 +227,12 @@ class SimplifiedURLTests(SimpleTestCase): "registered converters is deprecated and will be removed in Django 6.0." ) try: - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: register_converter(Base64Converter, "base64") register_converter(Base64Converter, "base64") finally: REGISTERED_CONVERTERS.pop("base64", None) + self.assertEqual(ctx.filename, __file__) def test_invalid_view(self): msg = "view must be a callable or a list/tuple in the case of include()." diff --git a/tests/utils_tests/test_itercompat.py b/tests/utils_tests/test_itercompat.py index e6ea278ab4..a95867c621 100644 --- a/tests/utils_tests/test_itercompat.py +++ b/tests/utils_tests/test_itercompat.py @@ -11,5 +11,6 @@ class TestIterCompat(SimpleTestCase): "django.utils.itercompat.is_iterable() is deprecated. " "Use isinstance(..., collections.abc.Iterable) instead." ) - with self.assertWarnsMessage(RemovedInDjango60Warning, msg): + with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: is_iterable([]) + self.assertEqual(ctx.filename, __file__)