[Django] #29024: `TestContextDecorator` never exits if `setUp` fails in tests
Django
---------------------------------------------+------------------------
Reporter: Anthony King | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
---------------------------------------------+------------------------
when using `TestContextDecorator` as a class decorator, `enable` is called
just before `Test.setUp`, and `disable` is called after `Test.tearDown`.
unfortunately, `tearDown` is not called in the event of a failure inside
`setUp`. This can result in unexpected behaviour.
Even though this class is not documented, there are decorators that use it
that are.
example:
{{{#!python
class example_decorator(TestContextDecorator):
some_var = False
def enable(self):
self.__class__.some_var = True
def disable(self):
self.__class__.some_var = False
class Example1TestCase(TestCase):
def test_var_is_false(self):
# some_var will be False
self.assertFalse(example_decorator.some_var)
@example_decorator()
class Example2TestCase(TestCase):
def setUp(self):
raise Exception
def test_var_is_true(self):
# won't be hit due to exception in setUp
self.assertTrue(example_decorator.some_var)
class Example3TestCase(TestCase):
def test_var_is_false(self):
# some_var will be True now due to no cleanup in Example2TestCase
self.assertFalse(example_decorator.some_var)
}}}
output:
{{{
.EF
======================================================================
ERROR: test_var_is_true (sharefile.tests.Example2TestCase)
----------------------------------------------------------------------
... Traceback
======================================================================
FAIL: test_var_is_false (sharefile.tests.Example3TestCase)
----------------------------------------------------------------------
...
AssertionError: True is not false
Ran 3 tests in 0.007s
FAILED (failures=1, errors=1)
}}}
There are 2 potential fixes here:
- decorate `setUpClass` and `tearDownClass`
- use `addCleanup` inside the `setUp` decorator
`addCleanup` will always run, and will only ever be registered if the
context manager was successfully enabled.
It would also be useful to have this class documented, however that's out
of scope of this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/29024>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Anthony King | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Simon Charette):
* version: 1.11 => master
* stage: Unreviewed => Accepted
Comment:
Using `addCleanup` instead of hooking up on `tearDown` makes sense here.
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:1>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Shahbaj Sayyad):
* owner: nobody => Shahbaj Sayyad
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:2>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Description changed by Anthony King:
Old description:
> example:
>
New description:
example:
}}}
output:
{{{
.EF
======================================================================
ERROR: test_var_is_true (example.tests.Example2TestCase)
----------------------------------------------------------------------
... Traceback
======================================================================
FAIL: test_var_is_false (example.tests.Example3TestCase)
----------------------------------------------------------------------
...
AssertionError: True is not false
Ran 3 tests in 0.007s
FAILED (failures=1, errors=1)
}}}
There are 2 potential fixes here:
- decorate `setUpClass` and `tearDownClass`
- use `addCleanup` inside the `setUp` decorator
`addCleanup` will always run, and will only ever be registered if the
context manager was successfully enabled.
It would also be useful to have this class documented, however that's out
of scope of this ticket.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:3>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Anthony King):
Hi Shahbaj,
I saw your post in the mailing list.
So how we've done it internally is like this:
{{{
#!python
class TestContextDecorator(DjangoTestContextDecorator):
def decorate_class(self, cls):
# https://code.djangoproject.com/ticket/29024
if issubclass(cls, TestCase):
decorated_setUp = cls.setUp
@wraps(decorated_setUp)
def setUp(inner_self):
context = self.enable()
if self.attr_name:
setattr(inner_self, self.attr_name, context)
inner_self.addCleanup(self.disable)
decorated_setUp(inner_self)
cls.setUp = setUp
return cls
raise TypeError('Can only decorate subclasses of
unittest.TestCase')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:4>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Shahbaj Sayyad):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:5>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Shahbaj Sayyad):
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:6>
Django
Reporter: Anthony King | Owner: Shahbaj
| Sayyad
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:6>
Django
Reporter: Anthony King | Owner: (none)
Type: Bug | Status: new
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Shahbaj Sayyad):
* owner: Shahbaj Sayyad => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:7>
Django
Reporter: Anthony King | Owner: Kamil
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Kamil):
* owner: (none) => Kamil
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:8>
Django
Reporter: Anthony King | Owner: Kamil
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
Changes (by Jeff):
* needs_better_patch: 0 => 1
Comment:
Recommendations present on PR
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:9>
Django
Reporter: Anthony King | Owner: Kamil
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3d4080f19c606865f8f76d30d91c49d989a7f76c" 3d4080f]:
{{{
#!CommitTicketReference repository=""
revision="3d4080f19c606865f8f76d30d91c49d989a7f76c"
Fixed #29024 -- Made TestContextDecorator call disable() if setUp() raises
an exception.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29024#comment:10>