diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c23ee9d3..34bf8fe4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,16 +1,19 @@ name: CI -# Run the test suite on every pull request and on pushes to main. Push is -# limited to main so PR branches don't run twice (once for push, once for the PR). +# Run the test suite only when changes land on main (a merged PR or a direct +# push). Tests intentionally do NOT run on pull requests — see ADR-027 for the +# trigger policy and its post-merge-feedback tradeoff. workflow_dispatch allows +# running the full battery grid manually (e.g. to check a branch before merge). on: push: branches: [main] - pull_request: + workflow_dispatch: -# Cancel superseded runs on the same ref (e.g. a new push to an open PR branch). +# Serialize runs on the same ref. Don't cancel an in-progress run: every merge +# to main should be fully tested, since these runs are what gate releases. concurrency: group: ci-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: false jobs: tests: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index fc7d9814..a78cecf7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,8 +1,13 @@ name: Tests # Reusable workflow — the single definition of how RAC's test suite runs. -# Called by ci.yml (push / pull_request) and by python-publish.yml (release gate), +# Called by ci.yml (push to main) and by python-publish.yml (release gate), # so the test steps live in one place rather than being duplicated. +# +# Tests are split into one "battery" per .py service (plus grouped core / cli / +# artifacts) and run across every supported Python version, so the Actions UI +# names the service + version that failed (e.g. "relationships (py3.11)") +# instead of a single opaque "pytest (3.11)". See ADR-027. on: workflow_call: @@ -12,11 +17,37 @@ permissions: jobs: pytest: + name: ${{ matrix.battery.name }} (py${{ matrix.python-version }}) runs-on: ubuntu-latest strategy: fail-fast: false matrix: python-version: ["3.11", "3.12", "3.13"] + # One battery per .py service, plus grouped core / cli / artifacts. + # Every tests/test_*.py belongs to exactly one battery (no orphans). + battery: + - name: core + paths: "tests/test_validate.py tests/test_parser.py tests/test_schema.py tests/test_identity.py" + - name: cli + paths: "tests/test_cli.py" + - name: artifacts + paths: "tests/test_design.py tests/test_roadmap.py tests/test_prompt.py tests/test_decision_metadata.py" + - name: diff + paths: "tests/test_diff.py" + - name: improve + paths: "tests/test_improve.py" + - name: index + paths: "tests/test_index.py" + - name: ingest + paths: "tests/test_ingest.py" + - name: inspect + paths: "tests/test_inspect.py" + - name: portfolio + paths: "tests/test_portfolio.py" + - name: relationships + paths: "tests/test_relationships.py tests/test_relationships_cmd.py tests/test_relationship_validation.py" + - name: stats + paths: "tests/test_stats.py" steps: - uses: actions/checkout@v4 with: @@ -35,5 +66,5 @@ jobs: # tests/test_ingest.py uses to generate fixtures. python -m pip install -e .[dev] - - name: Run tests - run: python -m pytest -q + - name: Run ${{ matrix.battery.name }} battery + run: python -m pytest -q ${{ matrix.battery.paths }} diff --git a/rac/decisions/adr-019-asset-management.md b/rac/decisions/adr-019-asset-management.md index 1c816ca7..9133029f 100644 --- a/rac/decisions/adr-019-asset-management.md +++ b/rac/decisions/adr-019-asset-management.md @@ -1,4 +1,4 @@ -# ADR-017 Asset References +# ADR-019: Asset References ## Status diff --git a/rac/decisions/adr024-rac-not-content-store.md b/rac/decisions/adr-024-rac-not-content-store.md similarity index 99% rename from rac/decisions/adr024-rac-not-content-store.md rename to rac/decisions/adr-024-rac-not-content-store.md index 91592560..a06670c2 100644 --- a/rac/decisions/adr024-rac-not-content-store.md +++ b/rac/decisions/adr-024-rac-not-content-store.md @@ -1,4 +1,4 @@ -# ADR-016: RAC Is Not a Content Store +# ADR-024: RAC Is Not a Content Store ## Status diff --git a/rac/decisions/adr-025-hybrid-artifact-metadata.md b/rac/decisions/adr-025-hybrid-artifact-metadata.md index e6b0d08c..c410c176 100644 --- a/rac/decisions/adr-025-hybrid-artifact-metadata.md +++ b/rac/decisions/adr-025-hybrid-artifact-metadata.md @@ -1,4 +1,4 @@ -# ADR-XXX Hybrid Artifact Metadata +# ADR-025: Hybrid Artifact Metadata ## Status diff --git a/rac/decisions/adr-026-opaque-artifact-identities.md b/rac/decisions/adr-026-opaque-artifact-identities.md index 000faaad..c9b16b5a 100644 --- a/rac/decisions/adr-026-opaque-artifact-identities.md +++ b/rac/decisions/adr-026-opaque-artifact-identities.md @@ -1,4 +1,4 @@ -# ADR-XXX Opaque System-Assigned Artifact Identity +# ADR-026: Opaque System-Assigned Artifact Identity ## Status diff --git a/rac/decisions/adr-027-ci-test-topology.md b/rac/decisions/adr-027-ci-test-topology.md new file mode 100644 index 00000000..7c384126 --- /dev/null +++ b/rac/decisions/adr-027-ci-test-topology.md @@ -0,0 +1,217 @@ +# ADR-027: CI Test Topology — Merge-Gated, Per-Service Batteries + +## Status + +Accepted + +## Category + +Process + +## Context + +RAC runs its test suite in GitHub Actions through a single reusable workflow, +`.github/workflows/tests.yml`, which is consumed by two callers: + +- `.github/workflows/ci.yml` — continuous integration. +- `.github/workflows/python-publish.yml` — the release/publish pipeline. + +Two aspects of this setup were never written down, and had begun to drift with each +CI edit: + +1. **When tests run.** `ci.yml` triggered on every `pull_request` *and* on pushes to + `main`. Every push to an open PR branch therefore re-ran the entire suite, and the + policy for *when* CI should run was never recorded — so it was liable to be widened + or narrowed incidentally. +2. **How the suite is shaped.** `tests.yml` ran the whole suite as a single `pytest` + job parameterized only by Python version (`["3.11", "3.12", "3.13"]`). The Actions + UI showed only `pytest (3.11)`, `pytest (3.12)`, `pytest (3.13)`. A failure named the + Python version but not the **service** at fault, and the run shape did not reflect + RAC's service-oriented architecture (ADR-008), where each capability is an isolated + `.py` service under `src/rac/services/` plus the `core`, `cli`, and artifact layers. + +The release path already gated publishing on the reusable test workflow +(`release-build: needs: test`), but that gate was likewise an unrecorded convention +rather than a stated policy. + +Absent a recorded decision, each of these choices invites silent regression — a +re-added PR trigger, a collapsed matrix, a release that builds before tests — on the +next person's CI edit. This ADR fixes the CI test topology so future changes are +deliberate. + +## Decision + +RAC's CI test topology is governed by three rules. + +### 1. Tests run on merge to `main`, not on pull requests + +`ci.yml` triggers on `push:` to `main` (a merged PR or a direct push) and on +`workflow_dispatch` (manual). It does **not** trigger on `pull_request`. + +The explicit, accepted consequence: **a pull request receives no automated test +feedback before it merges.** A regression is caught by the post-merge run on `main` +and then blocks the next release through the gate (rule 2), rather than being caught +on the PR itself. `workflow_dispatch` is the escape hatch — the full battery grid can +be run against any branch on demand from the Actions tab. + +Runs on `main` use `concurrency` with `cancel-in-progress: false`, so every merge is +fully tested and a later merge does not cancel an in-flight run. + +### 2. Releases are gated on the full suite + +`python-publish.yml` runs the reusable test workflow as a `test` job and makes +`release-build` depend on it (`needs: test`). Nothing is built or published unless +**every** battery passes. This protects the public contracts a release ships — the CLI +command surface (ADR-005) and the JSON output (ADR-007). + +### 3. The suite is organized as per-service batteries + +`tests.yml` defines a two-axis matrix — supported Python version × **battery** — where +each battery is one `.py` service (`diff`, `improve`, `index`, `ingest`, `inspect`, +`portfolio`, `relationships`, `stats`), plus the grouped `core`, `cli`, and `artifacts` +layers. Each battery runs on every supported Python version (currently 3.11–3.13, per +`pyproject` `requires-python`). Job names surface service and version, e.g. +`relationships (py3.11)`. + +The battery list is an **explicit, static enumeration** in the workflow — not +discovered dynamically — and every `tests/test_*.py` file maps to exactly one battery. + +## Principles + +### Principle 1 — Test topology mirrors the service architecture + +ADR-008 makes each capability a reusable service. CI makes each service a battery, so a +red check names the responsible module instead of an opaque interpreter version. + +### Principle 2 — One reusable workflow is the single source of truth + +`tests.yml` is the only definition of *how* tests run. Both CI and the release gate +consume it, so day-to-day testing and release testing can never diverge. + +### Principle 3 — The matrix is explicit and deterministic + +The battery list is readable in the YAML and changes only by deliberate edit. This is +consistent with RAC's preference for deterministic, inspectable structure over +generated or dynamically discovered behavior. + +### Principle 4 — Trigger policy is recorded, not incidental + +*When* tests run is a decision with real tradeoffs. It lives here so a future CI edit +changes it on purpose, with this context in view, rather than by accident. + +## Consequences + +### Positive + +- A failed check names the service and Python version, so triage starts at the right + module without opening logs. +- The run shape matches the architecture; adding a service is a one-line battery + addition. +- Releases cannot ship on a red suite, and the gate shares one definition with CI. +- PR pushes no longer trigger repeated full-suite runs, reducing Actions usage on + in-flight branches. + +### Negative + +- Pull requests get no pre-merge test signal by default; regressions surface + post-merge on `main`. Mitigated by `workflow_dispatch` and the release gate. +- More jobs per run (batteries × versions ≈ 33). They are short and run in parallel, + but the checks list is longer. +- The battery list must be kept in sync: a new `tests/test_*.py` that is not added to a + battery will not run in CI. Guarded by a coverage check at change time; a CI + self-check could enforce it later. + +## Alternatives Considered + +### Run tests on every pull request + +Keep the `pull_request` trigger so PRs are checked before they merge. + +#### Pros + +- Regressions are caught before landing — standard CI practice. + +#### Cons + +- The stated goal was to stop running the suite on every push to in-flight branches. +- Doubles runs (push + PR) and spends Actions minutes on branches that may be rebased + or abandoned. + +Deferred, not rejected — to be reconsidered if outside contributors need pre-merge +gating (see Review Date). + +### Single job parameterized only by Python version (the prior shape) + +#### Pros + +- Fewer jobs; simplest possible matrix. + +#### Cons + +- A failure shows only the Python version, not the service. +- The run shape ignores the service architecture. + +Rejected — it is exactly the opacity this decision removes. + +### Per-test-file batteries + +One job per `tests/test_*.py`. + +#### Pros + +- Finest possible granularity. + +#### Cons + +- ~19 batteries × 3 versions, with near-duplicates (e.g. three relationship files) and + no service-level grouping; noisier than per-service for no extra signal. + +Rejected. + +### Dynamically discovered matrix + +Generate the battery list from the filesystem at run time. + +#### Pros + +- No manual sync; new test files are included automatically. + +#### Cons + +- A generated matrix is opaque to read and non-deterministic; it contradicts RAC's + preference for explicit, inspectable structure. + +Rejected — the sync cost is instead paid by an explicit list plus a coverage check. + +## Relationship to Other ADRs + +### ADR-008 Agent-Ready Architecture + +Capabilities live in reusable services. The per-service battery topology is the CI +projection of that architecture — one service, one battery. + +### ADR-005 CLI First and ADR-007 JSON Contract Stability + +The CLI and JSON outputs are RAC's public contracts. The release gate (rule 2) exists +so no release ever ships with those contracts broken. + +### ADR-003 Structured Outputs First + +Typed, structured outputs are what make services testable in isolation, which is what +lets the suite split cleanly along service lines. + +## Success Measures + +Evidence that this decision is working: + +- A red CI check identifies the failing service and Python version without opening logs. +- Adding a new service adds exactly one battery entry — a one-line diff. +- No release builds or publishes while any battery is failing. +- Every `tests/test_*.py` belongs to exactly one battery; no test is silently unrun. +- The trigger policy changes only via a deliberate edit to this decision. + +## Review Date + +Review before v1.0.0, or sooner if RAC accepts outside contributors who need pre-merge +test feedback — which would warrant re-adding a `pull_request` trigger (rule 1) — or if +the job count from the battery × version grid becomes burdensome.