Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32437 closed Bug (fixed)

LiveServerTestCase.setUpClass() doesn't clean up fully on failure

Reported by: Chris Jerdonek Owned by: Chris Jerdonek
Component: Testing framework Version: 2.2
Severity: Normal Keywords: LiveServerTestCase, setUpClass
Cc: Florian Apolloner, Tobias McNulty 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 (last modified by Chris Jerdonek)

While working on #32417, I noticed in this comment that LiveServerTestCase.setUpClass() doesn't clean up all the way if setUpClass() errors out.

Specifically, LiveServerTestCase.setUpClass() looks like this:

@classmethod
def setUpClass(cls):
    super().setUpClass()
    ...
    cls._live_server_modified_settings = modify_settings(
        ALLOWED_HOSTS={'append': cls.allowed_host},
    )
    cls._live_server_modified_settings.enable()
    ...
    if cls.server_thread.error:
        # Clean up behind ourselves, since tearDownClass won't get called in
        # case of errors.
        cls._tearDownClassInternal()
        raise cls.server_thread.error

But _tearDownClassInternal() doesn't undo the call to cls._live_server_modified_settings.enable().

I will post a PR shortly.

Change History (7)

comment:1 by Chris Jerdonek, 4 years ago

Has patch: set
Owner: changed from nobody to Chris Jerdonek
Status: newassigned

comment:2 by Chris Jerdonek, 4 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 4 years ago

Cc: Florian Apolloner Tobias McNulty added
Triage Stage: UnreviewedAccepted

comment:4 by Florian Apolloner, 4 years ago

The cleanup precedes the commit that modified the settings by three years (https://github.com/django/django/commit/17e661641ddaf8266e7430d83cfb2039abc55df7 & https://github.com/django/django/commit/73a610d2a81bc3bf2d3834786b2458bc85953ed0) so it is more than likely that this was just an oversight at the time. I'll give the PR a review in a second.

Version 0, edited 4 years ago by Florian Apolloner (next)

comment:5 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 694deff8:

Fixed #32437 -- Fixed cleaning up ALLOWED_HOSTS in LiveServerTestCase on setUpClass() failure.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 65a62094:

[3.2.x] Fixed #32437 -- Fixed cleaning up ALLOWED_HOSTS in LiveServerTestCase on setUpClass() failure.

Backport of 694deff82f86e025561dfa724425f67e2ff7cbb7 from master

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