Skip to content

Commit 0258484

Browse files
RonnyPfannschmidtCursor AIclaude
committed
Address review: DeprecationWarning, add self to test, suppress pytest-timeout
- Change FutureWarning to DeprecationWarning for hookspec methods missing self - Add self to HookSpec methods in test_hookspec instead of suppressing the warning - Suppress the deprecation warning for pytest-timeout's TimeoutHooks (upstream fix exists but is unreleased) - Add changelog entry for the deprecation - Update all test references from FutureWarning to DeprecationWarning Co-authored-by: Cursor AI <ai@cursor.sh> Co-authored-by: Anthropic Claude Opus 4 <claude@anthropic.com>
1 parent dd20a85 commit 0258484

5 files changed

Lines changed: 52 additions & 18 deletions

File tree

changelog/632.removal.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Hookspec methods defined without ``self`` as the first parameter now emit a :class:`DeprecationWarning`. Add ``self`` as the first parameter or use ``@staticmethod``.

src/pluggy/_hooks.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,17 @@ def normalize_hookimpl_opts(opts: HookimplOpts) -> None:
291291
_PYPY = sys.implementation.name == "pypy"
292292
_IMPLICIT_NAMES = ("self", "cls", "obj") if _PYPY else ("self", "cls")
293293

294+
# Qualnames whose missing-self deprecation warning is suppressed because
295+
# their upstream code is already fixed but not yet released.
296+
# Remove entries once a release with the fix is available.
297+
_NOSELF_WARN_SUPPRESS: frozenset[str] = frozenset(
298+
{
299+
# pytest-timeout >=2.3.2 has the fix, but is unreleased as of 2026-05.
300+
"TimeoutHooks.pytest_timeout_set_timer",
301+
"TimeoutHooks.pytest_timeout_cancel_timer",
302+
}
303+
)
304+
294305

295306
def varnames(
296307
func: object, *, legacy_noself: bool = False
@@ -306,7 +317,7 @@ def varnames(
306317
:param legacy_noself:
307318
If ``True``, support hookspec classes whose methods omit ``self``.
308319
When the function looks like a class method but has no implicit first
309-
parameter, a :class:`FutureWarning` is emitted.
320+
parameter, a :class:`DeprecationWarning` is emitted.
310321
"""
311322
is_bound = False
312323
if inspect.isclass(func):
@@ -360,14 +371,15 @@ def varnames(
360371
elif _is_class_method and args[0] in _IMPLICIT_NAMES:
361372
args = args[1:]
362373
elif _is_class_method and legacy_noself:
363-
warnings.warn(
364-
f"{qualname} is a method but its first parameter"
365-
f" {args[0]!r} is not 'self'."
366-
f" Add 'self' as the first parameter or use @staticmethod."
367-
f" This will become an error in a future version of pluggy.",
368-
FutureWarning,
369-
stacklevel=2,
370-
)
374+
if _tail not in _NOSELF_WARN_SUPPRESS:
375+
warnings.warn(
376+
f"{qualname} is a method but its first parameter"
377+
f" {args[0]!r} is not 'self'."
378+
f" Add 'self' as the first parameter or use @staticmethod."
379+
f" This will become an error in a future version of pluggy.",
380+
DeprecationWarning,
381+
stacklevel=2,
382+
)
371383

372384
return args, kwargs
373385

testing/test_helpers.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def my_hook(item, extra) -> None:
201201
result = varnames(MySpecs.my_hook, legacy_noself=True)
202202
assert result == (("item", "extra"), ())
203203
assert len(w) == 1
204-
assert issubclass(w[0].category, FutureWarning)
204+
assert issubclass(w[0].category, DeprecationWarning)
205205
assert "'item' is not 'self'" in str(w[0].message)
206206

207207

@@ -235,6 +235,28 @@ def my_hook(item, extra) -> None:
235235
assert len(w) == 0
236236

237237

238+
def test_varnames_legacy_noself_suppressed_for_timeout_hooks() -> None:
239+
"""The deprecation warning is suppressed for known upstream-fixed packages."""
240+
import warnings
241+
242+
class TimeoutHooks:
243+
def pytest_timeout_set_timer(item, settings) -> None:
244+
pass # pragma: no cover
245+
246+
def pytest_timeout_cancel_timer(item) -> None:
247+
pass # pragma: no cover
248+
249+
with warnings.catch_warnings(record=True) as w:
250+
warnings.simplefilter("always")
251+
result_set = varnames(TimeoutHooks.pytest_timeout_set_timer, legacy_noself=True)
252+
result_cancel = varnames(
253+
TimeoutHooks.pytest_timeout_cancel_timer, legacy_noself=True
254+
)
255+
assert result_set == (("item", "settings"), ())
256+
assert result_cancel == (("item",), ())
257+
assert len(w) == 0
258+
259+
238260
def test_varnames_unresolvable_annotation() -> None:
239261
"""Test that varnames works with annotations that cannot be resolved.
240262

testing/test_hookcaller.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,19 +301,18 @@ def m13() -> None: ...
301301
]
302302

303303

304-
@pytest.mark.filterwarnings("ignore::FutureWarning")
305304
def test_hookspec(pm: PluginManager) -> None:
306305
class HookSpec:
307306
@hookspec()
308-
def he_myhook1(arg1) -> None:
307+
def he_myhook1(self, arg1) -> None:
309308
pass
310309

311310
@hookspec(firstresult=True)
312-
def he_myhook2(arg1) -> None:
311+
def he_myhook2(self, arg1) -> None:
313312
pass
314313

315314
@hookspec(firstresult=False)
316-
def he_myhook3(arg1) -> None:
315+
def he_myhook3(self, arg1) -> None:
317316
pass
318317

319318
pm.add_hookspecs(HookSpec)

testing/test_warnings.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,22 @@ def my_hook(self):
5151

5252

5353
def test_hookspec_missing_self_warns(pm: PluginManager) -> None:
54-
"""A hookspec defined as a method without ``self`` emits a FutureWarning."""
54+
"""A hookspec defined as a method without ``self`` emits a DeprecationWarning."""
5555

5656
class Api:
5757
@hookspec
5858
def my_hook(item, extra):
5959
pass
6060

6161
with pytest.warns(
62-
FutureWarning,
62+
DeprecationWarning,
6363
match=r"is a method but its first parameter 'item' is not 'self'",
6464
):
6565
pm.add_hookspecs(Api)
6666

6767

6868
def test_hookspec_with_self_no_warning(pm: PluginManager) -> None:
69-
"""A hookspec with ``self`` does not emit a FutureWarning."""
69+
"""A hookspec with ``self`` does not emit a DeprecationWarning."""
7070

7171
class Api:
7272
@hookspec
@@ -79,7 +79,7 @@ def my_hook(self, item, extra):
7979

8080

8181
def test_hookspec_staticmethod_no_warning(pm: PluginManager) -> None:
82-
"""A hookspec using @staticmethod does not emit a FutureWarning."""
82+
"""A hookspec using @staticmethod does not emit a DeprecationWarning."""
8383

8484
class Api:
8585
@staticmethod

0 commit comments

Comments
 (0)