From 50f50d7654a030affbcd41b97e180db6d66a1782 Mon Sep 17 00:00:00 2001 From: Andrew Barnes Date: Fri, 27 Mar 2026 08:20:31 -0400 Subject: [PATCH] test: add unit tests for skill frontmatter validation (#1309) * test: add unit tests for skill frontmatter validation Cover _validate_skill_frontmatter logic: - Valid minimal and full-field skills - Missing SKILL.md, missing frontmatter, invalid YAML - Required field validation (name, description) - Unexpected key rejection - Name format: hyphen-case, no leading/trailing/consecutive hyphens - Name and description length limits - Angle bracket rejection in description * test: fix unused variables flagged by ruff F841 Replace unused tuple elements with _ and add assertions on msg/name return values in success-path tests. * test: address review feedback on unused variables * test: consolidate validation tests into single module Move the UTF-8/windows-locale test from test_skills_router.py into test_skills_validation.py and remove test_skills_router.py to eliminate duplicated assertions and future maintenance drift. * fix: match assertion strings to actual validation messages --------- Co-authored-by: Willem Jiang --- backend/tests/test_skills_router.py | 88 ------------ backend/tests/test_skills_validation.py | 180 ++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 88 deletions(-) delete mode 100644 backend/tests/test_skills_router.py create mode 100644 backend/tests/test_skills_validation.py diff --git a/backend/tests/test_skills_router.py b/backend/tests/test_skills_router.py deleted file mode 100644 index 470ad69..0000000 --- a/backend/tests/test_skills_router.py +++ /dev/null @@ -1,88 +0,0 @@ -from collections.abc import Callable -from pathlib import Path -from typing import cast - -from deerflow.skills.validation import _validate_skill_frontmatter - -VALIDATE_SKILL_FRONTMATTER = cast( - Callable[[Path], tuple[bool, str, str | None]], - _validate_skill_frontmatter, -) - - -def _write_skill(skill_dir: Path, frontmatter: str) -> None: - skill_dir.mkdir(parents=True, exist_ok=True) - (skill_dir / "SKILL.md").write_text(frontmatter, encoding="utf-8") - - -def test_validate_skill_frontmatter_allows_standard_optional_metadata(tmp_path: Path) -> None: - skill_dir = tmp_path / "demo-skill" - _write_skill( - skill_dir, - """--- -name: demo-skill -description: Demo skill -version: 1.0.0 -author: example.com/demo -compatibility: OpenClaw >= 1.0 -license: MIT ---- - -# Demo Skill -""", - ) - - valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir) - - assert valid is True - assert message == "Skill is valid!" - assert skill_name == "demo-skill" - - -def test_validate_skill_frontmatter_still_rejects_unknown_keys(tmp_path: Path) -> None: - skill_dir = tmp_path / "demo-skill" - _write_skill( - skill_dir, - """--- -name: demo-skill -description: Demo skill -unsupported: true ---- - -# Demo Skill -""", - ) - - valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir) - - assert valid is False - assert "unsupported" in message - assert skill_name is None - - -def test_validate_skill_frontmatter_reads_utf8_on_windows_locale(tmp_path, monkeypatch) -> None: - skill_dir = tmp_path / "demo-skill" - _write_skill( - skill_dir, - """--- -name: demo-skill -description: "Curly quotes: \u201cutf8\u201d" ---- - -# Demo Skill -""", - ) - - original_read_text = Path.read_text - - def read_text_with_gbk_default(self, *args, **kwargs): - kwargs.setdefault("encoding", "gbk") - return original_read_text(self, *args, **kwargs) - - monkeypatch.setattr(Path, "read_text", read_text_with_gbk_default) - - valid, message, skill_name = VALIDATE_SKILL_FRONTMATTER(skill_dir) - - assert valid is True - assert message == "Skill is valid!" - assert skill_name == "demo-skill" diff --git a/backend/tests/test_skills_validation.py b/backend/tests/test_skills_validation.py new file mode 100644 index 0000000..fc0be18 --- /dev/null +++ b/backend/tests/test_skills_validation.py @@ -0,0 +1,180 @@ +"""Tests for skill frontmatter validation. + +Consolidates all _validate_skill_frontmatter tests (previously split across +test_skills_router.py and this module) into a single dedicated module. +""" + +from pathlib import Path + +from deerflow.skills.validation import ALLOWED_FRONTMATTER_PROPERTIES, _validate_skill_frontmatter + + +def _write_skill(tmp_path: Path, content: str) -> Path: + """Write a SKILL.md file and return its parent directory.""" + skill_file = tmp_path / "SKILL.md" + skill_file.write_text(content, encoding="utf-8") + return tmp_path + + +class TestValidateSkillFrontmatter: + def test_valid_minimal_skill(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill\ndescription: A valid skill\n---\n\nBody\n", + ) + valid, msg, name = _validate_skill_frontmatter(skill_dir) + assert valid is True + assert msg == "Skill is valid!" + assert name == "my-skill" + + def test_valid_with_all_allowed_fields(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill\ndescription: A skill\nlicense: MIT\nversion: '1.0'\nauthor: test\n---\n\nBody\n", + ) + valid, msg, name = _validate_skill_frontmatter(skill_dir) + assert valid is True + assert msg == "Skill is valid!" + assert name == "my-skill" + + def test_missing_skill_md(self, tmp_path): + valid, msg, name = _validate_skill_frontmatter(tmp_path) + assert valid is False + assert "not found" in msg + assert name is None + + def test_no_frontmatter(self, tmp_path): + skill_dir = _write_skill(tmp_path, "# Just markdown\n\nNo front matter.\n") + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "frontmatter" in msg.lower() + + def test_invalid_yaml(self, tmp_path): + skill_dir = _write_skill(tmp_path, "---\n[invalid yaml: {{\n---\n\nBody\n") + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "YAML" in msg + + def test_missing_name(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\ndescription: A skill without a name\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "name" in msg.lower() + + def test_missing_description(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "description" in msg.lower() + + def test_unexpected_keys_rejected(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill\ndescription: test\ncustom-field: bad\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "custom-field" in msg + + def test_name_must_be_hyphen_case(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: MySkill\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "hyphen-case" in msg + + def test_name_no_leading_hyphen(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: -my-skill\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "hyphen" in msg + + def test_name_no_trailing_hyphen(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill-\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "hyphen" in msg + + def test_name_no_consecutive_hyphens(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my--skill\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "hyphen" in msg + + def test_name_too_long(self, tmp_path): + long_name = "a" * 65 + skill_dir = _write_skill( + tmp_path, + f"---\nname: {long_name}\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "too long" in msg.lower() + + def test_description_no_angle_brackets(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: my-skill\ndescription: Has tags\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "angle brackets" in msg.lower() + + def test_description_too_long(self, tmp_path): + long_desc = "a" * 1025 + skill_dir = _write_skill( + tmp_path, + f"---\nname: my-skill\ndescription: {long_desc}\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "too long" in msg.lower() + + def test_empty_name_rejected(self, tmp_path): + skill_dir = _write_skill( + tmp_path, + "---\nname: ''\ndescription: test\n---\n\nBody\n", + ) + valid, msg, _ = _validate_skill_frontmatter(skill_dir) + assert valid is False + assert "empty" in msg.lower() + + def test_allowed_properties_constant(self): + assert "name" in ALLOWED_FRONTMATTER_PROPERTIES + assert "description" in ALLOWED_FRONTMATTER_PROPERTIES + assert "license" in ALLOWED_FRONTMATTER_PROPERTIES + + def test_reads_utf8_on_windows_locale(self, tmp_path, monkeypatch): + skill_dir = _write_skill( + tmp_path, + '---\nname: demo-skill\ndescription: "Curly quotes: \u201cutf8\u201d"\n---\n\n# Demo Skill\n', + ) + original_read_text = Path.read_text + + def read_text_with_gbk_default(self, *args, **kwargs): + kwargs.setdefault("encoding", "gbk") + return original_read_text(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text_with_gbk_default) + + valid, msg, name = _validate_skill_frontmatter(skill_dir) + assert valid is True + assert msg == "Skill is valid!" + assert name == "demo-skill"