153 lines
5.8 KiB
Python
153 lines
5.8 KiB
Python
"""
|
|
Tests for improved error messages in skill_manager_tool (issue #624).
|
|
Verifies that error messages include file paths, context, and suggestions.
|
|
"""
|
|
|
|
import pytest
|
|
from pathlib import Path
|
|
from unittest.mock import patch, MagicMock
|
|
from tools.skill_manager_tool import _format_error, _edit_skill, _patch_skill, skill_manage
|
|
|
|
|
|
class TestFormatError:
|
|
"""Test the _format_error helper function."""
|
|
|
|
def test_basic_error(self):
|
|
"""Test basic error formatting."""
|
|
result = _format_error("Something went wrong")
|
|
assert result["success"] is False
|
|
assert "Something went wrong" in result["error"]
|
|
assert result["skill_name"] is None
|
|
assert result["file_path"] is None
|
|
|
|
def test_with_skill_name(self):
|
|
"""Test error with skill name."""
|
|
result = _format_error("Failed", skill_name="test-skill")
|
|
assert "test-skill" in result["error"]
|
|
assert result["skill_name"] == "test-skill"
|
|
|
|
def test_with_file_path(self):
|
|
"""Test error with file path."""
|
|
result = _format_error("Failed", file_path="/path/to/SKILL.md")
|
|
assert "/path/to/SKILL.md" in result["error"]
|
|
assert result["file_path"] == "/path/to/SKILL.md"
|
|
|
|
def test_with_suggestion(self):
|
|
"""Test error with suggestion."""
|
|
result = _format_error("Failed", suggestion="Try again")
|
|
assert "Suggestion: Try again" in result["error"]
|
|
assert result["suggestion"] == "Try again"
|
|
|
|
def test_with_context(self):
|
|
"""Test error with context dict."""
|
|
result = _format_error("Failed", context={"line": 5, "found": "x"})
|
|
assert "line: 5" in result["error"]
|
|
assert "found: x" in result["error"]
|
|
|
|
def test_all_fields(self):
|
|
"""Test error with all fields."""
|
|
result = _format_error(
|
|
"Pattern match failed",
|
|
skill_name="my-skill",
|
|
file_path="/skills/my-skill/SKILL.md",
|
|
suggestion="Check whitespace",
|
|
context={"expected": "foo", "found": "bar"}
|
|
)
|
|
assert "Pattern match failed" in result["error"]
|
|
assert "Skill: my-skill" in result["error"]
|
|
assert "File: /skills/my-skill/SKILL.md" in result["error"]
|
|
assert "Suggestion: Check whitespace" in result["error"]
|
|
assert "expected: foo" in result["error"]
|
|
|
|
|
|
class TestEditSkillErrors:
|
|
"""Test improved error messages in _edit_skill."""
|
|
|
|
@patch('tools.skill_manager_tool._find_skill')
|
|
def test_skill_not_found(self, mock_find):
|
|
"""Test skill not found error includes suggestion."""
|
|
mock_find.return_value = None
|
|
# Provide valid content with frontmatter so it passes validation
|
|
valid_content = """---
|
|
name: test
|
|
description: Test skill
|
|
---
|
|
Body content here.
|
|
"""
|
|
result = _edit_skill("nonexistent", valid_content)
|
|
assert result["success"] is False
|
|
assert "nonexistent" in result["error"]
|
|
assert "skills_list()" in result.get("suggestion", "")
|
|
|
|
@patch('tools.skill_manager_tool._find_skill')
|
|
def test_yaml_parse_error_includes_file_path_and_line_number(self, mock_find, tmp_path):
|
|
"""Invalid YAML should report target file path and parser line information."""
|
|
skill_dir = tmp_path / "my-skill"
|
|
skill_dir.mkdir()
|
|
(skill_dir / "SKILL.md").write_text("old", encoding="utf-8")
|
|
mock_find.return_value = {"path": skill_dir}
|
|
|
|
bad_content = """---
|
|
name: my-skill
|
|
description: [broken
|
|
---
|
|
Body.
|
|
"""
|
|
result = _edit_skill("my-skill", bad_content)
|
|
assert result["success"] is False
|
|
assert str(skill_dir / "SKILL.md") in result["error"]
|
|
assert "line" in result["error"].lower()
|
|
|
|
|
|
class TestPatchSkillErrors:
|
|
"""Test improved error messages in _patch_skill."""
|
|
|
|
def test_old_string_required(self):
|
|
"""Test old_string required error includes suggestion."""
|
|
result = _patch_skill("test-skill", None, "new")
|
|
assert result["success"] is False
|
|
assert "old_string is required" in result["error"]
|
|
assert "suggestion" in result
|
|
|
|
def test_new_string_required(self):
|
|
"""Test new_string required error includes suggestion."""
|
|
result = _patch_skill("test-skill", "old", None)
|
|
assert result["success"] is False
|
|
assert "new_string is required" in result["error"]
|
|
assert "suggestion" in result
|
|
|
|
@patch('tools.skill_manager_tool._find_skill')
|
|
def test_skill_not_found(self, mock_find):
|
|
"""Test skill not found error includes suggestion."""
|
|
mock_find.return_value = None
|
|
result = _patch_skill("nonexistent", "old", "new")
|
|
assert result["success"] is False
|
|
assert "nonexistent" in result["error"]
|
|
assert "skills_list()" in result.get("suggestion", "")
|
|
|
|
@patch('tools.skill_manager_tool._find_skill')
|
|
def test_pattern_match_error_includes_state_info(self, mock_find, tmp_path):
|
|
"""Patch failures should include file path and patch state info."""
|
|
skill_dir = tmp_path / "state-skill"
|
|
skill_dir.mkdir()
|
|
target = skill_dir / "SKILL.md"
|
|
target.write_text("---\nname: state-skill\ndescription: desc\n---\n\nBody content here.\n", encoding="utf-8")
|
|
mock_find.return_value = {"path": skill_dir}
|
|
|
|
result = _patch_skill("state-skill", "missing pattern", "new text", replace_all=False)
|
|
assert result["success"] is False
|
|
assert str(target) in result["error"]
|
|
assert "replace_all" in result["error"]
|
|
assert "False" in result["error"]
|
|
|
|
|
|
class TestSkillManageEntryPoint:
|
|
def test_patch_missing_old_string_returns_json_error(self):
|
|
result = skill_manage(action="patch", name="demo-skill", old_string="", new_string="x")
|
|
assert isinstance(result, str)
|
|
assert "old_string is required" in result
|
|
|
|
|
|
if __name__ == "__main__":
|
|
pytest.main([__file__, "-v"])
|