From 7166a0e1cbf24885dd2840edcc5c598f5ea1b40d Mon Sep 17 00:00:00 2001 From: Joseph Yaksich <294273268+gitcommit90@users.noreply.github.com> Date: Sat, 20 Jun 2026 09:09:08 +0000 Subject: [PATCH] Add skill maintainer test layout coverage Refs #86 --- CHANGELOG.md | 3 + docs/TESTING.md | 11 +++ .../compliance}/test_mica_module.py | 0 tests/skills/dev_tools/test_issue_resolver.py | 71 +++++++++++++++++++ 4 files changed, 85 insertions(+) rename tests/{ => skills/compliance}/test_mica_module.py (100%) create mode 100644 tests/skills/dev_tools/test_issue_resolver.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 85f0b98..eee155e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Contributors add user-facing entries under `[Unreleased]` in the same PR. Mainta ## [Unreleased] +### Changed +- **Tests**: Standardized skill maintainer tests under `tests/skills//`, added loader-path coverage for `dev_tools/issue_resolver`, and documented the two-tier skill test convention (#86). + ### Fixed - **`finance/wallet_screening`**: Align examples and docs with `finance/wallet_screening` manifest tool name; fix `gemini_wallet_check.py` and `claude_wallet_check.py` to match tool name dynamically from manifest; correct `card.json` UI fields to match actual skill output schema; update `instructions.md`, provider snippets, Data Schema, and usage docs (#173). diff --git a/docs/TESTING.md b/docs/TESTING.md index 5660a84..a1152f4 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -64,6 +64,17 @@ pip install -r requirements.txt **Rule of thumb:** if it ships with the skill and must pass before merge → **bundle test** (CI + local). If it is extra regression depth for clone-repo work → **maintainer test** (optional). If it proves provider integration → **example**, not pytest. +## Two-tier skill test convention + +Skills use a two-tier layout inside the broader test model above. Use these tiers when adding or moving skill tests so future contributors can find coverage quickly. + +| Tier | Location | Purpose | Rules | +| :--- | :--- | :--- | :--- | +| **Tier 1 — skill-local bundle test** | `skills///test_skill.py` | Self-contained contract tests for the skill bundle. | Import the skill class directly, verify manifest integrity, validate inputs, and exercise deterministic output schema. Keep tests offline; mock external calls and avoid live APIs. | +| **Tier 2 — maintainer integration test** | `tests/skills//test_.py` | Clone-only regression depth through the framework loader path. | Load the skill with `SkillLoader.load_skill("/")`, use shared `tests/conftest.py` fixtures when useful, and mock providers or external services. | + +Keep top-level `tests/test_*.py` files for framework behavior only: loader, CLI, issuer rules, version policy, and other cross-cutting checks. Skill-specific integration tests belong under `tests/skills//`. + ## 1. Code Formatting (Black) We use **Black** as our uncompromising code formatter. It ensures that all code looks the same, regardless of who wrote it, eliminating discussions about style. diff --git a/tests/test_mica_module.py b/tests/skills/compliance/test_mica_module.py similarity index 100% rename from tests/test_mica_module.py rename to tests/skills/compliance/test_mica_module.py diff --git a/tests/skills/dev_tools/test_issue_resolver.py b/tests/skills/dev_tools/test_issue_resolver.py new file mode 100644 index 0000000..ff559bf --- /dev/null +++ b/tests/skills/dev_tools/test_issue_resolver.py @@ -0,0 +1,71 @@ +from skillware.core.loader import SkillLoader + +VALID_ISSUE_URL = "https://github.com/ARPAHLS/skillware/issues/56" + + +def get_skill(): + bundle = SkillLoader.load_skill("dev_tools/issue_resolver") + return bundle, bundle["module"].IssueResolverSkill() + + +def test_issue_resolver_manifest_loads(): + bundle, _ = get_skill() + manifest = bundle["manifest"] + + assert manifest["name"] == "dev_tools/issue_resolver" + assert "issue_url" in manifest["parameters"]["properties"] + assert "action" in manifest["parameters"]["properties"] + + +def test_issue_resolver_prepare_uses_loader_path(): + _, skill = get_skill() + + result = skill.execute({"issue_url": VALID_ISSUE_URL}) + + assert result["status"] == "ready" + assert result["action"] == "prepare" + assert result["issue"] == { + "url": VALID_ISSUE_URL, + "api_url": "https://api.github.com/repos/ARPAHLS/skillware/issues/56", + "owner": "ARPAHLS", + "repo": "skillware", + "number": "56", + } + assert result["repository"]["html_url"] == "https://github.com/ARPAHLS/skillware" + + +def test_issue_resolver_workflow_overview_uses_loader_path(): + _, skill = get_skill() + + result = skill.execute({"action": "workflow_overview"}) + + assert result["status"] == "ready" + assert result["workflow_version"] == "0.2" + assert result["stage_order"][0] == "discover_issue" + assert result["stage_order"][-1] == "pull_request" + + +def test_issue_resolver_stage_checklist_uses_loader_path(): + _, skill = get_skill() + + result = skill.execute({"action": "stage_checklist", "stage": "verify"}) + + assert result["status"] == "ready" + assert result["stage"] == "verify" + assert result["next_stage"] == "pre_commit" + assert any("Run verification commands" in step for step in result["steps"]) + + +def test_issue_resolver_rejects_ai_coauthor_via_loader_path(): + _, skill = get_skill() + + result = skill.execute( + { + "action": "validate_commit_message", + "message": "Fix issue workflow\n\nCo-authored-by: Claude ", + } + ) + + assert result["status"] == "rejected" + assert result["ok"] is False + assert result["violations"]