diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4058716b47d..5814c81fb30 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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" diff --git a/changelog/10404.bugfix.rst b/changelog/10404.bugfix.rst new file mode 100644 index 00000000000..4c98ea03d64 --- /dev/null +++ b/changelog/10404.bugfix.rst @@ -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. diff --git a/pyproject.toml b/pyproject.toml index 0a695e0247e..df633e0a092 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 = [ diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 6160f780b1b..0161f5952b8 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -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(): - 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: diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 412d850d2da..59839562031 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -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: @@ -188,7 +188,7 @@ def pytest_runtest_protocol(self, item: Item) -> Generator[None, object, object] "*** function {}:{}: {} ".format(*item.location), "See issue #2366", ] - item.warn(PytestWarning("\n".join(error))) + item.warn(PytestFDWarning("\n".join(error))) # used at least by pytest-xdist plugin diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index b8e9998cd2e..8c9ff2d9a36 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -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` diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py index 64ea3ff222d..806681a5020 100644 --- a/src/_pytest/warnings.py +++ b/src/_pytest/warnings.py @@ -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 @@ -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. @@ -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 - + 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) @@ -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: @@ -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) diff --git a/src/pytest/__init__.py b/src/pytest/__init__.py index f0c3516f4cc..70096d6593e 100644 --- a/src/pytest/__init__.py +++ b/src/pytest/__init__.py @@ -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 @@ -124,6 +125,7 @@ "PytestConfigWarning", "PytestDeprecationWarning", "PytestExperimentalApiWarning", + "PytestFDWarning", "PytestPluginManager", "PytestRemovedIn9Warning", "PytestUnhandledThreadExceptionWarning", diff --git a/testing/test_unraisableexception.py b/testing/test_unraisableexception.py index 5aa6aa773d9..3f191073e2b 100644 --- a/testing/test_unraisableexception.py +++ b/testing/test_unraisableexception.py @@ -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,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") + + +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(