[Django] #32557: Fail tests when unraisable exceptions occur
Django
---------------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
Following django-developers discussion: https://groups.google.com/g
/django-developers/c/ea21KjiGiY4/m/7HTnQaS8BwAJ
Django's test runner should install an unraisable exception hook and fail
the test run if it is ever triggered. The check could be done just once at
the end of the test run (in subprocesses when run in parallel), because
unraisable exceptions already print their traceback by default.
--
Ticket URL: <https://code.djangoproject.com/ticket/32557>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Johnson):
There's a reference implementation in pytest we might be able to copy:
https://github.com/pytest-dev/pytest/pull/8055
Additionally here's an implementation I've used - calling the context
manager in an overridden DiscoverRunner.run_tests to wrap the whole test
run:
{{{
@contextmanager
def fail_if_any_unraisable_exceptions():
orig_unraisable_hook = sys.unraisablehook
hook_called = multiprocessing.Value("b", lock=False)
hook_called.value = 0
def hook(unraisable, /):
hook_called.value = 1
orig_unraisable_hook(unraisable)
sys.unraisablehook = hook
try:
yield
finally:
sys.unraisablehook = orig_unraisable_hook
if hook_called.value:
print(
"❌ Unraisable exceptions reported, failing test run - see
above.",
file=sys.stderr,
)
sys.exit(1)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:1>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
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 Mariusz Felisiak):
* cc: Carlton Gibson, Tom Forbes (added)
* stage: Unreviewed => Accepted
Comment:
Tentatively accepted. I would like to check this in practice.
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:2>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Chris Jerdonek (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:3>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
One comment: the `sys.unraisablehook` docs say:
> Called when an exception has occurred but there is no way for Python to
handle it. For example, when a destructor raises an exception or during
garbage collection (`gc.collect()`).
Thus, with this patch, if we disable garbage collection as in this PR:
https://github.com/django/django/pull/14142
then it seems like we won't get the benefit of this patch in checking for
exceptions that occur during garbage collection.
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:4>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Johnson):
Exceptions in `__del__` are only one kind of unraisable exception. The
other kind I know of that motivated me to look into them are from async
tasks that were not joined by the task that spawned them.
Python already prints tracebacks for unraisable exceptions, the main point
of this ticket is to ensure that such tracebacks in tests do not get
missed because the tests exit with a 0 status code. The default behaviour
looks like this:
{{{
$ cat test.py
class Naughty:
def __del__(self):
1 / 0
Naughty()
$ python test.py
Exception ignored in: <function Naughty.__del__ at 0x1015da8b0>
Traceback (most recent call last):
File "/Users/chainz/tmp/test/test.py", line 3, in __del__
1 / 0
ZeroDivisionError: division by zero
$ echo $?
0
}}}
The PR disabling gc affects only objects in cycles, which are a minority.
Moreover all garbage is still collected at interpreter shutdown, so any
unraisable exceptions would have their traceback printed at that point,
although they indeed not be picked up by a sys.unraisablehook installed
only for the duration of the test run. That said, the gc PR affects only
the Django test suite which has no unraisable exceptions at the moment,
and it's likely if any were added someone would spot them.
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:5>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Thomas Grainger):
You should be able to run gc.collect in a loop until no more objects get
collected just before you unset your unraisablehook
--
Ticket URL: <https://code.djangoproject.com/ticket/32557#comment:6>