diff --git a/AGENTS.md b/AGENTS.md index 787033812..543db993b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,7 +119,7 @@ A feature is not done until integration tests pass and cover the new functionali ### Enforcing Colocation (testing-conventions) -The colocation rule is a blocking CI gate ([`testing-conventions`](https://github.com/thekevinscott/testing-conventions), wired in the per-domain CI workflows + `testing-conventions.toml`): every source file needs a colocated unit test (Rust: inline `#[cfg(test)]`), a PR-only co-change check flags modified sources whose test didn't change, and `unit lint` enforces **isolation** -- a unit test must mock every collaborator (Python `patch(...)`, TS `vi.mock`, Rust trait double or relocate the effectful test), never weaken the test. The gates also cover the two binding crates and the repo-tooling `internals/checks` / `internals/distcheck` uv packages. **The exemption count is zero and stays there**: barrels get a colocated surface test, logic gets extracted, dead shells get deleted -- an exemption is never the escape hatch. Full wiring, per-package gate map, and history: `agents/reference/testing-gates.md`. +The colocation rule is a blocking CI gate ([`testing-conventions`](https://github.com/thekevinscott/testing-conventions), wired in the per-domain CI workflows + `testing-conventions.toml`): every source file needs a colocated unit test (Rust: inline `#[cfg(test)]`), a PR-only co-change check flags modified sources whose test didn't change, and `unit lint` enforces **isolation** -- a unit test must mock every collaborator (Python `patch(...)`, TS `vi.mock`, Rust trait double or relocate the effectful test), never weaken the test. The gates also cover the two binding crates and the repo-tooling `internals/checks` / `internals/distcheck` uv packages. **There is exactly one exemption**: `no-first-party-patch` on the two `packages/python/tests/integration/hermetic/` suites, waived because patching the first-party `_RustDirSQL` core boundary *is* what makes that tier hermetic -- there is no third-party target the rule could redirect it to, so the rule does not describe that seam. Everywhere else the count stays zero and an exemption is never the escape hatch: barrels get a colocated surface test, logic gets extracted, dead shells get deleted. Full wiring, per-package gate map, and history: `agents/reference/testing-gates.md`. **Run every gate CI declares before pushing** with `just preflight` (#781), which *derives* the (source, gate) pairs from the CI workflows instead of restating them -- 40 pairs across 8 scan roots, versus the 6 across 3 the four hand-written recipes it replaced covered. It reads every `.github/workflows/*.yml` and folds together each job that calls the reusable workflow, so a lane added in a new per-domain file is a pair it runs. It also encodes the invocations that differ from the naive one (python suites via the package venv, typescript via `npx`, per-gate option support), each of which used to pass locally while failing in CI. `--dry-run` prints the matrix; `--gate ` and `--conventions ` narrow it (both repeatable); `packaging` reports SKIP since it needs a built artifact (`just test-packaging`). diff --git a/agents/reference/testing-gates.md b/agents/reference/testing-gates.md index 767bd7ea1..36d3c699b 100644 --- a/agents/reference/testing-gates.md +++ b/agents/reference/testing-gates.md @@ -38,7 +38,7 @@ flags. `packaging` is reported **SKIP**: it inspects a built artifact, so use **Exemptions.** The goal is **zero** exemptions, and barrels are no longer an excuse for one. A re-export barrel gets a **colocated test that asserts its public surface** (TS `src/index.ts` ↔ `index.test.ts`, python `dirsql/__init__.py` ↔ `__init___test.py`), exactly as any module would -- it is *tested*, not exempted, so a broken re-export is caught. An `__init__.py` carrying no executable logic is made **truly empty** (0 bytes), which the tool auto-skips with no config entry. A package shell left dead by a feature removal is **deleted**, not parked behind an exemption. When a "barrel" actually holds logic, the fix remains to **extract that logic into colocated-tested modules** (#239). The npm `bin` shim `src/cli/dirsql.ts` is likewise *not* exempt: its error-handling lives in the unit-tested `cli/run-cli.ts`, leaving a trivial `runCli()` shim covered by a mocked distcheck-test. -The exemption count is again **zero**. The three-tier conformance work (#517) restored it: the binding suites moved into the recognized tier `tests/integration/binding/` (#519) and the packaging distcheck flows moved to the `internals/distcheck` package (#520), so nothing trips `unknown-tier` anymore and the temporary `unknown-tier` waivers #518 added are all removed. Before that the last standing entry -- the python barrel-test isolation waiver (`[[python.exempt]] path = "__init___test.py" rules = ["unmocked-collaborator"]`) -- was removed once testing-conventions#382 / PR #384 brought the python `unmocked-collaborator` rule to parity with TS (a bare package-relative import `from . import ` in a barrel test now resolves to the SUT and passes). Exemption entries carry a `path` (relative to the scanned source dir for source-file rules, but the derived package root for the suite-tier `unknown-tier` rule), the `rules` waived, and a required `reason`; the CLI **rejects a stale entry whose `path` matches no file**, so it self-cleans. Adding a *new* untested source file fails the gate -- an exemption is never the escape hatch. +The exemption count is **one**: `no-first-party-patch` waived on `tests/integration/hermetic/dirsql_test.py` and `tests/integration/hermetic/extensions_test.py` in `packages/python/testing-conventions.toml`. Patching the first-party `_RustDirSQL` core boundary is the hermetic tier's definition -- it is why that subdir needs no native build -- and there is no third-party target to redirect the patch to, so the rule has nothing to say about that seam. It is not a way past a real failure: the two stdlib patches reported alongside it (`asyncio.to_thread`, `os.getcwd`, both reached through `dirsql._async`) were re-rooted at their own modules rather than waived, and everything else remains at zero. Before this entry the count was zero. The three-tier conformance work (#517) restored it then: the binding suites moved into the recognized tier `tests/integration/binding/` (#519) and the packaging distcheck flows moved to the `internals/distcheck` package (#520), so nothing trips `unknown-tier` anymore and the temporary `unknown-tier` waivers #518 added are all removed. Before that the last standing entry -- the python barrel-test isolation waiver (`[[python.exempt]] path = "__init___test.py" rules = ["unmocked-collaborator"]`) -- was removed once testing-conventions#382 / PR #384 brought the python `unmocked-collaborator` rule to parity with TS (a bare package-relative import `from . import ` in a barrel test now resolves to the SUT and passes). Exemption entries carry a `path` (relative to the scanned source dir for source-file rules, but the derived package root for `integration lint`'s suite subjects -- both `unknown-tier` and the mocking lints), the `rules` waived, and a required `reason`; the CLI **rejects a stale entry whose `path` matches no file**, so it self-cleans. Adding a *new* untested source file fails the gate -- an exemption is never the escape hatch. ## Mutation The rung above coverage is the **`unit mutation`** gate (#235 / epic #231): testing-conventions mutates the source and fails on any **surviving** mutant -- one no unit test caught. Engines: **cosmic-ray** (Python), **Stryker** (TypeScript), **cargo-mutants** (Rust). It is **PR-only and diff-scoped** (`--base ...HEAD`): only the lines a PR added/modified are mutated, so each PR's surface stays bounded. A PR that changes no SDK source has nothing to mutate and passes trivially. @@ -91,7 +91,7 @@ Keep the job count low enough that the build dirs fit: each *unscoped* job dir r **What the audit found.** Unit scope: 330 caught, **61 survivors**, 75 unviable, 4 timeouts. Re-running those 61 under CI's scoping, **40 are killed by the integration tier** -- the #233 split (effectful filesystem / subprocess / HTTP / signal code lives in the integration tier by design) showing up as mutation survivors rather than gate escapes -- 3 time out, and **18 escape every tier**. Those 18 divide three ways, and the split is what matters for anyone repeating this: - **7 were real gaps and are now closed** -- six operator mutations in `matcher.rs`'s `placeholder_spans` (its own grammar counter-examples were untested) and `lib.rs`'s `prepare_persist` reading `meta_current` as `compatible && lengths match`, where `&&` -> `||` survived. The tests added for them kill 14 mutants at those sites. -- **7 are equivalent mutants, proved rather than argued.** `|` -> `^` over disjoint bit fields is always a no-op: `function_flags`' `SQLITE_UTF8|SQLITE_DETERMINISTIC` (bits `0x1` and `0x800`), `base64_encode`'s `(b0<<16)|(b1<<8)|b2` (exhaustive over all 16,777,216 inputs), and `base64_decode`'s `(triple<<6)|value_of(b)` (all 67,108,864). In `placeholder_spans`, `j += 1` -> `j *= 1` is re-consumed by the following `while`, and `i = j + 1` -> `i = j * 1` / `i = j - 1` land on the closing brace or a name byte, never a `{` -- zero differing results over all 335,923 strings up to length 7 on `{ } a 1 _ -`. **No exemptions were added**: the gate is diff-scoped, so these only surface if someone edits those lines, and the exemption count stays zero. +- **7 are equivalent mutants, proved rather than argued.** `|` -> `^` over disjoint bit fields is always a no-op: `function_flags`' `SQLITE_UTF8|SQLITE_DETERMINISTIC` (bits `0x1` and `0x800`), `base64_encode`'s `(b0<<16)|(b1<<8)|b2` (exhaustive over all 16,777,216 inputs), and `base64_decode`'s `(triple<<6)|value_of(b)` (all 67,108,864). In `placeholder_spans`, `j += 1` -> `j *= 1` is re-consumed by the following `while`, and `i = j + 1` -> `i = j * 1` / `i = j - 1` land on the closing brace or a name byte, never a `{` -- zero differing results over all 335,923 strings up to length 7 on `{ } a 1 _ -`. **No exemptions were added**: the gate is diff-scoped, so these only surface if someone edits those lines, and the `mutation` exemption count stays zero. - **4 sat in effectful code no tier reached** -- `RootedFs::hash` (two), `cli/run.rs`'s `load_state` guard and `wait_for_shutdown`. Tracked as #1012 rather than re-litigating #233 there, and closed by it at the integration and e2e tiers (below). The headline: nothing merged behind the blind gate turned out to be a silent behavior regression. What it hid was **test debt** -- two pure functions whose own documented edge cases had no assertions. diff --git a/packages/python/testing-conventions.toml b/packages/python/testing-conventions.toml index ef8dcff95..8cd029a15 100644 --- a/packages/python/testing-conventions.toml +++ b/packages/python/testing-conventions.toml @@ -16,3 +16,19 @@ one_function_per_file = { max_lines = 100 } [python.coverage] fail_under = 100 branch = true + +# The hermetic integration tier is defined by patching the first-party core +# boundary: `dirsql._async._RustDirSQL` is the PyO3 class, and replacing it is +# what lets these tests run SQLite-free and native-build-free. There is no +# third-party target to re-root onto, so `no-first-party-patch` has nothing to +# say about this seam. Every other patch in both files targets stdlib and stays +# gated. +[[python.exempt]] +path = "tests/integration/hermetic/dirsql_test.py" +rules = ["no-first-party-patch"] +reason = "the hermetic tier IS the _RustDirSQL core-boundary patch; no third-party target exists to move it to" + +[[python.exempt]] +path = "tests/integration/hermetic/extensions_test.py" +rules = ["no-first-party-patch"] +reason = "the hermetic tier IS the _RustDirSQL core-boundary patch; no third-party target exists to move it to" diff --git a/packages/python/tests/integration/hermetic/dirsql_test.py b/packages/python/tests/integration/hermetic/dirsql_test.py index fd705e7d0..ae3c3602b 100644 --- a/packages/python/tests/integration/hermetic/dirsql_test.py +++ b/packages/python/tests/integration/hermetic/dirsql_test.py @@ -104,7 +104,7 @@ async def spy(func, *args, **kwargs): calls.append(getattr(func, "__name__", repr(func))) return await real_to_thread(func, *args, **kwargs) - with patch.object(async_mod.asyncio, "to_thread", spy): + with patch.object(asyncio, "to_thread", spy): yield calls diff --git a/packages/python/tests/integration/hermetic/extensions_test.py b/packages/python/tests/integration/hermetic/extensions_test.py index 401c06abf..128212685 100644 --- a/packages/python/tests/integration/hermetic/extensions_test.py +++ b/packages/python/tests/integration/hermetic/extensions_test.py @@ -59,7 +59,7 @@ def mock_core(): @pytest.fixture def mock_cwd(): """Pin the cwd the SDK resolves programmatic entries against.""" - with patch.object(async_mod.os, "getcwd", return_value="/cwd") as m: + with patch.object(os, "getcwd", return_value="/cwd") as m: yield m