Opened 10 years ago

Closed 4 years ago

Last modified 3 months ago

#18707 closed New feature (fixed)

Test client doesn't allow testing 500 responses content

Reported by: ricardokirkner@… Owned by: nobody
Component: Testing framework Version: 1.6
Severity: Normal Keywords:
Cc: Francis Devereux, d1fffuz0r@…, Stephane "Twidi" Angel, jon.dufresne@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (19)

comment:1 Changed 10 years ago by Łukasz Rekucki

Triage Stage: UnreviewedAccepted
Type: BugNew feature

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.

comment:2 Changed 10 years ago by Francis Devereux

Cc: Francis Devereux added

comment:3 Changed 10 years ago by Roman Gladkov

Cc: d1fffuz0r@… added

comment:4 Changed 9 years ago by Marc Tamlyn

Is there any reason why TemplateView(template_name='500.html') is not sufficient for doing this?

comment:5 Changed 9 years ago 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.

comment:6 Changed 8 years ago by Nicolas Bouliane

Version: 1.31.6

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.

comment:7 Changed 8 years ago by Pavel Sutyrin

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 sweet locmem email backend that I may use in unit tests).

Thanks!

Last edited 8 years ago by Pavel Sutyrin (previous) (diff)

comment:8 Changed 6 years ago by Tim Graham

I closed #27893 as a duplicate. It suggests the same approach as in comment 1.

comment:9 Changed 6 years ago 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?

comment:10 Changed 5 years ago by Stephane "Twidi" Angel

Cc: Stephane "Twidi" Angel added

comment:11 Changed 5 years ago 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.

comment:12 Changed 5 years ago by Stephane "Twidi" Angel

By the way I found a solution. I need this in a whole test case so:

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

comment:13 Changed 4 years ago by Jon Dufresne

Cc: jon.dufresne@… added

comment:14 Changed 4 years ago by Jon Dufresne

Has patch: set

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.

comment:15 Changed 4 years ago by Simon Charette

Triage Stage: AcceptedReady for checkin

Patch looks good to go.

comment:16 Changed 4 years ago by Carlton Gibson <carlton.gibson@…>

Resolution: fixed
Status: newclosed

In 7feddd8:

Fixed #18707 -- Added support for the test client to return 500 responses.

comment:17 Changed 4 years ago 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

comment:18 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 6f49b7b:

Refs #18707 -- Corrected django.test.Client signature in docs.

comment:19 Changed 3 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 2984e17c:

[4.1.x] Refs #18707 -- Corrected django.test.Client signature in docs.

Backport of 6f49b7b69b4e19b0362d4dff35ef7544b94c84a5 from main

Note: See TracTickets for help on using tickets.
Back to Top