Opened 12 years ago

Closed 5 years ago

Last modified 19 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 by Łukasz Rekucki, 11 years ago

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 by Francis Devereux, 11 years ago

Cc: Francis Devereux added

comment:3 by Roman Gladkov, 11 years ago

Cc: d1fffuz0r@… added

comment:4 by Marc Tamlyn, 11 years ago

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

comment:5 by David Avs <david@…>, 11 years ago

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 by Nicolas Bouliane, 10 years ago

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 by Pavel Sutyrin, 9 years ago

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 9 years ago by Pavel Sutyrin (previous) (diff)

comment:8 by Tim Graham, 7 years ago

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

comment:9 by Peter Zsoldos, 7 years ago

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 by Stephane "Twidi" Angel, 6 years ago

Cc: Stephane "Twidi" Angel added

comment:11 by Stephane "Twidi" Angel, 6 years ago

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 by Stephane "Twidi" Angel, 6 years ago

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 by Jon Dufresne, 5 years ago

Cc: jon.dufresne@… added

comment:14 by Jon Dufresne, 5 years ago

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 by Simon Charette, 5 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good to go.

comment:16 by Carlton Gibson <carlton.gibson@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 7feddd8:

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

comment:17 by Jon Dufresne, 5 years ago

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 by Mariusz Felisiak <felisiak.mariusz@…>, 19 months ago

In 6f49b7b:

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

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

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