From dbaa9f4dbeee09d83ddc581c83819e65c11e14ad Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 29 Aug 2020 10:34:13 -0300 Subject: [PATCH] Integrate warnings filtering directly into Config Warnings are a central part of Python, so much that Python itself has command-line and environtment variables to handle warnings. By moving the concept of warning handling into Config, it becomes natural to filter warnings issued as early as possible, even before the "_pytest.warnings" plugin is given a chance to spring into action. This also avoids the weird coupling between config and the warnings plugin that was required before. Fix #6681 Fix #2891 Fix #7620 Fix #7626 Close #7649 --- changelog/6681.improvement.rst | 3 + src/_pytest/assertion/rewrite.py | 4 +- src/_pytest/config/__init__.py | 138 +++++++++++++++++++++++++------ src/_pytest/faulthandler.py | 5 +- src/_pytest/main.py | 14 ++++ src/_pytest/warnings.py | 101 +++------------------- testing/test_config.py | 23 ++++-- testing/test_warnings.py | 23 ++++-- 8 files changed, 180 insertions(+), 131 deletions(-) create mode 100644 changelog/6681.improvement.rst diff --git a/changelog/6681.improvement.rst b/changelog/6681.improvement.rst new file mode 100644 index 00000000000..cc586e6a337 --- /dev/null +++ b/changelog/6681.improvement.rst @@ -0,0 +1,3 @@ +Internal pytest warnings issued during the early stages of initialization are now properly handled and can filtered through :confval:`filterwarnings` or ``--pythonwarnings/-W``. + +This also fixes a number of long standing issues: `#2891 `__, `#7620 `__, `#7426 `__. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 50fa4d405a2..48587b17e89 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -267,13 +267,11 @@ def mark_rewrite(self, *names: str) -> None: def _warn_already_imported(self, name: str) -> None: from _pytest.warning_types import PytestAssertRewriteWarning - from _pytest.warnings import _issue_warning_captured - _issue_warning_captured( + self.config.issue_config_time_warning( PytestAssertRewriteWarning( "Module already imported so cannot be rewritten: %s" % name ), - self.config.hook, stacklevel=5, ) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 1c6ad32882d..6473eedc22a 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -6,6 +6,7 @@ import enum import inspect import os +import re import shlex import sys import types @@ -15,6 +16,7 @@ from typing import Any from typing import Callable from typing import Dict +from typing import Generator from typing import IO from typing import Iterable from typing import Iterator @@ -342,6 +344,13 @@ def __init__(self) -> None: self._noconftest = False self._duplicatepaths = set() # type: Set[py.path.local] + # plugins that were explicitly skipped with pytest.skip + # list of (module name, skip reason) + # previously we would issue a warning when a plugin was skipped, but + # since we refactored warnings as first citizens of Config, they are + # just stored here to be used later. + self.skipped_plugins = [] # type: List[Tuple[str, str]] + self.add_hookspecs(_pytest.hookspec) self.register(self) if os.environ.get("PYTEST_DEBUG"): @@ -694,13 +703,7 @@ def import_plugin(self, modname: str, consider_entry_points: bool = False) -> No ).with_traceback(e.__traceback__) from e except Skipped as e: - from _pytest.warnings import _issue_warning_captured - - _issue_warning_captured( - PytestConfigWarning("skipped plugin {!r}: {}".format(modname, e.msg)), - self.hook, - stacklevel=2, - ) + self.skipped_plugins.append((modname, e.msg or "")) else: mod = sys.modules[importspec] self.register(mod, modname) @@ -1092,6 +1095,9 @@ def _preparse(self, args: List[str], addopts: bool = True) -> None: self._validate_args(self.getini("addopts"), "via addopts config") + args ) + self.known_args_namespace = self._parser.parse_known_args( + args, namespace=copy.copy(self.option) + ) self._checkversion() self._consider_importhook(args) self.pluginmanager.consider_preparse(args, exclude_only=False) @@ -1100,10 +1106,10 @@ def _preparse(self, args: List[str], addopts: bool = True) -> None: # plugins are going to be loaded. self.pluginmanager.load_setuptools_entrypoints("pytest11") self.pluginmanager.consider_env() - self.known_args_namespace = ns = self._parser.parse_known_args( - args, namespace=copy.copy(self.option) - ) + self._validate_plugins() + self._warn_about_skipped_plugins() + if self.known_args_namespace.confcutdir is None and self.inifile: confcutdir = py.path.local(self.inifile).dirname self.known_args_namespace.confcutdir = confcutdir @@ -1112,20 +1118,22 @@ def _preparse(self, args: List[str], addopts: bool = True) -> None: early_config=self, args=args, parser=self._parser ) except ConftestImportFailure as e: - if ns.help or ns.version: + if self.known_args_namespace.help or self.known_args_namespace.version: # we don't want to prevent --help/--version to work # so just let is pass and print a warning at the end - from _pytest.warnings import _issue_warning_captured - - _issue_warning_captured( + self.issue_config_time_warning( PytestConfigWarning( "could not load initial conftests: {}".format(e.path) ), - self.hook, stacklevel=2, ) else: raise + + @hookimpl(hookwrapper=True) + def pytest_collection(self) -> Generator[None, None, None]: + """Validate invalid ini keys after collection is done.""" + yield self._validate_keys() def _checkversion(self) -> None: @@ -1165,7 +1173,6 @@ def _validate_plugins(self) -> None: missing_plugins = [] for required_plugin in required_plugins: - spec = None try: spec = Requirement(required_plugin) except InvalidRequirement: @@ -1187,11 +1194,7 @@ def _warn_or_fail_if_strict(self, message: str) -> None: if self.known_args_namespace.strict_config: fail(message, pytrace=False) - from _pytest.warnings import _issue_warning_captured - - _issue_warning_captured( - PytestConfigWarning(message), self.hook, stacklevel=3, - ) + self.issue_config_time_warning(PytestConfigWarning(message), stacklevel=3) def _get_unknown_ini_keys(self) -> List[str]: parser_inicfg = self._parser._inidict @@ -1222,6 +1225,51 @@ def parse(self, args: List[str], addopts: bool = True) -> None: except PrintHelp: pass + def issue_config_time_warning(self, warning: Warning, stacklevel: int) -> None: + """Issue and handle a warning during the "configure" stage. + + During ``pytest_configure`` we can't capture warnings using the ``catch_warnings_for_item`` + function because it is not possible to have hookwrappers around ``pytest_configure``. + + :param warning: The warning instance. + :param stacklevel: stacklevel forwarded to warnings.warn. + """ + + cmdline_filters = self.known_args_namespace.pythonwarnings or [] + inifilters = self.getini("filterwarnings") + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always", type(warning)) + + # Make -W/filterwarnings apply to these warnings as well. + # This code is the same as in catch_warnings_for_item(). + for arg in inifilters: + warnings.filterwarnings(*parse_warning_filter(arg, escape=False)) + for arg in cmdline_filters: + warnings.filterwarnings(*parse_warning_filter(arg, escape=True)) + + warnings.warn(warning, stacklevel=stacklevel) + + if records: + frame = sys._getframe(stacklevel - 1) + location = frame.f_code.co_filename, frame.f_lineno, frame.f_code.co_name + self.hook.pytest_warning_captured.call_historic( + kwargs=dict( + warning_message=records[0], + when="config", + item=None, + location=location, + ) + ) + self.hook.pytest_warning_recorded.call_historic( + kwargs=dict( + warning_message=records[0], + when="config", + nodeid="", + location=location, + ) + ) + def addinivalue_line(self, name: str, line: str) -> None: """Add a line to an ini-file option. The option must have been declared but might not yet be set in which case the line becomes @@ -1365,8 +1413,6 @@ def getvalueorskip(self, name: str, path=None): def _warn_about_missing_assertion(self, mode: str) -> None: if not _assertion_supported(): - from _pytest.warnings import _issue_warning_captured - if mode == "plain": warning_text = ( "ASSERTIONS ARE NOT EXECUTED" @@ -1381,8 +1427,15 @@ def _warn_about_missing_assertion(self, mode: str) -> None: "by the underlying Python interpreter " "(are you using python -O?)\n" ) - _issue_warning_captured( - PytestConfigWarning(warning_text), self.hook, stacklevel=3, + self.issue_config_time_warning( + PytestConfigWarning(warning_text), stacklevel=3, + ) + + def _warn_about_skipped_plugins(self) -> None: + for module_name, msg in self.pluginmanager.skipped_plugins: + self.issue_config_time_warning( + PytestConfigWarning("skipped plugin {!r}: {}".format(module_name, msg)), + stacklevel=2, ) @@ -1435,3 +1488,38 @@ def _strtobool(val: str) -> bool: return False else: raise ValueError("invalid truth value {!r}".format(val)) + + +@lru_cache(maxsize=50) +def parse_warning_filter( + arg: str, *, escape: bool +) -> "Tuple[str, str, Type[Warning], str, int]": + """Parse a warnings filter string. + + This is copied from warnings._setoption, but does not apply the filter, + only parses it, and makes the escaping optional. + """ + parts = arg.split(":") + if len(parts) > 5: + raise warnings._OptionError("too many fields (max 5): {!r}".format(arg)) + while len(parts) < 5: + parts.append("") + action_, message, category_, module, lineno_ = [s.strip() for s in parts] + action = warnings._getaction(action_) # type: str # type: ignore[attr-defined] + category = warnings._getcategory( + category_ + ) # type: Type[Warning] # type: ignore[attr-defined] + if message and escape: + message = re.escape(message) + if module and escape: + module = re.escape(module) + r"\Z" + if lineno_: + try: + lineno = int(lineno_) + if lineno < 0: + raise ValueError + except (ValueError, OverflowError) as e: + raise warnings._OptionError("invalid lineno {!r}".format(lineno_)) from e + else: + lineno = 0 + return action, message, category, module, lineno diff --git a/src/_pytest/faulthandler.py b/src/_pytest/faulthandler.py index e4a952966af..d0cc0430c49 100644 --- a/src/_pytest/faulthandler.py +++ b/src/_pytest/faulthandler.py @@ -30,18 +30,15 @@ def pytest_configure(config: Config) -> None: # of enabling faulthandler before each test executes. config.pluginmanager.register(FaultHandlerHooks(), "faulthandler-hooks") else: - from _pytest.warnings import _issue_warning_captured - # Do not handle dumping to stderr if faulthandler is already enabled, so warn # users that the option is being ignored. timeout = FaultHandlerHooks.get_timeout_config_value(config) if timeout > 0: - _issue_warning_captured( + config.issue_config_time_warning( pytest.PytestConfigWarning( "faulthandler module enabled before pytest configuration step, " "'faulthandler_timeout' option ignored" ), - config.hook, stacklevel=2, ) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 29b1e5f7006..4ab91f82c07 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -69,6 +69,20 @@ def pytest_addoption(parser: Parser) -> None: const=1, help="exit instantly on first error or failed test.", ) + group = parser.getgroup("pytest-warnings") + group.addoption( + "-W", + "--pythonwarnings", + action="append", + help="set which warnings to report, see -W option of python itself.", + ) + parser.addini( + "filterwarnings", + type="linelist", + help="Each line specifies a pattern for " + "warnings.filterwarnings. " + "Processed after -W/--pythonwarnings.", + ) group._addoption( "--maxfail", metavar="num", diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py index 4478d8723c6..5045cd2ffed 100644 --- a/src/_pytest/warnings.py +++ b/src/_pytest/warnings.py @@ -1,77 +1,21 @@ -import re import sys import warnings from contextlib import contextmanager -from functools import lru_cache from typing import Generator from typing import Optional -from typing import Tuple import pytest from _pytest.compat import TYPE_CHECKING from _pytest.config import Config -from _pytest.config.argparsing import Parser +from _pytest.config import parse_warning_filter from _pytest.main import Session from _pytest.nodes import Item from _pytest.terminal import TerminalReporter if TYPE_CHECKING: - from typing import Type from typing_extensions import Literal -@lru_cache(maxsize=50) -def _parse_filter( - arg: str, *, escape: bool -) -> "Tuple[str, str, Type[Warning], str, int]": - """Parse a warnings filter string. - - This is copied from warnings._setoption, but does not apply the filter, - only parses it, and makes the escaping optional. - """ - parts = arg.split(":") - if len(parts) > 5: - raise warnings._OptionError("too many fields (max 5): {!r}".format(arg)) - while len(parts) < 5: - parts.append("") - action_, message, category_, module, lineno_ = [s.strip() for s in parts] - action = warnings._getaction(action_) # type: str # type: ignore[attr-defined] - category = warnings._getcategory( - category_ - ) # type: Type[Warning] # type: ignore[attr-defined] - if message and escape: - message = re.escape(message) - if module and escape: - module = re.escape(module) + r"\Z" - if lineno_: - try: - lineno = int(lineno_) - if lineno < 0: - raise ValueError - except (ValueError, OverflowError) as e: - raise warnings._OptionError("invalid lineno {!r}".format(lineno_)) from e - else: - lineno = 0 - return (action, message, category, module, lineno) - - -def pytest_addoption(parser: Parser) -> None: - group = parser.getgroup("pytest-warnings") - group.addoption( - "-W", - "--pythonwarnings", - action="append", - help="set which warnings to report, see -W option of python itself.", - ) - parser.addini( - "filterwarnings", - type="linelist", - help="Each line specifies a pattern for " - "warnings.filterwarnings. " - "Processed after -W/--pythonwarnings.", - ) - - def pytest_configure(config: Config) -> None: config.addinivalue_line( "markers", @@ -93,7 +37,7 @@ def catch_warnings_for_item( Each warning captured triggers the ``pytest_warning_recorded`` hook. """ - cmdline_filters = config.getoption("pythonwarnings") or [] + cmdline_filters = config.known_args_namespace.pythonwarnings or [] inifilters = config.getini("filterwarnings") with warnings.catch_warnings(record=True) as log: # mypy can't infer that record=True means log is not None; help it. @@ -107,16 +51,16 @@ def catch_warnings_for_item( # Filters should have this precedence: mark, cmdline options, ini. # Filters should be applied in the inverse order of precedence. for arg in inifilters: - warnings.filterwarnings(*_parse_filter(arg, escape=False)) + warnings.filterwarnings(*parse_warning_filter(arg, escape=False)) for arg in cmdline_filters: - warnings.filterwarnings(*_parse_filter(arg, escape=True)) + warnings.filterwarnings(*parse_warning_filter(arg, escape=True)) nodeid = "" if item is None else item.nodeid if item is not None: for mark in item.iter_markers(name="filterwarnings"): for arg in mark.args: - warnings.filterwarnings(*_parse_filter(arg, escape=False)) + warnings.filterwarnings(*parse_warning_filter(arg, escape=False)) yield @@ -189,30 +133,11 @@ def pytest_sessionfinish(session: Session) -> Generator[None, None, None]: yield -def _issue_warning_captured(warning: Warning, hook, stacklevel: int) -> None: - """A function that should be used instead of calling ``warnings.warn`` - directly when we are in the "configure" stage. - - At this point the actual options might not have been set, so we manually - trigger the pytest_warning_recorded hook so we can display these warnings - in the terminal. This is a hack until we can sort out #2891. - - :param warning: The warning instance. - :param hook: The hook caller. - :param stacklevel: stacklevel forwarded to warnings.warn. - """ - with warnings.catch_warnings(record=True) as records: - warnings.simplefilter("always", type(warning)) - warnings.warn(warning, stacklevel=stacklevel) - frame = sys._getframe(stacklevel - 1) - location = frame.f_code.co_filename, frame.f_lineno, frame.f_code.co_name - hook.pytest_warning_captured.call_historic( - kwargs=dict( - warning_message=records[0], when="config", item=None, location=location - ) - ) - hook.pytest_warning_recorded.call_historic( - kwargs=dict( - warning_message=records[0], when="config", nodeid="", location=location - ) - ) +@pytest.hookimpl(hookwrapper=True) +def pytest_load_initial_conftests( + early_config: "Config", +) -> Generator[None, None, None]: + with catch_warnings_for_item( + config=early_config, ihook=early_config.hook, when="config", item=None + ): + yield diff --git a/testing/test_config.py b/testing/test_config.py index 346edb3304c..579a46c8907 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -232,6 +232,7 @@ def test_confcutdir(self, testdir): ), ], ) + @pytest.mark.filterwarnings("default") def test_invalid_ini_keys( self, testdir, ini_file_text, invalid_keys, warning_output, exception_text ): @@ -250,10 +251,22 @@ def pytest_addoption(parser): result.stdout.fnmatch_lines(warning_output) if exception_text: - with pytest.raises(pytest.fail.Exception, match=exception_text): - testdir.runpytest("--strict-config") - else: - testdir.runpytest("--strict-config") + result = testdir.runpytest("--strict-config") + result.stdout.fnmatch_lines("INTERNALERROR>*" + exception_text) + + @pytest.mark.filterwarnings("default") + def test_silence_unknown_key_warning(self, testdir: Testdir) -> None: + """Unknown config key warnings can be silenced using filterwarnings (#7620)""" + testdir.makeini( + """ + [pytest] + filterwarnings = + ignore:Unknown config ini key:pytest.PytestConfigWarning + foobar=1 + """ + ) + result = testdir.runpytest() + result.stdout.no_fnmatch_line("*PytestConfigWarning*") @pytest.mark.parametrize( "ini_file_text, exception_text", @@ -1132,7 +1145,7 @@ def pytest_load_initial_conftests(self): pm.register(m) hc = pm.hook.pytest_load_initial_conftests values = hc._nonwrappers + hc._wrappers - expected = ["_pytest.config", m.__module__, "_pytest.capture"] + expected = ["_pytest.config", m.__module__, "_pytest.capture", "_pytest.warnings"] assert [x.function.__module__ for x in values] == expected diff --git a/testing/test_warnings.py b/testing/test_warnings.py index 550ebb4b8e7..b7a231094fd 100644 --- a/testing/test_warnings.py +++ b/testing/test_warnings.py @@ -240,9 +240,8 @@ def test_func(): def test_warning_captured_hook(testdir): testdir.makeconftest( """ - from _pytest.warnings import _issue_warning_captured def pytest_configure(config): - _issue_warning_captured(UserWarning("config warning"), config.hook, stacklevel=2) + config.issue_config_time_warning(UserWarning("config warning"), stacklevel=2) """ ) testdir.makepyfile( @@ -716,10 +715,22 @@ def test_issue4445_preparse(self, testdir, capwarn): assert "config{sep}__init__.py".format(sep=os.sep) in file assert func == "_preparse" + @pytest.mark.filterwarnings("default") + def test_conftest_warning_captured(self, testdir: Testdir) -> None: + """Warnings raised during importing of conftest.py files is captured (#2891).""" + testdir.makeconftest( + """ + import warnings + warnings.warn(UserWarning("my custom warning")) + """ + ) + result = testdir.runpytest() + result.stdout.fnmatch_lines( + ["conftest.py:2", "*UserWarning: my custom warning*"] + ) + def test_issue4445_import_plugin(self, testdir, capwarn): - """#4445: Make sure the warning points to a reasonable location - See origin of _issue_warning_captured at: _pytest.config.__init__.py:585 - """ + """#4445: Make sure the warning points to a reasonable location""" testdir.makepyfile( some_plugin=""" import pytest @@ -738,7 +749,7 @@ def test_issue4445_import_plugin(self, testdir, capwarn): assert "skipped plugin 'some_plugin': thing" in str(warning.message) assert "config{sep}__init__.py".format(sep=os.sep) in file - assert func == "import_plugin" + assert func == "_warn_about_skipped_plugins" def test_issue4445_issue5928_mark_generator(self, testdir): """#4445 and #5928: Make sure the warning from an unknown mark points to