From 4cddffa2f4c3f5944dd190dea0f53d1c8922b757 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 11 Oct 2026 13:56:36 +0200 Subject: [PATCH] refactor(warnings): emit deprecations through WarningTemplate.warn Follow-up to #14931, which made every constant in _pytest.deprecated an UnformattedWarning and had each call site do warnings.warn(X.format(), stacklevel=N). Rename UnformattedWarning to WarningTemplate, keeping the old name as an alias, and give it warn(): it builds the fresh instance, absorbs the +1 stacklevel for its own frame so callers keep the stacklevel they had, and hides that frame from tracebacks. format() only runs str.format() when kwargs are given, so constant messages may carry literal braces. A guard test walks every _pytest module for module-level Warning instances. It found INI_STRING_TYPE_NON_STR_VALUE, which #14931 missed and which still grew its traceback under -W error; it is a template now. Co-Authored-By: Claude Opus 5.5 via Claude Code --- changelog/15137.bugfix.rst | 1 + src/_pytest/config/__init__.py | 14 ++----- src/_pytest/deprecated.py | 48 ++++++++++----------- src/_pytest/fixtures.py | 24 ++++------- src/_pytest/mark/structures.py | 8 ++-- src/_pytest/monkeypatch.py | 4 +- src/_pytest/python.py | 2 +- src/_pytest/warning_types.py | 41 ++++++++++++++---- testing/deprecated_test.py | 77 ++++++++++++++++++++++++++++++++++ testing/test_warning_types.py | 55 ++++++++++++++++++++++++ 10 files changed, 207 insertions(+), 67 deletions(-) create mode 100644 changelog/15137.bugfix.rst diff --git a/changelog/15137.bugfix.rst b/changelog/15137.bugfix.rst new file mode 100644 index 00000000000..25d66cae6c6 --- /dev/null +++ b/changelog/15137.bugfix.rst @@ -0,0 +1 @@ +The deprecation warning for passing a non-string value to a ``string``-typed ini option no longer reuses one warning instance, so it no longer accumulates traceback frames when warnings are turned into errors -- the case :issue:`14912` fixed for the other deprecations. diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 60ba5f54a89..feeca449fc4 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -301,11 +301,9 @@ def console_main() -> int: This function is slated for removal in pytest 10. It is not meant for programmable use; use :func:`pytest.main` instead. """ - import warnings - from _pytest.deprecated import CONSOLE_MAIN - warnings.warn(CONSOLE_MAIN.format(), stacklevel=2) + CONSOLE_MAIN.warn(stacklevel=2) return _console_main() @@ -1283,10 +1281,7 @@ def inicfg(self) -> _DeprecatedInicfgProxy: @property def inicfg(self) -> _DeprecatedInicfgProxy: - warnings.warn( - _pytest.deprecated.CONFIG_INICFG.format(), - stacklevel=2, - ) + _pytest.deprecated.CONFIG_INICFG.warn(stacklevel=2) return _DeprecatedInicfgProxy(self) @property @@ -2052,10 +2047,7 @@ def _getini_ini( return _strtobool(str(value).strip()) elif type == "string": if not isinstance(value, str): - warnings.warn( - _pytest.deprecated.INI_STRING_TYPE_NON_STR_VALUE, - stacklevel=2, - ) + _pytest.deprecated.INI_STRING_TYPE_NON_STR_VALUE.warn(stacklevel=2) return value elif type == "int": if not isinstance(value, str): diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index a94aeb735e5..a66e9c08764 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -5,19 +5,16 @@ be removed when the time comes. All constants defined in this module should be instances of -:class:`UnformattedWarning`, whose ``format`` method returns a fresh warning -instance each time. Do not warn with shared warning instances: if a warning is -raised as an error, CPython appends to the exception's existing ``__traceback__``, -so a process-lifetime instance would accumulate stale frames from previous raises. +:class:`WarningTemplate`. Emit them with its ``warn`` method, or pass the fresh +instance from ``format`` to an API that takes a warning; see its docstring for +why a warning instance is never kept around. """ from __future__ import annotations -from warnings import warn - from _pytest.warning_types import PytestDeprecationWarning from _pytest.warning_types import PytestRemovedIn10Warning -from _pytest.warning_types import UnformattedWarning +from _pytest.warning_types import WarningTemplate # set of plugins which have been integrated into the core; we use this list to ignore @@ -31,13 +28,13 @@ # This could have been removed pytest 8, but it's harmless and common, so no rush to remove. -YIELD_FIXTURE = UnformattedWarning( +YIELD_FIXTURE = WarningTemplate( PytestRemovedIn10Warning, "@pytest.yield_fixture is deprecated.\n" "Use @pytest.fixture instead; they are the same.", ) -CLASS_FIXTURE_INSTANCE_METHOD = UnformattedWarning( +CLASS_FIXTURE_INSTANCE_METHOD = WarningTemplate( PytestRemovedIn10Warning, "{scope}-scoped fixtures defined as instance methods are deprecated.\n" "Instance attributes set in the {fixturename!r} fixture will NOT be visible to test methods,\n" @@ -47,13 +44,13 @@ ) # This deprecation is never really meant to be removed. -PRIVATE = UnformattedWarning( +PRIVATE = WarningTemplate( PytestDeprecationWarning, "A private pytest class or function was used.", ) -HOOK_LEGACY_MARKING = UnformattedWarning( +HOOK_LEGACY_MARKING = WarningTemplate( PytestRemovedIn10Warning, "The hook{type} {fullname} uses old-style configuration options (marks or attributes).\n" "Please use the pytest.hook{type}({hook_opts}) decorator instead\n" @@ -62,7 +59,7 @@ "#configuring-hook-specs-impls-using-markers", ) -MONKEYPATCH_LEGACY_NAMESPACE_PACKAGES = UnformattedWarning( +MONKEYPATCH_LEGACY_NAMESPACE_PACKAGES = WarningTemplate( PytestRemovedIn10Warning, "monkeypatch.syspath_prepend() called with pkg_resources legacy namespace packages detected.\n" "Legacy namespace packages (using pkg_resources.declare_namespace) are deprecated.\n" @@ -70,7 +67,7 @@ "See https://docs.pytest.org/en/stable/deprecations.html#monkeypatch-fixup-namespace-packages", ) -PARAMETRIZE_NON_COLLECTION_ITERABLE = UnformattedWarning( +PARAMETRIZE_NON_COLLECTION_ITERABLE = WarningTemplate( PytestRemovedIn10Warning, "Passing a non-Collection iterable to parametrize is deprecated.\n" "Test: {nodeid}, argvalues type: {type_name}\n" @@ -78,20 +75,20 @@ "See https://docs.pytest.org/en/stable/deprecations.html#parametrize-iterators", ) -CONSOLE_MAIN = UnformattedWarning( +CONSOLE_MAIN = WarningTemplate( PytestRemovedIn10Warning, "pytest.console_main() is deprecated and will be removed in pytest 10.\n" "It was never intended for programmatic use; use pytest.main() instead.\n" "See https://docs.pytest.org/en/stable/deprecations.html#console-main", ) -CONFIG_INICFG = UnformattedWarning( +CONFIG_INICFG = WarningTemplate( PytestRemovedIn10Warning, "config.inicfg is deprecated, use config.getini() to access configuration values instead.\n" "See https://docs.pytest.org/en/stable/deprecations.html#config-inicfg", ) -FIXTURE_GETFIXTUREVALUE_DURING_TEARDOWN = UnformattedWarning( +FIXTURE_GETFIXTUREVALUE_DURING_TEARDOWN = WarningTemplate( PytestRemovedIn10Warning, 'Calling request.getfixturevalue("{argname}") during teardown is deprecated.\n' "Please request the fixture before teardown begins, either by declaring it in the fixture signature " @@ -99,19 +96,20 @@ "See https://docs.pytest.org/en/stable/deprecations.html#dynamic-fixture-request-during-teardown", ) -PASTEBIN = UnformattedWarning( +PASTEBIN = WarningTemplate( PytestRemovedIn10Warning, "The --pastebin option is deprecated. " "The functionality is now available in an external plugin package, pytest-pastebin.\n" "See https://docs.pytest.org/en/stable/deprecations.html#the-pastebin-option", ) -INI_STRING_TYPE_NON_STR_VALUE = PytestRemovedIn10Warning( +INI_STRING_TYPE_NON_STR_VALUE = WarningTemplate( + PytestRemovedIn10Warning, "Passing a value that is not a string to a 'string'-typed ini option is deprecated.\n" "In a future version this will raise a TypeError, matching the behavior of the " "corresponding TOML config path.\n" "If your plugin intentionally accepts non-string values, declare an explicit type " - '(e.g. type="args") instead of relying on the implicit string default.' + '(e.g. type="args") instead of relying on the implicit string default.', ) # You want to make some `__init__` or function "private". @@ -133,30 +131,30 @@ # the warning (possibly error in the future). -FIXTURE_BASEID_DEPRECATED = UnformattedWarning( +FIXTURE_BASEID_DEPRECATED = WarningTemplate( PytestRemovedIn10Warning, "Passing baseid to FixtureDef is deprecated. Pass node instead for fixture scoping.", ) -FIXTURE_NODEID_DEPRECATED = UnformattedWarning( +FIXTURE_NODEID_DEPRECATED = WarningTemplate( PytestRemovedIn10Warning, "Passing nodeid to _register_fixture is deprecated. " "Pass node instead for fixture scoping.", ) -FIXTUREDEF_HAS_LOCATION_DEPRECATED = UnformattedWarning( +FIXTUREDEF_HAS_LOCATION_DEPRECATED = WarningTemplate( PytestRemovedIn10Warning, "FixtureDef.has_location is deprecated and will be removed in pytest 10. " "See https://docs.pytest.org/en/stable/deprecations.html#fixturedef-has-location-deprecated", ) -PARSEFACTORIES_NODEID_DEPRECATED = UnformattedWarning( +PARSEFACTORIES_NODEID_DEPRECATED = WarningTemplate( PytestRemovedIn10Warning, "Passing nodeid string to parsefactories is deprecated. " "Use parsefactories(holder=obj, node=node) instead.", ) -CALLSPEC2_RENAMED = UnformattedWarning( +CALLSPEC2_RENAMED = WarningTemplate( PytestRemovedIn10Warning, "_pytest.python.CallSpec2 has been renamed to CallSpec.\n" "The CallSpec2 alias will be removed in pytest 10.\n" @@ -167,4 +165,4 @@ def check_ispytest(ispytest: bool) -> None: if not ispytest: - warn(PRIVATE.format(), stacklevel=3) + PRIVATE.warn(stacklevel=3) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 20e43f89132..bdbadddaa4d 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -30,7 +30,6 @@ from typing import overload from typing import TYPE_CHECKING from typing import TypeVar -import warnings import _pytest from _pytest import nodes @@ -777,10 +776,7 @@ def _get_active_fixturedef(self, argname: str) -> FixtureDef[object]: if not self.session._setupstate.is_node_active(self.node): # TODO(pytest10.1): Remove the `warn` and `if` and call # _raise_teardown_lookup_error unconditionally. - warnings.warn( - FIXTURE_GETFIXTUREVALUE_DURING_TEARDOWN.format(argname=argname), - stacklevel=3, - ) + FIXTURE_GETFIXTUREVALUE_DURING_TEARDOWN.warn(stacklevel=3, argname=argname) if subrequest.node not in self.session._setupstate.stack: self._raise_teardown_lookup_error(argname) @@ -1132,7 +1128,7 @@ def __init__( check_ispytest(_ispytest) # Emit deprecation warning if deprecated baseid string is used. if node is NOTSET: - warnings.warn(FIXTURE_BASEID_DEPRECATED.format(), stacklevel=2) + FIXTURE_BASEID_DEPRECATED.warn(stacklevel=2) if baseid is NOTSET: baseid = None # The node where this fixture was defined, if available. @@ -1197,7 +1193,7 @@ def scope(self) -> ScopeName: @property def has_location(self) -> bool: - warnings.warn(FIXTUREDEF_HAS_LOCATION_DEPRECATED.format(), stacklevel=2) + FIXTUREDEF_HAS_LOCATION_DEPRECATED.warn(stacklevel=2) return self._has_location def addfinalizer(self, finalizer: Callable[[], object]) -> None: @@ -1350,12 +1346,10 @@ def __tracebackhide__(e): # classmethod: bound_to is the class itself (a type) # instance method: bound_to is an instance (not a type) if not isinstance(bound_to, type): - warnings.warn( - CLASS_FIXTURE_INSTANCE_METHOD.format( - scope=fixturedef._scope.name.capitalize(), - fixturename=request.fixturename, - ), + CLASS_FIXTURE_INSTANCE_METHOD.warn( stacklevel=2, + scope=fixturedef._scope.name.capitalize(), + fixturename=request.fixturename, ) if instance is not None: @@ -1620,7 +1614,7 @@ def yield_fixture( .. deprecated:: 3.0 Use :py:func:`pytest.fixture` directly instead. """ - warnings.warn(YIELD_FIXTURE.format(), stacklevel=2) + YIELD_FIXTURE.warn(stacklevel=2) return fixture( fixture_function, *args, @@ -2091,7 +2085,7 @@ def _register_fixture( """ # Emit deprecation warning if nodeid string. if nodeid is not NOTSET or node is NOTSET: - warnings.warn(FIXTURE_NODEID_DEPRECATED.format(), stacklevel=2) + FIXTURE_NODEID_DEPRECATED.warn(stacklevel=2) fixture_def = FixtureDef( config=self.config, baseid=nodeid, @@ -2280,7 +2274,7 @@ def parsefactories( raise TypeError("parsefactories() requires holder or node_or_obj") elif nodeid is not NOTSET: # Legacy: parsefactories(obj, nodeid) - string-based scoping only. - warnings.warn(PARSEFACTORIES_NODEID_DEPRECATED.format(), stacklevel=2) + PARSEFACTORIES_NODEID_DEPRECATED.warn(stacklevel=2) holderobj = node_or_obj effective_nodeid = nodeid else: diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 34a004f4881..572fac99fa0 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -202,12 +202,10 @@ def _for_parametrize( nodeid: str, ) -> tuple[Sequence[str], list[ParameterSet]]: if not isinstance(argvalues, Collection): - warnings.warn( - PARAMETRIZE_NON_COLLECTION_ITERABLE.format( - nodeid=nodeid, - type_name=type(argvalues).__name__, - ), + PARAMETRIZE_NON_COLLECTION_ITERABLE.warn( stacklevel=3, + nodeid=nodeid, + type_name=type(argvalues).__name__, ) argnames, force_tuple = cls._parse_parametrize_args(argnames, argvalues) diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py index 9b9bcd3fb95..52e4ca55145 100644 --- a/src/_pytest/monkeypatch.py +++ b/src/_pytest/monkeypatch.py @@ -384,9 +384,7 @@ def syspath_prepend(self, path) -> None: continue ns_pkg_path = path_obj / ns_pkg.replace(".", os.sep) if ns_pkg_path.is_dir(): - warnings.warn( - MONKEYPATCH_LEGACY_NAMESPACE_PACKAGES.format(), stacklevel=2 - ) + MONKEYPATCH_LEGACY_NAMESPACE_PACKAGES.warn(stacklevel=2) break fixup_namespace_packages(str(path)) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 1ce7f71a7bd..b8ec09a8b9c 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1852,6 +1852,6 @@ def runtest(self) -> None: def __getattr__(name: str) -> object: if name == "CallSpec2": - warnings.warn(CALLSPEC2_RENAMED.format(), stacklevel=2) + CALLSPEC2_RENAMED.warn(stacklevel=2) return CallSpec raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index c5b339dec4c..29eb44e2c85 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -118,21 +118,48 @@ class PytestUnhandledThreadExceptionWarning(PytestWarning): @final -@dataclasses.dataclass -class UnformattedWarning(Generic[_W]): - """A warning meant to be formatted during runtime. - - This is used to hold warnings that need to format their message at runtime, - as opposed to a direct message. +@dataclasses.dataclass(frozen=True) +class WarningTemplate(Generic[_W]): + """A warning category paired with its message template. + + Emitting a warning goes through this type so that a *fresh* warning instance + is built every time. + + Warning instances must never be kept around and reused: under an ``error`` + filter ``warnings.warn(instance)`` raises that very object, so CPython + appends to its existing ``__traceback__`` instead of replacing it, and + ``__context__`` picks up whatever exception happened to be in flight. A + shared instance therefore grows a traceback without bound, pins the locals + of unrelated frames, and shows frames of earlier, unrelated code in failure + reports (#14912). """ category: type[_W] template: str def format(self, **kwargs: Any) -> _W: - """Return an instance of the warning category, formatted with given kwargs.""" + """Return a new instance of the warning category. + + The template is only run through :meth:`str.format` when ``kwargs`` are + given, so constant messages may contain literal braces unescaped. + """ + if not kwargs: + return self.category(self.template) return self.category(self.template.format(**kwargs)) + def warn(self, *, stacklevel: int = 1, **kwargs: Any) -> None: + """Emit the warning, with ``stacklevel`` relative to the caller. + + The extra level compensates for this method's own frame, so callers pass + the same ``stacklevel`` they would pass to :func:`warnings.warn`. + """ + __tracebackhide__ = True + warnings.warn(self.format(**kwargs), stacklevel=stacklevel + 1) + + +#: Kept for third-party code that reached into this private module. +UnformattedWarning = WarningTemplate + @final class PytestApproxDecimalToleranceWarning(PytestWarning): diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 5de659a481f..bdfe73247c8 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -1,11 +1,16 @@ # mypy: allow-untyped-defs from __future__ import annotations +import importlib +import pkgutil +import string from types import TracebackType +import _pytest from _pytest import deprecated from _pytest.pytester import Pytester from _pytest.scope import Scope +from _pytest.warning_types import WarningTemplate import pytest from pytest import PytestDeprecationWarning from pytest import PytestRemovedIn10Warning @@ -401,3 +406,75 @@ def raise_and_count() -> int: # The traceback of a shared instance would grow with every raise. assert second <= first + + +class TestNoSharedWarningInstances: + """Deprecations must not be emitted from shared ``Warning`` instances (#14912). + + Under an ``error`` filter ``warnings.warn(instance)`` raises that very object, + so a module-level instance accumulates traceback frames and ``__context__`` + across unrelated emissions. + """ + + @staticmethod + def _raise_private() -> PytestDeprecationWarning: + with pytest.raises(PytestDeprecationWarning) as excinfo: + deprecated.check_ispytest(False) + return excinfo.value + + @pytest.mark.filterwarnings("error") + def test_unrelated_context_does_not_leak_into_later_emissions(self) -> None: + try: + raise ValueError("unrelated") + except ValueError: + first = self._raise_private() + assert isinstance(first.__context__, ValueError) + + second = self._raise_private() + + assert second.__context__ is None + first_frames = {tb.tb_frame for tb in _walk_traceback(first.__traceback__)} + second_frames = {tb.tb_frame for tb in _walk_traceback(second.__traceback__)} + assert not first_frames & second_frames + + def test_no_module_level_warning_instances(self) -> None: + """Guard against reintroducing shared instances anywhere in ``_pytest``.""" + offenders = [] + for module_info in pkgutil.walk_packages(_pytest.__path__, prefix="_pytest."): + module = importlib.import_module(module_info.name) + for name, value in vars(module).items(): + if isinstance(value, Warning): + offenders.append(f"{module_info.name}.{name}") + assert sorted(offenders) == [] + + @pytest.mark.parametrize( + ("name", "template"), + sorted( + (name, value) + for name, value in vars(deprecated).items() + if isinstance(value, WarningTemplate) + ), + ) + def test_templates_are_well_formed(self, name: str, template) -> None: + fields = { + field + for _, field, _, _ in string.Formatter().parse(template.template) + if field is not None + } + # ``{}`` would silently consume a positional argument that never arrives. + assert "" not in fields, name + # ``warn()`` takes ``stacklevel`` as a keyword, so a field of that name + # could never be filled in. + assert "stacklevel" not in fields, name + + warning = template.format(**dict.fromkeys(fields, "x")) + + assert isinstance(warning, template.category) + if not fields: + assert str(warning) == template.template + + +def _walk_traceback(tb): + while tb is not None: + yield tb + tb = tb.tb_next diff --git a/testing/test_warning_types.py b/testing/test_warning_types.py index 81d8785733c..b24bff8211e 100644 --- a/testing/test_warning_types.py +++ b/testing/test_warning_types.py @@ -2,6 +2,7 @@ from __future__ import annotations import inspect +import warnings from _pytest import warning_types from _pytest.pytester import Pytester @@ -48,3 +49,57 @@ def test_warn_explicit_for_annotates_errors_with_location(): pytest.raises, # type: ignore[arg-type] warning_types.PytestWarning("test"), ) + + +class TestWarningTemplate: + """A template must build a fresh warning per emission (#14912).""" + + template: warning_types.WarningTemplate[warning_types.PytestWarning] = ( + warning_types.WarningTemplate(warning_types.PytestWarning, "a message") + ) + + def test_format_returns_a_new_instance_each_call(self) -> None: + first = self.template.format() + second = self.template.format() + + assert first is not second + assert isinstance(first, warning_types.PytestWarning) + assert str(first) == str(second) == "a message" + + def test_constant_message_keeps_literal_braces(self) -> None: + """Messages without placeholders are not run through str.format().""" + template = warning_types.WarningTemplate( + warning_types.PytestWarning, "a {literal} brace and a }} one" + ) + + assert str(template.format()) == "a {literal} brace and a }} one" + + def test_format_substitutes_kwargs(self) -> None: + template = warning_types.WarningTemplate( + warning_types.PytestWarning, "hello {name}" + ) + + assert str(template.format(name="world")) == "hello world" + + def test_warn_reports_the_callers_location(self) -> None: + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + lineno = inspect.currentframe().f_lineno + 1 # type: ignore[union-attr] + self.template.warn(stacklevel=1) + + [record] = records + assert record.filename == __file__ + assert record.lineno == lineno + + def test_warn_stacklevel_skips_the_helper(self) -> None: + def helper() -> None: + self.template.warn(stacklevel=2) + + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + lineno = inspect.currentframe().f_lineno + 1 # type: ignore[union-attr] + helper() + + [record] = records + assert record.filename == __file__ + assert record.lineno == lineno