Load plugins referencing entry point name provided via config and env var

[mtelka]

This allows to load plugins via PYTEST_PLUGINS environment variable and pytest_plugins global variable using their names in installed package entry points.

Closes #12624.

[webknjaz]

Please add tests to prove that it works and demonstrate the difference with the previous behavior.

[mtelka]

I can probably start with some merge of test_missing_required_plugins (it looks like the pytest11 entry point is created there) and test_early_config_cmdline (some kind of PYTEST_PLUGINS handling). But then some questions arises, for example: why is there the pytester.syspathinsert() call? After reading the doc it looks like the call is just a no-op, but this conclusion is apparently wrong.

I'm very sorry, but I'll need to leave this for somebody else who actually knows Python. Disclaimer: I'm not a Python developer.

[webknjaz]

I'd imagine that it sets $PYTHONPATH just for the subprocess invocation of pytest, which is what makes it possible for that subprocess to import <tested_pytest_plugin> residing on that path.

[webknjaz]

I wouldn't merge tests at this point. Maybe, in a separate refactoring. But it's always easier to add a separate one and not care if you break other ones accidentally. Especially, if you're unsure.

As for the tips, using monkeypatch.setenv() should be a way to test the env var bit. It might be a bit more involved with generating a custom config, though. Still, I think it's a good starting point.

[mtelka]

Sorry, this is really out of my knowledge.

[webknjaz]

Okay, so don't mark it as resolved because hiding the discussion from people doesn't mean that it shouldn't be solved before merging.

[webknjaz]

Looks like the right place to put the tests in would be test_pluginmanager.py::TestPytestPluginManager

[webknjaz]

There's also test_early_load_setuptools_name @ acceptance_test.py

[webknjaz]

Could you also add description to the PR and the commit, perhaps?

[bluetech]

Sounds reasonable to me. Normally this doesn't matter because entrypoint plugins are loaded already by the time PYTEST_PLUGINS is considered, but it does matter when PYTEST_DISABLE_PLUGIN_AUTOLOAD is used. And then it seems OK to consider entry points as well. I don't think #4727 has caused any issues.

The PYTEST_PLUGINS doc in reference.rst currently says "list of modules", we should add "or entry points".

[mtelka]

The PYTEST_PLUGINS doc in reference.rst currently says "list of modules", we should add "or entry points".

Done.

[mtelka]

Could you also add description to the PR and the commit, perhaps?

What kind of description would you like to see?

[webknjaz]

Could you also add description to the PR and the commit, perhaps?

What kind of description would you like to see?

Most of the information from the issue. Things that will end up in the merged commit. You only added a link and a word “fixes” but any other information is only available though Git paleontology. In general, people checking out that commit shouldn't have to go to external places to understand what it does. It should be summarized clearly in the message itself. A good guide to follow could be https://cbea.ms/git-commit/, for example.

[mtelka]

Could you also add description to the PR and the commit, perhaps?

What kind of description would you like to see?

Most of the information from the issue. Things that will end up in the merged commit. You only added a link and a word “fixes” but any other information is only available though Git paleontology. In general, people checking out that commit shouldn't have to go to external places to understand what it does. It should be summarized clearly in the message itself. A good guide to follow could be https://cbea.ms/git-commit/, for example.

Changed. Is it better?

[webknjaz]

Could you also add description to the PR and the commit, perhaps?

What kind of description would you like to see?

Most of the information from the issue. Things that will end up in the merged commit. You only added a link and a word “fixes” but any other information is only available though Git paleontology. In general, people checking out that commit shouldn't have to go to external places to understand what it does. It should be summarized clearly in the message itself. A good guide to follow could be cbea.ms/git-commit, for example.

Changed. Is it better?

Yes. Much better. This also reveals that we forgot to mention the effect of such plugins being listed in the console output in the changelog fragment. I think it'd be useful for the users to know that this changed.

[mtelka]

Yes. Much better. This also reveals that we forgot to mention the effect of such plugins being listed in the console output in the changelog fragment. I think it'd be useful for the users to know that this changed.

This also reminds me that this PR does not fully fix #12615 because plugins loaded the old way are still not listed in the list. :-(

[mtelka]

This also reminds me that this PR does not fully fix #12615 because plugins loaded the old way are still not listed in the list. :-(

#12624 created to cover the entry points part of #12615 only. The initial issue reported in #12615 won't be fixed by this PR.

[webknjaz]

Wait a second... I'm confused. How are these two issues separate?

[mtelka]

Wait a second... I'm confused. How are these two issues separate?

They are separate because this PR does not fix #12615 at all. With this PR and without this PR the env - PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 PYTEST_PLUGINS=pytest_randomly pytest --setup-plan output will be same (modulo the random number in --randomly-seed).

What will differ is the env - PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 PYTEST_PLUGINS=randomly pytest --setup-plan output. Without this PR the command will fail like this:

$ env - PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 PYTEST_PLUGINS=randomly pytest --setup-plan
Traceback (most recent call last):
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 865, in import_plugin
    __import__(importspec)
