[Django] #18707: Test client doesn't allow testing 500 responses content
Django
-----------------------------------+--------------------
Reporter: ricardokirkner@… | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.3
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
The test client is re-raising almost every single exception (as per
https://docs.djangoproject.com/en/dev/topics/testing/#exceptions), which
means there is no way to get the response content back for responses
returned by the 500 error handler. This prevents using the test client for
testing an important area.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lrekucki):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Bug => New feature
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
The test client works as documented, so it's not a bug.
In most cases this behavior is more useful then returning getting a
generic error page. Using selenium is probably a better choice for testing
your 500 pages, but I guess that having an option in the Client class
could also be useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:1>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by frankoid):
* cc: frankoid (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:2>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by d1ffuz0r):
* cc: d1fffuz0r@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:3>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
Is there any reason why `TemplateView(template_name='500.html')` is not
sufficient for doing this?
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:4>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Avs <david@…>):
One reason is that handler500 sometimes could be a custom view. In this
case `TemplateView(...).as_view()` may be not sufficient, but the
`handler500` can be used instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:5>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by nicbou):
* version: 1.3 => 1.6
Comment:
I am in the same situation as David. Our 500 page is a redirect that needs
to be tested. Although it's a fairly easy to test for 404s and the like,
we can't test that our 500 handler works.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:6>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by spacediver):
Hi folks,
Currently I need to test not even 500 user-facing view, but email sending
on 500.
Is there any way to trigger production error handling from within unit
test with django client (or otherwise from unit test)?
I strongly hesitate to move this into Selenium integration tests due to
need of full-fledged email mocking (instead of neat and sweat locmem email
backend that I may use in unit tests).
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:7>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I closed #27893 as a duplicate. It suggests the same approach as in
comment 1.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:8>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Peter Zsoldos):
Would an alternative implementation approach of chaging
{{{Client.request}}}'s current {{{if self.exc_info:}}} check (after the
{{{response = self.handler(environ)}}}) to {{{if self.exc_info and
settings.DEBUG_PROPAGATE_EXCEPTIONS:}}} be appropriate? If so, the PR
should be against the {{{master}}} branch only, correct?
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:9>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Stephane "Twidi" Angel):
* cc: Stephane "Twidi" Angel (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:10>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Stephane "Twidi" Angel):
I like the proposal from `Peter Zsoldos`.
Having to use selenium for this is a non-sense. The behavior of django in
tests should be near the behavior in reality, else what is the point of
tests...
We have a logic based on the request headers to handle the error responses
(including the 500) and we *have* to test it, like we test other errors.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:11>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Stephane "Twidi" Angel):
By the way I found a solution. I need this in a whole test case so:
{{{#!python
from unittest.mock import patch
from django.test import TestCase
class MyTestCase(TestCase):
@classmethod
def setUpClass(cls):
super().setUpClass()
cls.patched_signal =
patch('django.core.signals.got_request_exception.connect')
cls.patched_signal.start()
@classmethod
def tearDownClass(cls):
cls.patched_signal.stop()
super().tearDownClass()
}}}
With this the signal is not connected so the "problematic" code is not
called.
Note: this also works by patching
`django.test.client.Client.store_exc_info`
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:12>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* cc: jon.dufresne@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:13>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10892 PR]
This PR takes the following approach:
Adds a new test client argument and attribute `raise_view_exception` to
allow controlling whether or not exceptions raised in a view should also
be raised in the test. The value defaults to `True` for backwards
compatibility. If it is `False` and an exception occurs in the view, the
test client will return a 500 response with the attribute `exc_info`, a
tuple providing information of the exception that occurred in the same
format as Python's `sys.exc_inf()` (type, value, traceback).
However, I would like to suggest that `raise_view_exception=False` become
the default after a deprecation period and perhaps eventually dropping the
old behavior (again, after a deprecation period). IMO, the test client
should try to interact with views as close to production as possible. By
injecting alternative code paths, such as eagerly raising exceptions, we
drift from the real world scenario to a different one. A default of
`raise_view_exception=False` would better allow full stack integration
testing with the test client. With the introduction of `exc_info`, testing
precisely what exception occurred is still available and easy to do. This
proposal is not included in the PR, as I'd like some agreement before
beginning work.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:14>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Accepted => Ready for checkin
Comment:
Patch looks good to go.
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:15>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: 1.6
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson <carlton.gibson@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"7feddd878cd0d4ad6cc98f0da2b597d603a211a7" 7feddd8]:
{{{
#!CommitTicketReference repository=""
revision="7feddd878cd0d4ad6cc98f0da2b597d603a211a7"
Fixed #18707 -- Added support for the test client to return 500 responses.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:16>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
> However, I would like to suggest that raise_view_exception=False become
the default after a deprecation period and perhaps eventually dropping the
old behavior (again, after a deprecation period).
I've followed up with this suggestion in #30249
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:17>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"6f49b7b69b4e19b0362d4dff35ef7544b94c84a5" 6f49b7b]:
{{{
#!CommitTicketReference repository=""
revision="6f49b7b69b4e19b0362d4dff35ef7544b94c84a5"
Refs #18707 -- Corrected django.test.Client signature in docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:18>
Django
Reporter: ricardokirkner@… | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"2984e17c6453fac63bac2610bad508b2e5bb5324" 2984e17c]:
{{{
#!CommitTicketReference repository=""
revision="2984e17c6453fac63bac2610bad508b2e5bb5324"
[4.1.x] Refs #18707 -- Corrected django.test.Client signature in docs.
Backport of 6f49b7b69b4e19b0362d4dff35ef7544b94c84a5 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18707#comment:19>