pytest
pytest copied to clipboard
Load plugins from paths in 'pythonpath' option
Real fix for #11118
Can't figure out how to resolve the codecov failure, which seems to be illegitimate anyway since it removed the incorrect "not covered by tests" annotations. Otherwise, this is ready for review.
@millerdev Since the issue is closed, could you explain your use case for this change? I'm asking because we generally prefer to put functionality in plugins, so we'd like to understand the reasoning before moving to the core.
@bluetech I am trying to load a plugin that implements early hooks like pytest_load_initial_conftests from within the package under test. I could find no other way to do it without this change. My requirements are:
- tests must be run against packages in the CWD (
pythonpathcontains.to enable this) - tests must be run with the
pytestcommand (notpython -h pytest). - a custom pytest plugin required to run the tests resides within one of the packages under test
- the packages under test are not
pip install-able
The python_path plugin where this functionality was implemented was using the earliest available pytest hook (pytest_load_initial_conftests) to update sys.path with values from the pythonpath option. Unfortunately that hook is not early enough in the plugin lifecycle to allow plugins to be loaded from pythonpath paths.
@bluetech were you able to consider my use case requirements? Is there anything else blocking for this PR that I should address before it can be merged?
@millerdev in summary your requirement is that you need to load plugins which are implemented in files in ., however for them to be able to be imported, their path needs to be in PYTHONPATH already?
@nicoddemus That is correct. Thank you for taking a look!
I see, thanks for clarifying.
I understand (from a glance, don't have much time now to look deeply into it) you basically removed the python_path.py plugin and made the pythonpath configuration value a feature of Config, right?
Hard to tell if this might cause regressions or not, my gut feeling is that this is OK because it only affects those already using pythonpath and is only configuring $PYTHONPATH a little earlier than before.
In general I think it makes sense to set $PYTHONPATH via the pythonpath confval as early as possible. I also believe it was implemented as a separate plugin because it was cleaner that way, but it seems it is not enough to cover all cases.
What do others think? @RonnyPfannschmidt @Pierre-Sassoulas @bluetech
We need to make note to turn this into a early plugin late (similar to what coverage needs)
Seems there is a use case for moving this functionality to core, but that means it can't be a plugin. We prefer functionality to be in plugins because it keeps the core cleaner and leaner. So the question is, is it worth complicating the core in order to accommodate the use case?
IMO the answer is no. The use case and constraints (not being able to set PYTHONPATH or sys.path) seem too niche to me. But I don't feel strongly about it, i.e. perfectly fine if one of the other maintainers wants to merge.
BTW: Apologies for being slow to follow up @millerdev.
I'm OK with this, given the implementation is really simple, and it solves one pain point that is avoiding have to configure PYTHONPATH externally.
So if it is OK with @bluetech I would like to get this in.
Thanks @millerdev for the contribution!
@nicoddemus Thank you for getting this in!
@nicoddemus in what version of pytest is this expected to be released?
Version 8.4, but we do not have an estimated release date yet.
Anxiously awaiting 8.4 release 😬
@wgordon17 you can subscribe to #12945 for any news regarding 8.4. 👍