Merge pull request #14824 from RonnyPfannschmidt/plugin-import-error-exit-codes-993

Classify plugin startup import failures (#993)

Author: RonnyPfannschmidt

changelog/993.breaking.rst

@@ -0,0 +1,6 @@
+Plugin import failures at startup are now reported and classified instead of escaping as a raw traceback with the accidental exit code ``1``:
+
+* a plugin that cannot be found -- via ``-p``, ``pytest_plugins`` or ``PYTEST_PLUGINS`` -- exits with :class:`pytest.ExitCode` ``USAGE_ERROR`` (``4``), like a ``conftest.py`` that fails to import;
+* a plugin that is found but raises while importing -- including a broken ``pytest11`` entry point or a missing plugin dependency -- exits with :class:`pytest.ExitCode` ``INTERNAL_ERROR`` (``3``), with the traceback preserved.
+
+:func:`pytest.main` now returns these codes instead of raising ``ImportError``. Also fixed an ``IndexError`` from pytest's own internals when the plugin raised an exception with no arguments.

doc/en/reference/exit-codes.rst

@@ -8,8 +8,8 @@ Running ``pytest`` can result in seven different exit codes:
 :Exit code 0: All tests were collected and passed successfully
 :Exit code 1: Tests were collected and run but some of the tests failed
 :Exit code 2: Test execution was interrupted by the user
-:Exit code 3: Internal error happened while executing tests
-:Exit code 4: pytest command line usage error
+:Exit code 3: Internal error happened while executing tests, or a plugin raised while importing
+:Exit code 4: pytest command line usage error, including a plugin that cannot be found or a ``conftest.py`` that fails to import
 :Exit code 5: No tests were collected
 :Exit code 6: Maximum number of warnings exceeded (see :option:`--max-warnings`)
 

src/_pytest/config/__init__.py

@@ -142,7 +142,17 @@ def __str__(self) -> str:
         return f"{type(self.cause).__name__}: {self.cause} (from {self.path})"
 
 
-def filter_traceback_for_conftest_import_failure(
+class PluginImportFailure(Exception):
+    """A plugin was found, but raised while being imported.
+
+    This is deliberately distinct from a plugin which could not be found at
+    all: not finding it means pytest was pointed at something that isn't there,
+    which is a :class:`UsageError`, while a plugin blowing up on import is a
+    defect in the plugin and reported as an internal error.
+    """
+
+
+def filter_traceback_for_import_failure(
     entry: _pytest._code.TracebackEntry,
 ) -> bool:
     """Filter tracebacks entries which point to pytest internals or importlib.
@@ -153,13 +163,11 @@ def filter_traceback_for_conftest_import_failure(
     return filter_traceback(entry) and "importlib" not in str(entry.path).split(os.sep)
 
 
-def print_conftest_import_error(e: ConftestImportFailure, file: TextIO) -> None:
-    exc_info = ExceptionInfo.from_exception(e.cause)
+def _print_import_error(header: str, cause: BaseException, file: TextIO) -> None:
+    exc_info = ExceptionInfo.from_exception(cause)
     tw = TerminalWriter(file)
-    tw.line(f"ImportError while loading conftest '{e.path}'.", red=True)
-    exc_info.traceback = exc_info.traceback.filter(
-        filter_traceback_for_conftest_import_failure
-    )
+    tw.line(header, red=True)
+    exc_info.traceback = exc_info.traceback.filter(filter_traceback_for_import_failure)
     exc_repr = (
         exc_info.getrepr(style="short", chain=False)
         if exc_info.traceback
@@ -170,6 +178,17 @@ def print_conftest_import_error(e: ConftestImportFailure, file: TextIO) -> None:
         tw.line(line.rstrip(), red=True)
 
 
+def print_conftest_import_error(e: ConftestImportFailure, file: TextIO) -> None:
+    _print_import_error(
+        f"ImportError while loading conftest '{e.path}'.", e.cause, file
+    )
+
+
+def print_plugin_import_error(e: PluginImportFailure, file: TextIO) -> None:
+    assert e.__cause__ is not None, f"{e!r} must be raised `from` the original error"
+    _print_import_error(f'Error while loading plugin "{e}".', e.__cause__, file)
+
+
 def print_usage_error(e: UsageError, file: TextIO) -> None:
     tw = TerminalWriter(file)
     for msg in e.args:
@@ -232,6 +251,9 @@ def _main(
         except ConftestImportFailure as e:
             print_conftest_import_error(e, file=sys.stderr)
             return ExitCode.USAGE_ERROR
+        except PluginImportFailure as e:
+            print_plugin_import_error(e, file=sys.stderr)
+            return ExitCode.INTERNAL_ERROR
 
         try:
             ret: ExitCode | int = config.hook.pytest_cmdline_main(config=config)
@@ -924,16 +946,46 @@ def import_plugin(self, modname: str, consider_entry_points: bool = False) -> No
                 # testing/test_config.py::test_disable_plugin_autoload.
                 __import__(importspec)
                 mod = sys.modules[importspec]
-        except ImportError as e:
-            raise ImportError(
-                f'Error importing plugin "{modname}": {e.args[0]}'
-            ).with_traceback(e.__traceback__) from e
-
         except Skipped as e:
             self.skipped_plugins.append((modname, e.msg or ""))
+        except ModuleNotFoundError as e:
+            if _is_missing_module(e, importspec):
+                # The plugin itself is nowhere to be found - pytest was pointed
+                # at something which does not exist, so this is a usage error.
+                raise UsageError(f'Error importing plugin "{modname}": {e}') from e
+            # Some *other* module the plugin imports is missing: the plugin was
+            # found, so this is a defect in the plugin, not a usage error.
+            raise PluginImportFailure(modname) from e
+        except UsageError:
+            raise
+        except Exception as e:
+            raise PluginImportFailure(modname) from e
         else:
             self.register(mod, modname)
 
+    def load_setuptools_entrypoints(self, group: str, name: str | None = None) -> int:
+        """:meta private:"""
+        try:
+            return super().load_setuptools_entrypoints(group, name=name)
+        except UsageError:
+            raise
+        except Exception as e:
+            # An installed plugin which cannot be loaded is a defect in that
+            # plugin - the user did nothing wrong by having it installed.
+            raise PluginImportFailure(name or group) from e
+
+
+def _is_missing_module(e: ModuleNotFoundError, importspec: str) -> bool:
+    """Whether ``e`` means that ``importspec`` itself could not be found.
+
+    A ``ModuleNotFoundError`` naming some other module means the plugin was
+    located but one of its own imports is unsatisfied.
+    """
+    if e.name is None:
+        return False
+    # A missing parent package also means importspec cannot be found.
+    return e.name == importspec or importspec.startswith(f"{e.name}.")
+
 
 def _get_plugin_specs_as_list(
     specs: types.ModuleType | str | Sequence[str] | None,

testing/acceptance_test.py

@@ -13,6 +13,7 @@
 import setuptools
 
 from _pytest.config import ExitCode
+from _pytest.monkeypatch import MonkeyPatch
 from _pytest.pathlib import symlink_or_skip
 from _pytest.pytester import Pytester
 import pytest
@@ -510,9 +511,10 @@ def test_plugins_given_as_strings(
     ) -> None:
         """Test that str values passed to main() as `plugins` arg are
         interpreted as module names to be imported and registered (#855)."""
-        with pytest.raises(ImportError) as excinfo:
-            pytest.main([str(pytester.path)], plugins=["invalid.module"])
-        assert "invalid" in str(excinfo.value)
+        # A plugin which cannot be found is a usage error, reported through the
+        # return value rather than raised out of pytest.main() (#993).
+        ret = pytest.main([str(pytester.path)], plugins=["invalid.module"])
+        assert ret == ExitCode.USAGE_ERROR
 
         p = pytester.path.joinpath("test_test_plugins_given_as_strings.py")
         p.write_text("def test_foo(): pass", encoding="utf-8")
@@ -1088,6 +1090,138 @@ def main():
     result.stdout.no_fnmatch_line("*INTERNALERROR>*")
 
 
+class TestStartupPluginImportErrors:
+    """Exit codes for plugins which fail to load at startup (#993).
+
+    A plugin which cannot be found means pytest was pointed at something which
+    is not there, which is a usage error; a plugin which is found but blows up
+    while importing is a defect in the plugin, reported as an internal error.
+    """
+
+    @pytest.fixture
+    def broken_plugin(self, pytester: Pytester) -> Pytester:
+        pytester.syspathinsert()
+        pytester.makepyfile(myplugin="raise ValueError('plugin is broken')")
+        pytester.makepyfile("def test_foo(): pass")
+        return pytester
+
+    @pytest.fixture
+    def missing_plugin(self, pytester: Pytester) -> Pytester:
+        pytester.syspathinsert()
+        pytester.makepyfile("def test_foo(): pass")
+        return pytester
+
+    def test_missing_via_cmdline(self, missing_plugin: Pytester) -> None:
+        result = missing_plugin.runpytest("-p", "nosuchplugin")
+        assert result.ret == ExitCode.USAGE_ERROR
+        result.stderr.fnmatch_lines(['*Error importing plugin "nosuchplugin"*'])
+
+    def test_missing_via_conftest(self, missing_plugin: Pytester) -> None:
+        missing_plugin.makeconftest("pytest_plugins = ['nosuchplugin']")
+        result = missing_plugin.runpytest()
+        assert result.ret == ExitCode.USAGE_ERROR
+
+    def test_missing_via_env(
+        self, missing_plugin: Pytester, monkeypatch: MonkeyPatch
+    ) -> None:
+        monkeypatch.setenv("PYTEST_PLUGINS", "nosuchplugin")
+        result = missing_plugin.runpytest()
+        assert result.ret == ExitCode.USAGE_ERROR
+
+    def test_broken_via_cmdline(self, broken_plugin: Pytester) -> None:
+        result = broken_plugin.runpytest("-p", "myplugin")
+        assert result.ret == ExitCode.INTERNAL_ERROR
+        result.stderr.fnmatch_lines(
+            [
+                'Error while loading plugin "myplugin".',
+                "*myplugin.py:1: in <module>*",
+                "E*ValueError: plugin is broken",
+            ]
+        )
+
+    def test_broken_via_conftest(self, broken_plugin: Pytester) -> None:
+        broken_plugin.makeconftest("pytest_plugins = ['myplugin']")
+        result = broken_plugin.runpytest()
+        assert result.ret == ExitCode.INTERNAL_ERROR
+
+    def test_broken_via_env(
+        self, broken_plugin: Pytester, monkeypatch: MonkeyPatch
+    ) -> None:
+        monkeypatch.setenv("PYTEST_PLUGINS", "myplugin")
+        result = broken_plugin.runpytest()
+        assert result.ret == ExitCode.INTERNAL_ERROR
+
+    def test_usage_error_passes_through(self, pytester: Pytester) -> None:
+        """A plugin raising UsageError at import keeps its usage-error semantics."""
+        pytester.syspathinsert()
+        pytester.makepyfile(
+            myplugin="import pytest\nraise pytest.UsageError('config trouble')"
+        )
+        result = pytester.runpytest("-p", "myplugin")
+        assert result.ret == ExitCode.USAGE_ERROR
+        result.stderr.fnmatch_lines(["ERROR: config trouble*"])
+
+    def test_broken_via_entry_point(
+        self, pytester: Pytester, monkeypatch: MonkeyPatch
+    ) -> None:
+        monkeypatch.delenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", raising=False)
+
+        class DummyEntryPoint:
+            name = "myplugin"
+            group = "pytest11"
+
+            def load(self):
+                raise ValueError("plugin is broken")
+
+        class Distribution:
+            version = "1.0"
+            files = ("foo.txt",)
+            metadata = {"name": "foo"}
+            entry_points = (DummyEntryPoint(),)
+
+        monkeypatch.setattr(
+            importlib.metadata, "distributions", lambda: (Distribution(),)
+        )
+        pytester.makepyfile("def test_foo(): pass")
+        result = pytester.runpytest()
+        assert result.ret == ExitCode.INTERNAL_ERROR
+
+    def test_import_error_without_args(self, pytester: Pytester) -> None:
+        """A bare ``raise ImportError`` used to crash with an IndexError (#993)."""
+        pytester.syspathinsert()
+        pytester.makepyfile(myplugin="raise ImportError")
+        pytester.makepyfile("def test_foo(): pass")
+        result = pytester.runpytest("-p", "myplugin")
+        assert result.ret == ExitCode.INTERNAL_ERROR
+        result.stderr.no_fnmatch_line("*IndexError*")
+        result.stderr.fnmatch_lines(['Error while loading plugin "myplugin".'])
+
+    def test_missing_dependency_is_not_a_usage_error(self, pytester: Pytester) -> None:
+        """The plugin was found; one of *its* imports is unsatisfied (#993)."""
+        pytester.syspathinsert()
+        pytester.makepyfile(myplugin="import nosuchdependency")
+        pytester.makepyfile("def test_foo(): pass")
+        result = pytester.runpytest("-p", "myplugin")
+        assert result.ret == ExitCode.INTERNAL_ERROR
+
+    def test_missing_submodule_of_existing_package(self, pytester: Pytester) -> None:
+        """The package exists but the requested plugin module within it does not."""
+        pytester.syspathinsert()
+        pytester.mkpydir("mypkg")
+        pytester.makepyfile("def test_foo(): pass")
+        result = pytester.runpytest("-p", "mypkg.nosuchmodule")
+        assert result.ret == ExitCode.USAGE_ERROR
+
+    def test_conftest_import_failure_stays_a_usage_error(
+        self, pytester: Pytester
+    ) -> None:
+        """conftest.py is not a plugin; it keeps reporting a usage error (#993)."""
+        pytester.makeconftest("raise ValueError('conftest is broken')")
+        pytester.makepyfile("def test_foo(): pass")
+        result = pytester.runpytest()
+        assert result.ret == ExitCode.USAGE_ERROR
+
+
 def test_import_plugin_unicode_name(pytester: Pytester) -> None:
     pytester.makepyfile(myplugin="")
     pytester.makepyfile("def test(): pass")

testing/python/collect.py

@@ -8,6 +8,7 @@
 
 import _pytest._code
 from _pytest.config import ExitCode
+from _pytest.config.exceptions import UsageError
 from _pytest.main import Session
 from _pytest.monkeypatch import MonkeyPatch
 from _pytest.nodes import Collector
@@ -80,7 +81,7 @@ def test_syntax_error_in_module(self, pytester: Pytester) -> None:
 
     def test_module_considers_pluginmanager_at_import(self, pytester: Pytester) -> None:
         modcol = pytester.getmodulecol("pytest_plugins='xasdlkj',")
-        with pytest.raises(ImportError):
+        with pytest.raises(UsageError):
             modcol.obj()
 
     def test_invalid_test_module_name(self, pytester: Pytester) -> None:

testing/test_config.py

@@ -22,6 +22,7 @@
 from _pytest.config import console_main
 from _pytest.config import ExitCode
 from _pytest.config import parse_warning_filter
+from _pytest.config import PluginImportFailure
 from _pytest.config.argparsing import get_ini_default_for_type
 from _pytest.config.argparsing import Parser
 from _pytest.config.exceptions import UsageError
@@ -1858,7 +1859,35 @@ def distributions():
         return (Distribution(),)
 
     monkeypatch.setattr(importlib.metadata, "distributions", distributions)
-    with pytest.raises(ImportError):
+    with pytest.raises(PluginImportFailure) as excinfo:
+        pytester.parseconfig()
+    assert "Don't hide me!" in str(excinfo.value.__cause__)
+
+
+def test_setuptools_usage_error_passes_through(
+    pytester: Pytester, monkeypatch: MonkeyPatch
+) -> None:
+    """A UsageError from an entry-point plugin is not reclassified (#993)."""
+    monkeypatch.delenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", raising=False)
+
+    class DummyEntryPoint:
+        name = "mytestplugin"
+        group = "pytest11"
+
+        def load(self):
+            raise UsageError("bad usage")
+
+    class Distribution:
+        version = "1.0"
+        files = ("foo.txt",)
+        metadata = {"name": "foo"}
+        entry_points = (DummyEntryPoint(),)
+
+    def distributions():
+        return (Distribution(),)
+
+    monkeypatch.setattr(importlib.metadata, "distributions", distributions)
+    with pytest.raises(UsageError, match="bad usage"):
         pytester.parseconfig()