-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
apply warnings filter as soon as possible, and remove it as late as possible #13057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
fef1f09
229a64b
10eb342
11714dd
cc5b555
a3881db
f37d07a
9a953f4
7581137
b32316c
72475f4
e2e81b2
5349709
89ff09c
b5e00fb
a00679a
e828fa9
003a51f
808ad3d
c5fb158
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
Apply filterwarnings from config/cli as soon as possible, and revert them as late as possible | ||
so that warnings as errors are collected throughout the pytest run and before the | ||
unraisable and threadexcept hooks are removed. | ||
|
||
This allows very late warnings and unraisable/threadexcept exceptions to fail the test suite. | ||
|
||
This also changes the warning that the lsof plugin issues from PytestWarning to the new warning PytestFDWarning so it can be more easily filtered. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -264,11 +264,11 @@ def directory_arg(path: str, optname: str) -> str: | |
"setuponly", | ||
"setupplan", | ||
"stepwise", | ||
"unraisableexception", | ||
"threadexception", | ||
"warnings", | ||
"logging", | ||
"reports", | ||
"unraisableexception", | ||
"threadexception", | ||
"faulthandler", | ||
) | ||
|
||
|
@@ -1112,9 +1112,7 @@ def add_cleanup(self, func: Callable[[], None]) -> None: | |
def _do_configure(self) -> None: | ||
assert not self._configured | ||
self._configured = True | ||
with warnings.catch_warnings(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure what this was for - it just disables the user specified warning filter during config. We don't want this. |
||
warnings.simplefilter("default") | ||
self.hook.pytest_configure.call_historic(kwargs=dict(config=self)) | ||
self.hook.pytest_configure.call_historic(kwargs=dict(config=self)) | ||
|
||
def _ensure_unconfigure(self) -> None: | ||
try: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
from __future__ import annotations | ||
|
||
from collections.abc import Generator | ||
import contextlib | ||
import gc | ||
import sys | ||
from unittest import mock | ||
|
@@ -203,7 +205,25 @@ class MyError(BaseException): | |
) | ||
|
||
|
||
def test_create_task_unraisable(pytester: Pytester) -> None: | ||
def _set_gc_state(enabled: bool) -> bool: | ||
was_enabled = gc.isenabled() | ||
if enabled: | ||
gc.enable() | ||
else: | ||
gc.disable() | ||
return was_enabled | ||
|
||
|
||
@contextlib.contextmanager | ||
def _disable_gc() -> Generator[None]: | ||
was_enabled = _set_gc_state(enabled=False) | ||
try: | ||
yield | ||
finally: | ||
_set_gc_state(enabled=was_enabled) | ||
|
||
|
||
def test_refcycle_unraisable(pytester: Pytester) -> None: | ||
# see: https://github.com/pytest-dev/pytest/issues/10404 | ||
pytester.makepyfile( | ||
test_it=""" | ||
|
@@ -221,13 +241,8 @@ def test_it(): | |
""" | ||
) | ||
|
||
was_enabled = gc.isenabled() | ||
gc.disable() | ||
try: | ||
with _disable_gc(): | ||
result = pytester.runpytest() | ||
finally: | ||
if was_enabled: | ||
gc.enable() | ||
|
||
# TODO: should be a test failure or error | ||
assert result.ret == pytest.ExitCode.INTERNAL_ERROR | ||
|
@@ -236,6 +251,68 @@ def test_it(): | |
result.stderr.fnmatch_lines("ValueError: del is broken") | ||
|
||
|
||
@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning") | ||
def test_refcycle_unraisable_warning_filter(pytester: Pytester) -> None: | ||
# note that the host pytest warning filter is disabled and the pytester | ||
# warning filter applies during config teardown of unraisablehook. | ||
# see: https://github.com/pytest-dev/pytest/issues/10404 | ||
pytester.makepyfile( | ||
test_it=""" | ||
import pytest | ||
|
||
class BrokenDel: | ||
def __init__(self): | ||
self.self = self # make a reference cycle | ||
|
||
def __del__(self): | ||
raise ValueError("del is broken") | ||
|
||
def test_it(): | ||
BrokenDel() | ||
""" | ||
) | ||
|
||
with _disable_gc(): | ||
result = pytester.runpytest("-Werror") | ||
|
||
# TODO: should be a test failure or error | ||
assert result.ret == pytest.ExitCode.INTERNAL_ERROR | ||
|
||
result.assert_outcomes(passed=1) | ||
result.stderr.fnmatch_lines("ValueError: del is broken") | ||
|
||
|
||
@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning") | ||
def test_create_task_raises_unraisable_warning_filter(pytester: Pytester) -> None: | ||
# note that the host pytest warning filter is disabled and the pytester | ||
# warning filter applies during config teardown of unraisablehook. | ||
# see: https://github.com/pytest-dev/pytest/issues/10404 | ||
# This is a dupe of the above test, but using the exact reproducer from | ||
# the issue | ||
pytester.makepyfile( | ||
test_it=""" | ||
import asyncio | ||
import pytest | ||
|
||
async def my_task(): | ||
pass | ||
|
||
def test_scheduler_must_be_created_within_running_loop() -> None: | ||
with pytest.raises(RuntimeError) as _: | ||
asyncio.create_task(my_task()) | ||
""" | ||
) | ||
|
||
with _disable_gc(): | ||
result = pytester.runpytest("-Werror") | ||
|
||
# TODO: should be a test failure or error | ||
assert result.ret == pytest.ExitCode.INTERNAL_ERROR | ||
|
||
result.assert_outcomes(passed=1) | ||
result.stderr.fnmatch_lines("RuntimeWarning: coroutine 'my_task' was never awaited") | ||
Comment on lines
+306
to
+313
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we run this without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah I broke it! Very good catch! I'll send a commit to fix it now |
||
|
||
|
||
@pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning") | ||
def test_possibly_none_excinfo(pytester: Pytester) -> None: | ||
pytester.makepyfile( | ||
|
Uh oh!
There was an error while loading. Please reload this page.