diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc40b20..173674b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,62 @@ jobs: - name: clang-tidy run: cmake --build build --target run-clang-tidy + # -------- Release contract (Linux only) -------- + # + # Every other job builds `Dev`, so the suite's `#ifdef NDEBUG` bodies compile to nothing: the + # cases run, pass, and assert precisely nothing. This job is the only place they are real code. + # Linux alone, because what they assert — what a writer returns once NDEBUG removes its + # assertion — does not vary by platform. + # + # It runs the SHARED stage body rather than restating the commands, so the Docker replica + # (`tools/ci/run-local-ci.sh release-contract`) and this job cannot drift. + + release-contract: + name: release contract (Linux) + runs-on: ubuntu-latest + env: + VCPKG_ROOT: ${{ github.workspace }}/vcpkg + steps: + - uses: actions/checkout@v4 + + - name: Install system packages + run: | + sudo apt-get update + # See the clang-tidy job above for why autotools are here: glfw3 3.5.1's `pthread-stubs` + # dependency is autotools-only on Linux. + sudo apt-get install -y ninja-build cmake libvulkan-dev \ + xorg-dev libxinerama-dev libxcursor-dev libglu1-mesa-dev pkg-config \ + autoconf autoconf-archive automake libtool + + - name: Checkout vcpkg + uses: actions/checkout@v4 + with: + repository: microsoft/vcpkg + path: vcpkg + + - name: Bootstrap vcpkg + run: ./vcpkg/bootstrap-vcpkg.sh + + # The same cache path as the Dev jobs, which is the point: the vcpkg-release preset points + # VCPKG_INSTALLED_DIR back at build/vcpkg_installed, so this job reuses those ports instead + # of building a second copy of every one of them under build-release/. + - name: Cache vcpkg packages + uses: actions/cache@v4 + with: + path: | + build/vcpkg_installed + vcpkg/downloads + vcpkg/buildtrees + vcpkg/packages + key: ${{ runner.os }}-vcpkg-${{ hashFiles('vcpkg.json', 'vcpkg-configuration.json') }} + restore-keys: | + ${{ runner.os }}-vcpkg- + + - name: Release contract + run: | + . tools/ci/ci-stages.sh + ci_release_contract + # -------- Per-platform build + test (each validates its own determinism golden) -------- build-test-linux: diff --git a/CLAUDE.md b/CLAUDE.md index d2e50ab..d0d7010 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -177,23 +177,48 @@ Reference class: `include/fire_engine/graphics/image.hpp`. Catch2 (v3, `Catch2::Catch2WithMain`). Single binary `test_fire_engine`. CTest registers `test_fire_engine` as the fast `~[slow]` entry, so plain `ctest` and `ctest --preset fast` -stay fast. The `tests-full` target runs the all-tags Catch2 binary plus the four build-time guards — -graphics-layer includes, shared shader blocks, the shadow bias law, and the shared GPU limits; from -the source root use `cmake --build --preset full`. Test files mirror +stay fast. The `tests-full` target runs the all-tags Catch2 binary plus the six build-time guards — +graphics-layer includes, shared shader blocks, the shadow bias law, the shared GPU limits, the +per-view shadow matrix, and the `[release-contract]` tags; from the source root use +`cmake --build --preset full`. Test files mirror source paths. Shared helpers/traits live in `tests/support/`. Test assets in `tests/assets/` → copied to `build/test_assets/`. Graphics-layer tests run without a GPU (opaque handles). -**CI has four parallel jobs** (GitHub Actions, all `FIRE_ENGINE_WARNINGS_AS_ERRORS=ON`): `clang-format` -+ `clang-tidy` (platform-independent lint, run once on Ubuntu) and `build-test-linux` + -`build-test-macos` (build + `tests-full` on Ubuntu and macOS/arm64). Each build job validates *its* -platform's determinism golden. The build/test/lint stage bodies are shared across the Docker replica -and the native macOS replica via `tools/ci/ci-stages.sh` — edit stages there, not in each script. - -**Local CI parity.** `tools/ci/run-local-ci.sh [format|configure|build|tidy|test|all|shell]` -reproduces the **Linux** checks (Ubuntu 24.04) in Docker; `tools/ci/run-local-macos.sh -[format|configure|build|tidy|test|all]` runs the same stages **natively on macOS** (no container — +**CI has five parallel jobs** (GitHub Actions, all `FIRE_ENGINE_WARNINGS_AS_ERRORS=ON`): `clang-format` ++ `clang-tidy` (platform-independent lint, run once on Ubuntu), `build-test-linux` + +`build-test-macos` (build + `tests-full` on Ubuntu and macOS/arm64), and `release-contract` (below). +Each build job validates *its* platform's determinism golden. The build/test/lint stage bodies are +shared across the Docker replica and the native macOS replica via `tools/ci/ci-stages.sh` — edit +stages there, not in each script, and the GitHub `release-contract` job sources that file rather +than restating its commands. + +**A test whose body is `#ifdef NDEBUG` is tagged `[release-contract]`.** Every preset here builds +`Dev`, so those bodies — the release half of a writer that asserts in Dev and returns `false` under +`NDEBUG` — compile to nothing: the cases run, pass, and assert nothing at all. The Linux-only +`release-contract` job is the only place they are real code (`cmake --preset vcpkg-release`, build +`test_fire_engine`, run `test_fire_engine "[release-contract]"`; locally, +`tools/ci/run-local-ci.sh release-contract`). Two things keep the job honest, and both must survive +any edit to it: Catch2 exits non-zero when a spec matches nothing, so a renamed tag fails rather +than passing quietly; and `tests/release_contract.cpp` holds a HIDDEN (`[.]`) sentinel case in the +same selection that fails unless `NDEBUG` is defined, so the job cannot pass by building `Dev` by +mistake. The `[.]` is what keeps it out of the Dev suites — `ctest` (`~[slow]`) and `tests-full` (no +filter) are both default runs, which exclude hidden cases, while an explicit tag selection includes +them. **Only tag a case that actually has an `#ifdef NDEBUG` in it**: the tag means "this asserts +something a Dev build cannot see". Both halves of that rule are enforced by the +`release_contract_guard` CTest case (`cmake/check_release_contract.cmake`), which fails on a +conditional case that is untagged — the silent failure, since nothing else would ever run it as real +code — on a tagged case with no conditional, on a missing sentinel, and on the tag vanishing +altogether; so the convention no longer depends on the next person knowing it. The `vcpkg-release` +preset builds into `build-release/` and points `VCPKG_INSTALLED_DIR` back at +`build/vcpkg_installed`, so it reuses the ports the Dev tree and the CI cache already hold instead +of building a second copy of every one. + +**Local CI parity.** `tools/ci/run-local-ci.sh [format|configure|build|tidy|test|release-contract|all|shell]` +reproduces the **Linux** checks (Ubuntu 24.04) in Docker — its `all` includes the Release contract; +`tools/ci/run-local-macos.sh [format|configure|build|tidy|test|all]` runs the same stages +**natively on macOS**, minus that one (it is platform-independent, so one job proves it) (no container — uses your existing vcpkg + C++ toolchain, installs nothing; Vulkan/GLFW/glslc all come from vcpkg). The Docker runner copies the working tree into volumes (host artifacts untouched), defaults to `linux/amd64` to match CI (`DOCKER_PLATFORM=linux/arm64` is faster but off-platform). Run the relevant one before committing diff --git a/CMakeLists.txt b/CMakeLists.txt index 3b4082c..42ae292 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -384,6 +384,9 @@ endif() enable_testing() add_executable(test_fire_engine + # The build-configuration sentinel for the [release-contract] gate — it mirrors no source + # file, because its subject is how this binary was compiled. + tests/release_contract.cpp tests/collision/test_aabb.cpp tests/collision/test_geometry.cpp tests/collision/test_sweep_and_prune_broad_phase.cpp @@ -527,6 +530,7 @@ add_custom_target(tests-full COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R shadow_bias_guard COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R gpu_limits_guard COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R shadow_matrix_guard + COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R release_contract_guard DEPENDS test_fire_engine COMMENT "Running the full test suite, including [slow] tests" ) @@ -582,6 +586,17 @@ add_test( -P ${PROJECT_SOURCE_DIR}/cmake/check_shadow_matrix.cmake ) +# Every NDEBUG-conditional test case carries `[release-contract]`, which is the only thing that +# selects it. An untagged one is not a weaker check but no check at all — every preset here builds +# Dev, so its guarded body compiles to nothing and it passes each suite while asserting nothing — +# and forgetting the tag is silent by construction, which is the gap this closes. +add_test( + NAME release_contract_guard + COMMAND ${CMAKE_COMMAND} + -DTESTS_DIR=${PROJECT_SOURCE_DIR}/tests + -P ${PROJECT_SOURCE_DIR}/cmake/check_release_contract.cmake +) + find_program(CLANG_TIDY_EXE NAMES clang-tidy) if(CLANG_TIDY_EXE) get_target_property(FIRE_ENGINE_TIDY_SOURCES fireengine SOURCES) diff --git a/CMakePresets.json b/CMakePresets.json index 3984f36..fd7313f 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -10,6 +10,17 @@ "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", "CMAKE_TOOLCHAIN_FILE": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" } + }, + { + "name": "vcpkg-release", + "inherits": "vcpkg", + "displayName": "Release — the [release-contract] gate", + "description": "A genuine Release build (NDEBUG defined), which is the only configuration in which the suite's #ifdef NDEBUG bodies are real code. VCPKG_INSTALLED_DIR deliberately points back at build/vcpkg_installed: vcpkg would otherwise install a second copy of every port under build-release/, which no CI cache populates and which takes as long as the ports themselves. Its own binaryDir so it never disturbs the Dev tree that ctest, tests-full and compile_commands.json all live in.", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "VCPKG_INSTALLED_DIR": "${sourceDir}/build/vcpkg_installed" + } } ], "buildPresets": [ @@ -23,6 +34,13 @@ "targets": [ "tests-full" ] + }, + { + "name": "release-contract", + "configurePreset": "vcpkg-release", + "targets": [ + "test_fire_engine" + ] } ], "testPresets": [ diff --git a/README.md b/README.md index e1e9048..f25248f 100644 --- a/README.md +++ b/README.md @@ -310,7 +310,9 @@ Or from the build directory: ctest --test-dir build --output-on-failure ``` -Run the full Catch2 suite, including `[slow]` tests, plus the graphics-layer include guard: +Run the full Catch2 suite, including `[slow]` tests, plus the build-time guards (graphics-layer +includes, shared shader blocks, the shadow bias law, the shared GPU limits, the per-view shadow +matrix, and the `[release-contract]` tags): ```bash cmake --build --preset full @@ -340,13 +342,19 @@ cmake --build build --target run-clang-tidy # if clang-tidy is installed `cmake --build build --target run-clang-tidy --parallel ` or `CMAKE_BUILD_PARALLEL_LEVEL` to cap local CPU/memory use. -CI (GitHub Actions, all `FIRE_ENGINE_WARNINGS_AS_ERRORS=ON`) runs four parallel jobs: +CI (GitHub Actions, all `FIRE_ENGINE_WARNINGS_AS_ERRORS=ON`) runs five parallel jobs: - **`clang-format`** and **`clang-tidy`** — platform-independent lint gates, run once (Ubuntu). - **`build-test-linux`** — build + `tests-full` on Ubuntu (validates the **Linux/x86_64** determinism golden). - **`build-test-macos`** — build + `tests-full` on macOS/arm64 (validates the **macOS/arm64** golden). Like Linux, it gets Vulkan + GLFW + `glslc` from vcpkg — the runner only adds `ninja`. +- **`release-contract`** — the only job that builds `Release`. Every other job builds `Dev`, so the + suite's `#ifdef NDEBUG` bodies (what a writer returns once its assertion is compiled away) + vanish; this one builds `test_fire_engine` from the `vcpkg-release` preset and runs + `test_fire_engine "[release-contract]"`. Ubuntu only — that behaviour does not vary by platform — + and deliberately not the whole Release suite, which would drag in the optimisation-sensitive + physics goldens. Each platform's `Determinism.GoldenHash` golden is now enforced by its own job — see [`docs/collision.md`](docs/collision.md) and CLAUDE.md § Testing. @@ -360,7 +368,8 @@ tools/ci/run-local-macos.sh all # macOS, native (no container) The **Docker** runner copies the working tree into an Ubuntu 24.04 container, keeps Linux build/vcpkg state in Docker volumes, and accepts `format`, `configure`, `build`, `tidy`, `test`, -`all`, or `shell` to isolate a stage. It defaults to `linux/amd64` to match GitHub Actions; set +`release-contract`, `all`, or `shell` to isolate a stage (`all` includes the Release contract on +Linux; the macOS runner has no such stage, by design). It defaults to `linux/amd64` to match GitHub Actions; set `DOCKER_PLATFORM=linux/arm64` for a faster native Apple Silicon check. The **native macOS** runner takes the same stages and runs them directly on your host toolchain (it installs nothing — Vulkan, GLFW, and `glslc` all come from vcpkg, so it just needs your existing vcpkg + compiler + `ninja`). diff --git a/cmake/check_release_contract.cmake b/cmake/check_release_contract.cmake new file mode 100644 index 0000000..d978155 --- /dev/null +++ b/cmake/check_release_contract.cmake @@ -0,0 +1,140 @@ +# Guard: a test case whose body is conditional on NDEBUG carries the `[release-contract]` tag, and +# nothing else does. +# +# The tag is what the Linux `release-contract` job selects, and it is the ONLY thing that selects +# it. An untagged `#ifdef NDEBUG` case is therefore not a weaker check, it is no check at all: every +# preset here builds `Dev`, so its guarded body compiles to nothing in every suite that runs it, and +# the case passes everywhere while asserting nothing anywhere. That is precisely the failure the tag +# was introduced to end, and it comes back silently the first time someone adds a conditional case +# without knowing the convention — which is the normal case, since the convention is invisible at +# the point where it matters. +# +# The other direction is checked too, and is not merely tidiness. `[release-contract]` means "this +# case asserts something a Dev build cannot see". A case carrying the tag without a conditional body +# runs identically in both configurations, so it pads the job's selection with work every other +# suite already covers and makes a green Release run look broader than it is. +# +# COMMENTS ARE STRIPPED FIRST, both forms. This file's subject is `#ifdef NDEBUG`, and so is the +# prose of nearly every case it guards: an unstripped scan is satisfied by a case that merely talks +# about release behaviour and — worse — by one whose real conditional has been commented out. +# +# Invoked as a CTest case; needs TESTS_DIR. + +if(NOT DEFINED TESTS_DIR) + message(FATAL_ERROR "TESTS_DIR must be set (path to tests/)") +endif() + +include("${CMAKE_CURRENT_LIST_DIR}/strip_glsl_comments.cmake") + +set(offenders "") +set(tag "[release-contract]") +set(sentinel_file "release_contract.cpp") +set(sentinel_seen FALSE) +set(tagged_count 0) + +file(GLOB_RECURSE test_sources "${TESTS_DIR}/*.cpp") +if(NOT test_sources) + message(FATAL_ERROR + "release contract guard: no .cpp files under ${TESTS_DIR} — the sweep found nothing to check, which is not a pass") +endif() + +foreach(source IN LISTS test_sources) + file(RELATIVE_PATH relative "${TESTS_DIR}" "${source}") + file(READ "${source}" raw) + strip_glsl_comments("${raw}" code) + + # Split the file into cases. CMake regexes cannot report a match POSITION, so the macro name + # becomes the list separator and the text is split on it: element 1 is whatever precedes the first + # case (includes, helpers) and is dropped; every later element is one case — its argument list + # followed by its body, up to the start of the next case. + # + # Semicolons and backslashes go first, and both matter: `;` is CMake's list separator, so a case + # containing one would be scanned in fragments; a trailing `\` escapes the separator itself. + # Neither character can join two identifiers, so replacing them with a space cannot manufacture or + # destroy a match below. + string(REPLACE "\\" " " scan "${code}") + string(REPLACE ";" " " scan "${scan}") + string(REPLACE "TEST_CASE(" ";" scan "${scan}") + + set(index 0) + foreach(case IN LISTS scan) + math(EXPR index "${index} + 1") + if(index EQUAL 1) + continue() + endif() + + # Argument list and body are separated at the first line that STARTS with `{`, not at the first + # `)`: 36 case names in this suite contain a parenthesis and one contains a brace, so a + # punctuation-based split would read part of a name as body (and then miss its tag). Allman + # braces make the line-initial `{` reliable, and they are themselves CI-gated by clang-format. + string(FIND "${case}" "\n{" brace) + if(brace LESS 0) + list(APPEND offenders + "${relative}: a TEST_CASE( has no body brace on its own line — the guard cannot tell its tags from its body") + continue() + endif() + string(SUBSTRING "${case}" 0 ${brace} arguments) + string(LENGTH "${case}" case_length) + math(EXPR body_start "${brace} + 2") + math(EXPR body_length "${case_length} - ${body_start}") + string(SUBSTRING "${case}" ${body_start} ${body_length} body) + + # The case's NAME, for the report — the first string literal, by Catch2's signature. + set(name "") + if(arguments MATCHES "\"([^\"]*)\"") + set(name "${CMAKE_MATCH_1}") + endif() + + string(FIND "${arguments}" "${tag}" tag_position) + if(tag_position GREATER_EQUAL 0) + set(tagged TRUE) + math(EXPR tagged_count "${tagged_count} + 1") + else() + set(tagged FALSE) + endif() + + # A preprocessor conditional ON NDEBUG, in any spelling. `#if !defined(NDEBUG)` counts: what + # matters is that the case's behaviour differs between the two configurations, not which half is + # which. + if(body MATCHES "#[ \t]*(ifdef|ifndef|if)[^\n]*NDEBUG") + set(conditional TRUE) + else() + set(conditional FALSE) + endif() + + if(conditional AND NOT tagged) + list(APPEND offenders + "${relative}: \"${name}\" has an NDEBUG-conditional body but is not tagged ${tag} — nothing builds it as real code, so it asserts nothing in any configuration") + elseif(tagged AND NOT conditional) + list(APPEND offenders + "${relative}: \"${name}\" is tagged ${tag} but has no NDEBUG conditional — the tag means 'this asserts something a Dev build cannot see', and every other suite already covers this case") + endif() + + # The sentinel: hidden, tagged, conditional. Its `[.]` is what keeps it out of the Dev suites, + # and removing the `[.]` fails them loudly — but removing the CASE is silent, and leaves the job + # unable to tell a Release build from a Dev one again. + if(relative STREQUAL "${sentinel_file}" AND tagged AND conditional AND arguments MATCHES "\\[\\.\\]") + set(sentinel_seen TRUE) + endif() + endforeach() +endforeach() + +if(NOT sentinel_seen) + list(APPEND offenders + "tests/${sentinel_file} has no hidden ([.]) ${tag} case with an NDEBUG conditional — that case is what fails the job when the binary was not built with NDEBUG, and without it a Dev build passes the selection silently") +endif() + +# A selection of nothing is not a green run. Catch2 catches that at job time (it exits non-zero when +# a spec matches no tests), but this fails in the build that removed the last tag, where the cause is +# still in front of whoever caused it. +if(tagged_count EQUAL 0) + list(APPEND offenders "no test case carries ${tag} — the Linux release-contract job now selects nothing") +endif() + +if(offenders) + string(REPLACE ";" "\n " report "${offenders}") + message(FATAL_ERROR "release contract guard failed:\n ${report}") +endif() + +message(STATUS + "release contract guard: ${tagged_count} case(s) tagged ${tag}, each with an NDEBUG-conditional body, plus the hidden sentinel") diff --git a/docs/onboarding.md b/docs/onboarding.md index dc1d855..d6387e3 100644 --- a/docs/onboarding.md +++ b/docs/onboarding.md @@ -1357,6 +1357,31 @@ cmake --build --preset full # all Catch2 tests + layering gua cmake --build build --target tests-full # build-dir equivalent ``` +**Some tests do not run in any of the above, and that is deliberate.** A case whose body is +`#ifdef NDEBUG` asserts RELEASE behaviour — what a writer returns once `NDEBUG` has compiled its +assertion away, which is the half that ships. Every preset here builds `Dev`, so those bodies are +empty: the cases run, pass, and assert nothing. They are tagged `[release-contract]` and run by one +Linux CI job, reproducible locally as: + +```bash +cmake --preset vcpkg-release # Release into build-release/, shared vcpkg tree +cmake --build --preset release-contract # test_fire_engine only +./build-release/test_fire_engine "[release-contract]" +tools/ci/run-local-ci.sh release-contract # or the whole thing in Docker +``` + +The selection contains a HIDDEN sentinel case (`tests/release_contract.cpp`) that fails unless +`NDEBUG` is defined, so running the tag against a Dev build reports the mistake instead of passing +vacuously. Hidden (`[.]`) is also why the sentinel never fires in the Dev suites above: they are +default runs, which exclude hidden cases. + +When you add a case with an `#ifdef NDEBUG` body, tag it — and `release_contract_guard` (part of +`tests-full`) fails if you don't. An untagged conditional case is not a weaker check but no check at +all: nothing selects it in Release, so its guarded body never becomes real code anywhere, and it +passes every suite while asserting nothing. The guard also rejects the tag on a case with no +conditional (it would run identically in both configurations, padding the job's selection with work +the Dev suites already do), and fails if the sentinel or the tag itself disappears. + Graphics-layer tests run headless because the layer only stores opaque handles — keep it that way so the suite stays GPU-free. Test files mirror their source path (`src/foo/bar.cpp` → `tests/foo/test_bar.cpp`). Shared Catch2 helpers and compile-time test traits live in diff --git a/docs/review-order.md b/docs/review-order.md index ee6c210..8192b83 100644 --- a/docs/review-order.md +++ b/docs/review-order.md @@ -17,9 +17,9 @@ Read these first when a change touches build configuration, CI, or local tooling | File | Pay attention to | |---|---| | `CMakePresets.json` | The `vcpkg` preset pins Apple Clang on macOS, selects `Dev`, and exports `compile_commands.json` for `clangd`. | -| `CMakeLists.txt` | `Dev` is the default build type (`-O2 -g`, no `NDEBUG`); `FIRE_ENGINE_WARNINGS_AS_ERRORS` is CI-only by default; CTest registers `test_fire_engine` (`~[slow]`) so plain CTest stays fast; `tests-full` runs the all-tags Catch2 binary plus the layering, shared-shader-block, shadow-bias and GPU-limits guards; `shaders/` is on the C++ include path (`BUILD_INTERFACE`, PUBLIC) because the public header `graphics/gpu_limits.hpp` includes `shaders/gpu_limits.glsl` — the dual-language limits file; `run-clang-tidy` appears only when `clang-tidy` is installed. **Read the include-order block near the top before touching dependency includes**: `CMAKE_NO_SYSTEM_FROM_IMPORTED` + `include_directories(BEFORE …)` force vcpkg's headers to arrive as `-I`, because clang searches `/usr/local/include` ahead of every `-isystem` path — with a Vulkan SDK installed there, our TUs compiled against ITS vulkan-hpp while includes resolved relative to a vcpkg header got vcpkg's, and the first RAII call aborted on a header-version assert with no bad C++ behind it (real: SDK 1.4.357 beside the pinned 1.4.335). The guard checks both `VCPKG_INSTALLED_DIR` spellings and hard-errors if the directory is missing, since a silent miss restores the mixed-header build. `SHADER_INCLUDES` lists the shared GLSL includes (`gpu_limits.glsl`, `light_ubo.glsl`, `material.glsl`, `shadow_push.glsl`, `shadow_depth.glsl`, `self_shadow_second.glsl`) so editing one rebuilds every shader — a stale `.spv` against a changed block is the layout bug the include exists to prevent. Consequence: third-party headers are no longer warning-suppressed, so a finding gets a narrow `#pragma` at the include site (`src/graphics/frame_capture.cpp`), never a weakened flag. | +| `CMakeLists.txt` | `Dev` is the default build type (`-O2 -g`, no `NDEBUG`); `FIRE_ENGINE_WARNINGS_AS_ERRORS` is CI-only by default; CTest registers `test_fire_engine` (`~[slow]`) so plain CTest stays fast; `tests-full` runs the all-tags Catch2 binary plus the layering, shared-shader-block, shadow-bias, GPU-limits, shadow-matrix and release-contract guards. Both of those are DEFAULT Catch2 runs, so neither sees a hidden (`[.]`) case — which is what keeps the `[release-contract]` sentinel (`tests/release_contract.cpp`, registered first in the test target) out of the Dev suites while an explicit tag selection picks it up; `shaders/` is on the C++ include path (`BUILD_INTERFACE`, PUBLIC) because the public header `graphics/gpu_limits.hpp` includes `shaders/gpu_limits.glsl` — the dual-language limits file; `run-clang-tidy` appears only when `clang-tidy` is installed. **Read the include-order block near the top before touching dependency includes**: `CMAKE_NO_SYSTEM_FROM_IMPORTED` + `include_directories(BEFORE …)` force vcpkg's headers to arrive as `-I`, because clang searches `/usr/local/include` ahead of every `-isystem` path — with a Vulkan SDK installed there, our TUs compiled against ITS vulkan-hpp while includes resolved relative to a vcpkg header got vcpkg's, and the first RAII call aborted on a header-version assert with no bad C++ behind it (real: SDK 1.4.357 beside the pinned 1.4.335). The guard checks both `VCPKG_INSTALLED_DIR` spellings and hard-errors if the directory is missing, since a silent miss restores the mixed-header build. `SHADER_INCLUDES` lists the shared GLSL includes (`gpu_limits.glsl`, `light_ubo.glsl`, `material.glsl`, `shadow_push.glsl`, `shadow_depth.glsl`, `self_shadow_second.glsl`) so editing one rebuilds every shader — a stale `.spv` against a changed block is the layout bug the include exists to prevent. Consequence: third-party headers are no longer warning-suppressed, so a finding gets a narrow `#pragma` at the include site (`src/graphics/frame_capture.cpp`), never a weakened flag. | | `.clang-format` / `.clang-tidy` | Formatting is CI-gated. Tidy is the first-pass static-analysis config for engine `src/` plus `include/fire_engine/`; disabled checks are documented inline. | -| `.github/workflows/ci.yml` | Builds with vcpkg + warnings-as-errors, runs `run-clang-tidy`, runs the full `tests-full` target, and has a separate clang-format dry-run job. | +| `.github/workflows/ci.yml` + `tools/ci/ci-stages.sh` | Five parallel jobs: a clang-format dry-run, `run-clang-tidy`, `tests-full` on Ubuntu and on macOS/arm64 (each validating its own determinism golden), and `release-contract` — the only job that builds `Release`, which is the only configuration where the suite's `#ifdef NDEBUG` bodies are real code. The stage BODIES live in `ci-stages.sh` and are shared with both local replicas, so a stage edited in one place cannot drift from the others; the `release-contract` job sources that file rather than restating its commands, and `all` includes the stage on Linux only (`CI_RELEASE_CONTRACT`). Read `ci_release_contract` together with the `vcpkg-release` preset: it builds `test_fire_engine` alone into `build-release/` while pointing `VCPKG_INSTALLED_DIR` back at `build/vcpkg_installed`, which is what lets the CI cache serve both trees. | | `tools/assetgen/geometry.py` | Primitive builders, all returning the same `(positions, normals, indices)` triple: box (24 verts, per-face normals), tetrahedron, UV sphere, flat-shaded mesh-from-triangles, and `combine_geometry` for compounds. Plus the shared vector helpers. Everything is authored at true size with node scale left at 1. | | `tools/assetgen/quaternions.py` | glTF `[x, y, z, w]` order throughout — the easiest thing to get wrong when hand-authoring. `look_at_quat` builds a camera orientation (glTF cameras look down −Z); `quat_from_to` is the "aim this at that" helper. | | `tools/assetgen/scene.py` | The `Scene` assembler: one self-contained `.gltf` with the binary embedded as a base64 data URI (no sidecar `.bin`, no textures). Note the two layers — `node`/`box`/`sphere` are generic and pass `extras` through verbatim, while `box_body`/`sphere_body`/`compound_body`/`static_mesh_body`/`static_floor` are physics wrappers that inject `extras.Physics` and default the collider to match the mesh. `generator` is a constructor argument, not a hard-coded string. `write_gltf`'s 2-space-plus-newline formatting is part of the contract: the build regenerates the committed assets, so a formatting change is churn in every one. | @@ -211,6 +211,7 @@ Read these first when a change touches build configuration, CI, or local tooling | `render/transmission.hpp` + `transmission.cpp` | **High-attention.** `KHR_materials_transmission` off the captured `sceneColor`. The `shader.frag` split (post-fix): clear/frosted glass does screen-space refraction (roughness-blurred by the sceneColor mip chain); a thin-walled surface that is **also emissive** (a self-lit paper lamp shade) instead scatters to a view-independent irradiance tint — so a bright bulb behind it doesn't beam a camera-tracking blob. Discriminator is the **emissive factor**, NOT thickness. Plus back-face normal flip. Its forward recorder shares the main recorder's descriptor-order invariant: after a pipeline transition, push set 0 before binding allocated sets 1/2 through the same layout. | | `render/debug_draw.hpp` + `debug_draw.cpp` | Physics debug wireframes. `PhysicsDebugData` (AABBs + `ClothCollider` shapes + `DebugContact`s + free-form `DebugLine`s `queryLines` (query probe + the ragdoll joint RGB gizmo) + `DebugLabel`s `jointLabels` (drawn as projected ImGui text by the overlay, not line geometry), all Vulkan-free) → CPU-built line list in a per-frame mapped `Vertex` buffer (`Resources::createMappedVertexBuffers`) → line-list pipeline (`Pipeline::debugLineConfig`, `PipelineConfig::topology` + `dynamicDepthTest`) drawn into HDR after particles. Brackets the HDR target ShaderReadOnly↔ColorAttachment + depth ReadOnly→Attachment. Physics side: `PhysicsWorld::debugColliderBounds()` / `debugContacts()` (captured in `step()` pre-resolve). | | `tests/support/vdpm.hpp` | Shared VDPM front validators — `foldoverCount` / `coverageFailures`, templated on any front exposing `forest()` + `active(v)`, so the sequential `ActiveFront` and parallel `ParallelFront` repairs are judged by the **same first-principles yardstick** (recomputed from the mesh, never the repair under test). Coverage keeps its geometry independent but shares the one runtime tuning knob (`detail::kMinCoverageScreenAreaPx`, viewport-derived) so the "worth-fixing" threshold tracks the repair's; checks only **projectable** coverage (near-plane straddles go to the separate path). | +| `tests/release_contract.cpp` + `cmake/check_release_contract.cmake` | The `[release-contract]` gate's two halves, and neither is a test of engine behaviour. The **sentinel** is one HIDDEN (`[.]`) case that fails unless `NDEBUG` is defined: it rides in the same tag selection the Linux job runs, so a job that built `Dev` by mistake fails instead of reporting a green run over bodies that compiled to nothing. Hidden is load-bearing — `ctest` and `tests-full` are default runs and skip it, an explicit tag selection does not. The **guard** enforces the tag in both directions across `tests/`: a case with an NDEBUG conditional must carry the tag (untagged, it is not a weaker check but no check at all — nothing ever builds it as real code), and a tagged case must have a conditional (otherwise it pads the selection with what the Dev suites already run). It also fails if the sentinel or the last tag disappears. It splits cases at the line-initial `{` rather than the first `)`, because 36 case names here contain a parenthesis and one contains a brace; comments are stripped first, since the prose it reads is *about* `#ifdef NDEBUG`. Nine mutations tested. | | `tests/physics/test_physics_determinism.cpp` + `tests/support/state_hash.hpp` | Determinism harness: FNV-1a body-state hash; ReplayIsBitIdentical / FreeFallMatchesClosedForm / GoldenHash / **BroadphasesAgree** (same scene through the tree + an injected SAP → identical hash). The GoldenHash constant is a behaviour tripwire — update it intentionally when the solver math changes. | | `render/ssao.hpp` + `ssao.cpp` | **High-attention.** SSAO + contact shadows. Runs after the depth prepass: borrows the shared scene depth (attachment → read-only → attachment), reconstructs view position+normal from depth alone (analytic unprojection from `proj`, no normal G-buffer), writes R8G8 (R = hemisphere-kernel AO, G = sun-direction contact-shadow ray-march; `ssao.frag`), then a **depth-aware bilateral blur** (`ssao_blur.frag`, view-space-Z edge-stop) into a second target. Always runs (disabled = intensity 0). Forward set 1 binding 13 samples the blurred target. The depth prepass itself is `Pipeline::depthPrepassConfig` + `Renderer::recordDepthPrepass` (reuses `shader.vert` w/ `invariant gl_Position`; forward loads depth `LESS_OR_EQUAL`). | | `render/taa.hpp` + `taa.cpp` | **High-attention.** Temporal AA subsystem. Owns the RG16F velocity target (written by the forward/transmission passes as a 2nd colour attachment), two ping-pong history HDR targets, and the resolve pass (`taa.frag`): reproject history along velocity → 3×3 neighbourhood clamp → blend → blit into the offscreen HDR target. `historyWritten_` guards the first frame after (re)create. Sub-pixel jitter lives in `Renderer::drawFrame`; motion vectors are jitter-free. | diff --git a/docs/roadmap.md b/docs/roadmap.md index abd7239..ef72386 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -96,9 +96,6 @@ review's priority order: enables — every view is still `Recorded`, which is what let the restructure be verified as decision-identical. Then #15 below, which is the same mechanism. - **Sequenced after arc 4** (see below) — the reuse stage adds conditional release behaviour, and the - job that exercises it should be in place first. - **The gate for that stage is agreed, and it is NOT the byte-identical row dump** — that gate worked precisely because nothing changed, and reuse changes what the recorder does. The claim to prove is: *on the second identical frame, every active CACHEABLE view is reused and every @@ -196,34 +193,6 @@ Further tiers of this review are expected to follow the [`review-order.md`](revi --- -## Arc 4 — Release-contract CI job - -**Trigger: after `shadow-static-cascade-cache` lands, and BEFORE arc 2 #4's residency/reuse work.** Its own branch. The order is deliberate: the reuse stage adds more conditional release behaviour to a set of checks nothing currently executes, so the job that proves those checks run should exist first. - -Rejection tests guarded by `#ifdef NDEBUG` — the release behaviour of every writer that asserts in -Dev and returns `false` under `NDEBUG` — **never run**. Both presets build `Dev`, locally and in CI, -so those blocks compile to nothing on every machine that has ever checked them. The gap was found -while adding `setPointLight`'s new range / one-light validation: a one-off local Release build was -needed to prove the new checks fire at all. - -**Death tests are not the fix.** They prove the Dev assertion fires; they say nothing about whether -the `NDEBUG` fallback returns `false` (or throws) correctly, which is the half that ships. - -The job: - -- Tag the conditional cases `[release-contract]`. -- Configure a genuine Release build with warnings-as-errors. -- Build the real library and test executable. -- Run `test_fire_engine "[release-contract]"` explicitly. -- Include a **sentinel assertion that `NDEBUG` is defined**, so a misconfigured job cannot pass by - selecting zero relevant cases. - -**Linux only, and deliberately not the whole suite.** These rejection semantics are -platform-independent, and running all tests under Release would mix this contract with the -optimisation-sensitive physics goldens. - ---- - ## Parked & revisit — trigger-based Not a backlog. Each item was investigated, has data behind the decision, and is picked up only on diff --git a/tests/graphics/test_shadow_diagnostics.cpp b/tests/graphics/test_shadow_diagnostics.cpp index d640b34..e3573a3 100644 --- a/tests/graphics/test_shadow_diagnostics.cpp +++ b/tests/graphics/test_shadow_diagnostics.cpp @@ -264,7 +264,7 @@ TEST_CASE("a focused view distinguishes 'ran and drew nothing' from 'never ran'" CHECK_FALSE(stats.focused(absent).found()); } -TEST_CASE("one diagnostic row belongs to one logical view", "[ShadowDiagnostics]") +TEST_CASE("one diagnostic row belongs to one logical view", "[ShadowDiagnostics][release-contract]") { // Slots are reused across frames, but WITHIN a frame a row is one view's counters. Two // identities landing on one slot would sum two views' draws, triangles and level distributions @@ -575,7 +575,7 @@ TEST_CASE("claiming a view and rasterising a layer are separate facts", "[Shadow } TEST_CASE("a raster pass cannot be counted for an unclaimed or mismatched row", - "[ShadowDiagnostics]") + "[ShadowDiagnostics][release-contract]") { #ifdef NDEBUG // Two refusals. GPU work attributed to NO view carries a cost with nothing to name it; work diff --git a/tests/graphics/test_shadow_lod_resolver.cpp b/tests/graphics/test_shadow_lod_resolver.cpp index 0480f64..fe01808 100644 --- a/tests/graphics/test_shadow_lod_resolver.cpp +++ b/tests/graphics/test_shadow_lod_resolver.cpp @@ -289,7 +289,8 @@ TEST_CASE("ShadowLodResolver.AGenerationChangeMissesTheOldHistory", "[ShadowLodR kNoPreviousShadowLod); } -TEST_CASE("ShadowLodResolver.AnInvalidKeyNeverEntersEitherStore", "[ShadowLodResolver]") +TEST_CASE("ShadowLodResolver.AnInvalidKeyNeverEntersEitherStore", + "[ShadowLodResolver][release-contract]") { // A caster with no identity still has to draw — leaving a hole in a shadow map would be worse // than the producer bug — but it must not be cached (it would collide with every other @@ -402,7 +403,7 @@ TEST_CASE("ShadowLodResolver.AnInvalidViewStillDraws", "[ShadowLodResolver]") } TEST_CASE("ShadowLodResolver.AMalformedCoarserLevelFallsBackInsteadOfBindingNothing", - "[ShadowLodResolver]") + "[ShadowLodResolver][release-contract]") { // The selector reasons about ERRORS; nothing upstream promises the level's CARRIERS. A half- // failed build can leave a coarser level with a null buffer or a zero count, and selecting it @@ -563,7 +564,8 @@ TEST_CASE("ShadowLodResolver.ProvenanceIsPerFamilyEvenWhenTheResolutionIsShared" CHECK(resolver.contentResolution(ShadowViewGroup::Cascade, key) == nullptr); } -TEST_CASE("ShadowLodResolver.MarkingAnUnresolvedCasterDrawnChangesNothing", "[ShadowLodResolver]") +TEST_CASE("ShadowLodResolver.MarkingAnUnresolvedCasterDrawnChangesNothing", + "[ShadowLodResolver][release-contract]") { // Provenance is a FIELD of a decision, not a record of its own: marking a caster the resolver // never resolved would manufacture attribution for a level nobody chose. (The shadow pass @@ -869,7 +871,7 @@ TEST_CASE("ShadowLodResolver.ADeformableCasterCannotSelectBelowFullDetail", "[Sh } TEST_CASE("ShadowLodResolver.DeformationOutranksTheChainButNotTheUserOrAProducerBug", - "[ShadowLodResolver]") + "[ShadowLodResolver][release-contract]") { const auto lods = chain(); ShadowRenderViewSet views = populatedViews(); diff --git a/tests/graphics/test_shadow_render_view.cpp b/tests/graphics/test_shadow_render_view.cpp index 1969d96..10834a7 100644 --- a/tests/graphics/test_shadow_render_view.cpp +++ b/tests/graphics/test_shadow_render_view.cpp @@ -127,7 +127,8 @@ TEST_CASE("ShadowRenderViewSet.EachWriterStampsItsOwnSlotsIdentity", "[ShadowRen CHECK(face->logicalId() == ShadowLogicalViewId::point(light, 4)); } -TEST_CASE("ShadowRenderViewSet.WritersRejectTheWrongProjectionKind", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.WritersRejectTheWrongProjectionKind", + "[ShadowRenderView][release-contract]") { // A perspective descriptor in a cascade slot (or an orthographic one on a point face) is not // merely wrong, it is meaningless — and would be read back as authoritative. Dev asserts; these @@ -181,7 +182,8 @@ TEST_CASE("ShadowRenderViewSet.PointFacesCarryTheLightTheirDepthIsMeasuredAgains CHECK_FALSE(views.find(ShadowViewGroup::Spot, 0)->pointLightDepth().has_value()); } -TEST_CASE("ShadowRenderViewSet.APointCubeNeedsOneLightAndAUsableRange", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.APointCubeNeedsOneLightAndAUsableRange", + "[ShadowRenderView][release-contract]") { // Both halves of the stored ratio are checked as strictly as the matrices. A zero or non-finite // range makes every texel of all six faces meaningless; six faces about DIFFERENT positions @@ -209,7 +211,8 @@ TEST_CASE("ShadowRenderViewSet.APointCubeNeedsOneLightAndAUsableRange", "[Shadow #endif } -TEST_CASE("ShadowRenderViewSet.WritersRejectAnUnkeyableIdentity", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.WritersRejectAnUnkeyableIdentity", + "[ShadowRenderView][release-contract]") { // An engaged entry must always be keyable: hysteresis keys on the identity, so an invalid one // would leave the view rendering but its history unreachable. @@ -252,7 +255,7 @@ TEST_CASE("ShadowRenderViewSet.AbsentMeansInactiveAndEngagedInvalidIsDifferent", } TEST_CASE("ShadowRenderViewSet.OutOfRangeAccessIsNullAndOutOfRangeWritesAreDropped", - "[ShadowRenderView]") + "[ShadowRenderView][release-contract]") { // Never clamped into a neighbouring valid slot: that would bill one view's matrix to another // and render wrongly instead of failing. @@ -332,7 +335,8 @@ TEST_CASE("ShadowRenderViewSet.WorldOnlyFollowsACascadeRefitAfterEnabling", "[Sh CHECK(worldOnly->projection().worldUnitsPerTexel() == 0.5f); } -TEST_CASE("ShadowRenderViewSet.WorldOnlyCannotOutliveOrPrecedeItsCascade", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.WorldOnlyCannotOutliveOrPrecedeItsCascade", + "[ShadowRenderView][release-contract]") { #ifdef NDEBUG ShadowRenderViewSet views; @@ -378,7 +382,8 @@ TEST_CASE("ShadowRenderViewSet.PointFacesOccupyDistinctFlatSlotsWithDistinctForw CHECK_FALSE(views.active(ShadowViewGroup::Point, shadowPointViewSlot(lightSlot + 1, 0))); } -TEST_CASE("ShadowRenderViewSet.PointLightSlotIsValidatedBeforeFlattening", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.PointLightSlotIsValidatedBeforeFlattening", + "[ShadowRenderView][release-contract]") { // `lightSlot * kCubeFaceCount + face` is unsigned arithmetic, so a large enough slot WRAPS back // into the valid range: with an even face count, the top bit times 6 is 0 modulo the word. @@ -411,7 +416,8 @@ TEST_CASE("ShadowRenderViewSet.PointLightSlotIsValidatedBeforeFlattening", "[Sha #endif } -TEST_CASE("ShadowRenderViewSet.ANonFiniteRenderMatrixIsNotAView", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.ANonFiniteRenderMatrixIsNotAView", + "[ShadowRenderView][release-contract]") { // The asymmetry that matters. An invalid PROJECTION is a reportable state: the view rasterises // and selection says InvalidView. A non-finite MATRIX is not a view at all — it is what the @@ -447,7 +453,8 @@ TEST_CASE("ShadowRenderViewSet.ANonFiniteRenderMatrixIsNotAView", "[ShadowRender #endif } -TEST_CASE("ShadowRenderViewSet.ARejectedReplacementClearsTheSlotItAddressed", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.ARejectedReplacementClearsTheSlotItAddressed", + "[ShadowRenderView][release-contract]") { // Rejecting a write is only half the job. A producer that ATTEMPTED to describe this view and // failed has invalidated whatever was there: keeping the previous entry would rasterise the @@ -609,7 +616,8 @@ TEST_CASE("ShadowViewMetrics.ADegenerateFitPacksNoConversionRatherThanAnInfinity CHECK(ShadowViewMetrics::pointLight(0.004f, 0.0f).packed()[1] == Catch::Approx(0.0f)); } -TEST_CASE("ShadowRenderViewSet.WritersRejectMetricsOfTheWrongKind", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.WritersRejectMetricsOfTheWrongKind", + "[ShadowRenderView][release-contract]") { // The three packings share a shape, so a mismatch is not caught by anything downstream: the // receiver would simply read a spot's near plane as a cascade's depth conversion and produce a @@ -653,7 +661,8 @@ TEST_CASE("ShadowRenderViewSet.APointCubeCarriesOneMetricForTheWholeLight", "[Sh } } -TEST_CASE("ShadowRenderViewSet.ARejectedCubeLeavesNoFaceBehind", "[ShadowRenderView]") +TEST_CASE("ShadowRenderViewSet.ARejectedCubeLeavesNoFaceBehind", + "[ShadowRenderView][release-contract]") { // "All six faces or none" is now the writer's contract rather than a comment in the renderer. A // five-face cube is not a usable caster: the missing face would sample whatever the previous diff --git a/tests/graphics/test_vdpm.cpp b/tests/graphics/test_vdpm.cpp index 258be62..0afffff 100644 --- a/tests/graphics/test_vdpm.cpp +++ b/tests/graphics/test_vdpm.cpp @@ -1502,7 +1502,7 @@ TEST_CASE("shadowDeviationForCut: takes the maximum over the EXACT prefix, not t } TEST_CASE("shadowDeviationForCut: a negative or NaN radius in the prefix yields infinity", - "[MeshSimplifier]") + "[MeshSimplifier][release-contract]") { // A deviation is a magnitude, so these are invalid data, not cheap collapses. std::max would // swallow both (NaN loses every comparison; a negative simply loses), leaving a plausible small diff --git a/tests/release_contract.cpp b/tests/release_contract.cpp new file mode 100644 index 0000000..6b46d39 --- /dev/null +++ b/tests/release_contract.cpp @@ -0,0 +1,41 @@ +#include + +// THE RELEASE CONTRACT, AND THE SENTINEL THAT PROVES IT WAS ACTUALLY CHECKED. +// +// A number of this suite's cases assert RELEASE behaviour: what a writer that asserts in a Dev +// build returns once `NDEBUG` compiles that assertion away. `ShadowRenderViewSet`'s writers are the +// clearest family — Dev stops at the assertion inside the writer, while under `NDEBUG` the writer +// must return `false` and clear the slot it addressed, which is the half that ships. Those bodies +// sit behind `#ifdef NDEBUG` and are tagged `[release-contract]`. +// +// Every preset in this repository builds `Dev`. Locally, in the Docker replica, and in every +// GitHub job before this one, those bodies therefore compiled to NOTHING — the cases ran, passed, +// and asserted precisely nothing, for as long as they have existed. That is what the Linux +// `release-contract` job exists to fix. +// +// This file is the sentinel, and it is here because a job that checks nothing looks exactly like a +// job that checks everything and finds no fault. Two failure modes are covered, by two different +// mechanisms: +// +// 1. THE SELECTION MATCHED NOTHING — a renamed or mistyped tag. Catch2 already exits non-zero when +// a test spec matches no tests, so this needs no help from us. +// 2. THE BUILD WAS NOT A RELEASE BUILD. That one is silent: the filter matches the tagged cases, +// they run, their guarded halves are empty, and the job reports success while proving nothing. +// The case below fails outright in that situation, so the job cannot pass by accident. +// +// HIDDEN (`[.]`) on purpose. The Dev suites — `ctest` (`~[slow]`) and `tests-full` (no filter) — +// are both default runs, and Catch2 excludes hidden cases from those, so this does not fail the +// builds it is designed to detect. Explicitly selecting `[release-contract]`, which is the only +// thing the release job does, runs it. Do not remove the `[.]`, and do not give the tag to a case +// that has no `#ifdef NDEBUG` in it: the tag's meaning is "this case asserts something a Dev build +// cannot see". +TEST_CASE("the release contract is only checked where NDEBUG is defined", "[.][release-contract]") +{ +#ifdef NDEBUG + SUCCEED("NDEBUG is defined — the guarded bodies in this selection are real code"); +#else + FAIL("[release-contract] was selected from a build WITHOUT NDEBUG: every guarded body in this " + "selection compiled to nothing, so the run proves nothing. Configure the vcpkg-release " + "preset (or any Release build) and re-run."); +#endif +} diff --git a/tools/ci/ci-stages.sh b/tools/ci/ci-stages.sh index a3ed048..c640c53 100644 --- a/tools/ci/ci-stages.sh +++ b/tools/ci/ci-stages.sh @@ -8,9 +8,15 @@ # Tunables via the environment: # CLANG_FORMAT clang-format binary (clang-format-22 in the container; clang-format on macOS) # WARNINGS_AS_ERRORS ON/OFF for the configure step (default ON — CI parity) +# CI_RELEASE_CONTRACT 1 to include the Release-contract stage in `all` (Linux only — see below) : "${CLANG_FORMAT:=clang-format}" : "${WARNINGS_AS_ERRORS:=ON}" +# LINUX ONLY, and deliberately so. The `[release-contract]` cases assert what a writer returns once +# NDEBUG compiles its assertion away, which is platform-independent behaviour — one job proves it, +# and a second would only spend runner minutes. The Linux runner sets this; the macOS one leaves it +# at 0, so `all` means "every gate this platform owns" on both. +: "${CI_RELEASE_CONTRACT:=0}" ci_print_versions() { @@ -51,6 +57,31 @@ ci_test() cmake --build build --target tests-full } +# The RELEASE CONTRACT: the suite's `#ifdef NDEBUG` bodies, in the only configuration where they are +# real code. +# +# Every other stage here builds `Dev`, so those bodies compile to nothing — the cases run, pass, and +# assert precisely nothing. This stage exists because that is indistinguishable from a clean run. +# +# A GENUINE Release build, not a flag bolted onto the Dev tree: the behaviour under test belongs to +# the LIBRARY (a writer that asserts in Dev must return false under NDEBUG), so the library has to +# be the one compiled with NDEBUG. `vcpkg-release` puts it in its own binaryDir while sharing +# build/vcpkg_installed — see the preset's own description. +# +# Only `test_fire_engine` is built. It already pulls in `fireengine` and the shaders; the +# application adds no contract coverage. And only the tagged cases run: the whole Release suite +# would drag in the optimisation-sensitive physics goldens, which are a different question with +# different failure modes. +ci_release_contract() +{ + cmake --preset vcpkg-release -DFIRE_ENGINE_WARNINGS_AS_ERRORS="${WARNINGS_AS_ERRORS}" + cmake --build --preset release-contract + # Catch2 exits non-zero when a spec matches nothing, so a renamed tag fails here rather than + # passing quietly; the hidden sentinel case inside the selection fails if this binary was not + # built with NDEBUG. Between them, a green run means the contract was actually checked. + ./build-release/test_fire_engine "[release-contract]" +} + # Run one named stage, composing prerequisites the same way CI does. ci_run_stage() { @@ -60,7 +91,15 @@ ci_run_stage() build) ci_configure && ci_build ;; tidy) ci_configure && ci_build && ci_tidy ;; test) ci_configure && ci_build && ci_test ;; - all) ci_format && ci_configure && ci_build && ci_tidy && ci_test ;; + release-contract) ci_release_contract ;; + all) + ci_format && ci_configure && ci_build && ci_tidy && ci_test || return $? + # Appended rather than folded into the chain above: `all` means "every gate this + # platform owns", and the Release contract is one Linux job by design. + if [ "${CI_RELEASE_CONTRACT}" = "1" ]; then + ci_release_contract + fi + ;; *) echo "unknown CI stage: $1" >&2 return 2 diff --git a/tools/ci/container-run.sh b/tools/ci/container-run.sh index a754298..4d3606d 100755 --- a/tools/ci/container-run.sh +++ b/tools/ci/container-run.sh @@ -10,9 +10,15 @@ mode="${1:-all}" sync_source() { + # Every build tree is excluded, and for two reasons at once: a host CMakeCache names host + # ABSOLUTE paths, so copying one in makes the container's configure fail outright ("the source + # /work/fireEngine/CMakeLists.txt does not match the source /Users/... used to generate + # cache"); and each of these is a mounted volume, which `--delete` would otherwise empty on + # every run. rsync -a --delete \ --exclude /.git \ --exclude /build \ + --exclude /build-release \ --exclude /vcpkg \ --exclude /vcpkg_installed \ /repo/ /work/fireEngine/ diff --git a/tools/ci/run-local-ci.sh b/tools/ci/run-local-ci.sh index 965b745..162aa86 100755 --- a/tools/ci/run-local-ci.sh +++ b/tools/ci/run-local-ci.sh @@ -3,17 +3,22 @@ set -euo pipefail usage() { cat <<'EOF' -Usage: tools/ci/run-local-ci.sh [format|configure|build|tidy|test|all|shell] +Usage: tools/ci/run-local-ci.sh [format|configure|build|tidy|test|release-contract|all|shell] Runs the GitHub Actions Linux checks inside Docker. The repository is copied into the container before each run; Docker volumes hold the Linux build tree and vcpkg checkout/cache so host build artifacts are left alone. + +`release-contract` builds a genuine Release tree and runs the [release-contract] +cases — the suite's #ifdef NDEBUG bodies, which every other stage compiles away. +`all` includes it; the macOS runner deliberately does not (the contract is +platform-independent, so one job proves it). EOF } mode="${1:-all}" case "${mode}" in - format|configure|build|tidy|test|all|shell) ;; + format|configure|build|tidy|test|release-contract|all|shell) ;; -h|--help|help) usage exit 0 @@ -29,6 +34,9 @@ platform="${DOCKER_PLATFORM:-linux/amd64}" platform_tag="${platform//\//-}" image="fireengine-local-ci:ubuntu-24.04-${platform_tag}" build_volume="fireengine-local-ci-build-${platform_tag}" +# The Release-contract tree gets its own volume for the same reason the Dev one has one: without it +# every `release-contract` run recompiles all ~270 translation units inside an emulated container. +release_build_volume="fireengine-local-ci-build-release-${platform_tag}" vcpkg_volume="fireengine-local-ci-vcpkg-${platform_tag}" build_parallel_level="${CMAKE_BUILD_PARALLEL_LEVEL:-2}" docker_run_flags=(--rm) @@ -50,9 +58,11 @@ docker run "${docker_run_flags[@]}" \ --platform "${platform}" \ --volume "${repo_root}:/repo:ro" \ --volume "${build_volume}:/work/fireEngine/build" \ + --volume "${release_build_volume}:/work/fireEngine/build-release" \ --volume "${vcpkg_volume}:/cache/vcpkg" \ --env VCPKG_ROOT=/cache/vcpkg \ --env VCPKG_DISABLE_METRICS=1 \ + --env CI_RELEASE_CONTRACT=1 \ --env CMAKE_BUILD_PARALLEL_LEVEL="${build_parallel_level}" \ --workdir /work/fireEngine \ "${image}" \ diff --git a/tools/ci/run-local-macos.sh b/tools/ci/run-local-macos.sh index 2ad368b..ec72684 100755 --- a/tools/ci/run-local-macos.sh +++ b/tools/ci/run-local-macos.sh @@ -7,7 +7,10 @@ set -euo pipefail # MoltenVK, glfw, a shader compiler (glslc or glslangValidator), and vcpkg via VCPKG_ROOT. It does # NOT install anything. # -# Stages mirror run-local-ci.sh: format | configure | build | tidy | test | all. +# Stages mirror run-local-ci.sh: format | configure | build | tidy | test | all. The one exception +# is `release-contract`, which is Linux-only ON PURPOSE — those cases assert what a writer returns +# once NDEBUG removes its assertion, which does not vary by platform, so a second job would spend +# runner minutes to re-prove it. `all` here therefore stays "every gate macOS owns". # # Note: clang-tidy on macOS uses Apple's clang-tidy, which can differ from the Linux one the CI / # Docker replica runs — treat the macos `tidy` stage as advisory; Linux is the source of truth.