Skip to content
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.

Sign up for GitHub

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

Jump to bottom

deprecate hook configuration via marks/attributes #9118

Merged
merged 7 commits into from
Oct 2, 2022
Merged

deprecate hook configuration via marks/attributes #9118

merged 7 commits into from
Oct 2, 2022

Conversation

RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Sep 23, 2021 •
edited
Loading

fixes #4562

must be merged after #8508

@RonnyPfannschmidt RonnyPfannschmidt added the type: deprecation feature that will be removed in the future label Sep 23, 2021
@RonnyPfannschmidt RonnyPfannschmidt changed the title deprecate hook configuration via marks/attributes [WIP] deprecate hook configuration via marks/attributes Sep 23, 2021
nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this pull request Sep 24, 2021
@nicoddemus
Use modern options configurations for hooks
da606e7 
Noticed while reviewing pytest-dev/pytest#9118
@nicoddemus nicoddemus mentioned this pull request Sep 24, 2021
Merged
nicoddemus
nicoddemus requested changes Sep 24, 2021
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks @RonnyPfannschmidt, this issue wasn't even in my radar. 😁

Left some comments, please take a look.

changelog/4562.deprecation.rst Outdated Show resolved Hide resolved
changelog/4562.deprecation.rst Outdated Show resolved Hide resolved
doc/en/deprecations.rst Outdated Show resolved Hide resolved
src/_pytest/config/__init__.py Outdated Show resolved Hide resolved
src/_pytest/config/__init__.py Outdated Show resolved Hide resolved
src/_pytest/deprecated.py Outdated Show resolved Hide resolved
src/_pytest/deprecated.py Outdated Show resolved Hide resolved
testing/deprecated_test.py Outdated Show resolved Hide resolved
testing/deprecated_test.py Outdated Show resolved Hide resolved
nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this pull request Sep 24, 2021
@nicoddemus
Use modern options configurations for hooks
a86b90f 
Noticed while reviewing pytest-dev/pytest#9118
nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this pull request Sep 24, 2021
@nicoddemus
Use modern options configurations for hooks
c7c7f9e 
Noticed while reviewing pytest-dev/pytest#9118
@RonnyPfannschmidt

This comment has been minimized.

Sign in to view
bluetech
bluetech reviewed Oct 2, 2021
View reviewed changes
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Left some comments on the docs/texts. I haven't reviewed the code, will try to later.

doc/en/deprecations.rst Outdated Show resolved Hide resolved
doc/en/deprecations.rst Outdated Show resolved Hide resolved
doc/en/deprecations.rst Outdated Show resolved Hide resolved
src/_pytest/deprecated.py Outdated Show resolved Hide resolved
src/_pytest/deprecated.py Outdated Show resolved Hide resolved
changelog/4562.deprecation.rst Show resolved Hide resolved
doc/en/deprecations.rst Outdated Show resolved Hide resolved
nicoddemus
nicoddemus reviewed Oct 4, 2021
View reviewed changes
doc/en/deprecations.rst Outdated Show resolved Hide resolved
@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as draft February 27, 2022 07:33
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-4562-deprecate-hookmarkers branch 3 times, most recently from 2dabd1b to 38f4561 Compare March 8, 2022 08:51
@RonnyPfannschmidt RonnyPfannschmidt changed the title [WIP] deprecate hook configuration via marks/attributes deprecate hook configuration via marks/attributes Mar 8, 2022
@RonnyPfannschmidt RonnyPfannschmidt marked this pull request as ready for review May 15, 2022 21:00
@RonnyPfannschmidt RonnyPfannschmidt added this to the 7.2 milestone Jul 11, 2022
@RonnyPfannschmidt RonnyPfannschmidt force-pushed the fix-4562-deprecate-hookmarkers branch 2 times, most recently from 22a9238 to 4052ce6 Compare July 30, 2022 19:59
@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus i finally got around reiterating this, please have a look

nicoddemus
nicoddemus approved these changes Sep 22, 2022
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Great work!

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus as far as i can tell, the only blocker is pytest-dev/pytest-cov#549

ionelmc pushed a commit to pytest-dev/pytest-cov that referenced this pull request Sep 22, 2022
@nicoddemus @ionelmc
Use modern approach to specify hook options
b077753 
The old way using marks is being deprecated in pytest 7.2:

pytest-dev/pytest#9118
@nicoddemus
Copy link
Member

That has just been merged. 👍

@RonnyPfannschmidt RonnyPfannschmidt merged commit 2be1b8f into pytest-dev:main Oct 2, 2022
@RonnyPfannschmidt RonnyPfannschmidt deleted the fix-4562-deprecate-hookmarkers branch October 2, 2022 10:59
This was referenced Oct 6, 2022
Closed
Closed
@The-Compiler
Copy link
Member

I'm afraid the error messages this generates for hookimpls are almost unusable due to having way too little information where the culprit is: #10342

This was referenced Oct 6, 2022
Merged
Merged
Merged
@The-Compiler
Copy link
Member

Quick analysis of the impact of this:

FWIW, I submitted PRs to the affected plugins I use:

The-Compiler added a commit to The-Compiler/pytest that referenced this pull request Oct 7, 2022
@The-Compiler
Mark tryfirst/trylast as deprecated in their docs.
72cac9b 
Follow-up to pytest-dev#9118
@The-Compiler The-Compiler mentioned this pull request Oct 7, 2022
Merged
@RonnyPfannschmidt
Copy link
Member Author

The impact is not surprising, this was intended to be deprecate back when the new decorators came up

Instead the old way was cargo culted all the way till now

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Oct 7, 2022
@The-Compiler
bleeding tests: Use forks of pytest-benchmark and -instafail
208be25 
See pytest-dev/pytest#9118
ionelmc pushed a commit to ionelmc/pytest-benchmark that referenced this pull request Oct 7, 2022
@The-Compiler @ionelmc
Use pytest.hookimpl/.hookspec to configure hooks
d135104 
See
pytest-dev/pytest#9118
https://docs.pytest.org/en/latest/deprecations.html#configuring-hook-specs-impls-using-markers

Closes #223
jpvanhal pushed a commit to pytest-dev/pytest-instafail that referenced this pull request Nov 11, 2022
@The-Compiler @jpvanhal
Use pytest.hookimpl to configure hooks
7d997d6 
See
pytest-dev/pytest#9118
https://docs.pytest.org/en/latest/deprecations.html#configuring-hook-specs-impls-using-markers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus approved these changes

@bluetech bluetech Awaiting requested review from bluetech

@The-Compiler The-Compiler Awaiting requested review from The-Compiler

Assignees
No one assigned
Labels
type: deprecation feature that will be removed in the future
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.

deprecate marker based hook metadata
4 participants
@RonnyPfannschmidt @nicoddemus @The-Compiler @bluetech