Trevin Chow
73dc6b9996
The same `^version:\s*"([^"]+)"\s*$` regex (or a slight variant) was
duplicated across three files: render.py inline, test_plugin_contract.py
local helper, test_version_consistency.py local helper. A future change
to the SKILL.md frontmatter version format would have needed to update
three places without any compile-time pressure to keep them in sync.
New skills/last30days/scripts/lib/skill_meta.py provides:
- `_VERSION_RE` private compiled pattern (accepts double-quoted,
single-quoted, or unquoted YAML version scalars per the widening
landed in 997708a)
- `read_skill_version(skill_md_path: Path) -> str | None` helper that
catches OSError + UnicodeDecodeError and returns None on miss
Callers updated:
- render.py::_skill_version now calls skill_meta.read_skill_version
inside the SKILL.md fallback loop, returning `read_skill_version(...) or "?"`.
Semantically equivalent to the old break-after-first-SKILL.md logic.
- test_plugin_contract.py and test_version_consistency.py import the
helper instead of defining the regex inline. Both files use the
established sys.path.insert pattern.
Added tests/test_skill_meta.py with 6 direct unit tests covering the
helper's full contract: missing file, undecodable bytes, no-version-line,
and all three quoting styles (double, single, unquoted). Previously the
helper was only exercised transitively through render._skill_version().
Added test_skill_md_uses_double_quoted_version to
test_version_consistency.py — the old per-test regex incidentally
asserted "this repo's SKILL.md uses double-quotes" by being strict;
the shared helper accepts all three styles, so the assertion is now
explicit instead of implicit.
Code-reviewed by ce-code-review (8 reviewers); safe_auto fixes applied
inline (rename to _VERSION_RE, group or-chain instead of generator,
docstring tightened, dropped unnecessary `from __future__ import
annotations`, tightened signature to Path-only).
Conftest.py refactor for the sys.path.insert duplication across ~20 test
files filed as issue #411 — out of scope for this PR (touches many
files, separate concern).
Test results: 23 passed in the affected test set (16 prior + 6 new
test_skill_meta tests + 1 new double-quote assertion). Full suite shows
same 13 pre-existing failures as main; zero new failures.
83 lines
3.3 KiB
Python
83 lines
3.3 KiB
Python
import re
|
|
import sys
|
|
import unittest
|
|
from pathlib import Path
|
|
|
|
|
|
ROOT = Path(__file__).resolve().parents[1]
|
|
SKILL_ROOT = ROOT / "skills" / "last30days"
|
|
|
|
sys.path.insert(0, str(SKILL_ROOT / "scripts"))
|
|
from lib.skill_meta import read_skill_version # noqa: E402
|
|
|
|
|
|
def _skill_version() -> str:
|
|
version = read_skill_version(SKILL_ROOT / "SKILL.md")
|
|
if not version:
|
|
raise AssertionError("SKILL.md version frontmatter not found")
|
|
return version
|
|
|
|
|
|
class TestVersionConsistency(unittest.TestCase):
|
|
def test_skill_md_uses_double_quoted_version(self) -> None:
|
|
# The shared VERSION_RE in skill_meta.py accepts double-quoted,
|
|
# single-quoted, and unquoted YAML version scalars. This repo's
|
|
# SKILL.md must use the double-quoted form so the badge string stays
|
|
# deterministic and contributors don't accidentally introduce a
|
|
# quoting style that's harder for downstream tooling to parse.
|
|
text = (SKILL_ROOT / "SKILL.md").read_text(encoding="utf-8")
|
|
self.assertRegex(
|
|
text,
|
|
re.compile(r'^version:\s*"[^"]+"\s*$', re.MULTILINE),
|
|
msg="SKILL.md frontmatter version must use double-quoted form",
|
|
)
|
|
|
|
def test_root_skill_header_matches_frontmatter_version(self) -> None:
|
|
text = (SKILL_ROOT / "SKILL.md").read_text(encoding="utf-8")
|
|
version = _skill_version()
|
|
self.assertIn(f"# last30days v{version}:", text)
|
|
|
|
def test_memory_save_dir_uses_single_env_variable(self) -> None:
|
|
skill_text = (SKILL_ROOT / "SKILL.md").read_text(encoding="utf-8")
|
|
compare_text = (SKILL_ROOT / "scripts" / "compare.sh").read_text(encoding="utf-8")
|
|
default_assignment = 'LAST30DAYS_MEMORY_DIR="${LAST30DAYS_MEMORY_DIR:-$HOME/Documents/Last30Days}"'
|
|
|
|
self.assertIn(default_assignment, skill_text)
|
|
self.assertIn(default_assignment, compare_text)
|
|
self.assertNotIn("--save-dir=~/Documents/Last30Days", skill_text)
|
|
self.assertIn('--save-dir="${LAST30DAYS_MEMORY_DIR}"', skill_text)
|
|
|
|
def test_no_stray_hardcoded_memory_dir_paths(self) -> None:
|
|
allowed_suffixes = {".md", ".py", ".sh", ".txt", ".yml", ".yaml", ".json"}
|
|
skip_dirs = {".git", "assets", "fixtures", "docs"}
|
|
offenders = []
|
|
|
|
for path in ROOT.rglob("*"):
|
|
if not path.is_file() or path.suffix not in allowed_suffixes:
|
|
continue
|
|
if skip_dirs.intersection(path.relative_to(ROOT).parts):
|
|
continue
|
|
if path.relative_to(ROOT) == Path("tests/test_version_consistency.py"):
|
|
continue
|
|
|
|
try:
|
|
lines = path.read_text(encoding="utf-8").splitlines()
|
|
except UnicodeDecodeError:
|
|
continue
|
|
|
|
for line_number, line in enumerate(lines, start=1):
|
|
if "~/Documents/Last30Days" not in line and "$HOME/Documents/Last30Days" not in line:
|
|
continue
|
|
allowed_default = (
|
|
"LAST30DAYS_MEMORY_DIR" in line
|
|
and ("defaults to" in line or "${LAST30DAYS_MEMORY_DIR:-$HOME/Documents/Last30Days}" in line)
|
|
)
|
|
if not allowed_default:
|
|
offenders.append(f"{path.relative_to(ROOT)}:{line_number}: {line.strip()}")
|
|
|
|
self.assertEqual([], offenders)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|