Classify plugin startup import failures (#993)

changelog/993.bugfix.rst

@@ -0,0 +1,15 @@
+Fixed a plugin which fails to load during startup escaping as an unhandled exception, printing a raw traceback and exiting with the accidental exit code ``1``
+-- indistinguishable from :attr:`ExitCode.TESTS_FAILED <pytest.ExitCode.TESTS_FAILED>`. The failure is now reported and classified:
+
+* A plugin which cannot be found -- requested via ``-p``, ``pytest_plugins`` or the ``PYTEST_PLUGINS`` environment variable --
+  now exits with :attr:`ExitCode.USAGE_ERROR <pytest.ExitCode.USAGE_ERROR>` (``4``), the same code already used for a ``conftest.py`` which fails to import.
+
+* A plugin which is found but raises while being imported -- including an installed plugin loaded through a ``pytest11`` entry point,
+  and a plugin whose own dependencies are missing -- now exits with :attr:`ExitCode.INTERNAL_ERROR <pytest.ExitCode.INTERNAL_ERROR>` (``3``),
+  since this is a defect in the plugin rather than a misuse of pytest. The plugin's traceback is still shown.
+
+As a consequence, :func:`pytest.main` now returns these exit codes instead of propagating the exception to its caller.
+
+A ``conftest.py`` which fails to import is unaffected and keeps returning :attr:`ExitCode.USAGE_ERROR <pytest.ExitCode.USAGE_ERROR>` (``4``).
+
+This also fixes an ``IndexError`` raised from pytest's internals when a plugin raised an exception with no arguments, such as a bare ``raise ImportError``.

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 failed to load
+:Exit code 4: pytest command line usage error, including a ``conftest.py`` which fails to import or a plugin which cannot be found
 :Exit code 5: No tests were collected
 :Exit code 6: Maximum number of warnings exceeded (see :option:`--max-warnings`)
 

src/_pytest/config/__init__.py

@@ -141,7 +141,27 @@ 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 __init__(self, modname: str, *, cause: BaseException) -> None:
+        self.modname = modname
+        self.cause = cause
+
+    def __str__(self) -> str:
+        return (
+            f"{type(self.cause).__name__}: {self.cause} "
+            f"(importing plugin {self.modname!r})"
+        )
+
+
+def filter_traceback_for_import_failure(
     entry: _pytest._code.TracebackEntry,
 ) -> bool:
     """Filter tracebacks entries which point to pytest internals or importlib.
@@ -152,13 +172,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
@@ -169,6 +187,16 @@ 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:
+    _print_import_error(f'Error while loading plugin "{e.modname}".', e.cause, file)
+
+
 def print_usage_error(e: UsageError, file: TextIO) -> None:
     tw = TerminalWriter(file)
     for msg in e.args:
@@ -227,6 +255,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)
@@ -919,16 +950,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, cause=e) from e
+        except UsageError:
+            raise
+        except Exception as e:
+            raise PluginImportFailure(modname, cause=e) 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, cause=e) 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: None | types.ModuleType | str | Sequence[str],

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,128 @@ 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_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
@@ -1771,8 +1772,9 @@ 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)
 
 
 def test_importlib_metadata_broken_distribution(