Pytest verifies if hooks are valid before processing "pytest_plugins" variable

[nicoddemus]

Consider:

# content of plugin.py
class Hooks:
    def pytest_my_hook(self, config):
        pass

def pytest_configure(config):
    config.pluginmanager.add_hookspec(Hooks)

# content of conftest.py
pytest_plugins = ['plugin']

def pytest_my_hook(self, config):
    pass

# content of test_foo.py
def test():
    pass

This produces this error:

pytest test_foo.py
Traceback (most recent call last):
  File "E:\Miniconda\envs\pytest\Scripts\pytest-script.py", line 9, in <module>
    load_entry_point('pytest==3.0.0.dev1', 'console_scripts', 'pytest')()
  File "c:\users\bruno\pytest\_pytest\config.py", line 56, in main
    config.pluginmanager.check_pending()
  File "c:\users\bruno\pytest\_pytest\vendored_packages\pluggy.py", line 490, in check_pending
    (name, hookimpl.plugin))
_pytest.vendored_packages.pluggy.PluginValidationError: unknown hook 'pytest_my_hook' in plugin <module 'conftest' from 'C:\Users\bruno\pytest\tmp\conftest.py'>

It seems pytest is validating hooks before actually processing all hooks.

This is using the features branch just pre-release of 3.0 (be9356a).

[leifurhauks]

I think I'm seeing the same issue when using pytest-factoryboy (with a django project):

# conftest.py
from factory import django as factory_django
import pytest_factoryboy


@pytest_factoryboy.register
class UserFactory(factory_django.DjangoModelFactory):
    class Meta:
        model = 'auth.User'

    username = 'test_user'

I get the following error:

$ pytest           
Traceback (most recent call last):
  File "/home/leifur/.virtualenvs/eboxtvapi/bin/pytest", line 11, in <module>
    sys.exit(main())
  File "/home/leifur/.virtualenvs/eboxtvapi/lib/python3.6/site-packages/_pytest/config.py", line 56, in main
    config.pluginmanager.check_pending()
  File "/home/leifur/.virtualenvs/eboxtvapi/lib/python3.6/site-packages/_pytest/vendored_packages/pluggy.py", line 498, in check_pending
    (name, hookimpl.plugin))
_pytest.vendored_packages.pluggy.PluginValidationError: unknown hook 'pytest_factoryboy' in plugin <module 'conftest' (<_pytest.assertion.rewrite.AssertionRewritingHook object at 0x7f1c13129828>)>

The simple workaround is to use from pytest_factoryboy import register instead of importing pytest_factoryboy directly.

I don't get this error when I import pytest_factoryboy in conftest files in inline testing directories in my app; it only happens with the conftest at the top of the project.

I'm using pytest 3.0.6, pytest-factoryboy 1.3.0 (although I confirmed that the same error happened with several prior releases) and pytest-django 3.1.2.

[RonnyPfannschmidt]

oh - @nicoddemus the behaviour as seen is correct, hooks that may not be declared in plugin order must be marked as optional

[goodboy]

@nicoddemus @RonnyPfannschmidt I just ran into this as well with packaging our new project pytestlab.

I'm wondering is maybe check_pending() is being called slightly too early?
Would it not make more sense for it be called at least after the first historic call to pytest_addhooks() (or better yet after collection altogether)?
I'm realizing now it is called after pytest_addhooks().
My issue is actually to do with assuming a pytest_plugins declaration in an entrypoints registered plugin works the same as in conftest.py files, which it doesn't.

I'm stuck at the moment marking nearly all pytestlab hooks as optionalhook=True in pytestlab plugins which are loaded using pytest_plugins .
I'm wondering if it makes sense to have a recursive loading mechanism such as pytest_plugins supported for entry points plugins in pluggy?

[goodboy]

Guys unless I'm misunderstanding this might be actually kind of bad.

Currently check_pending() is only called once through the entire code base. That means there's no hook validation after pytest_cmdline_main() is entered.

Therefore hook validation is never done except for the initial plugin set loaded by _prepareconfig().

This also makes me wonder if hook validation is something that should be done on every plugin loaded with pluggy.

[RonnyPfannschmidt]

@tgoodlet potentially yes, this may need a discussion/brainstorm

[goodboy]

@RonnyPfannschmidt I've opened pytest-dev/pluggy#48 for a follow up regarding the validation question.

What do you think about the recursive loading suggestion I made?

[nicoddemus]

Here are the updated files:

# content of conftest.py
pytest_plugins = ['plugin']

def pytest_my_hook(config):
    assert 0

# content of test_foo.py
def test(request):
    request.config.hook.pytest_my_hook(config=request.config)

# content of plugin.py
class Hooks:
    def pytest_my_hook(self, config):
        pass

def pytest_configure(config):
    config.pluginmanager.add_hookspecs(Hooks)

I think this can be fixed by calling check_pending as late as possible in the process, instead of at pytest_cmdline_main. I propose calling it right after collection:

diff --git a/_pytest/config.py b/_pytest/config.py
index c73416f..8af3908 100644
--- a/_pytest/config.py
+++ b/_pytest/config.py
@@ -53,7 +53,7 @@ def main(args=None, plugins=None):
             return 4
         else:
             try:
-                config.pluginmanager.check_pending()
+
                 return config.hook.pytest_cmdline_main(config=config)
             finally:
                 config._ensure_unconfigure()
diff --git a/_pytest/main.py b/_pytest/main.py
index b66b661..1bc80b2 100644
--- a/_pytest/main.py
+++ b/_pytest/main.py
@@ -598,6 +598,7 @@ class Session(FSCollector):
         hook = self.config.hook
         try:
             items = self._perform_collect(args, genitems)
+            self.config.pluginmanager.check_pending()
             hook.pytest_collection_modifyitems(session=self,
                 config=self.config, items=items)
         finally:

This gives a chance for all plugins to be loaded and validated properly. We then don't need the slighly hackish way of deferring the use of hooks.

[nicoddemus]

With the above patch, my example works (the assertion inside pytest_my_hook is triggered) and if I don't register the spec by commenting out pytest_configure I get the same validation error.

[goodboy]

@nicoddemus nice, yes this is the fix I wanted to see and I think it addresses the bulk of the issue :)

I still think it's not necessarily great that validation only ever happens once.
There could be cases where plugins are registered after collection no? An example might be choosing an appropriate reporting hook depending on a fixtures calculation of the target platform.

I feel like we should at least allow the plugin provider the option to specify how "strict" (or maybe how often?) validations should be.

[nicoddemus]

nice, yes this is the fix I wanted to see and I think it addresses the bulk of the issue :)

Great, then unless @RonnyPfannschmidt has any objections I will put together a PR.

There could be cases where plugins are registered after collection no?

Not that I'm aware of, but I'm not 100% sure either.

I feel like we should at least allow the plugin provider the option to specify how "strict" (or maybe how often?) validations should be.

I see, thanks. Do you have an idea on how pluggy can provide that functionality though? It seems like it depends completely on how pluggy is being used. As an example, check_pending was being called in pytest's main, and my code above proposes it to be called after collection, both which are outside of pluggy's control.

[goodboy]

@nicoddemus see my comments in pytest-dev/pluggy#48 for a deeper explanation :)