diff --git a/.gitignore b/.gitignore index 41bf358..6d0923b 100644 --- a/.gitignore +++ b/.gitignore @@ -115,3 +115,4 @@ environment.yml _ /docs/build/ /_build/ +/.claude/ diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f19112a..f518b8a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,14 @@ UNRELEASED ========== - Added full type annotations and exported types (via ``py.typed``). +- Suppress the Windows hard-error dialog that ``LoadLibraryW`` would raise when a plugin + DLL has unresolved imports (e.g. a conflicting dependency version bundled inside the + plugin). The OS error is now captured and raised as the new ``SharedLibraryLoadError`` + exception instead of blocking the process with a modal dialog. +- Plugin discovery is now resilient to individual load failures: ``get_plugins_available`` + skips broken plugins with a warning, and the new ``get_plugins_available_and_failures`` + method returns the successful plugins together with a list of ``PluginLoadFailure`` + records so callers can surface the reason to the user. 0.8.0 (2025-08-18) ================== diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..2388276 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,76 @@ +# hookman — Developer Tooling Reference + +## Environment setup + +This project uses a plain virtualenv (not pixi, despite the presence of `pixi.devenv.toml`). + +```bash +python -m venv .venv + +# Windows +.venv/Scripts/pip install -e . -r requirements_dev.txt + +# Linux +.venv/bin/pip install -e . -r requirements_dev.txt +``` + +## Type checking + +```bash +prek run mypy -a +``` + +## Running tests + +```bash +# Windows +.venv/Scripts/pytest tests/test_hooks.py tests/test_hookman_utils.py -v + +# Linux +.venv/bin/pytest tests/test_hooks.py tests/test_hookman_utils.py -v +``` + +Some tests require compiled C++ artifacts (see **Building** below). The +`add_artifacts_to_path` autouse fixture in `conftest.py` adds `build/artifacts` +to `sys.path` automatically, so no `PYTHONPATH` setup is needed. + +## Building C++ artifacts + +```bash +# Windows +.venv/Scripts/inv build + +# Linux +.venv/bin/inv build +``` + +This compiles the test plugins (`.dll`/`.so`) and the pybind11 binding +(`.pyd`/`.so`), installing them to `build/artifacts/`. Must be re-run whenever +C++ source files change. + +On Windows, requires Visual Studio with MSVC. On Linux, requires a C++ compiler +(e.g. `g++`) and `ninja`. + +## Regenerating file regression snapshots + +Some tests in `test_hookman_generator.py` use `pytest-regressions` to compare +generated files against stored snapshots. When the generator output changes +intentionally, regenerate the snapshots with: + +```bash +# Windows +.venv/Scripts/pytest tests/test_hookman_generator.py --force-regen + +# Linux +.venv/bin/pytest tests/test_hookman_generator.py --force-regen +``` + +Then verify the regenerated files look correct before committing them. + +## Pre-commit / linting + +```bash +prek run --all-files +``` + +This runs mypy, ruff, and other hooks defined in `.pre-commit-config.yaml`. diff --git a/setup.cfg b/setup.cfg index a321561..e89ffcc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,6 +3,3 @@ exclude = docs [aliases] test = pytest - -[tool:pytest] -collect_ignore = ['setup.py'] diff --git a/src/hookman/exceptions.py b/src/hookman/exceptions.py index b6dd0c3..d29a8a3 100644 --- a/src/hookman/exceptions.py +++ b/src/hookman/exceptions.py @@ -1,3 +1,6 @@ +from pathlib import Path + + class HookmanError(Exception): """ Base class for all hookman exceptions. @@ -11,6 +14,24 @@ class SharedLibraryNotFoundError(HookmanError): """ +class SharedLibraryLoadError(HookmanError): + """ + Exception raised when a shared library exists but fails to load. + + This typically occurs when the plugin DLL has unresolved imports or + incompatible entry points — for example, when a plugin bundles a + conflicting version of a dependency that the host also provides. + + :param shared_lib_path: Path to the shared library that failed to load. + :param reason: Human-readable OS error description. + """ + + def __init__(self, shared_lib_path: Path, reason: str) -> None: + self.shared_lib_path = shared_lib_path + self.reason = reason + super().__init__(f"Failed to load '{shared_lib_path}': {reason}") + + class InvalidDestinationPathError(HookmanError): """ Exception raised when the destination path to install the plugin is not one of the paths used diff --git a/src/hookman/hookman_generator.py b/src/hookman/hookman_generator.py index 41b7847..1d00134 100644 --- a/src/hookman/hookman_generator.py +++ b/src/hookman/hookman_generator.py @@ -856,7 +856,12 @@ def _generate_windows_body(hooks: list[Hook]) -> list[str]: " std::wstring w_filename = utf8_to_wstring(utf8_filename);", " auto handle = this->load_dll(w_filename);", " if (handle == NULL) {", - ' throw std::runtime_error("Error loading library " + utf8_filename + ": " + std::to_string(GetLastError()));', + " DWORD error_code = GetLastError();", + " char error_buf[512] = {};", + " FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error_code, 0, error_buf, sizeof(error_buf), nullptr);", + " std::string error_msg(error_buf);", + " while (!error_msg.empty() && (error_msg.back() <= ' ')) { error_msg.pop_back(); }", + ' throw std::runtime_error("Error loading library " + utf8_filename + ": " + error_msg + " (code " + std::to_string(error_code) + ")");', " }", " this->handles.push_back(handle);", "", @@ -929,8 +934,19 @@ def _generate_windows_body(hooks: list[Hook]) -> list[str]: " HMODULE load_dll(const std::wstring& filename) {", " // Path Modifier", " PathGuard path_guard{ filename };", - " // Load library (DLL)", - " return LoadLibraryW(filename.c_str());", + " // Suppress the Windows hard-error dialog that LoadLibraryW would otherwise", + " // show for unresolved DLL imports before returning NULL.", + " // NOTE: the same suppression logic exists in suppress_dll_error_dialog() in", + " // hookman_utils.py - keep both in sync when changing flags or error handling.", + " DWORD old_error_mode = 0;", + " if (!SetThreadErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX, &old_error_mode)) {", + ' throw std::runtime_error("SetThreadErrorMode failed: " + std::to_string(GetLastError()));', + " }", + " HMODULE handle = LoadLibraryW(filename.c_str());", + " if (!SetThreadErrorMode(old_error_mode, nullptr)) {", + ' throw std::runtime_error("SetThreadErrorMode restore failed: " + std::to_string(GetLastError()));', + " }", + " return handle;", " }", "", "", diff --git a/src/hookman/hookman_utils.py b/src/hookman/hookman_utils.py index f74b5e9..b5f6cc6 100644 --- a/src/hookman/hookman_utils.py +++ b/src/hookman/hookman_utils.py @@ -6,6 +6,8 @@ from contextlib import contextmanager from pathlib import Path +from hookman.exceptions import SharedLibraryLoadError + def find_config_files( plugin_dirs: Sequence[Path] | Path, *, ignored_sub_dir_names: Sequence[str] = () @@ -52,13 +54,66 @@ def change_path_env(shared_lib_path: str) -> Iterator[None]: handle.close() # pragma: no cover +@contextmanager +def suppress_dll_error_dialog() -> Iterator[None]: # pragma: no cover + """ + Suppress the Windows hard-error dialog that the OS loader shows when a + DLL fails to resolve its imports or entry points. + + No-op on non-Windows platforms. + + The same suppression logic is duplicated in the C++ `load_dll` function + generated by `hookman_generator.py` — keep both in sync when changing + the error-mode flags or the error-handling strategy. + """ + if sys.platform != "win32": + yield + return + + # Without this, `LoadLibraryW` raises a modal dialog for certain load + # failures (missing procedure, missing module) that blocks the process until + # manually dismissed. Setting `SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX` + # via `SetThreadErrorMode` makes the loader return NULL to the caller instead, + # so the error is handled programmatically via the normal `OSError`/`GetLastError` + # path. + SEM_FAILCRITICALERRORS = 0x0001 + SEM_NOOPENFILEERRORBOX = 0x8000 + # ctypes.WinDLL is Windows-only; mypy on non-Windows platforms does not resolve this attribute. + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) # type: ignore[attr-defined] + old_mode = ctypes.c_uint(0) + # The thread-local `SetThreadErrorMode` is used rather than the process-wide + # `SetErrorMode` to avoid interfering with other threads. + if not kernel32.SetThreadErrorMode( + SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX, + ctypes.byref(old_mode), + ): + raise OSError(ctypes.get_last_error(), "SetThreadErrorMode failed") + try: + yield + finally: + if not kernel32.SetThreadErrorMode(old_mode.value, None): + raise OSError(ctypes.get_last_error(), "SetThreadErrorMode restore failed") + + @contextmanager def load_shared_lib(shared_lib_path: str) -> Iterator[ctypes.CDLL]: """ Load a shared library using ctypes freeing the resource at end. + + On Windows, the OS hard-error dialog for unresolved DLL imports is + suppressed so that load failures surface as `SharedLibraryLoadError` + instead of blocking modal dialogs. + + :raises SharedLibraryLoadError: + If the shared library exists but fails to load, e.g. because of an + incompatible or missing dependency DLL. """ with change_path_env(shared_lib_path): - plugin_dll = ctypes.cdll.LoadLibrary(shared_lib_path) + with suppress_dll_error_dialog(): + try: + plugin_dll = ctypes.cdll.LoadLibrary(shared_lib_path) + except OSError as error: + raise SharedLibraryLoadError(Path(shared_lib_path), str(error)) from error try: yield plugin_dll diff --git a/src/hookman/hooks.py b/src/hookman/hooks.py index e905ab3..bfc3f92 100644 --- a/src/hookman/hooks.py +++ b/src/hookman/hooks.py @@ -1,4 +1,5 @@ import inspect +import logging import shutil from collections.abc import Callable from collections.abc import Sequence @@ -12,9 +13,13 @@ from hookman import hookman_utils from hookman.exceptions import InvalidDestinationPathError from hookman.exceptions import PluginAlreadyInstalledError +from hookman.exceptions import SharedLibraryLoadError +from hookman.exceptions import SharedLibraryNotFoundError from hookman.hookman_utils import change_path_env from hookman.plugin_config import PluginInfo +_logger = logging.getLogger(__name__) + @dataclass(frozen=True) class InstalledPluginInfo: @@ -26,6 +31,20 @@ class InstalledPluginInfo: version: Version +@dataclass(frozen=True) +class PluginLoadFailure: + """Information about a plugin that was found but failed to load during discovery.""" + + yaml_location: Path + """Path to the plugin's plugin.yaml file.""" + + plugin_id: str + """The plugin id, read from the plugin's YAML config file.""" + + reason: str + """Human-readable description of why the plugin failed to load.""" + + class HookSpecs: """ A class that holds the specification of the hooks, currently the following specification are available: @@ -203,6 +222,49 @@ def remove_plugin(self, caption: str, version: Version | None = None) -> None: self._try_clear_trash(root_dir) break + def get_plugins_available_and_failures( + self, ignored_plugins: Sequence[str] = () + ) -> tuple[list[PluginInfo], list[PluginLoadFailure]]: + """ + Return all plugins that loaded successfully, plus a list of those that failed. + + A single incompatible plugin DLL will not abort discovery of the remaining + plugins — `SharedLibraryLoadError` is caught per-plugin and reported in the + failures list instead. + + Optionally you can pass a list of plugin ids to exclude from both lists. + + :returns: + A tuple of (successful `PluginInfo` list, `PluginLoadFailure` list). + """ + plugin_ids_and_files = [ + (plugin_id, f) + for f in hookman_utils.find_config_files( + self.plugins_dirs, ignored_sub_dir_names=[self._TRASH_DIR_NAME] + ) + if (plugin_id := PluginInfo.parse_id(f)) not in ignored_plugins + ] + + plugins: list[PluginInfo] = [] + failures: list[PluginLoadFailure] = [] + for plugin_id, plugin_file in plugin_ids_and_files: + try: + plugin_info = PluginInfo(plugin_file, self.hooks_available) + except (SharedLibraryLoadError, SharedLibraryNotFoundError) as error: + reason = str(error) + _logger.warning("Plugin at '%s' failed to load: %s", plugin_file, reason) + failures.append( + PluginLoadFailure( + yaml_location=plugin_file, + plugin_id=plugin_id, + reason=reason, + ) + ) + continue + else: + plugins.append(plugin_info) + return plugins, failures + def get_plugins_available(self, ignored_plugins: Sequence[str] = ()) -> Sequence[PluginInfo]: """ Return a list of :ref:`plugin-info-api-section` that are available on ``plugins_dirs`` @@ -211,20 +273,11 @@ def get_plugins_available(self, ignored_plugins: Sequence[str] = ()) -> Sequence When informed, the `ignored_plugins` must be a list with the names of the plugins (same as shared_lib_name) instead of the plugin caption. - The :ref:`plugin-info-api-section` is a object that holds all information related to the plugin. + Plugins whose DLL fails to load are silently skipped with a warning logged. + Use `get_plugins_available_and_failures` to receive the failure details. """ - plugin_config_files = hookman_utils.find_config_files( - self.plugins_dirs, ignored_sub_dir_names=[self._TRASH_DIR_NAME] - ) - - plugins_available = [ - PluginInfo(plugin_file, self.hooks_available) for plugin_file in plugin_config_files - ] - return [ - plugin_info - for plugin_info in plugins_available - if plugin_info.id not in ignored_plugins - ] + plugins, _failures = self.get_plugins_available_and_failures(ignored_plugins) + return plugins def get_hook_caller(self, ignored_plugins: Sequence[str] = ()) -> HookCaller: """ @@ -233,6 +286,10 @@ def get_hook_caller(self, ignored_plugins: Sequence[str] = ()) -> HookCaller: When informed, the `ignored_plugins` must be a list with the names of the plugins (same as shared_lib_name) instead of the plugin caption. + + Plugins whose DLL fails to load are silently skipped with a warning logged — they + do not raise an exception here. Use `get_plugins_available_and_failures` to + inspect load failures. """ assert self.specs.pyd_name is not None, f"Specs {self.specs!r}.pyd_name must be set" _hookman = __import__(self.specs.pyd_name) diff --git a/src/hookman/plugin_config.py b/src/hookman/plugin_config.py index 00834db..912ff3a 100644 --- a/src/hookman/plugin_config.py +++ b/src/hookman/plugin_config.py @@ -116,6 +116,12 @@ def _get_hooks_implemented(self) -> Sequence[str]: ] return hooks_implemented + @classmethod + def parse_id(cls, yaml_location: Path) -> str: + """Read the plugin id from the YAML config file without loading the shared library.""" + content = cls._load_yaml_file(yaml_location.read_text(encoding="utf-8")) + return str(content["id"]) + @classmethod def is_implemented_on_plugin(cls, plugin_dll: ctypes.CDLL, hook_name: str) -> bool: """ diff --git a/tasks.py b/tasks.py index 26df6d1..e2956ac 100644 --- a/tasks.py +++ b/tasks.py @@ -97,11 +97,14 @@ def compile_build_files(ctx): os.makedirs(artifacts_dir) os.makedirs(ninja_dir) + import pybind11 + call_cmake = ( f"cmake " f"-DCMAKE_BUILD_TYPE=Release " f'-G Ninja "{build_dir}" ' f"-DPYTHON_EXECUTABLE={sys.executable} " + f"-DCMAKE_PREFIX_PATH={pybind11.get_cmake_dir()} " ) call_ninja = "ninja -j 8" call_install = "ninja install" @@ -123,6 +126,7 @@ def compile_build_files(ctx): ), # Path for vcvars on GithubAction r"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat", + r"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars64.bat", ) for msvc_path in paths: if os.path.isfile(msvc_path): diff --git a/tests/conftest.py b/tests/conftest.py index a5087aa..bd2554a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,9 +1,20 @@ # mypy: allow-untyped-defs +import sys +from collections.abc import Iterator from pathlib import Path import pytest +@pytest.fixture(autouse=True) +def add_artifacts_to_path() -> Iterator[None]: + artifacts = Path(__file__).parents[1] / "build/artifacts" + assert artifacts.is_dir(), f"{artifacts} not found, run 'inv build' first" + sys.path.insert(0, str(artifacts)) + yield + sys.path.remove(str(artifacts)) + + @pytest.fixture def plugins_zip_folder(): return Path(__file__).parents[1] / "build/plugin_zip" diff --git a/tests/test_hookman_generator/HookCaller.hpp b/tests/test_hookman_generator/HookCaller.hpp index ca7426d..ea85655 100644 --- a/tests/test_hookman_generator/HookCaller.hpp +++ b/tests/test_hookman_generator/HookCaller.hpp @@ -70,7 +70,12 @@ class HookCaller { std::wstring w_filename = utf8_to_wstring(utf8_filename); auto handle = this->load_dll(w_filename); if (handle == NULL) { - throw std::runtime_error("Error loading library " + utf8_filename + ": " + std::to_string(GetLastError())); + DWORD error_code = GetLastError(); + char error_buf[512] = {}; + FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error_code, 0, error_buf, sizeof(error_buf), nullptr); + std::string error_msg(error_buf); + while (!error_msg.empty() && (error_msg.back() <= ' ')) { error_msg.pop_back(); } + throw std::runtime_error("Error loading library " + utf8_filename + ": " + error_msg + " (code " + std::to_string(error_code) + ")"); } this->handles.push_back(handle); @@ -141,8 +146,19 @@ class HookCaller { HMODULE load_dll(const std::wstring& filename) { // Path Modifier PathGuard path_guard{ filename }; - // Load library (DLL) - return LoadLibraryW(filename.c_str()); + // Suppress the Windows hard-error dialog that LoadLibraryW would otherwise + // show for unresolved DLL imports before returning NULL. + // NOTE: the same suppression logic exists in suppress_dll_error_dialog() in + // hookman_utils.py - keep both in sync when changing flags or error handling. + DWORD old_error_mode = 0; + if (!SetThreadErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX, &old_error_mode)) { + throw std::runtime_error("SetThreadErrorMode failed: " + std::to_string(GetLastError())); + } + HMODULE handle = LoadLibraryW(filename.c_str()); + if (!SetThreadErrorMode(old_error_mode, nullptr)) { + throw std::runtime_error("SetThreadErrorMode restore failed: " + std::to_string(GetLastError())); + } + return handle; } diff --git a/tests/test_hookman_generator/HookCallerNoPyd.hpp b/tests/test_hookman_generator/HookCallerNoPyd.hpp index b957ba5..0a5ab61 100644 --- a/tests/test_hookman_generator/HookCallerNoPyd.hpp +++ b/tests/test_hookman_generator/HookCallerNoPyd.hpp @@ -38,7 +38,12 @@ class HookCaller { std::wstring w_filename = utf8_to_wstring(utf8_filename); auto handle = this->load_dll(w_filename); if (handle == NULL) { - throw std::runtime_error("Error loading library " + utf8_filename + ": " + std::to_string(GetLastError())); + DWORD error_code = GetLastError(); + char error_buf[512] = {}; + FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error_code, 0, error_buf, sizeof(error_buf), nullptr); + std::string error_msg(error_buf); + while (!error_msg.empty() && (error_msg.back() <= ' ')) { error_msg.pop_back(); } + throw std::runtime_error("Error loading library " + utf8_filename + ": " + error_msg + " (code " + std::to_string(error_code) + ")"); } this->handles.push_back(handle); @@ -99,8 +104,19 @@ class HookCaller { HMODULE load_dll(const std::wstring& filename) { // Path Modifier PathGuard path_guard{ filename }; - // Load library (DLL) - return LoadLibraryW(filename.c_str()); + // Suppress the Windows hard-error dialog that LoadLibraryW would otherwise + // show for unresolved DLL imports before returning NULL. + // NOTE: the same suppression logic exists in suppress_dll_error_dialog() in + // hookman_utils.py - keep both in sync when changing flags or error handling. + DWORD old_error_mode = 0; + if (!SetThreadErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX, &old_error_mode)) { + throw std::runtime_error("SetThreadErrorMode failed: " + std::to_string(GetLastError())); + } + HMODULE handle = LoadLibraryW(filename.c_str()); + if (!SetThreadErrorMode(old_error_mode, nullptr)) { + throw std::runtime_error("SetThreadErrorMode restore failed: " + std::to_string(GetLastError())); + } + return handle; } diff --git a/tests/test_hookman_utils.py b/tests/test_hookman_utils.py index 9bd3cd6..eb6f36a 100644 --- a/tests/test_hookman_utils.py +++ b/tests/test_hookman_utils.py @@ -1,12 +1,16 @@ # mypy: allow-untyped-defs +import os import sys import pytest +from hookman.exceptions import SharedLibraryLoadError +from hookman.hookman_utils import change_path_env +from hookman.hookman_utils import find_config_files +from hookman.hookman_utils import load_shared_lib -def test_find_config_files(datadir) -> None: - from hookman.hookman_utils import find_config_files +def test_find_config_files(datadir) -> None: config_files = find_config_files(datadir) assert len(config_files) == 2 @@ -27,9 +31,6 @@ def test_find_config_files(datadir) -> None: not sys.platform.startswith("win"), reason="path only needs changing on Windows" ) def test_change_path_env(simple_plugin_dll) -> None: - import os - from hookman.hookman_utils import change_path_env - dll_dir = str(simple_plugin_dll.parent) assert dll_dir not in os.environ["PATH"] with change_path_env(str(simple_plugin_dll)): @@ -42,8 +43,22 @@ def test_change_path_env(simple_plugin_dll) -> None: ) def test_path_change_when_load_library(simple_plugin_dll, mocker) -> None: mocked = mocker.patch("hookman.hookman_utils.change_path_env") - from hookman.hookman_utils import load_shared_lib with load_shared_lib(str(simple_plugin_dll)): pass mocked.assert_called_once() + + +def test_load_shared_lib_raises_shared_library_load_error_for_corrupt_file(tmp_path) -> None: + """A file that exists but is not a valid shared library raises SharedLibraryLoadError.""" + + lib_name = "my_plugin.dll" if sys.platform == "win32" else "libmy_plugin.so" + corrupt_lib = tmp_path / lib_name + corrupt_lib.write_text("not a real shared library") + + with pytest.raises(SharedLibraryLoadError) as exc_info: + with load_shared_lib(str(corrupt_lib)): + pass # pragma: no cover + + assert exc_info.value.shared_lib_path == corrupt_lib + assert exc_info.value.reason # Non-empty OS-dependent error description. diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 90726f1..fb09743 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,5 +1,7 @@ # mypy: allow-untyped-defs import dataclasses +import sys +from pathlib import Path import pytest from packaging.version import Version @@ -134,6 +136,96 @@ def test_plugins_available_plain(simple_plugin, simple_plugin_2) -> None: assert len(plugins) == 1 +def _make_broken_plugin_dir(root: Path) -> Path: + """Create a plugin directory with a valid yaml but an unloadable shared library.""" + broken_plugin_dir = root / "broken_plugin-1.0.0" + (broken_plugin_dir / "assets").mkdir(parents=True) + (broken_plugin_dir / "assets" / "plugin.yaml").write_text( + "caption: 'Broken Plugin'\nversion: '1.0.0'\nauthor: 'a'\nemail: 'a@a.com'\nid: 'broken_plugin'\n" + ) + artifacts_dir = broken_plugin_dir / "artifacts" + artifacts_dir.mkdir() + lib_name = "broken_plugin.dll" if sys.platform == "win32" else "libbroken_plugin.so" + (artifacts_dir / lib_name).write_text("not a real shared library") + return broken_plugin_dir + + +def test_get_plugins_available_and_failures_with_broken_plugin( + tmp_path, simple_plugin, acme_hook_specs +) -> None: + """A plugin with an unloadable DLL appears in the failures list; valid plugins still load.""" + # Use a dedicated subdirectory so that pytest-datadir content written to tmp_path + # by other fixtures (e.g. multiple_plugins without compiled .so files) is not + # picked up by find_config_files' recursive glob. + plugins_root = tmp_path / "plugins" + plugins_root.mkdir() + broken_plugin_dir = _make_broken_plugin_dir(plugins_root) + + hm = HookMan(specs=acme_hook_specs, plugin_dirs=[simple_plugin["path"], plugins_root]) + plugins, failures = hm.get_plugins_available_and_failures() + + assert [p.id for p in plugins] == ["simple_plugin"] + [failure] = failures + assert failure.plugin_id == "broken_plugin" + assert failure.yaml_location == broken_plugin_dir / "assets" / "plugin.yaml" + assert failure.reason # Non-empty OS-dependent error message. + + +def test_get_plugins_available_skips_failures(tmp_path, simple_plugin, acme_hook_specs) -> None: + """get_plugins_available silently skips plugins that fail to load.""" + plugins_root = tmp_path / "plugins" + plugins_root.mkdir() + _make_broken_plugin_dir(plugins_root) + + hm = HookMan(specs=acme_hook_specs, plugin_dirs=[simple_plugin["path"], plugins_root]) + plugins = hm.get_plugins_available() + + # The broken plugin is skipped; the valid one still loads. + assert [p.id for p in plugins] == ["simple_plugin"] + + +def _make_missing_dll_plugin_dir(root: Path) -> Path: + """Create a plugin directory with a valid YAML but no shared library file.""" + plugin_dir = root / "missing_dll_plugin-1.0.0" + (plugin_dir / "assets").mkdir(parents=True) + (plugin_dir / "assets" / "plugin.yaml").write_text( + "caption: 'Missing DLL Plugin'\nversion: '1.0.0'\nauthor: 'a'\nemail: 'a@a.com'\nid: 'missing_dll_plugin'\n" + ) + (plugin_dir / "artifacts").mkdir() + return plugin_dir + + +def test_get_plugins_available_and_failures_with_missing_dll(tmp_path, acme_hook_specs) -> None: + """A plugin whose DLL is absent (SharedLibraryNotFoundError) lands in the failures list.""" + plugins_root = tmp_path / "plugins" + plugins_root.mkdir() + missing_plugin_dir = _make_missing_dll_plugin_dir(plugins_root) + + hm = HookMan(specs=acme_hook_specs, plugin_dirs=[plugins_root]) + plugins, failures = hm.get_plugins_available_and_failures() + + assert plugins == [] + [failure] = failures + assert failure.plugin_id == "missing_dll_plugin" + assert failure.yaml_location == missing_plugin_dir / "assets" / "plugin.yaml" + assert failure.reason + + +def test_get_plugins_available_and_failures_ignored_plugin_excluded_from_failures( + tmp_path, acme_hook_specs +) -> None: + """An ignored plugin that also fails to load must not appear in failures.""" + plugins_root = tmp_path / "plugins" + plugins_root.mkdir() + _make_broken_plugin_dir(plugins_root) + + hm = HookMan(specs=acme_hook_specs, plugin_dirs=[plugins_root]) + plugins, failures = hm.get_plugins_available_and_failures(ignored_plugins=["broken_plugin"]) + + assert plugins == [] + assert failures == [] + + def test_plugins_available_ignore_trash(datadir, simple_plugin, simple_plugin_2) -> None: plugin_dirs = [simple_plugin["path"], simple_plugin_2["path"]] hm = HookMan(specs=simple_plugin["specs"], plugin_dirs=plugin_dirs) diff --git a/tests/test_plugin_config.py b/tests/test_plugin_config.py index 68d47cd..6fd3e6b 100644 --- a/tests/test_plugin_config.py +++ b/tests/test_plugin_config.py @@ -39,6 +39,10 @@ def test_get_shared_libs_path(datadir, mocker, mock_plugin_id_from_dll) -> None: assert plugin_config.shared_lib_path == expected_path +def test_parse_id(datadir) -> None: + assert PluginInfo.parse_id(datadir / "assets/plugin.yaml") == "name_of_the_shared_lib" + + def test_plugin_id_conflict(simple_plugin, datadir) -> None: yaml_file = simple_plugin["path"] / "assets/plugin.yaml" assert PluginInfo(yaml_file, None) diff --git a/tox.ini b/tox.ini index 7c48c4c..e8dbe51 100644 --- a/tox.ini +++ b/tox.ini @@ -7,8 +7,6 @@ envlist = 310 [testenv] passenv = TOXENV -setenv = - PYTHONPATH = {toxinidir}/build/artifacts deps = -r{toxinidir}/requirements_dev.txt