diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 60356385..8500465a 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -195,18 +195,36 @@ class PluginManager: continue manifest_path = item / "manifest.json" - if manifest_path.exists(): - try: - with open(manifest_path, 'r', encoding='utf-8') as f: - manifest = json.load(f) - plugin_id = manifest.get('id') - if plugin_id: - plugin_ids.append(plugin_id) - new_manifests[plugin_id] = manifest - new_directories[plugin_id] = item - except (json.JSONDecodeError, PermissionError, OSError) as e: - self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True) - continue + if not manifest_path.exists(): + # A directory here that carries no manifest is not a + # plugin. Said once, because the alternative is a plugin + # that is enabled in config, enabled in plugin state, + # present on disk, and simply absent from the running + # process with nothing anywhere to say why. Working that + # out afterwards means reading cache-file mtimes. + self.logger.warning( + "Skipping %s: no manifest.json, so it cannot be loaded " + "as a plugin", item.name) + continue + try: + with open(manifest_path, 'r', encoding='utf-8') as f: + manifest = json.load(f) + except (json.JSONDecodeError, PermissionError, OSError) as e: + self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True) + continue + + plugin_id = manifest.get('id') + if not plugin_id: + # Parsed but unusable. This was the quietest path of all: + # the manifest is read successfully and then dropped. + self.logger.warning( + "Skipping %s: its manifest.json has no \"id\", so there " + "is nothing to register it under", item.name) + continue + + plugin_ids.append(plugin_id) + new_manifests[plugin_id] = manifest + new_directories[plugin_id] = item except (OSError, PermissionError) as e: self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True) diff --git a/test/test_plugin_discovery_reports_skips.py b/test/test_plugin_discovery_reports_skips.py new file mode 100644 index 00000000..080c780f --- /dev/null +++ b/test/test_plugin_discovery_reports_skips.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +"""Discovery must say when it skips a directory. + +A plugin can be enabled in config, enabled in plugin state, present on disk +with a valid entry point -- and simply absent from the running process, with +nothing in the journal to say why. Working that out afterwards meant comparing +cache-file mtimes to find when it had last run. + +Two paths were silent. A directory with no manifest.json was ignored, and -- +quieter still -- a manifest that parsed but carried no "id" was read +successfully and then dropped on the floor. +""" +import json +import logging +import sys +from pathlib import Path +from unittest.mock import MagicMock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from src.plugin_system.plugin_manager import PluginManager # noqa: E402 + + +def _manager(tmp_path): + pm = PluginManager.__new__(PluginManager) + pm.plugins_dir = tmp_path + pm.logger = logging.getLogger("test.discovery") + pm.plugin_manifests = {} + pm.plugin_directories = {} + pm._discovery_lock = __import__("threading").RLock() + pm.schema_manager = MagicMock() + return pm + + +def test_a_directory_without_a_manifest_is_reported(tmp_path, caplog): + (tmp_path / "not-a-plugin").mkdir() + pm = _manager(tmp_path) + with caplog.at_level(logging.WARNING, logger="test.discovery"): + pm._scan_directory_for_plugins(tmp_path) + joined = " ".join(r.message for r in caplog.records) + assert "not-a-plugin" in joined and "manifest" in joined, ( + f"skip was silent; log said: {joined!r}") + + +def test_a_manifest_without_an_id_is_reported(tmp_path, caplog): + d = tmp_path / "idless" + d.mkdir() + (d / "manifest.json").write_text(json.dumps({"name": "No Id", "version": "1.0.0"})) + pm = _manager(tmp_path) + with caplog.at_level(logging.WARNING, logger="test.discovery"): + pm._scan_directory_for_plugins(tmp_path) + joined = " ".join(r.message for r in caplog.records) + assert "idless" in joined and "id" in joined, ( + f"a parsed-but-unusable manifest vanished silently; log said: {joined!r}") + + +def test_a_good_plugin_still_registers(tmp_path, caplog): + d = tmp_path / "real-plugin" + d.mkdir() + (d / "manifest.json").write_text(json.dumps( + {"id": "real-plugin", "name": "Real", "version": "1.0.0"})) + pm = _manager(tmp_path) + pm._scan_directory_for_plugins(tmp_path) + assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"