chore(skills): clean up PR #862 — simplify manifest guard, DRY up tests
Follow-up to PR #862 (local skills classification by arceus77-7): - Remove unnecessary isinstance guard on _read_manifest() return value — it always returns Dict[str, str], so set() on it suffices. - Extract repeated hub-dir monkeypatching into a shared pytest fixture (hub_env). - Add three_source_env fixture for source-classification tests. - Add _read_manifest monkeypatch to test_do_list_initializes_hub_dir (was fragile — relied on empty skills list masking the real manifest). - Add test coverage for --source hub and --source builtin filters. - Extract _capture() helper to reduce console/StringIO boilerplate. 5 tests, all green.
This commit is contained in:
teknium1
@@ -416,8 +416,7 @@ def do_list(source_filter: str = "all", console: Optional[Console] = None) -> No
|
||||
ensure_hub_dirs()
|
||||
lock = HubLockFile()
|
||||
hub_installed = {e["name"]: e for e in lock.list_installed()}
|
||||
bundled_manifest = _read_manifest()
|
||||
builtin_names = set(bundled_manifest.keys()) if isinstance(bundled_manifest, dict) else set()
|
||||
builtin_names = set(_read_manifest())
|
||||
|
||||
all_skills = _find_all_skills()
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
from io import StringIO
|
||||
|
||||
import pytest
|
||||
from rich.console import Console
|
||||
|
||||
from hermes_cli.skills_hub import do_list
|
||||
@@ -13,9 +14,10 @@ class _DummyLockFile:
|
||||
return self._installed
|
||||
|
||||
|
||||
def test_do_list_initializes_hub_dir(monkeypatch, tmp_path):
|
||||
@pytest.fixture()
|
||||
def hub_env(monkeypatch, tmp_path):
|
||||
"""Set up isolated hub directory paths and return (monkeypatch, tmp_path)."""
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_tool as skills_tool
|
||||
|
||||
hub_dir = tmp_path / "skills" / ".hub"
|
||||
monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "skills")
|
||||
@@ -25,13 +27,63 @@ def test_do_list_initializes_hub_dir(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(hub, "AUDIT_LOG", hub_dir / "audit.log")
|
||||
monkeypatch.setattr(hub, "TAPS_FILE", hub_dir / "taps.json")
|
||||
monkeypatch.setattr(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache")
|
||||
|
||||
return hub_dir
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures for common skill setups
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_HUB_ENTRY = {"name": "hub-skill", "source": "github", "trust_level": "community"}
|
||||
|
||||
_ALL_THREE_SKILLS = [
|
||||
{"name": "hub-skill", "category": "x", "description": "hub"},
|
||||
{"name": "builtin-skill", "category": "x", "description": "builtin"},
|
||||
{"name": "local-skill", "category": "x", "description": "local"},
|
||||
]
|
||||
|
||||
_BUILTIN_MANIFEST = {"builtin-skill": "abc123"}
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def three_source_env(monkeypatch, hub_env):
|
||||
"""Populate hub/builtin/local skills for source-classification tests."""
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
|
||||
monkeypatch.setattr(hub, "HubLockFile", lambda: _DummyLockFile([_HUB_ENTRY]))
|
||||
monkeypatch.setattr(skills_tool, "_find_all_skills", lambda: list(_ALL_THREE_SKILLS))
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: dict(_BUILTIN_MANIFEST))
|
||||
|
||||
return hub_env
|
||||
|
||||
|
||||
def _capture(source_filter: str = "all") -> str:
|
||||
"""Run do_list into a string buffer and return the output."""
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
do_list(source_filter=source_filter, console=console)
|
||||
return sink.getvalue()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_do_list_initializes_hub_dir(monkeypatch, hub_env):
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
|
||||
monkeypatch.setattr(skills_tool, "_find_all_skills", lambda: [])
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: {})
|
||||
|
||||
console = Console(file=StringIO(), force_terminal=False, color_system=None)
|
||||
|
||||
hub_dir = hub_env
|
||||
assert not hub_dir.exists()
|
||||
|
||||
do_list(console=console)
|
||||
_capture()
|
||||
|
||||
assert hub_dir.exists()
|
||||
assert (hub_dir / "lock.json").exists()
|
||||
@@ -39,80 +91,34 @@ def test_do_list_initializes_hub_dir(monkeypatch, tmp_path):
|
||||
assert (hub_dir / "index-cache").is_dir()
|
||||
|
||||
|
||||
def test_do_list_distinguishes_hub_builtin_and_local(monkeypatch, tmp_path):
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
def test_do_list_distinguishes_hub_builtin_and_local(three_source_env):
|
||||
output = _capture()
|
||||
|
||||
hub_dir = tmp_path / "skills" / ".hub"
|
||||
monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "skills")
|
||||
monkeypatch.setattr(hub, "HUB_DIR", hub_dir)
|
||||
monkeypatch.setattr(hub, "LOCK_FILE", hub_dir / "lock.json")
|
||||
monkeypatch.setattr(hub, "QUARANTINE_DIR", hub_dir / "quarantine")
|
||||
monkeypatch.setattr(hub, "AUDIT_LOG", hub_dir / "audit.log")
|
||||
monkeypatch.setattr(hub, "TAPS_FILE", hub_dir / "taps.json")
|
||||
monkeypatch.setattr(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache")
|
||||
|
||||
monkeypatch.setattr(
|
||||
hub,
|
||||
"HubLockFile",
|
||||
lambda: _DummyLockFile([
|
||||
{"name": "hub-skill", "source": "github", "trust_level": "community"},
|
||||
]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
skills_tool,
|
||||
"_find_all_skills",
|
||||
lambda: [
|
||||
{"name": "hub-skill", "category": "x", "description": "hub"},
|
||||
{"name": "builtin-skill", "category": "x", "description": "builtin"},
|
||||
{"name": "local-skill", "category": "x", "description": "local"},
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: {"builtin-skill": "abc123"})
|
||||
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
|
||||
do_list(console=console)
|
||||
|
||||
output = sink.getvalue()
|
||||
assert "hub-skill" in output
|
||||
assert "builtin-skill" in output
|
||||
assert "local-skill" in output
|
||||
assert "1 hub-installed, 1 builtin, 1 local" in output
|
||||
|
||||
|
||||
def test_do_list_local_filter(monkeypatch, tmp_path):
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
def test_do_list_filter_local(three_source_env):
|
||||
output = _capture(source_filter="local")
|
||||
|
||||
hub_dir = tmp_path / "skills" / ".hub"
|
||||
monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "skills")
|
||||
monkeypatch.setattr(hub, "HUB_DIR", hub_dir)
|
||||
monkeypatch.setattr(hub, "LOCK_FILE", hub_dir / "lock.json")
|
||||
monkeypatch.setattr(hub, "QUARANTINE_DIR", hub_dir / "quarantine")
|
||||
monkeypatch.setattr(hub, "AUDIT_LOG", hub_dir / "audit.log")
|
||||
monkeypatch.setattr(hub, "TAPS_FILE", hub_dir / "taps.json")
|
||||
monkeypatch.setattr(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache")
|
||||
|
||||
monkeypatch.setattr(hub, "HubLockFile", lambda: _DummyLockFile([]))
|
||||
monkeypatch.setattr(
|
||||
skills_tool,
|
||||
"_find_all_skills",
|
||||
lambda: [
|
||||
{"name": "builtin-skill", "category": "x", "description": "builtin"},
|
||||
{"name": "local-skill", "category": "x", "description": "local"},
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: {"builtin-skill": "abc123"})
|
||||
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
|
||||
do_list(source_filter="local", console=console)
|
||||
|
||||
output = sink.getvalue()
|
||||
assert "local-skill" in output
|
||||
assert "builtin-skill" not in output
|
||||
assert "hub-skill" not in output
|
||||
|
||||
|
||||
def test_do_list_filter_hub(three_source_env):
|
||||
output = _capture(source_filter="hub")
|
||||
|
||||
assert "hub-skill" in output
|
||||
assert "builtin-skill" not in output
|
||||
assert "local-skill" not in output
|
||||
|
||||
|
||||
def test_do_list_filter_builtin(three_source_env):
|
||||
output = _capture(source_filter="builtin")
|
||||
|
||||
assert "builtin-skill" in output
|
||||
assert "hub-skill" not in output
|
||||
assert "local-skill" not in output
|
||||
|
||||
Reference in New Issue
Block a user