diff --git a/pyproject.toml b/pyproject.toml index c211d46..cf00fbb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,18 +61,21 @@ ignore_missing_imports = true module = "snakemake" follow_untyped_imports = true +[tool.pytest.ini_options] +python_files = ["test_*.py", "tests.py"] + [tool.pixi.feature.dev.tasks.test] cmd = [ "pytest", "--cov=snakemake_interface_common", "--cov-report=xml:coverage-report/coverage.xml", "--cov-report=term-missing", - "tests/tests.py" + "tests/", ] description = "Run tests and generate coverage report" [tool.pixi.feature.dev.tasks] -format = "ruff format src" +format = "ruff format src tests" lint = "ruff check" type-check = "mypy src/" qc = { depends-on = ["format", "lint", "type-check"] } diff --git a/src/snakemake_interface_common/plugin_registry/__init__.py b/src/snakemake_interface_common/plugin_registry/__init__.py index a991b8e..c86b6bb 100644 --- a/src/snakemake_interface_common/plugin_registry/__init__.py +++ b/src/snakemake_interface_common/plugin_registry/__init__.py @@ -8,7 +8,7 @@ import types import pkgutil import importlib -from typing import Dict, List, Mapping, TYPE_CHECKING, Type, TypeVar, Generic +from typing import Dict, List, Mapping, TYPE_CHECKING, TypeVar, Generic from snakemake_interface_common.exceptions import InvalidPluginException from snakemake_interface_common.plugin_registry.plugin import PluginBase @@ -21,23 +21,54 @@ class PluginRegistryBase(ABC, Generic[TPlugin]): - """This class is a singleton that holds all registered executor plugins.""" + """Base class to discover and record all available plugins of a given type. + + This class is a singleton, all calls to the constructor will return the same instance. + ``__init__()`` should not take any arguments. + + Derived class names are expected to end with ``PluginRegistry``, where the prefix is the type of + plugin (e.g. ``ExecutorPluginRegistry``). This is returned by :meth:`get_plugin_type()`. + + Package discovery works by searching through all importable top-level modules in ``sys.path`` + and selecting those where the full name matches ``"{self.module_prefix}_{plugin_name}"``, where + :attr:`module_prefix` should be ``"snakemake_{plugin_type}_plugin_"``. The plugin will be + registered under the name ``plugin_name``, but with underscores replaced with dashes. Note that + this will not detect packages installed in editable mode ``(pip install -e .)``. + + Example: a package named ``snakemake_executor_plugin_my_executor`` will be discovered by the + ``executor`` registry (with ``module_prefix = "snakemake_executor_plugin_"``) and registered + under the name ``"my-executor"``. The corresponding Pip/distribution package should be named + ``snakemake-executor-plugin-my-executor``, although this is not enforced. + """ _instance = None plugins: Dict[str, TPlugin] - def __new__( - cls: Type["PluginRegistryBase[TPlugin]"], - ) -> "PluginRegistryBase[TPlugin]": + def __new__(cls): if cls._instance is None: - cls._instance = super().__new__(cls) - return cls._instance + instance = super().__new__(cls) + instance.collect_plugins() + cls._instance = instance + # The following can cause issues with the type checker, ignore the line to make it assume + # standard behavior of returning an instance of cls (which should be the case). + return cls._instance # type: ignore - def __init__(self) -> None: - if hasattr(self, "plugins"): - # init has been called before - return - self.collect_plugins() + ######## Abstract methods ######## + + @property + @abstractmethod + def module_prefix(self) -> str: + """Prefix used to identify plugins by importable module name.""" + + @abstractmethod + def load_plugin(self, name: str, module: types.ModuleType) -> TPlugin: + """Instantiate the plugin object given its name and imported module.""" + + @abstractmethod + def expected_attributes(self) -> Mapping[str, AttributeType]: + """Get expected attributes of imported plugin module.""" + + ######## Other methods ######## def get_registered_plugins(self) -> List[str]: """Return a list of registered plugin names.""" @@ -47,29 +78,41 @@ def is_installed(self, plugin_name: str) -> bool: """Return True if the plugin is registered.""" return plugin_name in self.plugins - def get_plugin(self, plugin_name: str) -> PluginBase: - """Get a plugin by name.""" + def get_plugin(self, plugin_name: str) -> TPlugin: + """Get a registered plugin by name. + + Raises + ------ + InvalidPluginException + If the plugin is not registered. + """ try: return self.plugins[plugin_name] except KeyError: + pkgname = self.get_plugin_package_name(plugin_name) raise InvalidPluginException( plugin_name, - f"The package {self.module_prefix.replace('_', '-')}{plugin_name} is " - "not installed.", + f"The package {pkgname} is not installed.", ) def get_plugin_package_name(self, plugin_name: str) -> str: - """Get the package name of a plugin by name.""" + """Get the package name of a plugin by name. + + This is the pip-installable package name, not the name used to import the plugin module. + """ return f"{self.module_prefix.replace('_', '-')}{plugin_name}" def register_cli_args(self, argparser: "ArgumentParser") -> None: - """Add arguments derived from self.executor_settings to given - argparser.""" + """Add arguments derived from all registered plugins to given argparser.""" plugin_type = self.get_plugin_type() for _, plugin in self.plugins.items(): plugin.register_cli_args(argparser, plugin_type) def get_plugin_type(self) -> str: + """Get a string describing the type of plugin tracked by the registry. + + This is derived from the class name. + """ m = re.match(r"(?P.+)PluginRegistry", self.__class__.__name__) if m is not None: return m.group("type").lower() @@ -80,43 +123,63 @@ def get_plugin_type(self) -> str: ) def collect_plugins(self) -> None: - """Collect plugins and call register_plugin for each.""" + """Collect plugins, import their modules, and call :meth:`register_plugin` for each.""" self.plugins = dict() - # Executor plugins are externally installed plugins named - # "snakemake_executor_". - # They should follow the same convention if on pip, - # snakemake-executor-. - # Note that these will not be detected installed in editable - # mode (pip install -e .). for moduleinfo in pkgutil.iter_modules(): if not moduleinfo.ispkg or not moduleinfo.name.startswith( self.module_prefix ): continue + + name = moduleinfo.name.removeprefix(self.module_prefix).replace("_", "-") module = importlib.import_module(moduleinfo.name) - self.register_plugin(moduleinfo.name, module) + self.register_plugin(name, module) - def register_plugin(self, name: str, plugin: types.ModuleType) -> None: + def register_plugin(self, name: str, module: types.ModuleType) -> None: """Validate and register a plugin. Does nothing if the plugin is already registered. + + Parameters + ---------- + name + The name to register the plugin under. If derived from the module name, it should have + :attr:`module_prefix` removed. + module + The plugin's imported module. + + Raises + ------ + InvalidPluginException + If validation fails. """ + name = name.removeprefix(self.module_prefix).replace("_", "-") if name in self.plugins: return - self.validate_plugin(name, plugin) - - # Derive the shortened name for future access - plugin_name = name.removeprefix(self.module_prefix).replace("_", "-") - - self.plugins[plugin_name] = self.load_plugin(plugin_name, plugin) + self.validate_plugin(name, module) + self.plugins[name] = self.load_plugin(name, module) def is_valid_plugin_package_name(self, name: str) -> bool: return True def validate_plugin(self, name: str, module: types.ModuleType) -> None: - """Validate a plugin for attributes and naming""" + """Validate a plugin module for attributes and naming. + + Parameters + ---------- + name + The name the plugin is to be registered under. + module + The plugin's imported module. + + Raises + ------ + InvalidPluginException + If any module attributes have an incorrect type or a required attribute is missing. + """ + expected_attributes = self.expected_attributes() for attr, attr_type in expected_attributes.items(): # check if attr is missing and fail if it is not optional @@ -128,7 +191,9 @@ def validate_plugin(self, name: str, module: types.ModuleType) -> None: attr_value = getattr(module, attr) if attr_type.is_class: # check for class type - if not issubclass(attr_value, attr_type.cls): + if not isinstance(attr_value, type) or not issubclass( + attr_value, attr_type.cls + ): raise InvalidPluginException( name, f"{attr} must be a subclass of " @@ -140,15 +205,3 @@ def validate_plugin(self, name: str, module: types.ModuleType) -> None: raise InvalidPluginException( name, f"{attr} must be of type {attr_type.cls.__name__}." ) - - @property - @abstractmethod - def module_prefix(self) -> str: ... - - @abstractmethod - def load_plugin(self, name: str, module: types.ModuleType) -> TPlugin: - """Load a plugin by name.""" - ... - - @abstractmethod - def expected_attributes(self) -> Mapping[str, AttributeType]: ... diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..28ee3fb --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +# Make test dir a package to allow for relative imports diff --git a/tests/example_plugin.py b/tests/example_plugin.py new file mode 100644 index 0000000..8fbd5b7 --- /dev/null +++ b/tests/example_plugin.py @@ -0,0 +1,65 @@ +"""Define example plugin type for testing.""" + +from dataclasses import dataclass +from types import ModuleType +from pathlib import Path + +from snakemake_interface_common.plugin_registry import PluginRegistryBase +from snakemake_interface_common.plugin_registry.plugin import PluginBase, SettingsBase +from snakemake_interface_common.plugin_registry.attribute_types import ( + AttributeType, + AttributeMode, + AttributeKind, +) + + +@dataclass +class ExamplePlugin(PluginBase): + _name: str + _settings_cls: type[SettingsBase] | None + file: Path + string_attr: str + + @property + def name(self) -> str: + return self._name + + @property + def cli_prefix(self) -> str: + return "example-plugin-" + self.name + + @property + def settings_cls(self) -> type[SettingsBase] | None: + return self._settings_cls + + +class ExamplePluginRegistry(PluginRegistryBase[ExamplePlugin]): + + @property + def module_prefix(self) -> str: + return "snakemake_example_plugin_" + + def load_plugin(self, name: str, module: ModuleType) -> ExamplePlugin: + settings_cls = getattr(module, "ExampleSettings", None) + string_attr = module.example_string + + return ExamplePlugin( + _name=name, + _settings_cls=settings_cls, + file=Path(module.__file__), + string_attr=string_attr, + ) + + def expected_attributes(self): + return { + "ExampleSettings": AttributeType( + cls=SettingsBase, + mode=AttributeMode.OPTIONAL, + kind=AttributeKind.CLASS, + ), + "example_string": AttributeType( + cls=str, + mode=AttributeMode.REQUIRED, + kind=AttributeKind.OBJECT, + ), + } diff --git a/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py b/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py new file mode 100644 index 0000000..ab80c04 --- /dev/null +++ b/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py @@ -0,0 +1,7 @@ +"""Plugin module with ExampleSettings class that does not inherit from correct base class.""" + +example_string = "foo" + + +class ExampleSettings: + pass diff --git a/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py b/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py new file mode 100644 index 0000000..af33f94 --- /dev/null +++ b/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py @@ -0,0 +1,3 @@ +"""Plugin module with wrong type for "example_string" attribute.""" + +example_string = 1 diff --git a/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py b/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py new file mode 100644 index 0000000..baf8bcb --- /dev/null +++ b/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py @@ -0,0 +1 @@ +"""Plugin module missing required "example_string" attribute.""" diff --git a/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py b/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py new file mode 100644 index 0000000..fdd108d --- /dev/null +++ b/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py @@ -0,0 +1,10 @@ +"""Valid plugin with optional settings class.""" + +from snakemake_interface_common.plugin_registry.plugin import SettingsBase + + +example_string = "valid 1" + + +class ExampleSettings(SettingsBase): + pass # TODO: add attributes diff --git a/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py b/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py new file mode 100644 index 0000000..a3c3a9d --- /dev/null +++ b/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py @@ -0,0 +1,3 @@ +"""Valid plugin without optional settings class.""" + +example_string = "valid 2" diff --git a/tests/test_registry.py b/tests/test_registry.py new file mode 100644 index 0000000..7ff16ea --- /dev/null +++ b/tests/test_registry.py @@ -0,0 +1,107 @@ +"""Test the PluginRegistry class.""" + +from pathlib import Path + +import pytest + +from snakemake_interface_common.plugin_registry.plugin import SettingsBase +from snakemake_interface_common.exceptions import InvalidPluginException +from .example_plugin import ExamplePlugin, ExamplePluginRegistry + + +# Directory containing importable example plugins +PLUGIN_DIR = Path(__file__).parent / "plugins" + + +@pytest.fixture(autouse=True) +def _reset_example_registry_singleton(): + """Reset the singleton instance of ExamplePluginRegistry before/after all tests.""" + ExamplePluginRegistry._instance = None + yield + ExamplePluginRegistry._instance = None + + +def test_basic(): + """Test basic attributes and behavior.""" + + registry = ExamplePluginRegistry() + assert registry.get_plugin_type() == "example" + + # Check singleton + assert ExamplePluginRegistry() is registry + + # Check plugin not found + assert not registry.is_installed("foo") + with pytest.raises(InvalidPluginException): + registry.get_plugin("foo") + + +def test_discovery(monkeypatch: pytest.MonkeyPatch): + """Test plugin discovery and initialization.""" + + # Add directory of valid plugins to import path so they can be discovered by module name + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "valid")) + + registry = ExamplePluginRegistry() + + expected_plugins = {"valid-1", "valid-2"} + plugins: dict[str, ExamplePlugin] = {} + + # Check valid plugins + assert set(registry.get_registered_plugins()) == expected_plugins + + for name in expected_plugins: + assert registry.is_installed(name) + plugins[name] = registry.get_plugin(name) + assert isinstance(plugins[name], ExamplePlugin) + assert plugins[name].name == name + assert plugins[name].file.is_relative_to(PLUGIN_DIR) + + # Valid plugin 1 + assert plugins["valid-1"].string_attr == "valid 1" + assert plugins["valid-1"].settings_cls is not None + assert issubclass(plugins["valid-1"].settings_cls, SettingsBase) + + # Valid plugin 2 + assert plugins["valid-2"].string_attr == "valid 2" + assert plugins["valid-2"].settings_cls is None + + +def test_missing_attr(monkeypatch: pytest.MonkeyPatch): + """Test plugin with missing required attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "missing-attr")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry() + + errmsg = str(exc_info.value) + assert "missing-attr" in errmsg + assert "plugin does not define example_string" in errmsg + + +def test_invalid_object(monkeypatch: pytest.MonkeyPatch): + """Test plugin with invalid object attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "invalid-object")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry() + + errmsg = str(exc_info.value) + assert "invalid-object" in errmsg + assert "example_string must be of type str" in errmsg + + +def test_invalid_class(monkeypatch: pytest.MonkeyPatch): + """Test plugin with invalid class attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "invalid-class")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry() + + errmsg = str(exc_info.value) + assert "invalid-class" in errmsg + assert "ExampleSettings must be a subclass of" in errmsg + assert "SettingsBase" in errmsg diff --git a/tests/tests.py b/tests/tests.py index a6ad793..3fb8edb 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -4,7 +4,11 @@ from snakemake_interface_common import at_least_snakemake_version from snakemake_interface_common.exceptions import ApiError, WorkflowError -from snakemake_interface_common.plugin_registry.plugin import TaggedSettings, SettingsBase, PluginBase +from snakemake_interface_common.plugin_registry.plugin import ( + TaggedSettings, + SettingsBase, + PluginBase, +) from snakemake_interface_common.rules import RuleInterface from snakemake_interface_common.settings import SettingsEnumBase @@ -81,11 +85,15 @@ def test_snakemake_version(): @dataclass class TestSettings(SettingsBase): + __test__ = False + required_int: int = 42 optional_int: Optional[int] = None class TestPlugin(PluginBase[TestSettings]): + __test__ = False + @property def name(self) -> str: return "test_plugin"