Opened 59 minutes ago

Last modified 55 minutes ago

#37205 assigned Cleanup/optimization

Tests asserting result isinstance of HttpResponse do not fail if result is a subclass

Reported by: Jonathan Biemond Owned by: Jonathan Biemond
Component: Uncategorized Version: dev
Severity: Normal Keywords: tests, internal
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Tests in the Django test suite that use self.assertIsInstance() to check if a response is an HttpResponse will pass for any object that is a subclass of HttpResponse, such as HttpResponseNotAllowed. This can lead to uncaught errors when the expectation is that the result is only and exactly HttpResponse.

For example the following test will continue to pass after these changes:

     def test_require_http_methods_methods(self):
-        @require_http_methods(["GET", "PUT"])
+        @require_http_methods(["PUT"])
         def my_view(request):
             return HttpResponse("OK")

         request = HttpRequest()
         request.method = "GET"
         self.assertIsInstance(my_view(request), HttpResponse)
         request.method = "POST"
         self.assertIsInstance(my_view(request), HttpResponseNotAllowed)

Both responses are HttpResponseNotAllowed and both assert statements are true.
Majority of occurrences may be found here: ​https://github.com/django/django/blob/main/tests/decorators/test_http.py

Change History (1)

in reply to:  description comment:1 by Jonathan Biemond, 55 minutes ago

I propose we explicitly assert against the HTTP status code instead. This has the downside of breaking tests if a HttpResponse's status code ever changes, but that seems unlikely.

     def test_require_http_methods_methods(self):
       @require_http_methods(["GET", "PUT"])
         def my_view(request):
             return HttpResponse("OK")

         request = HttpRequest()
         request.method = "GET"
-        self.assertIsInstance(my_view(request), HttpResponse)
+        self.assertEqual(my_view(request).status_code, HTTPStatus.OK)
         request.method = "POST"
-        self.assertIsInstance(my_view(request), HttpResponseNotAllowed)
+        self.assertEqual(my_view(request).status_code, HTTPStatus.METHOD_NOT_ALLOWED)
Note: See TracTickets for help on using tickets.
Back to Top