From dad88fce3f880fc9d58bed369507f56f5638069d Mon Sep 17 00:00:00 2001 From: sami Date: Thu, 10 Sep 2026 14:58:10 +0400 Subject: [PATCH] pkg: run conformance through ctest as the one canonical path PROTO wired the suite into ctest (label "conformance": the end-to-end `conformance` test that shells to run.sh, plus the native `conformance_cpp`); the CI job called run.sh directly. Two entry points, and the required M0 gate exercised only one of them, so the ctest registration could rot. The conformance job now configures, builds velox_conformance_cpp, and runs `ctest --preset dev -L conformance`. The dev test preset's noTestsAction: error is the rot guard: an empty label match exits non-zero instead of the old silent pass. build/sanitizers exclude the heavy e2e test with -E '^conformance$' (the dedicated job owns that run; conformance_cpp still runs under every sanitizer). ADR 0014 records the decision. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_0143aKiohmDiyefJBwHDJJqw --- .github/BRANCH_PROTECTION.md | 6 ++- .github/workflows/ci.yml | 41 +++++++++------ .../0014-conformance-runs-through-ctest.md | 50 +++++++++++++++++++ 3 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 docs/adr/0014-conformance-runs-through-ctest.md 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.