Opened 4 years ago
Last modified 2 years ago
#32557 new New feature
Fail tests when unraisable exceptions occur
Reported by: | Adam Johnson | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Carlton Gibson, Tom Forbes, Chris Jerdonek | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
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.
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 , 2 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: