Skip to content

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

Merged
merged 20 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fef1f09
apply warnings filter as soon as possible, and remove it as late as p…
graingert Dec 13, 2024
229a64b
apply unraisablehook and threadexception before the warning plugin, s…
graingert Dec 13, 2024
10eb342
apply the warning filter during pytest_configure
graingert Dec 13, 2024
11714dd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 13, 2024
cc5b555
restore the filterwarnings marker config
graingert Dec 13, 2024
a3881db
move lsof warning to something ignorable and ignore it
graingert Dec 13, 2024
f37d07a
add newsfragment
graingert Dec 13, 2024
9a953f4
add test
graingert Dec 13, 2024
7581137
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 13, 2024
b32316c
simplify PytestFDWarning filter
graingert Dec 13, 2024
72475f4
fix PytestFDWarning module
graingert Dec 13, 2024
e2e81b2
get more patch coverage
graingert Dec 13, 2024
5349709
Merge branch 'main' into apply-warnings-filter-asap-remove-alap
graingert Dec 16, 2024
89ff09c
Merge branch 'main' into apply-warnings-filter-asap-remove-alap
graingert Dec 17, 2024
b5e00fb
Update changelog/10404.bugfix.rst
graingert Dec 17, 2024
a00679a
add more realistic create_task/unawaited coro warning test
graingert Dec 17, 2024
e828fa9
update comments
graingert Dec 17, 2024
003a51f
enable coverage on lsof tests
graingert Dec 17, 2024
808ad3d
ensure global warning filter application does not swallow default war…
graingert Dec 18, 2024
c5fb158
Update testing/test_unraisableexception.py
graingert Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ jobs:
python: "3.9"
os: ubuntu-latest
tox_env: "py39-lsof-numpy-pexpect"
use_coverage: true

- name: "ubuntu-py39-pluggy"
python: "3.9"
Expand Down
7 changes: 7 additions & 0 deletions changelog/10404.bugfix.rst
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.
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,9 @@ filterwarnings = [
"ignore:VendorImporter\\.find_spec\\(\\) not found; falling back to find_module\\(\\):ImportWarning",
# https://github.com/pytest-dev/execnet/pull/127
"ignore:isSet\\(\\) is deprecated, use is_set\\(\\) instead:DeprecationWarning",
# https://github.com/pytest-dev/pytest/issues/2366
# https://github.com/pytest-dev/pytest/pull/13057
"default::pytest.PytestFDWarning",
]
pytester_example_dir = "testing/example_scripts"
markers = [
Expand Down
8 changes: 3 additions & 5 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ def directory_arg(path: str, optname: str) -> str:
"setuponly",
"setupplan",
"stepwise",
"unraisableexception",
"threadexception",
"warnings",
"logging",
"reports",
"unraisableexception",
"threadexception",
"faulthandler",
)

Expand Down Expand Up @@ -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():
Copy link
Member Author

Choose a reason for hiding this comment

The 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:
Expand Down
4 changes: 2 additions & 2 deletions src/_pytest/pytester.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
from _pytest.reports import CollectReport
from _pytest.reports import TestReport
from _pytest.tmpdir import TempPathFactory
from _pytest.warning_types import PytestWarning
from _pytest.warning_types import PytestFDWarning


if TYPE_CHECKING:
Expand Down Expand Up @@ -188,7 +188,7 @@
"*** function {}:{}: {} ".format(*item.location),
"See issue #2366",
]
item.warn(PytestWarning("\n".join(error)))
item.warn(PytestFDWarning("\n".join(error)))

Check warning on line 191 in src/_pytest/pytester.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/pytester.py#L191

Added line #L191 was not covered by tests


# used at least by pytest-xdist plugin
Expand Down
7 changes: 7 additions & 0 deletions src/_pytest/warning_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ def format(self, **kwargs: Any) -> _W:
return self.category(self.template.format(**kwargs))


@final
class PytestFDWarning(PytestWarning):
"""When the lsof plugin finds leaked fds."""

__module__ = "pytest"


def warn_explicit_for(method: FunctionType, message: PytestWarning) -> None:
"""
Issue the warning :param:`message` for the definition of the given :param:`method`
Expand Down
59 changes: 39 additions & 20 deletions src/_pytest/warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from collections.abc import Generator
from contextlib import contextmanager
from contextlib import ExitStack
import sys
from typing import Literal
import warnings
Expand All @@ -17,20 +18,14 @@
import pytest


def pytest_configure(config: Config) -> None:
config.addinivalue_line(
"markers",
"filterwarnings(warning): add a warning filter to the given test. "
"see https://docs.pytest.org/en/stable/how-to/capture-warnings.html#pytest-mark-filterwarnings ",
)


@contextmanager
def catch_warnings_for_item(
config: Config,
ihook,
when: Literal["config", "collect", "runtest"],
item: Item | None,
*,
record: bool = True,
) -> Generator[None]:
"""Context manager that catches warnings generated in the contained execution block.

Expand All @@ -40,10 +35,7 @@ def catch_warnings_for_item(
"""
config_filters = config.getini("filterwarnings")
cmdline_filters = config.known_args_namespace.pythonwarnings or []
with warnings.catch_warnings(record=True) as log:
# mypy can't infer that record=True means log is not None; help it.
assert log is not None

Comment on lines -43 to -46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at typeshed I think the comment was previously incorrect and the assert wasn't needed https://github.com/python/typeshed/blob/54e1c6ad58961c1faebc6842698eec78f1298f92/stdlib/warnings.pyi#L82
But ye with current logic mypy can't infer if record -> log is not None later in the func

with warnings.catch_warnings(record=record) as log:
if not sys.warnoptions:
# If user is not explicitly configuring warning filters, show deprecation warnings by default (#2908).
warnings.filterwarnings("always", category=DeprecationWarning)
Expand All @@ -64,15 +56,19 @@ def catch_warnings_for_item(
try:
yield
finally:
for warning_message in log:
ihook.pytest_warning_recorded.call_historic(
kwargs=dict(
warning_message=warning_message,
nodeid=nodeid,
when=when,
location=None,
if record:
# mypy can't infer that record=True means log is not None; help it.
assert log is not None

for warning_message in log:
ihook.pytest_warning_recorded.call_historic(
kwargs=dict(
warning_message=warning_message,
nodeid=nodeid,
when=when,
location=None,
)
)
)


def warning_record_to_str(warning_message: warnings.WarningMessage) -> str:
Expand Down Expand Up @@ -131,3 +127,26 @@ def pytest_load_initial_conftests(
config=early_config, ihook=early_config.hook, when="config", item=None
):
return (yield)


def pytest_configure(config: Config) -> None:
with ExitStack() as stack:
stack.enter_context(
catch_warnings_for_item(
config=config,
ihook=config.hook,
when="config",
item=None,
# this disables recording because the terminalreporter has
# finished by the time it comes to reporting logged warnings
# from the end of config cleanup. So for now, this is only
# useful for setting a warning filter with an 'error' action.
record=False,
)
)
config.addinivalue_line(
"markers",
"filterwarnings(warning): add a warning filter to the given test. "
"see https://docs.pytest.org/en/stable/how-to/capture-warnings.html#pytest-mark-filterwarnings ",
)
config.add_cleanup(stack.pop_all().close)
2 changes: 2 additions & 0 deletions src/pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
from _pytest.warning_types import PytestConfigWarning
from _pytest.warning_types import PytestDeprecationWarning
from _pytest.warning_types import PytestExperimentalApiWarning
from _pytest.warning_types import PytestFDWarning
from _pytest.warning_types import PytestRemovedIn9Warning
from _pytest.warning_types import PytestUnhandledThreadExceptionWarning
from _pytest.warning_types import PytestUnknownMarkWarning
Expand Down Expand Up @@ -124,6 +125,7 @@
"PytestConfigWarning",
"PytestDeprecationWarning",
"PytestExperimentalApiWarning",
"PytestFDWarning",
"PytestPluginManager",
"PytestRemovedIn9Warning",
"PytestUnhandledThreadExceptionWarning",
Expand Down
124 changes: 117 additions & 7 deletions testing/test_unraisableexception.py
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
Expand Down Expand Up @@ -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="""
Expand All @@ -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
Expand All @@ -236,6 +251,101 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we run this without -Werror we can check that normal warning functionality works.

Copy link
Member Author

Choose a reason for hiding this comment

The 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



def test_refcycle_unraisable_warning_filter_default(pytester: Pytester) -> None:
# note this time we use a default warning filter for pytester
# and run it in a subprocess, because the warning can only go to the
# sys.stdout rather than the terminal reporter, which has already
# finished.
# see: https://github.com/pytest-dev/pytest/pull/13057#discussion_r1888396126
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_subprocess("-Wdefault")

assert result.ret == pytest.ExitCode.OK

# TODO: should be warnings=1, but the outcome has already come out
# by the time the warning triggers
result.assert_outcomes(passed=1)
result.stderr.fnmatch_lines("ValueError: del is broken")


@pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning")
def test_possibly_none_excinfo(pytester: Pytester) -> None:
pytester.makepyfile(
Expand Down
Loading