diff --git a/.github/AUTOMATION.md b/.github/AUTOMATION.md index e23c99af..82902678 100644 --- a/.github/AUTOMATION.md +++ b/.github/AUTOMATION.md @@ -21,7 +21,7 @@ workflows change. | Workflow | File | Trigger | Responsibility | GitHub state | | --- | --- | --- | --- | --- | -| Validate repository | [`workflows/validate.yml`](workflows/validate.yml) | `pull_request` | Set up Python 3.13, validate both Skills' metadata (`scripts/validation/validate-skill-metadata.py`), run `python -m unittest discover -s tests` | Read-only (`contents: read`) | +| Validate repository | [`workflows/validate.yml`](workflows/validate.yml) | `pull_request`, `push` to `main` | `test` (required): set up Python 3.13, extract the trusted router (`scripts/validation/ci_test_route.py`) from the PR base SHA through a separate blobless clone and pick the FAST or FULL tier (FULL by default, on any error, and on every push to `main`), validate both Skills' metadata (`scripts/validation/validate-skill-metadata.py`), build and spec-validate the Skill trees, then run `python -m unittest discover -s tests -t .` (FULL) or the same discovery minus `tests.integration.*` (FAST). Skill-tree shell/PowerShell parity on both tiers | Read-only (`contents: read`) | | Validate PR description length | [`workflows/pr-description-length.yml`](workflows/pr-description-length.yml) | `pull_request` (opened, edited, synchronize) | Check out the trusted validator from the PR base SHA (bootstrapping from head only for the PR that introduces the script), enforce the useful-content limit and the canonical PR-template structure via `scripts/validation/pr_description_length.py` | Read-only (`contents: read`) | | Sync Engineering Task labels | [`workflows/sync-issue-labels.yml`](workflows/sync-issue-labels.yml) | `issues` (opened, edited) | Compute managed-label changes from the issue body (`scripts/governance/sync_issue_labels.py`), then `gh issue edit` to apply the add/remove set; per-issue `concurrency` with cancel-in-progress | Mutates issue labels (`issues: write`) | | Claim contribution issue | [`workflows/claim-issue.yml`](workflows/claim-issue.yml) | `issue_comment` (created) | On `/claim` or `/unclaim` on a non-PR issue: check out trusted default-branch automation, read the issue and comment history, plan via `scripts/governance/claim_issue.py` with churn/cooldown thresholds, persist a trusted receipt and a reconciled-state checkpoint comment, then project state onto the `claimed` label; repo-wide serialized `concurrency` queue | Mutates issue comments + the `claimed` label (`issues: write`) | @@ -36,8 +36,11 @@ Runs on the pull request, read-only. `validate.yml` and the `Release worthiness` `release-gate` job are the required status checks on the `main` ruleset ([`../docs/RELEASE.md`](../docs/RELEASE.md)): -- **`validate.yml`** — Skill metadata validation plus the full - `tests/` suite. +- **`validate.yml`** — Skill metadata validation plus the `tests/` + suite, routed per PR: FULL by default, or FAST (omitting only + `tests/integration/`) when every changed path is on the router's + allowlist; every push to `main` runs FULL. Contract: + [`../policies/validation-and-clean-exit.md`](../policies/validation-and-clean-exit.md#routed-ci-tests). - **`pr-description-length.yml`** — enforces the PR-description useful-content limit and the canonical PR-template structure (required headings/fields, unresolved placeholders), checking out the validator diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 7c702853..a3b8eb18 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -2,6 +2,10 @@ name: Validate repository on: pull_request: + # Every merged commit runs the full suite (the router resolves any + # non-pull_request event to FULL), so a stale FAST allowlist turns main red. + push: + branches: [main] permissions: contents: read @@ -20,6 +24,33 @@ jobs: with: python-version: "3.13" + - name: Route tests (FAST/FULL) + id: route + # The router is trusted from the base, never the head: a PR that + # introduces or edits it is FULL (#533). A separate blobless clone keeps + # the checkout under test unchanged; any failure leaves the tier FULL. + continue-on-error: true + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REPO_URL: ${{ github.server_url }}/${{ github.repository }} + run: | + route_repo="$RUNNER_TEMP/ci-route" + router="$RUNNER_TEMP/ci_test_route.py" + if [ "$EVENT_NAME" != "pull_request" ]; then + reason="non-pull_request event ($EVENT_NAME)" + elif git init -q "$route_repo" \ + && git -C "$route_repo" remote add origin "$REPO_URL" \ + && git -C "$route_repo" fetch -q --no-tags --filter=blob:none origin "$BASE_SHA" "$HEAD_SHA" \ + && git -C "$route_repo" show "$BASE_SHA:scripts/validation/ci_test_route.py" > "$router"; then + exec python "$router" route --event-name "$EVENT_NAME" --base "$BASE_SHA" --head "$HEAD_SHA" --repo "$route_repo" + else + reason="no router at the base commit, or the base/head could not be fetched" + fi + printf '### CI test route\n\n- Tier: **FULL**\n- Reason: %s\n' "$reason" >> "$GITHUB_STEP_SUMMARY" + echo "tier=full" >> "$GITHUB_OUTPUT" + - name: Set up Node.js # The distribution consumer check drives the pinned `skills` CLI (#511). uses: actions/setup-node@v4 @@ -45,10 +76,19 @@ jobs: for tree in dist/skills/*/; do agentskills validate "$tree"; done - name: Run repository tests + # FULL unless the router positively selected FAST. + if: ${{ steps.route.outputs.tier != 'fast' }} env: DISTRIBUTION_INSTALL_CHECK: "1" run: python -m unittest discover -s tests -t . + - name: Run repository tests except tests.integration (FAST tier) + if: ${{ steps.route.outputs.tier == 'fast' }} + env: + DISTRIBUTION_INSTALL_CHECK: "1" + # The base's router, extracted by the route step, selects the tests. + run: python "$RUNNER_TEMP/ci_test_route.py" run-fast + skill-tree-parity: name: Skill tree (${{ matrix.shell }} on ${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 89206c25..efb2e137 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,8 +79,11 @@ bot replies. Maintainers can tune the three named `CLAIM_*` values in 3. Implement the issue without adding unrelated changes. 4. Run the checks relevant to your change from [README § Local validation before opening a PR](README.md#local-validation-before-opening-a-pr). - The full test suite is not required locally before every push or PR — it - runs on every PR in CI, which is the authoritative regression gate. + The full test suite is not required locally before every push or PR — CI + is the authoritative regression gate. It runs the full suite unless every + changed path is provably not an integration input, in which case it omits + only `tests/integration/` + ([routed CI tests](policies/validation-and-clean-exit.md#routed-ci-tests)). 5. Open a pull request against this repository and use `Fixes #` when the pull request should close the issue. diff --git a/README.md b/README.md index 64562138..492e8cc8 100644 --- a/README.md +++ b/README.md @@ -178,10 +178,14 @@ python -m pip install -r requirements-dev.txt Local validation is **targeted**, not a fixed sequence: run only the checks relevant to what you changed, for fast feedback before pushing. The full -suite below is **not** required locally before every push or PR — it runs -automatically in [`.github/workflows/validate.yml`](.github/workflows/validate.yml) -on every PR, which remains the authoritative regression gate before merge +suite below is **not** required locally before every push or PR — CI runs +it in [`.github/workflows/validate.yml`](.github/workflows/validate.yml), +which remains the authoritative regression gate before merge (see [`policies/git-pr-merge-policy.md`](policies/git-pr-merge-policy.md)). +CI routes each PR to FULL (the default) or to FAST, which omits only +`tests/integration/` when every changed path is provably not an +integration input; every push to `main` runs FULL +(see [`policies/validation-and-clean-exit.md`](policies/validation-and-clean-exit.md#routed-ci-tests)). | Change | Run | | --- | --- | diff --git a/policies/validation-and-clean-exit.md b/policies/validation-and-clean-exit.md index fecd9206..93053d3e 100644 --- a/policies/validation-and-clean-exit.md +++ b/policies/validation-and-clean-exit.md @@ -76,12 +76,51 @@ relevant to the change — see targeted mapping from change type to command. The full suite (`python3 -m unittest discover -s tests -t .` plus both metadata validations, link validation, and packaging) is not a local precondition; -it runs on every PR in `.github/workflows/validate.yml`, which is the +CI runs it through `.github/workflows/validate.yml`, which is the authoritative regression gate before merge -([`git-pr-merge-policy.md`](git-pr-merge-policy.md)). Running the full -sequence locally beforehand is optional, never required, and appropriate -when a change genuinely spans multiple areas or a shared cross-cutting -contract. +([`git-pr-merge-policy.md`](git-pr-merge-policy.md)), routed as described +under [Routed CI tests](#routed-ci-tests). Running the full sequence +locally beforehand is optional, never required, and appropriate when a +change genuinely spans multiple areas or a shared cross-cutting contract. + +### Routed CI tests + +The required `test` job in `validate.yml` routes every PR to exactly one +tier. **FULL is the default**: + +- **FULL** runs the full suite exactly as above + (`python -m unittest discover -s tests -t .`). +- **FAST** runs the same discovery minus exactly the tests whose ID starts + with `tests.integration.`. Every unit, policy, and repository test — and + any new top-level test directory — still runs, as do metadata + validation, the canonical build, Agent Skills spec validation, and the + shell/PowerShell parity jobs. FAST never removes a contract or + correctness test. + +A PR is FAST only when **every** changed path (a three-dot merge-base +diff, renames counted as both paths) is on the allowlist in +[`../scripts/validation/ci_test_route.py`](../scripts/validation/ci_test_route.py), +the single canonical home of that list. Every entry must have positive, +repository-backed evidence that no integration test copies, reads, or +packages it; anything unknown, mixed, empty, or erroring is FULL, and +there is no label or flag that selects FAST. The `test` job extracts the +router from the PR's base commit (via a separate blobless clone, so the +checkout under test is unchanged), so a PR that edits it (or +`validate.yml`) is FULL. + +`tests/policy/governance/test_ci_test_routing.py` is a narrow tripwire, +not proof that an allowlisted path is inert. On every PR it fails if the +shared temp-root copy list (`TEMP_ROOT_INPUTS` in +`tests/integration/packaging/_shared.py`) or a literal +`REPO_ROOT / "…"` read under `tests/integration/` overlaps the allowlist. +It does not see an integration test's own inline copy list, other path +forms (`joinpath`, `Path(REPO_ROOT, …)`, f-strings), or files read by a +script an integration test invokes. Admitting a path to the allowlist +therefore stays a maintainer decision made in its own PR with that +positive evidence gathered by hand +([#533](https://github.com/amirbena/code-review-skill/issues/533)); the +safety net for a missed consumer is that every push to `main` runs FULL, +so a stale entry surfaces at the first merge that exposes it. Repository-owned Python that any of these steps touch follows [`python_scripts_coding_policy.md`](python_scripts_coding_policy.md). diff --git a/scripts/validation/ci_test_route.py b/scripts/validation/ci_test_route.py new file mode 100644 index 00000000..a568deea --- /dev/null +++ b/scripts/validation/ci_test_route.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +"""Route a CI run to the FAST or FULL test tier from its changed paths. + +FULL is the default. FAST omits only `tests.integration.*`, and only when every +changed path is on the allowlist below; see policies/validation-and-clean-exit.md. +""" + +from __future__ import annotations + +import argparse +import os +import subprocess +import sys +import unittest +from dataclasses import dataclass +from pathlib import Path +from typing import Iterator, Sequence + +FAST = "fast" +FULL = "full" + +# Exact, case-sensitive matches. Each entry needs positive evidence that no +# integration test copies, reads, or packages it (issue #533). +FAST_FILES = frozenset( + { + ".gitignore", + "AGENTS.md", + "CLAUDE.md", + "CONTRIBUTING.md", + "SECURITY.md", + "README.md", + ".github/PULL_REQUEST_TEMPLATE.md", + ".github/AUTOMATION.md", + } +) +FAST_DIRS = ("policies/", ".github/ISSUE_TEMPLATE/") + +INTEGRATION_PREFIX = "tests.integration." + + +@dataclass(frozen=True) +class Route: + tier: str + reason: str + first_full_path: str | None = None + + +def is_fast_path(path: str) -> bool: + return path in FAST_FILES or any(path.startswith(d) and len(path) > len(d) for d in FAST_DIRS) + + +def classify(paths: Sequence[str]) -> Route: + if not paths: + return Route(FULL, "empty change set") + for path in paths: + if not is_fast_path(path): + return Route(FULL, "a changed path is not on the FAST allowlist", path) + return Route(FAST, "every changed path is on the FAST allowlist") + + +def changed_paths(repo: Path, base: str, head: str) -> list[str]: + # Three-dot: the PR's own changes since the merge-base; renames split into D + A. + out = subprocess.run( + ["git", "-C", str(repo), "diff", "--name-only", "--no-renames", "-z", f"{base}...{head}"], + capture_output=True, + check=True, + ).stdout + return [p for p in out.decode("utf-8", "surrogateescape").split("\0") if p] + + +def route(event_name: str, repo: Path, base: str | None, head: str | None) -> Route: + if event_name != "pull_request": + return Route(FULL, f"non-pull_request event ({event_name or 'unknown'})") + if not base or not head: + return Route(FULL, "missing base or head SHA") + try: + paths = changed_paths(repo, base, head) + except (OSError, subprocess.CalledProcessError): + return Route(FULL, "git diff failed") + return classify(paths) + + +def safe_route(event_name: str, repo: Path, base: str | None, head: str | None) -> Route: + try: + result = route(event_name, repo, base, head) + except Exception as exc: # noqa: BLE001 - any router failure must resolve to FULL + return Route(FULL, f"router exception ({type(exc).__name__})") + if result.tier not in (FAST, FULL): + return Route(FULL, f"unrecognized tier {result.tier!r}") + return result + + +def summary_markdown(result: Route) -> str: + lines = ["### CI test route", "", f"- Tier: **{result.tier.upper()}**", f"- Reason: {result.reason}"] + if result.first_full_path is not None: + lines.append(f"- First path that forced FULL: `{result.first_full_path}`") + return "\n".join(lines) + "\n" + + +def _append(path: str | None, text: str) -> None: + if path: + with open(path, "a", encoding="utf-8") as handle: + handle.write(text) + + +def _iter_tests(suite: unittest.TestSuite) -> Iterator[unittest.TestCase]: + for item in suite: + if isinstance(item, unittest.TestSuite): + yield from _iter_tests(item) + else: + yield item + + +def fast_suite(start_dir: str | Path = "tests", top_level_dir: str | Path = ".") -> unittest.TestSuite: + # Same discovery as FULL, minus exactly the integration tests. + discovered = unittest.defaultTestLoader.discover(str(start_dir), top_level_dir=str(top_level_dir)) + return unittest.TestSuite(t for t in _iter_tests(discovered) if not t.id().startswith(INTEGRATION_PREFIX)) + + +def _cmd_route(args: argparse.Namespace) -> int: + result = safe_route(args.event_name, Path(args.repo), args.base, args.head) + print(summary_markdown(result), end="") + _append(args.step_summary, summary_markdown(result)) + # Last, so a failure anywhere above leaves the tier unset, which runs FULL. + _append(args.github_output, f"tier={result.tier}\n") + return 0 + + +def _cmd_run_fast(args: argparse.Namespace) -> int: + suite = fast_suite() + if args.list: + for test in _iter_tests(suite): + print(test.id()) + return 0 + result = unittest.TextTestRunner().run(suite) + return 0 if result.wasSuccessful() and result.testsRun > 0 else 1 + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + commands = parser.add_subparsers(dest="command", required=True) + + route_cmd = commands.add_parser("route", help="classify the change set and emit tier=fast|full") + route_cmd.add_argument("--event-name", required=True) + route_cmd.add_argument("--base") + route_cmd.add_argument("--head") + route_cmd.add_argument("--repo", default=".") + route_cmd.add_argument("--github-output", default=os.environ.get("GITHUB_OUTPUT")) + route_cmd.add_argument("--step-summary", default=os.environ.get("GITHUB_STEP_SUMMARY")) + route_cmd.set_defaults(func=_cmd_route) + + fast_cmd = commands.add_parser("run-fast", help="run tests/ from the repository root minus tests.integration.*") + fast_cmd.add_argument("--list", action="store_true", help="print the FAST test IDs instead of running them") + fast_cmd.set_defaults(func=_cmd_run_fast) + + args = parser.parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/integration/packaging/_shared.py b/tests/integration/packaging/_shared.py index 1d6261ac..124fa550 100644 --- a/tests/integration/packaging/_shared.py +++ b/tests/integration/packaging/_shared.py @@ -36,6 +36,10 @@ def _reference_module_path(name: str) -> Path: GITHUB_SKILL_DIR = REPO_ROOT / "skills" / "github-pr-review" DIST_DIR = REPO_ROOT / "dist" +# Repository paths copied into the temp build roots of the packaging and +# release version tests; the CI routing guard keeps these off the FAST allowlist. +TEMP_ROOT_INPUTS = ("skills", "shared", "scripts", "capabilities", "docs", "LICENSE", "CHANGELOG.md") + # The reference modules this test guards — none is a runtime dependency. REFERENCE_TEST_MODULES = ( "current_evidence.py", diff --git a/tests/integration/packaging/test_ordinary_packaging_version.py b/tests/integration/packaging/test_ordinary_packaging_version.py index 5073ab21..3b03d5b5 100644 --- a/tests/integration/packaging/test_ordinary_packaging_version.py +++ b/tests/integration/packaging/test_ordinary_packaging_version.py @@ -20,6 +20,7 @@ import tests.unit.package_domain._shared # noqa: F401 - sys.path wiring from package_domain.version import frontmatter_version, newest_release_version +from tests.integration.packaging._shared import TEMP_ROOT_INPUTS from tests.support.paths import REPO_ROOT PACKAGE = "scripts/packaging/package-skills.sh" @@ -45,7 +46,7 @@ def setUp(self) -> None: tmp = tempfile.TemporaryDirectory() self.addCleanup(tmp.cleanup) self.root = Path(tmp.name) - for rel in ("skills", "shared", "scripts", "capabilities", "docs", "LICENSE", "CHANGELOG.md"): + for rel in TEMP_ROOT_INPUTS: _copy(rel, self.root) def _set_committed_version(self, version: str) -> None: diff --git a/tests/integration/release/test_skill_archive_release_version.py b/tests/integration/release/test_skill_archive_release_version.py index ec7a9f98..6c2e0208 100644 --- a/tests/integration/release/test_skill_archive_release_version.py +++ b/tests/integration/release/test_skill_archive_release_version.py @@ -14,6 +14,7 @@ import zipfile from pathlib import Path +from tests.integration.packaging._shared import TEMP_ROOT_INPUTS from tests.support.paths import REPO_ROOT SCRIPT = "scripts/release/verify-skill-archives.sh" @@ -96,8 +97,9 @@ def setUp(self) -> None: tmp = tempfile.TemporaryDirectory() self.addCleanup(tmp.cleanup) self.root = Path(tmp.name) - for rel in ("skills", "shared", "scripts", "capabilities", "docs", "LICENSE"): - _copy(rel, self.root) + for rel in TEMP_ROOT_INPUTS: + if rel != "CHANGELOG.md": # each test writes its own rolled changelog + _copy(rel, self.root) def _roll_changelog(self, version: str) -> None: (self.root / "CHANGELOG.md").write_text( diff --git a/tests/policy/governance/test_ci_test_routing.py b/tests/policy/governance/test_ci_test_routing.py new file mode 100644 index 00000000..83939f37 --- /dev/null +++ b/tests/policy/governance/test_ci_test_routing.py @@ -0,0 +1,133 @@ +"""Contracts for fail-safe FAST/FULL routing in validate.yml and its integration-input guard.""" + +from __future__ import annotations + +import re +import unittest + +import yaml + +from scripts.validation import ci_test_route as router +from tests.integration.packaging._shared import TEMP_ROOT_INPUTS +from tests.support.paths import REPO_ROOT + +WORKFLOW = REPO_ROOT / ".github" / "workflows" / "validate.yml" +INTEGRATION_DIR = REPO_ROOT / "tests" / "integration" +FULL_COMMAND = "python -m unittest discover -s tests -t ." + +_REPO_ROOT_READ = re.compile(r'REPO_ROOT((?:\s*/\s*"[^"]+")+)') + + +def _load_workflow() -> dict: + return yaml.safe_load(WORKFLOW.read_text(encoding="utf-8")) + + +def _on(workflow: dict) -> dict: + return workflow.get("on", workflow.get(True)) + + +def _step(job: dict, name: str) -> dict: + return next(step for step in job["steps"] if step.get("name") == name) + + +def _overlaps_allowlist(path: str) -> bool: + # A read of a path, or of any directory containing an allowlisted path, overlaps. + path = path.strip("/") + entries = [*router.FAST_FILES, *(d.rstrip("/") for d in router.FAST_DIRS)] + return any(path == e or e.startswith(path + "/") or path.startswith(e + "/") for e in entries) + + +def _integration_repo_root_reads() -> set[str]: + reads = set() + for module in INTEGRATION_DIR.rglob("*.py"): + for match in _REPO_ROOT_READ.finditer(module.read_text(encoding="utf-8")): + reads.add("/".join(re.findall(r'"([^"]+)"', match.group(1)))) + return reads + + +class ValidateWorkflowRoutingTests(unittest.TestCase): + def setUp(self) -> None: + self.workflow = _load_workflow() + self.jobs = self.workflow["jobs"] + + def test_required_test_job_is_always_created_and_routes_itself(self) -> None: + test = self.jobs["test"] + self.assertNotIn("name", test) + self.assertNotIn("needs", test) + self.assertNotIn("if", test) + self.assertEqual(set(self.jobs), {"test", "skill-tree-parity", "skill-tree-hash-equality"}) + + def test_no_path_filters(self) -> None: + for event, config in _on(self.workflow).items(): + with self.subTest(event=event): + self.assertFalse({"paths", "paths-ignore"} & set(config or {})) + + def test_runs_full_on_push_to_main(self) -> None: + on = _on(self.workflow) + self.assertIn("pull_request", on) + self.assertEqual(on["push"], {"branches": ["main"]}) + + def test_router_runs_from_the_base_sha_outside_the_checkout(self) -> None: + test = self.jobs["test"] + self.assertEqual(test["steps"][0]["with"], {"persist-credentials": False}) + route = _step(test, "Route tests (FAST/FULL)") + self.assertEqual(route["id"], "route") + self.assertIs(route["continue-on-error"], True) + run = route["run"] + self.assertIn('route_repo="$RUNNER_TEMP/ci-route"', run) + self.assertIn('fetch -q --no-tags --filter=blob:none origin "$BASE_SHA" "$HEAD_SHA"', run) + self.assertIn('show "$BASE_SHA:scripts/validation/ci_test_route.py" > "$router"', run) + self.assertIn('--repo "$route_repo"', run) + self.assertIn('echo "tier=full"', run) + self.assertNotIn("HEAD_SHA:scripts", run) + steps = [step.get("name") for step in test["steps"]] + self.assertLess(steps.index("Route tests (FAST/FULL)"), steps.index("Run repository tests")) + + def test_integration_runs_unless_tier_is_exactly_fast(self) -> None: + test = self.jobs["test"] + full = _step(test, "Run repository tests") + self.assertEqual(full["if"], "${{ steps.route.outputs.tier != 'fast' }}") + self.assertEqual(full["run"], FULL_COMMAND) + self.assertEqual(full["env"], {"DISTRIBUTION_INSTALL_CHECK": "1"}) + fast = _step(test, "Run repository tests except tests.integration (FAST tier)") + self.assertEqual(fast["if"], "${{ steps.route.outputs.tier == 'fast' }}") + self.assertEqual(fast["run"], 'python "$RUNNER_TEMP/ci_test_route.py" run-fast') + + def test_non_test_validation_runs_on_both_tiers(self) -> None: + test = self.jobs["test"] + for name in ( + "Validate Skill metadata", + "Build the canonical Skill trees", + "Validate the built Skill trees against the Agent Skills spec", + ): + with self.subTest(step=name): + self.assertNotIn("if", _step(test, name)) + for job in ("skill-tree-parity", "skill-tree-hash-equality"): + with self.subTest(job=job): + self.assertNotIn("if", self.jobs[job]) + + +class IntegrationInputGuardTests(unittest.TestCase): + def test_overlap_helper_catches_parent_and_child_reads(self) -> None: + for path in ("policies", "policies/x.md", ".github", "README.md", ".github/ISSUE_TEMPLATE/a.yml"): + with self.subTest(path=path): + self.assertTrue(_overlaps_allowlist(path)) + for path in ("docs", "skills", "policiesx", "tests/README.md"): + with self.subTest(path=path): + self.assertFalse(_overlaps_allowlist(path)) + + def test_temp_root_copy_list_shares_no_path_with_the_fast_allowlist(self) -> None: + for path in TEMP_ROOT_INPUTS: + with self.subTest(path=path): + self.assertFalse(_overlaps_allowlist(path)) + + def test_integration_repo_root_reads_share_no_path_with_the_fast_allowlist(self) -> None: + reads = _integration_repo_root_reads() + self.assertIn("CHANGELOG.md", reads) + for path in sorted(reads): + with self.subTest(path=path): + self.assertFalse(_overlaps_allowlist(path)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/governance/test_ci_test_route.py b/tests/unit/governance/test_ci_test_route.py new file mode 100644 index 00000000..54a6462a --- /dev/null +++ b/tests/unit/governance/test_ci_test_route.py @@ -0,0 +1,237 @@ +"""Tests for the fail-safe FAST/FULL CI test router.""" + +from __future__ import annotations + +import subprocess +import sys +import tempfile +import unittest +from contextlib import redirect_stdout +from io import StringIO +from pathlib import Path +from unittest import mock + +from scripts.validation import ci_test_route as router +from tests.support.paths import REPO_ROOT + +ROUTER = REPO_ROOT / "scripts" / "validation" / "ci_test_route.py" + +ALLOWLISTED = ( + ".gitignore", + "policies/validation-and-clean-exit.md", + "policies/nested/x.md", + "AGENTS.md", + "CLAUDE.md", + "CONTRIBUTING.md", + "SECURITY.md", + "README.md", + ".github/ISSUE_TEMPLATE/bug.yml", + ".github/PULL_REQUEST_TEMPLATE.md", + ".github/AUTOMATION.md", +) + +INTEGRATION_SENSITIVE = ( + "skills/local-code-review/SKILL.md", + "shared/policies/review-scope.md", + "capabilities/x/capability.yaml", + "scripts/packaging/package-skills.sh", + "scripts/release/release_worthiness.py", + "scripts/skill_metadata/x.py", + "scripts/sandbox/x.py", + "scripts/validation/validate-skill-metadata.py", + "CHANGELOG.md", + "LICENSE", + "distribution/x.md", + "dist/skills/x/SKILL.md", + "runtime_platform/benchmark/x.py", + "docs/ARCHITECTURE.md", + "tests/unit/test_x.py", + ".github/workflows/release-publish.yml", + ".github/CODEOWNERS", + "requirements-dev.txt", +) + +NEAR_MISSES = ( + "Docs/a.md", + "./README.md", + "policiesx/a.md", + "policies", + "policies/", + "sub/README.md", + "README.md.bak", + "readme.md", + "skills/local-code-review/README.md", + ".github/ISSUE_TEMPLATE", + "new/x", + "NEW.md", +) + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run(["git", "-C", str(repo), *args], capture_output=True, text=True, check=True).stdout.strip() + + +class ClassifyTests(unittest.TestCase): + def test_each_allowlisted_path_is_fast(self) -> None: + for path in ALLOWLISTED: + with self.subTest(path=path): + self.assertEqual(router.classify([path]).tier, router.FAST) + + def test_each_integration_sensitive_category_is_full(self) -> None: + for path in INTEGRATION_SENSITIVE: + with self.subTest(path=path): + result = router.classify([path]) + self.assertEqual(result.tier, router.FULL) + self.assertEqual(result.first_full_path, path) + + def test_unknown_paths_and_near_misses_are_full(self) -> None: + for path in NEAR_MISSES: + with self.subTest(path=path): + self.assertEqual(router.classify([path]).tier, router.FULL) + + def test_router_and_workflow_changes_are_full(self) -> None: + for path in ("scripts/validation/ci_test_route.py", ".github/workflows/validate.yml"): + with self.subTest(path=path): + self.assertEqual(router.classify([path]).tier, router.FULL) + + def test_mixed_change_set_is_full_and_names_first_forcing_path(self) -> None: + result = router.classify(["README.md", "shared/x.md", "skills/y.md"]) + self.assertEqual(result.tier, router.FULL) + self.assertEqual(result.first_full_path, "shared/x.md") + + def test_empty_change_set_is_full(self) -> None: + self.assertEqual(router.classify([]).tier, router.FULL) + + +class RouteTests(unittest.TestCase): + def setUp(self) -> None: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.repo = Path(tmp.name) + _git(self.repo, "init", "-q", "-b", "main") + _git(self.repo, "config", "user.email", "t@example.com") + _git(self.repo, "config", "user.name", "t") + self._write("policies/x.md", "p\n") + self._write("skills/y.md", "s\n") + self._write("shared/z.md", "z\n") + self.base = self._commit("base") + + def _write(self, rel: str, text: str) -> None: + path = self.repo / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + def _commit(self, message: str) -> str: + _git(self.repo, "add", "-A") + _git(self.repo, "commit", "-q", "--allow-empty", "-m", message) + return _git(self.repo, "rev-parse", "HEAD") + + def _route(self, head: str) -> router.Route: + return router.safe_route("pull_request", self.repo, self.base, head) + + def test_allowlisted_edit_is_fast(self) -> None: + self._write("policies/x.md", "changed\n") + self.assertEqual(self._route(self._commit("edit")).tier, router.FAST) + + def test_renames_across_the_boundary_are_full(self) -> None: + for old, new in (("policies/x.md", "skills/x.md"), ("skills/y.md", "policies/y.md")): + with self.subTest(old=old, new=new): + _git(self.repo, "checkout", "-q", "--detach", self.base) + _git(self.repo, "mv", old, new) + head = self._commit("rename") + self.assertEqual(set(router.changed_paths(self.repo, self.base, head)), {old, new}) + self.assertEqual(self._route(head).tier, router.FULL) + + def test_deletions(self) -> None: + _git(self.repo, "rm", "-q", "policies/x.md") + self.assertEqual(self._route(self._commit("delete policy")).tier, router.FAST) + _git(self.repo, "rm", "-q", "shared/z.md") + self.base = _git(self.repo, "rev-parse", "HEAD") + self.assertEqual(self._route(self._commit("delete shared")).tier, router.FULL) + + def test_three_dot_diff_ignores_base_commits_merged_after_the_fork(self) -> None: + _git(self.repo, "checkout", "-q", "-b", "pr") + self._write("README.md", "r\n") + head = self._commit("pr") + _git(self.repo, "checkout", "-q", "main") + self._write("shared/z.md", "moved on\n") + self.base = self._commit("main moves") + self.assertEqual(self._route(head).tier, router.FAST) + + def test_empty_change_set_is_full(self) -> None: + self.assertEqual(self._route(self._commit("empty")).tier, router.FULL) + + def test_git_failure_is_full(self) -> None: + result = self._route("0" * 40) + self.assertEqual((result.tier, result.reason), (router.FULL, "git diff failed")) + + def test_missing_sha_or_non_pull_request_event_is_full(self) -> None: + self._write("README.md", "r\n") + head = self._commit("readme") + self.assertEqual(router.safe_route("pull_request", self.repo, "", head).tier, router.FULL) + self.assertEqual(router.safe_route("pull_request", self.repo, self.base, None).tier, router.FULL) + for event in ("push", "workflow_dispatch", ""): + with self.subTest(event=event): + self.assertEqual(router.safe_route(event, self.repo, self.base, head).tier, router.FULL) + + def test_router_exception_is_full(self) -> None: + with mock.patch.object(router, "changed_paths", side_effect=RuntimeError("boom")): + result = self._route("HEAD") + self.assertEqual(result.tier, router.FULL) + self.assertIn("RuntimeError", result.reason) + + def test_unrecognized_tier_is_full(self) -> None: + with mock.patch.object(router, "classify", return_value=router.Route("FAST", "x")): + self.assertEqual(self._route("HEAD").tier, router.FULL) + + +class CliTests(unittest.TestCase): + def test_route_writes_tier_output_and_summary(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + output, summary = Path(tmp) / "out", Path(tmp) / "summary" + with redirect_stdout(StringIO()): + code = router.main( + ["route", "--event-name", "push", "--github-output", str(output), "--step-summary", str(summary)] + ) + self.assertEqual(code, 0) + self.assertEqual(output.read_text(encoding="utf-8"), "tier=full\n") + text = summary.read_text(encoding="utf-8") + self.assertIn("Tier: **FULL**", text) + self.assertIn("Reason: non-pull_request event (push)", text) + + def test_tier_is_written_last_so_a_failed_write_leaves_it_unset(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + output = Path(tmp) / "out" + with redirect_stdout(StringIO()), self.assertRaises(OSError): + router.main(["route", "--event-name", "push", "--github-output", str(output), "--step-summary", tmp]) + self.assertFalse(output.exists()) + + def test_summary_names_the_first_path_that_forced_full(self) -> None: + text = router.summary_markdown(router.classify(["README.md", "shared/x.md"])) + self.assertIn("First path that forced FULL: `shared/x.md`", text) + + +class FastSuiteTests(unittest.TestCase): + def test_fast_ids_are_full_ids_minus_exactly_integration(self) -> None: + full = {t.id() for t in router._iter_tests(unittest.defaultTestLoader.discover(str(REPO_ROOT / "tests"), top_level_dir=str(REPO_ROOT)))} + fast = {t.id() for t in router._iter_tests(router.fast_suite(REPO_ROOT / "tests", REPO_ROOT))} + self.assertTrue(any(i.startswith(router.INTEGRATION_PREFIX) for i in full)) + self.assertEqual(fast, {i for i in full if not i.startswith(router.INTEGRATION_PREFIX)}) + + def test_a_new_test_directory_is_not_skipped(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + body = "import unittest\n\nclass T(unittest.TestCase):\n def test_a(self):\n pass\n" + for package in ("tests", "tests/newdir", "tests/integration"): + (root / package).mkdir(parents=True, exist_ok=True) + (root / package / "__init__.py").write_text("", encoding="utf-8") + (root / "tests/newdir/test_x.py").write_text(body, encoding="utf-8") + (root / "tests/integration/test_y.py").write_text(body, encoding="utf-8") + listed = subprocess.run( + [sys.executable, str(ROUTER), "run-fast", "--list"], cwd=root, capture_output=True, text=True, check=True + ).stdout.split() + self.assertEqual(listed, ["tests.newdir.test_x.T.test_a"]) + + +if __name__ == "__main__": + unittest.main()