diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a49ebab3..9d6a02e2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,34 @@ jobs: npm --prefix client run build - run: cargo build --workspace --all-features --locked + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: Swatinem/rust-cache@v2 + - uses: actions/setup-node@v7 + with: + node-version: 24 + - name: Build embedded web assets + run: | + npm --prefix ui ci --no-audit --no-fund + npm --prefix ui run build + npm --prefix client ci --no-audit --no-fund + npm --prefix client run build + - uses: taiki-e/install-action@cargo-llvm-cov + - name: Record per-crate coverage + run: > + cargo llvm-cov --workspace --all-features --locked + --ignore-filename-regex '(^|/)(tests|benches|examples)/' + --lcov --output-path target/lcov.info + - name: Enforce the coverage ratchet + run: python3 scripts/coverage_ratchet.py --lcov target/lcov.info + - uses: actions/upload-artifact@v5 + if: always() + with: + name: coverage-lcov + path: target/lcov.info + ui: runs-on: ubuntu-latest strategy: @@ -102,7 +130,7 @@ jobs: container: if: github.event_name == 'push' - needs: [lint, test, build, ui, ui-e2e] + needs: [lint, test, build, coverage, ui, ui-e2e] runs-on: [self-hosted, node-b, linux, x64, publish, docker] permissions: contents: read diff --git a/AGENTS.md b/AGENTS.md index 51045073..29df9afc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,8 @@ business rules in the core. coverage for user-visible flows. - Run `cargo fmt --all -- --check`, `cargo clippy --workspace --all-features -- -D warnings`, and `cargo test --workspace --all-features` before committing. +- Keep `coverage-baseline.json` current. Per-crate coverage is a ratchet: CI + fails when a crate drops below its recorded percentage. See `docs/TESTING.md`. - Do not add an undocumented `#[allow]`. - Do not vendor published crates or add private dependency sources. - Do not add AI co-author trailers or list AI systems as contributors. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a4bd6922..07bd40d5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,6 +45,11 @@ cargo clippy --workspace --all-features -- -D warnings cargo test --workspace --all-features ``` +CI additionally measures line coverage per crate and fails when a crate falls +below the percentage recorded in `coverage-baseline.json`. Run `just coverage` +locally when a change adds code, and `just coverage-update` when the baseline +legitimately moves; both are documented in [docs/TESTING.md](docs/TESTING.md). + When a change affects the UI, compatibility contracts, or container runtime, also run the applicable tier documented in [docs/TESTING.md](docs/TESTING.md). The `justfile` provides the canonical task names as each phase lands. diff --git a/coverage-baseline.json b/coverage-baseline.json new file mode 100644 index 00000000..9170305c --- /dev/null +++ b/coverage-baseline.json @@ -0,0 +1,7 @@ +{ + "metric": "percent of executable lines covered per workspace crate", + "produced_by": "just coverage (cargo llvm-cov --workspace --all-features)", + "policy": "coverage may rise; it may not fall. See docs/TESTING.md.", + "tolerance_percent": 0.1, + "crates": {} +} diff --git a/docs/TESTING.md b/docs/TESTING.md index 3e9bf8bf..74b0b4ce 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -74,3 +74,30 @@ cargo test --workspace --all-features Coverage is a per-crate ratchet: a change may improve coverage but may not lower the checked-in baseline without an explicit, reviewed justification. + +## Coverage ratchet + +`coverage-baseline.json` records line coverage for every workspace crate. The +CI `coverage` job regenerates the numbers with `cargo llvm-cov` and fails when +any crate falls below its recorded percentage, so a change that adds untested +code has to add tests with it. + +```bash +just coverage +``` + +The recipe needs `cargo-llvm-cov` (`cargo install cargo-llvm-cov`). Integration +tests, benches, and examples are excluded from the measurement; they exercise +library code rather than being library code. The baseline allows a 0.1 point +tolerance so instrumentation noise does not fail a build. + +When a change legitimately moves the numbers — new tests, or a deletion that +removes well-covered code — re-record the baseline and include the diff in +review: + +```bash +just coverage-update +``` + +A crate that is missing from the baseline fails the job rather than passing +silently, so a new workspace member cannot land unrecorded. diff --git a/docs/UNIFIED-ARR-PLAN.md b/docs/UNIFIED-ARR-PLAN.md index 54095e63..81474535 100644 --- a/docs/UNIFIED-ARR-PLAN.md +++ b/docs/UNIFIED-ARR-PLAN.md @@ -507,7 +507,10 @@ specification *already exists in machine-readable form*. 6. **Gates are non-negotiable.** `fmt --check`, `clippy -- -D warnings`, `test --workspace` block merge. No exceptions, no `#[allow]` without a comment naming the reason. 7. **Coverage as a ratchet, not a target.** Record it per crate; the number may not go - down. `coverage-watchdog` already exists in this workspace — wire it in. + down. Landed: `cargo llvm-cov` feeds `scripts/coverage_ratchet.py`, which compares + every crate against `coverage-baseline.json`. The workspace's `coverage-watchdog` + app is not the tool for this — it tracks indexer *catalogue* coverage against TMDB, + not test coverage. 8. **A `justfile`**, mirroring rdpapp: `check`, `test`, `test-compat`, `test-e2e`, `conformance`, `smoke`. @@ -526,7 +529,8 @@ than the abandoned local branch suggested — the migration to GitHub happened o 2. **Self-hosted only.** Every job requires the `node-b` runner. An outside contributor cannot get CI on a fork — which defeats the adoption argument for being public at all. At minimum the `rust` and `ui` jobs should run on `ubuntu-latest`. -3. **No coverage ratchet.** `coverage-watchdog` exists in the workspace and is unused here. +3. ~~**No coverage ratchet.**~~ Closed: the `coverage` job records per-crate line + coverage and fails when a crate drops below `coverage-baseline.json`. 4. **No multi-arch build.** linux/arm64 and musl static builds matter for the NAS audience. **Target pipeline** (`.github/workflows/ci.yml`): diff --git a/justfile b/justfile index df33a51b..5679ccce 100644 --- a/justfile +++ b/justfile @@ -19,6 +19,16 @@ check: web-assets test: web-assets cargo test --workspace --all-features --locked +# Record per-crate coverage and enforce the ratchet against coverage-baseline.json. +coverage: web-assets + cargo llvm-cov --workspace --all-features --locked --ignore-filename-regex '(^|/)(tests|benches|examples)/' --lcov --output-path target/lcov.info + python3 scripts/coverage_ratchet.py --lcov target/lcov.info + +# Re-record the baseline after a change that legitimately moves coverage. +coverage-update: web-assets + cargo llvm-cov --workspace --all-features --locked --ignore-filename-regex '(^|/)(tests|benches|examples)/' --lcov --output-path target/lcov.info + python3 scripts/coverage_ratchet.py --lcov target/lcov.info --update + # P2 replaces this smoke assertion with generated façade contract tests. test-compat: test -f docs/API-COMPATIBILITY.md diff --git a/scripts/coverage_ratchet.py b/scripts/coverage_ratchet.py new file mode 100644 index 00000000..1fe20628 --- /dev/null +++ b/scripts/coverage_ratchet.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Per-crate line-coverage ratchet. + +Reads an LCOV report, attributes every covered line to the workspace member +that owns the file, and compares the result against the checked-in baseline in +`coverage-baseline.json`. Coverage may rise; it may not fall. Regenerate the +baseline with `--update` and review the diff like any other change. +""" + +from __future__ import annotations + +import argparse +import collections +import json +import pathlib +import sys +import tomllib + + +ROOT = pathlib.Path(__file__).resolve().parents[1] +BASELINE = ROOT / "coverage-baseline.json" +NON_LIBRARY_DIRS = {"tests", "benches", "examples"} + + +def fail(message: str) -> None: + sys.stdout.flush() + print(f"coverage ratchet: {message}", file=sys.stderr) + raise SystemExit(1) + + +def package_name(manifest_dir: pathlib.Path) -> str: + manifest = tomllib.loads((manifest_dir / "Cargo.toml").read_text()) + return manifest["package"]["name"] + + +def workspace_packages() -> dict[str, pathlib.Path]: + """Map every workspace package name to its directory, root package included. + + The root package owns `src/` only, so that files it does not compile — the + `crates/` members above all — are never attributed to it. + """ + manifest = tomllib.loads((ROOT / "Cargo.toml").read_text()) + packages = {manifest["package"]["name"]: ROOT / "src"} + for member in manifest["workspace"]["members"]: + directory = ROOT / member + packages[package_name(directory)] = directory + return packages + + +def owning_package(path: pathlib.Path, packages: dict[str, pathlib.Path]) -> str | None: + """Return the package owning `path`, or None for files outside library code.""" + owner: str | None = None + owned: pathlib.Path | None = None + depth = -1 + for name, directory in packages.items(): + try: + relative = path.relative_to(directory) + except ValueError: + continue + if len(directory.parts) > depth: + owner, owned, depth = name, relative, len(directory.parts) + if owned is None or set(owned.parts[:-1]) & NON_LIBRARY_DIRS: + return None + return owner + + +def parse_lcov(report: pathlib.Path) -> dict[pathlib.Path, dict[int, int]]: + """Merge an LCOV report into hit counts per source line.""" + hits: dict[pathlib.Path, dict[int, int]] = collections.defaultdict(dict) + current: dict[int, int] | None = None + for raw in report.read_text().splitlines(): + line = raw.strip() + if line.startswith("SF:"): + source = pathlib.Path(line[3:]) + if not source.is_absolute(): + source = ROOT / source + current = hits[pathlib.Path(source.as_posix())] + elif line.startswith("DA:") and current is not None: + number, _, count = line[3:].partition(",") + executions = int(count.split(",")[0]) + key = int(number) + current[key] = max(current.get(key, 0), executions) + elif line == "end_of_record": + current = None + return hits + + +def measure(report: pathlib.Path, packages: dict[str, pathlib.Path]) -> dict[str, dict]: + """Aggregate an LCOV report into per-package line coverage.""" + totals = {name: [0, 0] for name in packages} + for source, lines in parse_lcov(report).items(): + owner = owning_package(source, packages) + if owner is None: + continue + totals[owner][0] += len(lines) + totals[owner][1] += sum(1 for executions in lines.values() if executions > 0) + return { + name: { + "lines": lines, + "covered": covered, + # A crate the report says nothing about is a measurement gap, not a + # perfect score: record 0 so the ratchet can only be raised by real + # numbers arriving later. + "percent": round(100.0 * covered / lines, 2) if lines else 0.0, + } + for name, (lines, covered) in sorted(totals.items()) + } + + +def report_table(measured: dict[str, dict], baseline: dict[str, dict] | None) -> None: + width = max(len(name) for name in measured) + for name, entry in measured.items(): + recorded = (baseline or {}).get(name) + delta = "" + if recorded is not None: + delta = f" ({entry['percent'] - recorded['percent']:+.2f})" + print( + f" {name:<{width}} {entry['percent']:6.2f}%" + f" {entry['covered']}/{entry['lines']} lines{delta}" + ) + + +def check(measured: dict[str, dict], document: dict) -> list[str]: + baseline = document["crates"] + tolerance = document["tolerance_percent"] + problems = [] + for name in sorted(baseline.keys() - measured.keys()): + problems.append(f"{name} is recorded in the baseline but absent from the report") + for name in sorted(measured.keys() - baseline.keys()): + problems.append(f"{name} has no recorded baseline; rerun with --update") + for name in sorted(baseline.keys() & measured.keys()): + recorded = baseline[name]["percent"] + current = measured[name]["percent"] + if current < recorded - tolerance: + problems.append( + f"{name} dropped from {recorded:.2f}% to {current:.2f}% " + f"(tolerance {tolerance:.2f} points)" + ) + return problems + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--lcov", type=pathlib.Path, required=True, help="LCOV report to read") + parser.add_argument( + "--update", + action="store_true", + help="rewrite the baseline from this report instead of checking it", + ) + arguments = parser.parse_args() + + if not arguments.lcov.is_file(): + fail(f"{arguments.lcov} does not exist; generate it with `just coverage`") + + packages = workspace_packages() + measured = measure(arguments.lcov, packages) + document = json.loads(BASELINE.read_text()) + + if arguments.update: + report_table(measured, document["crates"]) + document["crates"] = measured + BASELINE.write_text(json.dumps(document, indent=2) + "\n") + print(f"coverage baseline updated: {len(measured)} crates recorded") + return + + report_table(measured, document["crates"]) + problems = check(measured, document) + if problems: + fail("coverage may not go down\n " + "\n ".join(problems)) + print(f"coverage ratchet held: {len(measured)} crates at or above baseline") + + +if __name__ == "__main__": + main()