Make it possible to call hooks in pytest_addoption

[joshuastorck]

Suggestion

Make it possible to call hooks in pytest_addoption by passing the pluginmanager as an argument.

I have a plugin that is used across a number of projects. That plugin has a command line option that is added in pytest_addoption, but the default values for those options will change based on the project/conftest.py that's using them (think: configuration file). While I could use the parser's extra_info field to inject the default values, that seems like a hack and not the field's intended use. Instead, it would seem to make more sense to have the conftest.py plugin in each project implement a hook or hooks that return the default values. That way there is a nicely defined contract between the plugin and the conftest.py that depends on it. Unfortunately, the pytest_addoption hook does not make the pluginmanager available as an argument. The plugin can cheat by storing a reference to the pluginmanager in pytest_addhooks, which works like this but feels like a hack:

In the plugin file, myplugin.py:

import hooks

global thepluginmanager

def pytest_addhooks(pluginmanager):
    global thepluginmanager
    pluginmanager.add_hookspecs(hooks)
    thepluginmanager = pluginmanager

def pytest_addoption(parser):
    global thepluginmanager
    parser.addoption("--some-option", help="Some option, defaults to %(default)s",
                     default=thepluginmanager.hook.pytest_myplugin_default_value()[0])

In the conftest.py file:

pytest_plugins = ["myplugin"]

def pytest_myplugin_default_value():
    return 1

In the hooks.py file:

def pytest_myplugin_default_value():
    """
    Returns the default value for the command line option
    """

One possible solution would be to modify the signature to:

def pytest_addoption(parser, pluginmanager)

It appears the pluggy can handle this in a backwards compatible fashion, though I could imagine there is a deprecation policy in this project that requires defining a newly named function. Alternatively, add a new hook and call them both:

def pytest_addoption_with_plugins(parser, pluginmanager)

pytest Version: 3.7.4

[RonnyPfannschmidt]

Why not just use a ini option for the per project default and warn in pytest configure if that's not set

[joshuastorck]

I don’t want to repeat the help documentation in each project and want the —help to display the default value.

…

On Sat, Oct 26, 2019 at 4:51 AM Ronny Pfannschmidt ***@***.***> wrote: Why not just use a ini option for the per project default and warn in pytest configure if that's not set — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6061?email_source=notifications&email_token=AAHFEPIOLIHL55HXI4MM4D3QQQAKHA5CNFSM4JFJS3EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKDM6Q#issuecomment-546584186>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFEPI2ZX66QHEKBUM3QDTQQQAKHANCNFSM4JFJS3EA> .

[RonnyPfannschmidt]

in that case, how about a helper function in the main project, and then each project can just call helper(parser)

this has the added benefit of failing early if someone sets it up multiple times by accident

[joshuastorck]

I tried that but it requires importing the plugin in conftest.py, which emits the warning about assertion rewriting. I can put a sibling module next to the plugin module with the helper function, though I would prefer to keep the code in one place for readability and maintainability. Is there a danger in making the pluginmanager available to addoption? Or is adding a new hook not desirable? I could imagine there are a lot of ways this could be useful beyond default values. For example, your contest.py could signal to the plugin whether or not it needed certain features and the plugin could conditionally add command line arguments. I would be happy to put together a PR for the feature if folks are open to adding the new hook.

…

On Sat, Oct 26, 2019 at 11:29 AM Ronny Pfannschmidt < ***@***.***> wrote: in that case, how about a helper function in the main project, and then each project can just call helper(parser) this has the added benefit of failing early if someone sets it up multiple times by accident — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6061?email_source=notifications&email_token=AAHFEPPMA2NEAAEUX3FT2XDQQRO6BA5CNFSM4JFJS3EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKKLOA#issuecomment-546612664>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFEPN7AMEMEQ7L43OJMHDQQRO6BANCNFSM4JFJS3EA> .

[nicoddemus]

Unless @RonnyPfannschmidt has strong motives otherwise, I think it is fine to add pluginmanager as parameter to pytest_addoption, we already pass the plugin manager to pytest_addhooks and pytest_cmdline_parse, so I don't see any harm.

[RonnyPfannschmidt]

ok, lets add it as a experimental hook

[nicoddemus]

I actually meant just adding pluginmanager parameter to pytest_addoption, not adding a new hook.

[blueyed]

What about passing config?

[joshuastorck]

I’m happy to make the PR. What’s the process for making it experimental?

…

On Sat, Oct 26, 2019 at 1:17 PM Ronny Pfannschmidt ***@***.***> wrote: ok, lets add it as a experimental hook — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6061?email_source=notifications&email_token=AAHFEPPTZQ7IRQDOKSK5H23QQR3S7A5CNFSM4JFJS3EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKMX7Y#issuecomment-546622463>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFEPP2WDNQV7LGTAENIITQQR3S7ANCNFSM4JFJS3EA> .

[joshuastorck]

I think when pytest_addoption is called config is not fully initialized, that’s why I recommended passing the pluginmanager instead.

…

On Sat, Oct 26, 2019 at 5:02 PM Daniel Hahler ***@***.***> wrote: What about passing config? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6061?email_source=notifications&email_token=AAHFEPPCDIPJK5EZAZOYYPLQQSV5DA5CNFSM4JFJS3EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKQZ6A#issuecomment-546639096>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFEPOZTYAQQXJOE7Z7BLDQQSV5DANCNFSM4JFJS3EA> .

[blueyed]

What’s the process for making it experimental?

You can use a draft PR, but feel free to have it tested already to see failing tests etc.
IIRC that happens also for Draft PRs via Azure anyway.

[blueyed]

I guess you could try passing config=self - then the pluginmanager is also available from there then:

pytest/src/_pytest/config/__init__.py

Lines 698 to 707 in fea57bf

	self.pluginmanager = pluginmanager
	self.trace = self.pluginmanager.trace.root.get("config")
	self.hook = self.pluginmanager.hook
	self._inicache = {} # type: Dict[str, Any]
	self._override_ini = () # type: Sequence[str]
	self._opt2dest = {} # type: Dict[str, str]
	self._cleanup = [] # type: List[Callable[[], None]]
	self.pluginmanager.register(self, "pytestconfig")
	self._configured = False
	self.hook.pytest_addoption.call_historic(kwargs=dict(parser=self._parser))

[joshuastorck]

I don't think config should be passed at that point, as it's in a half-initialized state. Note how _configured is set to False. Most of the methods would be useless/wrong (e.g., getoption). Perhaps an argument could be made for including InvocationParams as a parameter.

[RonnyPfannschmidt]

at that point config is entirely incomplete, so no passing it please