diff --git a/.github/workflows/dirsql-python-ci.yml b/.github/workflows/dirsql-python-ci.yml index 0df152c44..cbffde713 100644 --- a/.github/workflows/dirsql-python-ci.yml +++ b/.github/workflows/dirsql-python-ci.yml @@ -2,6 +2,10 @@ name: dirsql Python CI # packages/rust/** fans in: the core compiles into the binding, and # [e2e].extra_scope means a core change stales this package's attestation. +# +# The gate config is packages/python/testing-conventions.toml, not the root file: +# [e2e] is per-config, so this lane's freshness scope (core + PyO3 glue) lives +# with the package. No job here reads the root file, so it is not a trigger. on: push: branches: [main] @@ -10,7 +14,6 @@ on: - 'packages/rust/**' - 'Cargo.toml' - 'Cargo.lock' - - 'testing-conventions.toml' - '.github/workflows/dirsql-python-ci.yml' pull_request: paths: @@ -24,7 +27,6 @@ on: - '!packages/rust/README.md' - 'Cargo.toml' - 'Cargo.lock' - - 'testing-conventions.toml' - '.github/workflows/dirsql-python-ci.yml' concurrency: @@ -40,7 +42,7 @@ jobs: with: languages: '["python"]' source: packages/python/dirsql - config: testing-conventions.toml + config: packages/python/testing-conventions.toml base: ${{ github.event.pull_request.base.sha || 'origin/main' }} rust-python-binding: @@ -48,7 +50,7 @@ jobs: with: languages: '["rust"]' source: packages/python - config: testing-conventions.toml + config: packages/python/testing-conventions.toml base: ${{ github.event.pull_request.base.sha || 'origin/main' }} gates: '["colocated-test", "unit-lint"]' diff --git a/.github/workflows/dirsql-typescript-ci.yml b/.github/workflows/dirsql-typescript-ci.yml index b2bd5bef3..fba2240c7 100644 --- a/.github/workflows/dirsql-typescript-ci.yml +++ b/.github/workflows/dirsql-typescript-ci.yml @@ -2,6 +2,10 @@ name: dirsql TypeScript CI # packages/rust/** fans in: the core compiles into the binding, and # [e2e].extra_scope means a core change stales this package's attestation. +# +# The gate config is packages/ts/testing-conventions.toml, not the root file: +# [e2e] is per-config, so this lane's freshness scope (core + napi binding) lives +# with the package. No job here reads the root file, so it is not a trigger. on: push: branches: [main] @@ -10,7 +14,6 @@ on: - 'packages/rust/**' - 'Cargo.toml' - 'Cargo.lock' - - 'testing-conventions.toml' - '.github/workflows/dirsql-typescript-ci.yml' pull_request: paths: @@ -24,7 +27,6 @@ on: - '!packages/rust/README.md' - 'Cargo.toml' - 'Cargo.lock' - - 'testing-conventions.toml' - '.github/workflows/dirsql-typescript-ci.yml' concurrency: @@ -40,7 +42,7 @@ jobs: with: languages: '["typescript"]' source: packages/ts/src - config: testing-conventions.toml + config: packages/ts/testing-conventions.toml base: ${{ github.event.pull_request.base.sha || 'origin/main' }} rust_toolchain: true @@ -49,7 +51,7 @@ jobs: with: languages: '["rust"]' source: packages/ts/napi - config: testing-conventions.toml + config: packages/ts/testing-conventions.toml base: ${{ github.event.pull_request.base.sha || 'origin/main' }} gates: '["colocated-test", "unit-lint"]' diff --git a/AGENTS.md b/AGENTS.md index e407dbaec..86fa6d151 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -193,7 +193,7 @@ For docs/lint/typo-only PRs, include the section with a single line: `N/A - docs ### E2E Attestation -CI never runs the e2e suites; instead `conventions.yml`'s `e2e-verify` gate checks, per package, a committed attestation (`packages/python/e2e-attestation.json`, `packages/ts/e2e-attestation.json`) recording that the suite ran against the package's current code. **Regenerate the attestation for each package you changed, as the last commit touching that package before you push**: `just e2e-attest-python` / `just e2e-attest-ts` (needs `pip install testing-conventions`). Multi-package PRs: attest each package right after finishing *its* changes -- attesting both at the very end leaves the second one stale. **Any `packages/rust/src` change (including `cli/`) stales BOTH bindings' attestations** (CI-enforced via `[e2e].extra_scope`), so re-attest both after core changes. Full mechanics and debugging: `agents/reference/e2e-attestation.md`. +CI never runs the e2e suites; instead `conventions.yml`'s `e2e-verify` gate checks, per package, a committed attestation (`packages/python/e2e-attestation.json`, `packages/ts/e2e-attestation.json`) recording that the suite ran against the package's current code. **Regenerate the attestation for each package you changed, as the last commit touching that package before you push**: `just e2e-attest-python` / `just e2e-attest-ts` (needs `pip install testing-conventions`). Multi-package PRs: attest each package right after finishing *its* changes -- attesting both at the very end leaves the second one stale. **Any `packages/rust/src` change (including `cli/`) stales BOTH bindings' attestations** (CI-enforced via `[e2e].extra_scope`), so re-attest both after core changes. **A binding-crate change stales only its own package** -- `packages/python/src` (PyO3) the python attestation, `packages/ts/napi` the ts one (#933); that per-package scope is why each SDK package carries its own `testing-conventions.toml`. Full mechanics and debugging: `agents/reference/e2e-attestation.md`. ### Docs as Reference diff --git a/agents/environments/remote.md b/agents/environments/remote.md index 5a85ae494..93c37f62f 100644 --- a/agents/environments/remote.md +++ b/agents/environments/remote.md @@ -47,7 +47,7 @@ Do **NOT** hardcode `/home/duncan/...`. Use `$PWD` or the actual sandbox root (e The testing-conventions gates are the trap here: `pip install testing-conventions` cannot build in the sandbox, so `uvx testing-conventions ` is the natural reach -- and for the **python mutation** gate it is wrong. `uvx` runs the tool from its own environment, so cosmic-ray's `python3 -m pytest` resolves to an interpreter with no pytest instead of `packages/python/.venv`, and the gate dies on a baseline failure that names no cause (#706). Run that one through the project venv instead: ```bash -cd packages/python && uv run --with testing-conventions testing-conventions unit mutation --language python --base origin/main --config ../../testing-conventions.toml dirsql +cd packages/python && uv run --with testing-conventions testing-conventions unit mutation --language python --base origin/main --config testing-conventions.toml dirsql ``` `uvx` is fine for the gates that do not execute the python suite (`unit colocated-test`, `unit lint`, `e2e verify`, `e2e attest`). Full detail: `agents/reference/testing-gates.md`. diff --git a/agents/reference/e2e-attestation.md b/agents/reference/e2e-attestation.md index d312b3d85..73bf74e6d 100644 --- a/agents/reference/e2e-attestation.md +++ b/agents/reference/e2e-attestation.md @@ -5,7 +5,7 @@ Extracted from AGENTS.md (see "E2E Attestation" there for the summary). CI does not run the e2e suites -- they need real binaries, and some need live LLM calls -- but it enforces, **per package**, that they *were* run against that package's current code. Each SDK package carries its own attestation at its root -- `packages/python/e2e-attestation.json` and `packages/ts/e2e-attestation.json` -- recording (via [`testing-conventions`](https://github.com/thekevinscott/testing-conventions)) the e2e command, its exit code, and the commit it ran against. The freshness check runs **inside the reusable workflow** (`conventions.yml`, the `python-sdk` / `typescript-sdk` `e2e-verify` gate): `e2e verify "$PACKAGE_ROOT" --scope "$SCAN_PATH" --base "$BASE"` measures freshness over the **`base..HEAD` scoped-source diff**, so a PR that does not touch the SDK source has nothing to verify. It is a **freshness gate, not a test runner** -- no suite, no build, and no LLM run in CI, so it does not violate the E2E Test Policy above. The bespoke `e2e-attestation.yml` this replaced is deleted (whole-hog adoption -- the reusable `e2e-verify` gate owns per-package freshness). -The `--scope` + `--base` diff-scoping makes the gate per-SDK by construction: a change under `packages/python/dirsql` stales only the python attestation, a change under `packages/ts/src` only the ts one, and a PR that does not touch a package's scoped source never verifies it. +The `--scope` + `--base` diff-scoping makes the gate per-SDK by construction: a change under `packages/python/dirsql` stales only the python attestation, a change under `packages/ts/src` only the ts one, and a PR that does not touch a package's scoped source never verifies it. Each package's `[e2e].extra_scope` widens *its own* scope by the code compiled into it (below) without breaking that property. **Regenerate the attestation for each package you changed**, as the last commit touching that package before you push. From the repo root: @@ -18,6 +18,13 @@ just e2e-attest-ts # cd packages/ts && testing-conventions e2e attest 'pnp **Multi-package PRs:** because `attest` records `HEAD`, attest each package right after finishing *its* changes (complete + attest python, then complete + attest ts). Attesting both only at the very end leaves whichever you attest second naming the other's attestation commit -- outside its subtree -- which verify rejects. -**Shared-core changes stale both bindings -- CI-enforced via `[e2e].extra_scope`.** The shared Rust core (`packages/rust/src`) is compiled into both bindings but lives in neither binding's subtree, and `e2e verify --scope` requires the scope to be a **descendant of the caller's `path`** -- so a binding caller cannot point its freshness `--scope` at the core. That case is solved by the `e2e verify --extra-scope` / `--exclude` flags (upstream #333): `testing-conventions.toml`'s `[e2e]` block declares `extra_scope = ["packages/rust/src"]`, `detect.py` reads it, and the reusable `e2e-verify` job passes it through -- so **any `packages/rust/src` change now stales BOTH bindings' attestations in CI** and the gate demands re-attestation. Pure config; the former bespoke `e2e_core_freshness.py` gate is not needed (proven: over a `packages/rust/src`-only diff, `e2e verify` is exit 0 without `--extra-scope`, exit 1 with it). **After any shared-core change, re-attest each binding** (`just e2e-attest-python`, `just e2e-attest-ts`) -- CI now enforces this, it is no longer a by-hand-only promise. **`cli`-only** core source (`packages/rust/src/cli/**`, `packages/rust/src/bin/**`) is feature-gated out of the binding *libraries*, but the language packages ship the compiled `dirsql` binary and their e2e suites spawn it -- a CLI change alters what those suites exercise, so it stales the attestations like any other core change (no `exclude` carve-out). +**Everything compiled into the binding is in scope -- CI-enforced via `[e2e].extra_scope`.** The shared Rust core (`packages/rust/src`) is compiled into both bindings but lives in neither binding's subtree, and `e2e verify --scope` requires the scope to be a **descendant of the caller's `path`** -- so a binding caller cannot point its freshness `--scope` at the core. That case is solved by the `e2e verify --extra-scope` / `--exclude` flags (upstream #333): an `[e2e]` block declares the extra roots, `detect.py` reads it, and the reusable `e2e-verify` job passes it through. Pure config; the former bespoke `e2e_core_freshness.py` gate is not needed. Two roots per binding: + +- **The core, `packages/rust/src` -- stales BOTH bindings** (#337). **After any shared-core change, re-attest each binding** (`just e2e-attest-python`, `just e2e-attest-ts`) -- CI enforces this, it is no longer a by-hand-only promise. **`cli`-only** core source (`packages/rust/src/cli/**`, `packages/rust/src/bin/**`) is feature-gated out of the binding *libraries*, but since #721 each launcher calls the core's `run_cli` **in-process through its binding** -- a CLI change alters what those suites exercise, so it stales the attestations like any other core change (no `exclude` carve-out). +- **The binding crate itself -- stales ONLY its own package** (#933): `packages/python/src` (the PyO3 glue) for python, `packages/ts/napi` (the napi-rs crate, manifest and `build.rs` included) for typescript. Same #721 argument one layer out: `run_cli` in each binding does argv framing, GIL detach / exit-code handling, and runs in every e2e case, so a binding-only diff changes what the suites exercise with no core change at all. A napi change has no bearing on the python suite and does not stale it, and vice versa. + +**That per-package split is why `packages/python` and `packages/ts` each carry their own `testing-conventions.toml`** (passed explicitly as the lane's `config:`, the pattern `internals/checks` established in #550). `[e2e]` is a single global table applied to every caller reading a given config, so "core + *my* binding" is not expressible in one shared file: declaring both bindings at the repo root would make a napi change demand a python re-attest (and force `packages/ts/napi/**` into the python lane's path triggers to enforce it at all). The root `[e2e]` block stays as the default for a caller with no config of its own. + +Proven over synthetic single-root diffs (`e2e verify packages/ --scope --base main`): a `packages/ts/napi`-only diff is exit 0 with the core root alone and exit 1 once `--extra-scope packages/ts/napi` is added, while the python lane's flags stay exit 0 over that same diff; the mirror holds for `packages/python/src`. CI installs the latest `testing-conventions` release (unpinned); install it locally before attesting: `pip install testing-conventions`. diff --git a/agents/reference/testing-gates.md b/agents/reference/testing-gates.md index 5d7c94be9..567d284d6 100644 --- a/agents/reference/testing-gates.md +++ b/agents/reference/testing-gates.md @@ -50,7 +50,7 @@ Run a language locally (after building its native artifact), against your PR's b ```bash # from packages/python -- `uv run` is REQUIRED, see below -cd packages/python && uv run --with testing-conventions testing-conventions unit mutation --language python --base origin/main --config ../../testing-conventions.toml dirsql +cd packages/python && uv run --with testing-conventions testing-conventions unit mutation --language python --base origin/main --config testing-conventions.toml dirsql # from packages/ts (after pnpm build) npx -y testing-conventions unit mutation --language typescript --base origin/main src # from repo root (the tool provisions cargo-mutants itself) @@ -68,10 +68,10 @@ The message names no cause because cosmic-ray hands back an empty `output` for t The Rust arm handles `packages/rust` being a member of the repo-root cargo workspace: testing-conventions ≥ 0.0.88 rebases the diff it feeds cargo-mutants onto the workspace root (dirsql#659, fixed upstream in thekevinscott/testing-conventions#467), and `[rust].features` in `testing-conventions.toml` supplies `--features cli` to the engine's build. A passing run states its evidence — `(N mutant(s) tested)` with N > 0 whenever the diff touched mutatable Rust lines; treat a `0 mutant(s) tested` pass on a diff that changed Rust source as a bug, not a pass. -**Survivors.** The fix is almost always a **new assertion** that kills the mutant. Only a genuinely *equivalent* or intentionally-defensive mutant is lifted, via a `[[.exempt]]` entry in `testing-conventions.toml` whose `rules` includes `"mutation"` (with a real `path` and a `reason`) -- never weaken a test to make a survivor pass. There are none today. +**Survivors.** The fix is almost always a **new assertion** that kills the mutant. Only a genuinely *equivalent* or intentionally-defensive mutant is lifted, via a `[[.exempt]]` entry whose `rules` includes `"mutation"` -- in the config that lane actually reads (`packages/python/testing-conventions.toml` / `packages/ts/testing-conventions.toml` since #933; the root file for the rust core) (with a real `path` and a `reason`) -- never weaken a test to make a survivor pass. There are none today. ## Coverage Floor -Coverage enforcement must stay explicit in CI for each SDK package. All three SDKs are now enforced by [`testing-conventions`](https://github.com/thekevinscott/testing-conventions) `unit coverage`; the per-package floors live in `testing-conventions.toml`: +Coverage enforcement must stay explicit in CI for each SDK package. All three SDKs are now enforced by [`testing-conventions`](https://github.com/thekevinscott/testing-conventions) `unit coverage`; the per-package floors live in the config each lane reads -- `packages/python/testing-conventions.toml` and `packages/ts/testing-conventions.toml` since #933, the root `testing-conventions.toml` for the rust core: - **Python / TypeScript** run full tree + a PR-only `--base` changed-lines check, now **inside the reusable workflow** (`conventions.yml`, the `python-sdk` / `typescript-sdk` `unit-coverage` gate) after upstream #284 taught the coverage suite job to install/build from the derived package root (python via `python_env=uv` → `uv sync` builds maturin; ts via `build_command: pnpm build`). This retired the bespoke coverage jobs in `python-test.yml` / `ts-test.yml` (#412). Floors: `[python.coverage]` = `fail_under` / `branch`; `[typescript.coverage]` = `lines` / `branches` / `functions` / `statements` -- both held at the stricter **100%** (every line unit-reachable). - **Rust core** (#295) is still measured bespoke -- unit-only via `cargo llvm-cov --lib --features cli --branch`, wired into `rust-test.yml`'s coverage job (nightly toolchain for `--branch`, scoped to that job's step alone -- a **permanent** bespoke exception, not pending upstream: the crate-root `rust-toolchain.toml` the reusable job would need breaks release cross-compile, see *Adoption state* above / #437). The CLI now scopes to `--lib` and passes `[rust].features` through (testing-conventions #269/#270/#271). Floors in `[rust.coverage]`: `lines` **94**, `regions` **93**, `functions` **91**, `branch` **75**. #354 first added pure unit tests for the reachable unit-tier gaps (`cli/router.rs`'s `parse_sql_body`, `cli/mod.rs`'s `with_query_timeout`/`From`, `cli/init.rs`'s error `Display` arms), lifting measured unit-only coverage to lines 94.1% / regions 93.2% / functions 91.6% / branch 75.6%; the floors are then set as high as they robustly go -- the highest integer under each actual. `lines`/`regions`/`functions` are above the ≥90% ideal. **`branch` keeps a documented sub-90 floor as an accepted exception** to the ≥90% rule, for two structural reasons. **These floors are pinned near their actuals (<1pt slack) by maintainer request**, so the whole-tree gate goes red on any PR that lowers a dimension -- most fragile for `branch`, where merely adding well-formed unit tests can lower the number (reason (2)); the fix when that happens is to nudge the affected floor down to the new actual, not to chase the dip as a regression. (1) The effectful production branches (filesystem / subprocess / HTTP / `notify`) live in the integration/e2e tier by design (#233): `cli/router.rs`'s async HTTP handlers (0% branch), `cli/init.rs`'s effectful `std::fs::write` success/`--force` arms, and `lib.rs`'s racy-window / watcher / persist error arms are unreachable from a unit test, and covering them re-litigates #233. (2) `--branch` instruments the crate *with its `#[cfg(test)]` modules*, so every `matches!(…, if a && b)` / `assert!(x && y)` guard in a unit test contributes short-circuit sub-condition branches whose False arm is unreachable by construction (the assertion is written to pass) -- `differ.rs` is the clearest case: 100% line/region/function coverage but 58% branch, with every uncovered branch inside a test-module guard, not production code. Because well-formed new unit tests therefore tend to *lower* branch %, the branch floor stays conservative on purpose -- a tight floor would fail legitimate test-adding PRs. The Rust job is **whole-tree only** (no `--base` changed-lines check) for the same reason -- a per-PR changed-lines floor would fail legitimate integration-tier edits. diff --git a/packages/python/testing-conventions.toml b/packages/python/testing-conventions.toml new file mode 100644 index 000000000..40bf0dd6b --- /dev/null +++ b/packages/python/testing-conventions.toml @@ -0,0 +1,15 @@ +# Per-package config so `[e2e]` can be per-package: it is one global table +# applied to every caller reading a given config, so "the core plus MY binding" +# has nowhere else to live. Folded back into the root file, a napi change would +# start staling this package. +# +# The PyO3 glue is in scope because the console script runs the CLI in-process +# through it (#721), so every e2e case crosses it. packages/ts/napi is not: it +# cannot reach this suite. +[e2e] +extra_scope = ["packages/rust/src", "packages/python/src"] + +# Same floor as the root file, which this lane no longer reads. +[python.coverage] +fail_under = 100 +branch = true diff --git a/packages/ts/testing-conventions.toml b/packages/ts/testing-conventions.toml new file mode 100644 index 000000000..b7c773bd1 --- /dev/null +++ b/packages/ts/testing-conventions.toml @@ -0,0 +1,18 @@ +# Per-package config so `[e2e]` can be per-package: it is one global table +# applied to every caller reading a given config, so "the core plus MY binding" +# has nowhere else to live. Folded back into the root file, a PyO3 change would +# start staling this package. +# +# The napi crate is in scope because the npm launcher runs the CLI in-process +# through it (#721), so every e2e case crosses it -- the whole crate dir, since +# Cargo.toml and build.rs decide what lands in the addon. packages/python/src is +# not: it cannot reach this suite. +[e2e] +extra_scope = ["packages/rust/src", "packages/ts/napi"] + +# Same floors as the root file, which this lane no longer reads. +[typescript.coverage] +lines = 100 +functions = 100 +statements = 100 +branches = 100 diff --git a/testing-conventions.toml b/testing-conventions.toml index 1ed502573..b97f39d5a 100644 --- a/testing-conventions.toml +++ b/testing-conventions.toml @@ -1,10 +1,10 @@ # testing-conventions CLI config (https://github.com/thekevinscott/testing-conventions). # -# Drives the testing-conventions gates wired up in the reusable-workflow calls -# in .github/workflows/conventions.yml (colocated-test + unit lint + -# integration-lint), the per-package `unit coverage` jobs in python-test.yml / -# ts-test.yml / rust-test.yml, and the per-language `unit mutation` jobs in -# {python,ts,rust}-mutation.yml. The CLI reads this file by default. +# The repo-wide DEFAULT config: the CLI reads this file when a caller names no +# other, which today means the rust core lane (dirsql-rust-ci.yml) and +# internals/distcheck. Lanes carrying their own `testing-conventions.toml` +# (packages/python, packages/ts, internals/checks, the embeddings plugin) do not +# read it, and restate the tables their gates need. # # The `[[.exempt]]` array lists files waived from a rule (the colocation # gate fails on any source file lacking a colocated unit test that is not @@ -64,17 +64,24 @@ # shipped as #333). Proven locally: over a `packages/rust/src`-only diff, # `e2e verify` is exit 0 without `--extra-scope` and exit 1 with it. # -# `cli`-only core (`src/cli`, `src/bin`) is feature-gated out of the binding -# *libraries*, but the language packages SHIP the compiled `dirsql` binary and -# their e2e suites spawn it -- so a CLI change alters what those suites -# exercise just as a core change does. No carve-out: every -# `packages/rust/src` change stales both bindings' attestations. +# `cli`-only core (`src/cli`, `src/bin`) is NOT carved out. It is feature-gated +# out of the binding *libraries*, but since #721 each launcher calls the core's +# `run_cli` in-process through its binding, so a CLI change alters what those +# suites exercise just as any other core change does. Every `packages/rust/src` +# change stales both bindings' attestations. +# +# Neither binding reads this block since #933: each needs the core PLUS its own +# glue, which only a per-package config can express, so both SDK lanes pass +# their own `packages//testing-conventions.toml`. It stays as the default, +# so a caller that declares no config still inherits the core root. [e2e] extra_scope = ["packages/rust/src"] # --- Python (scanned: packages/python/dirsql) ------------------------------ # Unit-only coverage floor (branch coverage on, matching the prior --cov-branch). +# The default for a config-less python caller (internals/distcheck); the +# packages/python lane holds the identical floor in its own config. [python.coverage] fail_under = 100 branch = true