From 9a074b262d71f90ae2a66408ec9485fdc4841df2 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 19:59:22 +0200 Subject: [PATCH] test(lint-skills): cover check_skill and check_command, not just settings The linter's main job - frontmatter keys, allowed-tools targets, the command title rule - had zero assertions; deleting the missing- allowed-tools error left the suite green. The fixture's yaml stub now parses the flat frontmatter the fixtures write instead of returning a canned mapping, and four new cases pin both check functions. Mutation-verified against the real linter. Review finding F23 (2026-08-19). Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++++ tests/test_lint_skills.py | 70 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99aa0fd..7099c93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ per-file diff commands. ### Added +- **Tests for `lint_skills.py`'s skill and command checks** - only `check_settings()` + had coverage; the linter's main job (frontmatter keys, `allowed-tools` targets + existing, the `# /` command title rule) was unasserted, so deleting the + missing-allowed-tools error survived the whole suite. Four new cases in + `tests/test_lint_skills.py`, with the fixture's yaml stub upgraded to parse the real + frontmatter. Mutation-verified. - **Discriminating tests for `robots_check`'s tie-break and browser-UA fallback** - the existing tie test put Disallow first, the one ordering that cannot detect deletion of the tie-break clause; and the browser-readback recovery that `09-web-research.md` diff --git a/tests/test_lint_skills.py b/tests/test_lint_skills.py index 640e4aa..a7f57b3 100644 --- a/tests/test_lint_skills.py +++ b/tests/test_lint_skills.py @@ -29,11 +29,19 @@ class LinterRepoFixture(unittest.TestCase): shutil.copy(LINTER_SCRIPT, tools / "lint_skills.py") # The Python-test CI job does not install PyYAML; the separate lint job # does. These settings-focused tests only need a valid frontmatter map. + # The stub parses simple "key: value" lines, enough for the flat + # frontmatter these fixtures write, so the checks under test see the + # actual file content instead of a canned mapping. (tools / "yaml.py").write_text( "class YAMLError(Exception):\n" " pass\n\n" - "def safe_load(_text):\n" - " return {'name': 'example', 'description': 'Example skill'}\n", + "def safe_load(text):\n" + " result = {}\n" + " for line in (text or '').splitlines():\n" + " if ':' in line:\n" + " key, _, value = line.partition(':')\n" + " result[key.strip()] = value.strip()\n" + " return result\n", encoding="utf-8", ) @@ -103,5 +111,63 @@ class SettingsShapeTests(LinterRepoFixture): self.assertEqual(result.returncode, 1) self.assertIn("expected permissions.allow to be a list", result.stdout) self.assertNotIn("Traceback", result.stderr) +class SkillAndCommandCheckTests(LinterRepoFixture): + """check_skill()/check_command() are the linter's main job and were + previously untested - only check_settings() had coverage, so deleting + e.g. the missing-allowed-tools error survived the whole suite (review + finding F23, 2026-08-19).""" + + def write_skill(self, frontmatter: str): + skill = self.root / ".claude" / "skills" / "example" / "SKILL.md" + skill.write_text(frontmatter, encoding="utf-8") + + def test_allowed_tools_referencing_a_missing_file_fails(self): + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/DOES_NOT_EXIST.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("allowed-tools references a missing file", result.stdout) + self.assertIn("DOES_NOT_EXIST.ts", result.stdout) + + def test_allowed_tools_referencing_an_existing_file_passes(self): + target = self.root / ".claude" / "skills" / "example" / "cli.ts" + target.write_text("// present\n", encoding="utf-8") + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/cli.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_frontmatter_missing_description_fails(self): + self.write_skill("---\nname: example\ndescription:\n---\n") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("missing required key 'description'", result.stdout) + + def test_command_without_slash_title_fails(self): + command = self.root / ".claude" / "commands" / "setup.md" + command.write_text("# setup - missing the slash\n", encoding="utf-8") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("must start with a '# /' title", result.stdout) + + if __name__ == "__main__": unittest.main()