pytest icon indicating copy to clipboard operation
pytest copied to clipboard

Load plugins from paths in 'pythonpath' option

Open millerdev opened this issue 1 year ago • 3 comments

Real fix for #11118

millerdev avatar Jun 26 '24 16:06 millerdev

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 avatar Jun 27 '24 12:06 millerdev

@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 avatar Jun 28 '24 21:06 bluetech

@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 (pythonpath contains . to enable this)
  • tests must be run with the pytest command (not python -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.

millerdev avatar Jun 29 '24 02:06 millerdev

@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 avatar Jul 23 '24 11:07 millerdev

@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 avatar Jul 31 '24 12:07 nicoddemus

@nicoddemus That is correct. Thank you for taking a look!

millerdev avatar Jul 31 '24 12:07 millerdev

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

nicoddemus avatar Jul 31 '24 15:07 nicoddemus

We need to make note to turn this into a early plugin late (similar to what coverage needs)

RonnyPfannschmidt avatar Aug 04 '24 09:08 RonnyPfannschmidt

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.

bluetech avatar Aug 04 '24 13:08 bluetech

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.

nicoddemus avatar Aug 05 '24 13:08 nicoddemus

Thanks @millerdev for the contribution!

nicoddemus avatar Aug 06 '24 10:08 nicoddemus

@nicoddemus Thank you for getting this in!

millerdev avatar Aug 07 '24 16:08 millerdev

@nicoddemus in what version of pytest is this expected to be released?

millerdev avatar Oct 17 '24 17:10 millerdev

Version 8.4, but we do not have an estimated release date yet.

nicoddemus avatar Oct 17 '24 21:10 nicoddemus

Anxiously awaiting 8.4 release 😬

wgordon17 avatar Jan 07 '25 20:01 wgordon17

@wgordon17 you can subscribe to #12945 for any news regarding 8.4. 👍

nicoddemus avatar Jan 07 '25 20:01 nicoddemus