ModuleNotFoundError: No module named 'randomly'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/bin/pytest", line 8, in <module>
    sys.exit(console_main())
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 206, in console_main
    code = main()
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 159, in main
    config = _prepareconfig(args, plugins)
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 346, in _prepareconfig
    config = pluginmanager.hook.pytest_cmdline_parse(
  File "/usr/lib/python3.9/vendor-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  File "/usr/lib/python3.9/vendor-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/usr/lib/python3.9/vendor-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/usr/lib/python3.9/vendor-packages/pluggy/_callers.py", line 122, in _multicall
    teardown.throw(exception)  # type: ignore[union-attr]
  File "/usr/lib/python3.9/vendor-packages/_pytest/helpconfig.py", line 106, in pytest_cmdline_parse
    config = yield
  File "/usr/lib/python3.9/vendor-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 1152, in pytest_cmdline_parse
    self.parse(args)
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 1501, in parse
    self._preparse(args, addopts=addopts)
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 1389, in _preparse
    self.pluginmanager.consider_env()
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 827, in consider_env
    self._import_plugin_specs(os.environ.get("PYTEST_PLUGINS"))
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 838, in _import_plugin_specs
    self.import_plugin(import_spec)
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 867, in import_plugin
    raise ImportError(
  File "/usr/lib/python3.9/vendor-packages/_pytest/config/__init__.py", line 865, in import_plugin
    __import__(importspec)
ImportError: Error importing plugin "randomly": No module named 'randomly'
$

with this PR merged we will get this:

$ env - PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 PYTEST_PLUGINS=randomly pytest --setup-plan
============================= test session starts ==============================
platform sunos5 -- Python 3.9.19, pytest-8.2.2, pluggy-1.5.0
Using --randomly-seed=1218696459
rootdir: /tmp/test
plugins: randomly-3.15.0
collected 0 items                                                              

============================ no tests ran in 0.03s =============================
$

[webknjaz]

@mtelka okay, thank you! Now I understand the scope of the PR better. May I suggest changing the title of the PR/commit to something along the lines of “Load plugins referencing entry point name provided via config and env var”. I think, this would better reflect what the effect of applying your patch is.

[mtelka]

@mtelka okay, thank you! Now I understand the scope of the PR better. May I suggest changing the title of the PR/commit to something along the lines of “Load plugins referencing entry point name provided via config and env var”. I think, this would better reflect what the effect of applying your patch is.

I do not mind. Will do the change in a minute...

[mtelka]

... but, honestly, the new one is more confusing and less clear to me. But as I said, I do not mind.

[mtelka]

@webknjaz, should I squash (I prefer to)? Not sure what are rules for this project...

[webknjaz]

Could you also drop the mention of “entry_points.txt files” from the description? People usually don't know what they are or that they exist. Just saying something like “installed package entry points” should be enough. (well, we could add “metadata” too, but that's not mandatory)

[webknjaz]

@mtelka feel free to squash. If the commits are nicely crafted, the natural merge is typically used most of the time. If they are a mess, the maintainers may choose to use the squash strategy.

[mtelka]

I found that when I run tests with this change in place and with the following modification to tox.ini:

diff --git a/tox.ini b/tox.ini
index 61563ca2c..7089b5404 100644
--- a/tox.ini
+++ b/tox.ini
@@ -51,6 +51,8 @@ passenv =
     TERM
     SETUPTOOLS_SCM_PRETEND_VERSION_FOR_PYTEST
 setenv =
+    PYTEST_DISABLE_PLUGIN_AUTOLOAD=1
+    PYTEST_PLUGINS=hypothesispytest
     _PYTEST_TOX_DEFAULT_POSARGS={env:_PYTEST_TOX_POSARGS_DOCTESTING:} {env:_PYTEST_TOX_POSARGS_LSOF:} {env:_PYTEST_TOX_POSARGS_XDIST:}
 
     # See https://docs.python.org/3/library/io.html#io-encoding-warning

then the following 16 tests fails because they are unable to find hypothesispytest:

=========================== short test summary info ============================
FAILED testing/test_assertion.py::TestImportHookInstallation::test_installed_plugin_rewrite[plain]
FAILED testing/test_assertion.py::TestImportHookInstallation::test_installed_plugin_rewrite[rewrite]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[2-missing]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[2-missing-1-ok]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[1-ok]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[1-ok-pin-exact]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[1-ok-pin-loose]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[1-ok-prerelease]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[missing-version]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[missing-versions]
FAILED testing/test_config.py::TestParseIni::test_missing_required_plugins[invalid-header]
FAILED testing/test_config.py::test_importlib_metadata_broken_distribution - ...
FAILED testing/test_config.py::test_plugin_preparse_prevents_setuptools_loading[True]
FAILED testing/test_config.py::test_plugin_preparse_prevents_setuptools_loading[False]
FAILED testing/test_config.py::test_disable_plugin_autoload[parse_args0-True]
FAILED testing/test_config.py::test_disable_plugin_autoload[parse_args1-False]
XPASS testing/_py/test_local.py::TestLocalPath::test_make_numbered_dir_multiprocess_safe - #11603
= 16 failed, 3596 passed, 112 skipped, 11 xfailed, 1 xpassed in 216.23s (0:03:36) =

The fix seems obvious and we do need to remove the PYTEST_PLUGINS variable from the environment while we run the failing tests. Actually, in most cases there is already the PYTEST_DISABLE_PLUGIN_AUTOLOAD env var removal, so we will just add one more monkeypatch.delenv() call there.