From 288c2856758d5ca78b158627ae47949868e0ced7 Mon Sep 17 00:00:00 2001 From: gpBlockchain <744158715@qq.com> Date: Tue, 1 Sep 2026 16:07:47 +0800 Subject: [PATCH] Centralize reviews and suite-owned automation --- SKILL.md | 6 +- assets/repo-tests/root/AGENTS.md | 2 +- assets/repo-tests/root/README.md | 3 +- assets/repo-tests/suite/AGENTS.md | 6 +- assets/repo-tests/suite/README.md | 4 +- assets/repo-tests/suite/reviews/README.md | 3 - references/initialize.md | 11 +-- references/review-cases.md | 4 +- scripts/check_test_map.py | 7 +- scripts/init_repo_tests.py | 104 +++++++--------------- tests/test_project_tools.py | 73 +++++++++++++-- 11 files changed, 120 insertions(+), 103 deletions(-) delete mode 100644 assets/repo-tests/suite/reviews/README.md diff --git a/SKILL.md b/SKILL.md index bc8b83e..0de2cf3 100644 --- a/SKILL.md +++ b/SKILL.md @@ -7,8 +7,8 @@ description: Maintain reviewer-first standalone test projects that use concise c Use two sources of truth: -- `reviews/` contains behavior a human can judge. -- Executable tests contain a nearby `TEST-MAP: ` comment. +- Root `reviews/` centrally contains behavior a human can judge across every module. +- Executable tests live under `suites//` and contain a nearby `TEST-MAP: ` comment. Do not create coverage ledgers, approval statuses, or internal ID chains. @@ -22,7 +22,7 @@ Keep each invocation bounded: Keep context small: -- Read the nearest `AGENTS.md`, affected review document, feedback file, mapped tests, and only the source or diff needed for the current behavior. +- Read the nearest `AGENTS.md`, affected root review document, root feedback file, mapped suite tests, and only the source or diff needed for the current behavior. - Locate with `rg` or equivalent before reading targeted ranges. Avoid repository-wide file dumps and repeated reads of unchanged instructions. - Batch related reads and checks. - After context compaction, recover from current files, `git diff`, and the affected document rather than replaying the whole repository. diff --git a/assets/repo-tests/root/AGENTS.md b/assets/repo-tests/root/AGENTS.md index e34294e..0b2baca 100644 --- a/assets/repo-tests/root/AGENTS.md +++ b/assets/repo-tests/root/AGENTS.md @@ -44,7 +44,7 @@ Split larger scopes coherently instead of omitting behavior. Group related field ## Feedback and mapping -Read the nearest `reviews/review-feedback.md` before revising cases. On corrective feedback, append: +Read root `reviews/review-feedback.md` before revising cases. On corrective feedback, append: ```text - model: | cases: | feedback: diff --git a/assets/repo-tests/root/README.md b/assets/repo-tests/root/README.md index 249b79e..e392d9e 100644 --- a/assets/repo-tests/root/README.md +++ b/assets/repo-tests/root/README.md @@ -7,7 +7,8 @@ This repository stores human-reviewed test intent and automation mapped by `TEST {{SUITE_LIST}} - `source/{{PROJECT_SLUG}}/`: ignored product checkout. -- `reviews/`: concise behavior tables. +- `reviews/`: centralized behavior tables for every suite. +- `suites//`: module-owned executable automation, commands, and fixtures. - `scripts/check_test_map.py`: computed mapping coverage. Work on one review document at a time: present changed rows, stop for confirmation, implement confirmed cases, then run a focused test and the mapping checker. Stable commands follow. diff --git a/assets/repo-tests/suite/AGENTS.md b/assets/repo-tests/suite/AGENTS.md index 0ce4d10..6da6c78 100644 --- a/assets/repo-tests/suite/AGENTS.md +++ b/assets/repo-tests/suite/AGENTS.md @@ -3,9 +3,9 @@ Read root `../../AGENTS.md`; this file only routes suite-specific work. - Focus: {{SUITE_FOCUS}}. -- Reviews: `reviews/`; executable areas: {{CODE_DIRS}}; reusable inputs: `fixtures/`. -- Read `reviews/review-feedback.md` when present. +- Reviews: root `../../reviews/`; executable areas: {{CODE_DIRS}}; reusable inputs: `fixtures/`. +- Read root `../../reviews/review-feedback.md` when present. - Keep case IDs globally unique, map tests with `TEST-MAP: `, and mirror that mapping with `- [x]`/`- [ ]` at the start of the scenario cell. - After confirmation, implement direct, readable tests; add an abstraction only when it improves readability without hiding behavior or assertions that prove the expected behavior. -- Verify with the focused suite command and `python3 ../../scripts/check_test_map.py --root ../..`. +- Verify with the focused suite command and `python3 ../../scripts/check_test_map.py`. - Do not advance to another review document automatically or poll CI repeatedly. diff --git a/assets/repo-tests/suite/README.md b/assets/repo-tests/suite/README.md index 3f9f93a..0790389 100644 --- a/assets/repo-tests/suite/README.md +++ b/assets/repo-tests/suite/README.md @@ -4,7 +4,7 @@ Focus: {{SUITE_FOCUS}}. ## Layout -- `reviews/`: concise reviewer-facing case tables. +- `../../reviews/`: centralized reviewer-facing case tables for every suite. - {{CODE_DIRS}}: executable tests or targets. - `fixtures/`: suite-owned reusable inputs. @@ -21,5 +21,5 @@ Focused run: pending target integration Mapping check from this directory: ```bash -python3 ../../scripts/check_test_map.py --root ../.. +python3 ../../scripts/check_test_map.py ``` diff --git a/assets/repo-tests/suite/reviews/README.md b/assets/repo-tests/suite/reviews/README.md deleted file mode 100644 index cc318f5..0000000 --- a/assets/repo-tests/suite/reviews/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# {{SUITE_LABEL}} Review Documents - -Create one concise document per coherent behavior from root `templates/test-review.md`. Present new, deleted, or materially changed rows for confirmation before mapped automation changes. Read `review-feedback.md` when present and use the root feedback format only for corrective feedback. diff --git a/references/initialize.md b/references/initialize.md index 115369d..8d84042 100644 --- a/references/initialize.md +++ b/references/initialize.md @@ -28,9 +28,9 @@ After map confirmation, select one coherent review document and follow `review-c After row confirmation, follow `automation-maintenance.md`. Implement only confirmed IDs. -## Layout choice +## Layout -Use the flat layout for one runner or assurance approach: +Keep reviewer-facing intent centralized at the root and group all executable automation by module or runner under `suites/`, even when the project starts with one suite: ```text -tests/ @@ -38,11 +38,12 @@ Use the flat layout for one runner or assurance approach: ├── README.md ├── source// ├── reviews//.md -├── tests/ or benchmarks/ or targets/ -├── fixtures/ +├── suites// +│ ├── tests/ or case/ or benchmarks/ or targets/ +│ └── fixtures/ └── scripts/check_test_map.py ``` -Create `suites//` only for genuinely independent runners such as API, P2P, performance, or fuzz. Keep case IDs unique across the project. +Do not create suite-local `reviews/` copies. Add another `suites//` directory when a new module or runner needs separate commands, fixtures, or executable ownership. Keep case IDs unique across the project. Reuse a matching checkout under `source//`; clone only when absent and never overwrite a conflicting path. diff --git a/references/review-cases.md b/references/review-cases.md index f865db7..d52f3b0 100644 --- a/references/review-cases.md +++ b/references/review-cases.md @@ -12,7 +12,7 @@ Prefix every scenario cell with `- [ ]` for a case without mapped automation or ## Corrective feedback -Before revising cases, read the nearest `reviews/review-feedback.md` when it exists. +Before revising cases, read root `reviews/review-feedback.md` when it exists. When a human corrects AI-authored cases—missing or unnecessary cases, scenario or expectation errors, priority changes, merges, splits, renames, or deletions—append one physical line: @@ -26,4 +26,4 @@ When a human corrects AI-authored cases—missing or unnecessary cases, scenario - Do not record approval without a correction. - Keep the file as reusable analysis feedback, not a case status, approval ledger, resolution log, or run history. -In a single-suite project use root `reviews/review-feedback.md`; in a multi-suite project use the affected suite's file. +Use one root `reviews/review-feedback.md` across all suites so corrections remain centralized with the review documents. diff --git a/scripts/check_test_map.py b/scripts/check_test_map.py index f9acc7e..667dd75 100644 --- a/scripts/check_test_map.py +++ b/scripts/check_test_map.py @@ -16,7 +16,7 @@ ) TASK_CHECKBOX = re.compile(r"^-\s+\[(?P[ xX])\]\s+") TEST_MAP = re.compile(rf"\bTEST-MAP:\s*(?P{CASE_TOKEN})\b") -CODE_DIR_NAMES = {"tests", "benchmarks", "targets"} +CODE_DIR_NAMES = {"case", "tests", "benchmarks", "targets"} CODE_SUFFIXES = { ".c", ".cc", @@ -40,6 +40,7 @@ ".tsx", } SKIP_DIRS = {".git", ".idea", ".pytest_cache", ".venv", "node_modules", "source", "__pycache__"} +PROJECT_ROOT = Path(__file__).resolve().parent.parent def parse_args() -> argparse.Namespace: @@ -49,8 +50,8 @@ def parse_args() -> argparse.Namespace: parser.add_argument( "--root", type=Path, - default=Path.cwd(), - help="Test-project root (default: current directory)", + default=PROJECT_ROOT, + help="Test-project root (default: repository containing this script)", ) parser.add_argument( "--require-complete", diff --git a/scripts/init_repo_tests.py b/scripts/init_repo_tests.py index a1b10ef..d9eef62 100644 --- a/scripts/init_repo_tests.py +++ b/scripts/init_repo_tests.py @@ -198,34 +198,16 @@ def main() -> int: if missing: raise SystemExit(f"required skill assets are missing: {', '.join(missing)}") - single_suite = len(suite_specs) == 1 - if single_suite: - suite, metadata = suite_specs[0] - code_dirs = metadata["code_dirs"] - layout = ( - f"A single {metadata['label']} approach uses root `reviews/`, " - f"{', '.join(f'`{name}/`' for name in code_dirs)}, and `fixtures/`." - ) - suite_list = ( - f"- Root review and execution scope: {metadata['label']} — {metadata['focus']}." - ) - review_locations = "- `reviews//.md`" - commands = ( - "Replace after target integration:\n\n" - "```text\nSetup: pending target integration\nRun: pending target integration\n" - "Focused run: pending target integration\n```" - ) - else: - layout = "Independent runners and assurance approaches live under `suites//`." - suite_list = "\n".join( - f"- `suites/{suite}/`: {metadata['label']} — {metadata['focus']}." - for suite, metadata in suite_specs - ) - review_locations = "\n".join( - f"- `suites/{suite}/reviews//.md`" - for suite, _ in suite_specs - ) - commands = "Setup and run commands live in each suite README." + layout = ( + "Reviewer-facing case documents are centralized under root `reviews/`; " + "executable automation and fixtures are grouped by module under `suites//`." + ) + suite_list = "\n".join( + f"- `suites/{suite}/`: {metadata['label']} — {metadata['focus']}." + for suite, metadata in suite_specs + ) + review_locations = "- `reviews//.md`" + commands = "Setup and run commands live in each suite README." common = { "PROJECT_NAME": args.project.strip(), @@ -269,65 +251,45 @@ def main() -> int: ) checker_output.chmod(0o755) - if single_suite: - _, metadata = suite_specs[0] + for suite, metadata in suite_specs: + code_dirs = metadata["code_dirs"] + replacements = { + **common, + "SUITE_TYPE": suite, + "SUITE_LABEL": metadata["label"], + "SUITE_FOCUS": metadata["focus"], + "CODE_DIRS": ", ".join(f"`{name}/`" for name in code_dirs), + } + suite_output = output / "suites" / suite + render_tree( + suite_template, + suite_output, + replacements, + force=args.force, + created=created, + replaced=replaced, + skipped=skipped, + ) write_file( - output / "fixtures" / "README.md", + suite_output / "fixtures" / "README.md", fixtures_readme(metadata["label"]), force=args.force, created=created, replaced=replaced, skipped=skipped, ) - for directory, purpose in metadata["code_dirs"].items(): + for directory, purpose in code_dirs.items(): write_file( - output / directory / "README.md", + suite_output / directory / "README.md", readme_for_directory(metadata["label"], directory, purpose), force=args.force, created=created, replaced=replaced, skipped=skipped, ) - else: - for suite, metadata in suite_specs: - code_dirs = metadata["code_dirs"] - replacements = { - **common, - "SUITE_TYPE": suite, - "SUITE_LABEL": metadata["label"], - "SUITE_FOCUS": metadata["focus"], - "CODE_DIRS": ", ".join(f"`{name}/`" for name in code_dirs), - } - suite_output = output / "suites" / suite - render_tree( - suite_template, - suite_output, - replacements, - force=args.force, - created=created, - replaced=replaced, - skipped=skipped, - ) - write_file( - suite_output / "fixtures" / "README.md", - fixtures_readme(metadata["label"]), - force=args.force, - created=created, - replaced=replaced, - skipped=skipped, - ) - for directory, purpose in code_dirs.items(): - write_file( - suite_output / directory / "README.md", - readme_for_directory(metadata["label"], directory, purpose), - force=args.force, - created=created, - replaced=replaced, - skipped=skipped, - ) print(f"test-project: {output.resolve()}") - print(f"layout: {'single-suite' if single_suite else 'multi-suite'}") + print("layout: centralized-reviews-with-suites") print(f"created: {len(created)}") print(f"replaced: {len(replaced)}") print(f"preserved: {len(skipped)}") diff --git a/tests/test_project_tools.py b/tests/test_project_tools.py index bad4ce8..63b25cf 100644 --- a/tests/test_project_tools.py +++ b/tests/test_project_tools.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +import shutil import subprocess import sys import tempfile @@ -27,7 +28,7 @@ def run(*args: str, cwd: Path | None = None) -> subprocess.CompletedProcess[str] class ProjectGeneratorTests(unittest.TestCase): - def test_single_suite_uses_flat_review_and_test_layout(self) -> None: + def test_single_suite_uses_central_reviews_and_suite_automation(self) -> None: with tempfile.TemporaryDirectory() as temporary: output = Path(temporary) / "demo-tests" first = run( @@ -42,19 +43,27 @@ def test_single_suite_uses_flat_review_and_test_layout(self) -> None: str(output), ) self.assertEqual(first.returncode, 0, first.stdout) - self.assertIn("layout: single-suite", first.stdout) + self.assertIn("layout: centralized-reviews-with-suites", first.stdout) for relative in [ "AGENTS.md", "README.md", "reviews/README.md", - "tests/README.md", - "fixtures/README.md", + "suites/api/AGENTS.md", + "suites/api/README.md", + "suites/api/tests/README.md", + "suites/api/fixtures/README.md", "scripts/check_test_map.py", "templates/test-review.md", ]: self.assertTrue((output / relative).is_file(), relative) - self.assertFalse((output / "suites").exists()) + self.assertFalse((output / "tests").exists()) + self.assertFalse((output / "fixtures").exists()) + self.assertFalse((output / "suites" / "api" / "reviews").exists()) self.assertNotIn("MOD -> FUNC -> COV -> TP", (output / "README.md").read_text()) + self.assertIn( + "centralized under root `reviews/`", + (output / "README.md").read_text(), + ) self.assertIn( "| `[AREA-01]` | - [ ] ", (output / "templates" / "test-review.md").read_text(), @@ -77,7 +86,7 @@ def test_single_suite_uses_flat_review_and_test_layout(self) -> None: self.assertIn("sentinel", (output / "README.md").read_text()) self.assertIn("created: 0", second.stdout) - def test_multiple_suites_are_isolated_only_when_needed(self) -> None: + def test_multiple_suites_share_root_reviews(self) -> None: with tempfile.TemporaryDirectory() as temporary: output = Path(temporary) / "demo-tests" result = run( @@ -93,17 +102,18 @@ def test_multiple_suites_are_isolated_only_when_needed(self) -> None: str(output), ) self.assertEqual(result.returncode, 0, result.stdout) - self.assertIn("layout: multi-suite", result.stdout) + self.assertIn("layout: centralized-reviews-with-suites", result.stdout) for relative in [ - "suites/integration/reviews/README.md", + "reviews/README.md", "suites/integration/tests/README.md", "suites/integration/config/README.md", - "suites/performance/reviews/README.md", "suites/performance/benchmarks/README.md", "suites/performance/workloads/README.md", ]: self.assertTrue((output / relative).is_file(), relative) self.assertFalse((output / "tests").exists()) + self.assertFalse((output / "suites" / "integration" / "reviews").exists()) + self.assertFalse((output / "suites" / "performance" / "reviews").exists()) def test_source_repository_is_required(self) -> None: result = run( @@ -192,6 +202,35 @@ def test_missing_or_stale_scenario_checkbox_fails(self) -> None: self.assertEqual(stale.returncode, 1, stale.stdout) self.assertIn("automation marker mismatches: RPC-01 (expected - [x])", stale.stdout) + def test_default_root_and_suite_case_directory_are_supported(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) / "demo-tests" + scripts = root / "scripts" + scripts.mkdir(parents=True) + shutil.copy2(CHECKER, scripts / "check_test_map.py") + + review = root / "reviews" / "p2p" / "ping.md" + review.parent.mkdir(parents=True) + review.write_text( + """# P2P Review + +| 用例 | 场景 | 预期结果 | 防止的问题 | 优先级 | +| --- | --- | --- | --- | --- | +| `P2P-PING-01` | - [x] valid ping | pong | liveness failure | P0 | +""", + encoding="utf-8", + ) + case = root / "suites" / "p2p" / "runner" / "src" / "case" / "ping.rs" + case.parent.mkdir(parents=True) + case.write_text("// TEST-MAP: P2P-PING-01\n", encoding="utf-8") + unrelated = Path(temporary) / "elsewhere" + unrelated.mkdir() + + result = run(str(scripts / "check_test_map.py"), cwd=unrelated) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertIn("automation coverage: 1/1", result.stdout) + self.assertIn("orphan mappings: none", result.stdout) + class SkillContractTests(unittest.TestCase): def read_contract(self) -> str: @@ -274,6 +313,22 @@ def test_token_budget_rules_and_progressive_disclosure(self) -> None: self.assertTrue((ROOT / "references" / reference).is_file(), reference) self.assertIn(f"references/{reference}", skill) + def test_reviews_are_central_and_automation_is_suite_owned(self) -> None: + skill = (ROOT / "SKILL.md").read_text(encoding="utf-8") + initialize = (ROOT / "references" / "initialize.md").read_text(encoding="utf-8") + suite_agents = (ROOT / "assets" / "repo-tests" / "suite" / "AGENTS.md").read_text( + encoding="utf-8" + ) + checker = (ROOT / "scripts" / "check_test_map.py").read_text(encoding="utf-8") + + self.assertIn("Root `reviews/` centrally", skill) + self.assertIn("under `suites//`", skill) + self.assertIn("Do not create suite-local `reviews/` copies", initialize) + self.assertIn("root `../../reviews/`", suite_agents) + self.assertFalse((ROOT / "assets" / "repo-tests" / "suite" / "reviews").exists()) + self.assertIn('CODE_DIR_NAMES = {"case", "tests", "benchmarks", "targets"}', checker) + self.assertIn("default=PROJECT_ROOT", checker) + if __name__ == "__main__": unittest.main()