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.

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

support plugins as a class #11300

Open
DetachHead opened this issue Aug 9, 2023 · 7 comments
Open

support plugins as a class #11300

DetachHead opened this issue Aug 9, 2023 · 7 comments
Labels
topic: typing type-annotation issue type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@DetachHead
Copy link
Contributor

What's the problem this feature will solve?

currently, there is no way to type check your hook functions:

from pytest import Item

def pytest_runtestloop(session: Item): # no type error, it should be Session
    ...

Describe the solution you'd like

expose a base class for plugins to extend, so they can be defined like so:

from typing_extensions import override
from pytest import Plugin

class SomePlugin(Plugin):
    @override
    def pytest_runtestloop(self, session: Item): # type error: signature does not match base method
        ...

Alternative Solutions

Additional context

@RonnyPfannschmidt
Copy link
Member

No base class is needed,any object instance can be registered as a class

@RonnyPfannschmidt
Copy link
Member

As for type checking, as Plugins can add new hooks,a mypy plugin is needed in any case

@KotlinIsland
Copy link

KotlinIsland commented Aug 9, 2023

@RonnyPfannschmidt A thin interface would likely help the majority of use cases of typechecking and API discovery with minimal effort. A mypy plugin, while not neccissarily a bad idea, sounds like a much harder task with many more considerations to make.

from typing import Protocol

class Plugin(Protocol):
    def pytest_runtestloop(self, session: Session):
        ...
    # etc

@The-Compiler
Copy link
Member

I'm somewhat lukewarm on this as well, for two reasons:

  • I'd need the entire pytest hookspec to be duplicated somewhere, unless pluggy supports collecting hookspecs from a class maybe?
  • It wouldn't work with hook functions in a file, which probably is "the majority of use cases" of the plugin API.

@KotlinIsland
Copy link

It wouldn't work with hook functions in a file, which probably is "the majority of use cases" of the plugin API.

Personally, knowing that my types are correct is 100x more beneficial than the convenience of defining my code as a module over a class. And this would just be an option, the suggestion is not to force it.

@The-Compiler
Copy link
Member

That wasn't my point. Of course it's helpful if you know it exists and want to use it — but something most likely not benefiting the majority of current plugin users needs to be weighted up against the additional development effort (and, most likely, future issues with things getting out of sync) of having to duplicate everything.

Then again, it looks like pluggy already supports declaring hook specifications in a class, so perhaps pytest could indeed declare a pytest.Plugin which servers both as an abstract base class for plugin implementations wanting type checking, as well as a hook specification for pluggy.

FWIW, I'm not sure using a Protocol would work, though: Doesn't a Protocol mandate that the complete interface of the protocol class needs to be implemented? This wouldn't be the case here: A pytest plugin doesn't need to implement all hooks.

Additionally, and this might make this entire thing completely impossible anyways, hook arguments can be left out. You might say you're okay with just also declaring the unused arguments in exchange of getting type checking, but sometimes this is done deliberately for backwards compatibility. For example, various hooks have two different path arguments, and implementing something like:

def pytest_ignore_collect(collection_path: pathlib.Path, path: py.path.local, config: pytest.Config) -> bool | None:
    # do something with collection_path, ignore the rest
    pass

while working fine currently, might start raising a deprecation warning at some point in the future.

@RonnyPfannschmidt
Copy link
Member

Based on how pluggy works that's likely going to require some type of white elephant to make it sensible

As far as I am concerned a mypy plugin is most likely easier than the hack's needed for plugin spec's to stay peformant

But it would be nice for someone to disprove me

@Zac-HD Zac-HD added type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature topic: typing type-annotation issue labels Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: typing type-annotation issue type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

5 participants