diff --git a/.github/BRANCH_PROTECTION.md b/.github/BRANCH_PROTECTION.md index 77891ae..6e5ca5d 100644 --- a/.github/BRANCH_PROTECTION.md +++ b/.github/BRANCH_PROTECTION.md @@ -40,5 +40,7 @@ Their guards fail **loudly** (non-zero) once the lane is half-present — e.g. a filename is how a required check ends up green over nothing; the skip branch is only for a lane that is genuinely absent. -`conformance` no longer has a skip branch. The suite (`tests/conformance/run.sh`) has -landed, so the job runs it unconditionally and fails if the entrypoint is missing. +`conformance` has no skip branch. It runs `ctest -L conformance` (see +`docs/adr/0014-conformance-runs-through-ctest.md`); the `dev` test preset's +`noTestsAction: error` fails the job if that label ever matches nothing, so a deleted or +renamed registration goes red instead of passing vacuously. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6451a3d..04446fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,7 +131,10 @@ jobs: - name: Build run: cmake --build --preset ci - name: Test - run: ctest --preset ci --output-on-failure + # -E '^conformance$' drops the end-to-end run.sh test (npm installs, its own + # mockd, ~24 s); the dedicated `conformance` job owns that one run. The native + # `conformance_cpp` test is not excluded and still runs on every matrix leg. + run: ctest --preset ci --output-on-failure -E '^conformance$' sanitizers: runs-on: ubuntu-latest @@ -148,7 +151,10 @@ jobs: - name: Build run: cmake --build --preset ${{ matrix.preset }} - name: Test - run: ctest --preset ${{ matrix.preset }} --output-on-failure + # See the build job: the end-to-end run.sh test is the dedicated `conformance` + # job's; sanitizing a suite that shells out to its own unsanitized g++ build and + # a node process buys nothing. `conformance_cpp` still runs here under the sanitizer. + run: ctest --preset ${{ matrix.preset }} --output-on-failure -E '^conformance$' env: ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 @@ -190,24 +196,27 @@ jobs: # against the other. Required on every PR — branch protection is a repo setting, # recorded in .github/BRANCH_PROTECTION.md. # - # The suite ships as tests/conformance/run.sh (its own mockd, its own C++ build). - # There is no CMakeLists or package.json to guard on, and no ctest label to select; - # the earlier guard for those was always false, so this job passed running nothing. + # Canonical entry point is `ctest -L conformance`. tests/conformance/CMakeLists.txt + # (owned by PROTO) registers two tests under that label: `conformance`, which shells + # out to run.sh end to end, and `conformance_cpp`, the finer-grained native runner. + # CI drives it exactly as a developer does — one definition of "the suite passed", + # and PROTO's registration is on the exercised path so it cannot rot. See + # docs/adr/0014-conformance-runs-through-ctest.md. + # + # `noTestsAction: error` in the dev test preset is the rot guard: if the label ever + # matches nothing (registration deleted, typo), ctest exits non-zero instead of + # passing vacuously. runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Guard — the suite must be present - run: | - if [ ! -x tests/conformance/run.sh ]; then - echo "::error::tests/conformance/run.sh is missing or not executable. The" \ - "conformance suite is the M0 exit gate and this check must not pass" \ - "without running it. If PROTO moved the entrypoint, update this job." - exit 1 - fi - name: Bootstrap toolchain run: sudo ./tools/bootstrap.sh - uses: actions/setup-node@v4 with: - node-version: '22' # apt ships < 20; run.sh's TS runner needs >= 20 - - name: Run conformance suite - run: ./tests/conformance/run.sh + node-version: '22' # apt ships < 20; run.sh's TS replay runner needs >= 20 + - name: Configure + run: cmake --preset dev + - name: Build the native conformance runner + run: cmake --build --preset dev --target velox_conformance_cpp + - name: Run conformance (ctest -L conformance) + run: ctest --preset dev -L conformance --output-on-failure diff --git a/docs/adr/0014-conformance-runs-through-ctest.md b/docs/adr/0014-conformance-runs-through-ctest.md new file mode 100644 index 0000000..2106c5c --- /dev/null +++ b/docs/adr/0014-conformance-runs-through-ctest.md @@ -0,0 +1,50 @@ +# ADR 0014 — The conformance suite has one entry point: `ctest -L conformance` + +**Status:** accepted · **Date:** 2026-09-10 · **Lane:** PKG/QA + +## Context + +The conformance suite could be started two ways, and both were live: + +* `tests/conformance/run.sh` — the script a developer runs by hand; the CI `conformance` + job called it directly. +* `ctest -L conformance` — PROTO wired `tests/conformance/CMakeLists.txt` to register + `conformance` (which shells out to `run.sh`) and `conformance_cpp` (the finer-grained + native runner) under that label. + +Nothing was wrong with either, but the CI `conformance` job — the one named as the M0 +exit gate in `.github/BRANCH_PROTECTION.md` — exercised only the script. The ctest +registration was reached only incidentally, by the unfiltered `ctest` run in the `build` +and `sanitizers` jobs. A change that broke the registration (a deleted `add_test`, a +renamed label, a `WORKING_DIRECTORY` regression) would not fail the check that is supposed +to be about conformance; it would fail a build job, or nothing. + +Two entry points also means two answers to "did the suite pass", free to drift. + +## Decision + +`ctest -L conformance` is the canonical entry point. `run.sh` remains the thing it runs — +`tests/conformance/CMakeLists.txt` registers it verbatim — but CI reaches it only through +ctest, the same way a developer does. + +* The `conformance` job runs `cmake --preset dev`, builds just the `velox_conformance_cpp` + target, then `ctest --preset dev -L conformance --output-on-failure`. This puts PROTO's + registration on the exercised path. +* The rot guard is `noTestsAction: error` in the `dev` test preset: if the label ever + matches zero tests, ctest exits non-zero instead of the old silent "0 tests, exit 0". +* The `build` and `sanitizers` jobs exclude the end-to-end test with `-E '^conformance$'`. + It installs two npm trees and runs a node process against a self-compiled, unsanitized + `g++` binary — running that once, in the dedicated job, is enough; sanitizing it buys + nothing. `conformance_cpp` is *not* excluded and still runs on every matrix leg, which + is where ASan/UBSan/TSan coverage of the generated wire code is worth having. + +## Consequences + +* One definition of "the suite passed". +* `run.sh` still works standalone and unchanged for local use and for + `run.sh --uds … --ws-port …` against a real daemon; that path is out of scope for this + ADR, which is only about what CI treats as authoritative. +* If PROTO ever wants the two labelled tests separated (e.g. a `conformance-e2e` label so + jobs can select them independently without a name regex), that is a `tests/conformance/` + change on their side; the `-E '^conformance$'` here is the PKG/QA-side stopgap until + then.