Opened 4 years ago
Last modified 3 years ago
#32557 new New feature
Fail tests when unraisable exceptions occur
Description ¶
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.
According to the ticket's flags, the next step(s) to move this issue forward are:
- To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (6)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Tentatively accepted. I would like to check this in practice.
comment:3 by , 4 years ago
Cc: | added |
---|
comment:4 by , 4 years ago
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.
comment:5 by , 4 years ago
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.
comment:6 by , 3 years ago
You should be able to run gc.collect in a loop until no more objects get collected just before you unset your unraisablehook
Or just a constant number of gc.collect cycles 4 should be enough for anyone
https://github.com/python-trio/trio/blob/7b86d20549bba2629eb929a72022a19b999c91d5/trio/_core/tests/tutil.py#L55
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: