From 43d3b42155fb1514429e594753440620b5f4514e Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 16:33:24 +0200 Subject: [PATCH 1/3] tests: give the strand race its own drain-and-re-arm case, because the load test never reaches the boundary (fixes #668) `tests/test_strand_race.cpp` is the named regression test for "a strand never runs two tasks for one key concurrently" -- the invariant most of `docs/spec/concurrency_and_lifetimes.md` rests on. It did not detect that invariant's regression. Reproduced before changing anything. With the pre-fix two-step drain restored in `scheduleNext` (flip `running` under `strand->mtx`, release it, then erase under `_mapMtx` in a separate critical section), on this branch's base `7a343e6f`, x86-64 Linux, clang 22.1.8, `clang-tsan` preset, `TSAN_OPTIONS=suppressions=cmake/tsan.supp`, binary carrying 173 `__tsan_` symbols: run 1..10: PASS All tests passed (41 assertions in 2 test cases) === [race] : 10 pass / 0 fail out of 10 === The miss is the load *shape*, not the load volume. The erase only fires when a drain finds the pending queue empty, and eight threads posting 3200 tasks back-to-back onto one key keep it non-empty almost throughout. The header comment's claim that "very short tasks maximise the drain/re-arm interleaving" is false in the direction that matters: they maximise re-arm, and never produce the quiet moment a drain needs. That claim is corrected in place. The new case manufactures the rendezvous instead of hoping for it. A pilot task is posted alone on the key and publishes the round number as its last act; four chaser threads spin on that publication and post the instant it flips, i.e. while the drain block following the pilot's body is deciding keep-running vs. erase, each with a per-thread stagger that walks the post across that window. A round ends only once every one of its tasks has run, so the strand really does empty before the next pilot. Three detectors, because the defect and its symptom are not the same event: `maxInFlight` (kept -- the symptom); plain, non-atomic state touched by every task (the defect: two strands for one key leave those accesses unordered by any happens-before edge, which TSan reports whether or not the tasks overlap, and whose lost updates are visible with no sanitizer at all); and per-producer FIFO, which an orphaned strand breaks without any two tasks overlapping. Mutation-tested, with the parameters as committed: clang-tsan, mutant, 10 runs: 0 pass / 10 fail 2-3 TSan data races per run; outOfOrder 65-114; maxInFlight 2 clang-debug (no sanitizer), 15 runs: 0 pass / 15 fail outOfOrder 1-9; maxInFlight 2 clang-tsan, mutant, pinned to 4 CPUs: 0 pass / 10 fail clang-tsan, mutant, pinned to 2 CPUs: 0 pass / 10 fail the OLD case, same mutant binary, 10 runs: 10 pass / 0 fail And it does not flake on the fixed tree: `[race]` passes 20/20 under `clang-tsan` and 20/20 under `clang-debug`, plus the full `morph_tests` binary (1562 cases, 23007 assertions). Cost: 1.0 s under TSan on 4 CPUs, 1.3 s on 2, 0.098 s without a sanitizer -- against the suite's `TIMEOUT 120`. `include/morph/core/strand.hpp` is untouched; the mutant was applied to a scratch copy and reverted (md5 verified against the pristine file). Not verified: the `Kanban / ThreadSanitizer` CI leg, which needs Qt and the ladder, and every non-Linux leg. The 2-CPU run is the closest stand-in here for a four-core runner under load. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- tests/test_strand_race.cpp | 189 ++++++++++++++++++++++++++++++++++++- 1 file changed, 185 insertions(+), 4 deletions(-) diff --git a/tests/test_strand_race.cpp b/tests/test_strand_race.cpp index 1df6a3919..d1f64a3e4 100644 --- a/tests/test_strand_race.cpp +++ b/tests/test_strand_race.cpp @@ -1,8 +1,10 @@ // SPDX-License-Identifier: Apache-2.0 +#include #include #include #include +#include #include #include #include @@ -19,10 +21,26 @@ // both strands dispatched tasks for that key concurrently — breaking the // per-model serialisation guarantee. // -// The test hammers post() on a single key from many threads. Every task bumps a -// per-key in-flight counter on entry and drops it on exit; if two tasks for the -// same key ever run concurrently the counter exceeds 1 and the test fails. Very -// short tasks maximise the drain/re-arm interleaving that triggered the bug. +// This file holds two cases, and they cover different things. This first one is +// a *load* test: it hammers post() on a single key from many threads, and every +// task bumps a per-key in-flight counter on entry and drops it on exit, so if +// two tasks for the same key ever run concurrently the counter exceeds 1 and the +// test fails. +// +// What it does **not** cover is the drain-and-erase boundary the bug above lives +// on. The erase only fires when a drain finds the pending queue empty, and eight +// threads posting 3200 tasks back-to-back onto one key keep it non-empty almost +// throughout: the quiet moment the defect needs never arrives. Measured, not +// assumed -- with the pre-fix two-step drain restored in `scheduleNext` (flip +// `running` under `strand->mtx`, release it, then erase under `_mapMtx` in a +// separate critical section), this case passed 10/10 under ThreadSanitizer on +// x86-64 Linux / clang 22.1.8 (morph#668). Short tasks maximise *re-arm*; they do +// not produce a drain. Turning the thread or post counts up makes that worse, +// not better. +// +// The second case below produces the shape this one cannot, and is the one that +// fails against that mutant. Keep both: saturation and the drain boundary are +// different failure modes of the same invariant. TEST_CASE("StrandExecutor never runs two tasks for one key concurrently under contention", "[strand][race]") { constexpr int kThreads = 8; constexpr int kPostsPerThread = 400; @@ -115,6 +133,169 @@ TEST_CASE("StrandExecutor never runs two tasks for one key concurrently under co } } +// The drain-and-re-arm boundary (morph#668), which the case above never reaches. +// +// Shape, not volume. The defect needs a strand to reach *empty* while a post is +// arriving, so this case manufactures that rendezvous instead of hoping for it: +// +// 1. A pilot task is posted alone on the key. Its last act is to publish the +// round number, so the chaser threads learn the strand is about to drain. +// 2. `kChasers` threads spin on that publication and post the instant it +// flips -- that is, while the drain block following the pilot's body is +// deciding "keep running vs. erase". A per-thread stagger walks each post +// across the handful of instructions that decision spans, so the window is +// sampled at many offsets rather than one. +// 3. The round ends only once every one of its tasks has run, so the strand +// really does empty before the next pilot. The gap is the point of the +// test, and is exactly what sustained saturation destroys. +// +// Three detectors, because the defect and its symptom are not the same event: +// +// * `maxInFlight` -- the *symptom*, as in the case above: two tasks for one +// key running at the same wall-clock moment. +// * plain, non-atomic state touched by every task -- the *defect*. Two strands +// for one key leave those accesses unordered by any happens-before edge, +// which ThreadSanitizer reports whether or not the two tasks ever overlap in +// wall clock. Lost updates to the same state are visible without a sanitizer +// at all, which is why the cells are checked as well as raced on. +// * per-producer FIFO -- a strand orphaned mid-burst can run one producer's +// later task before its earlier one, and an ordinary build sees that too. +// +// The tasks deliberately do a little plain work rather than none: an empty task +// gives an overlap a window a few instructions wide, which is why the case above +// can be wrong about serialisation and still report `maxInFlight == 1`. +TEST_CASE("StrandExecutor keeps one strand per key when a post races the drain", "[strand][race]") { + constexpr int kChasers = 4; + constexpr int kBurst = 3; + constexpr int kRounds = 600; + constexpr int kIterations = 6; + constexpr int kCells = 24; + constexpr int kPerRound = 1 + (kChasers * kBurst); + + morph::exec::detail::ModelId const key{7}; + + for (int iter = 0; iter < kIterations; ++iter) { + // Same ordering rule as the case above: the pool outlives the strand. + morph::exec::ThreadPoolExecutor pool{4}; + + // Deliberately plain -- no atomic, no mutex. Under the invariant this + // file exists for, the strand *is* the synchronisation: every handoff + // between two tasks for one key passes through `_mapMtx`/`strand->mtx`, + // so each task's writes happen-before the next task's reads and these + // are data-race-free. A second strand for the same key breaks that + // chain, and then they are not. + std::vector cells(static_cast(kCells), 0); + std::vector lastSeq(static_cast(kChasers) + 1, -1); + long long executedPlain = 0; + + std::atomic inFlight{0}; + std::atomic maxInFlight{0}; + std::atomic outOfOrder{0}; + std::atomic completed{0}; + // Round number whose pilot task has finished its body. Release/acquire: + // the chasers must not start posting for round r before it is set. + std::atomic gate{0}; + + auto body = [&](int producer, int seq) { + int const cur = inFlight.fetch_add(1) + 1; + int prev = maxInFlight.load(); + while (cur > prev && !maxInFlight.compare_exchange_weak(prev, cur)) { + } + auto const slot = static_cast(producer); + if (seq <= lastSeq[slot]) { + outOfOrder.fetch_add(1); + } + lastSeq[slot] = seq; + for (auto& cell : cells) { + cell += 1; + } + ++executedPlain; + inFlight.fetch_sub(1); + completed.fetch_add(1, std::memory_order_release); + }; + + { + morph::exec::detail::StrandExecutor strand{pool}; + + std::vector chasers; + chasers.reserve(kChasers); + for (int chaser = 0; chaser < kChasers; ++chaser) { + chasers.emplace_back([&, chaser] { + int const producer = chaser + 1; + // Per-thread LCG, so the stagger below differs per thread + // and per round without pulling in or a shared + // engine that would itself synchronise the threads. + auto rng = (static_cast(chaser) * 2654435761U) + 1U; + for (int round = 0; round < kRounds; ++round) { + // Spin rather than yield: the window this case aims at + // is a few instructions wide, and a yield overshoots it + // by orders of magnitude. The periodic yield is only a + // starvation guard for hosts with fewer cores than this + // case has threads (a CI runner has four); it fires once + // per 4096 spins, so it costs the rendezvous nothing. + for (unsigned spins = 0; gate.load(std::memory_order_acquire) <= round; ++spins) { + if ((spins & 0xFFFU) == 0xFFFU) { + std::this_thread::yield(); + } + } + for (int post = 0; post < kBurst; ++post) { + rng = (rng * 1664525U) + 1013904223U; + int const stagger = static_cast((rng >> 16U) & 0x3FU); + for (int step = 0; step < stagger; ++step) { + // Busy work, folded back into `rng` so it cannot + // be optimised away, walking this post to a + // different offset inside the drain window. + rng = (rng * 1103515245U) + 12345U; + } + int const seq = (round * kBurst) + post; + strand.post(key, [&body, producer, seq] { body(producer, seq); }); + } + } + }); + } + + for (int round = 0; round < kRounds; ++round) { + strand.post(key, [&body, &gate, round] { + body(0, round); + // Published last: the chasers' posts have to arrive while + // the drain that follows this body is running, not before. + gate.store(round + 1, std::memory_order_release); + }); + // Wait out the round rather than pipelining it. This is the + // quiet moment -- the strand drains to empty here, which is the + // only state from which the erase can fire at all. + while (completed.load(std::memory_order_acquire) < (round + 1) * kPerRound) { + std::this_thread::yield(); + } + } + + for (auto& chaser : chasers) { + chaser.join(); + } + // Closing this scope runs `~StrandExecutor`, which blocks until + // `_inFlight == 0`; see the case above for why that is a complete + // drain and not a deadline. + } + + constexpr int kExpected = kRounds * kPerRound; + INFO("iteration " << iter << ": completed " << completed.load() << " of " << kExpected); + // `CHECK`, not `REQUIRE`, for everything but the last line: each of + // these answers a different question about the same run and stopping at + // the first one would hide the others (see the case above). + CHECK(completed.load() == kExpected); + // Lost updates to the plain state: the sanitizer-free reading of the + // same defect the TSan legs see as a data race on it. + CHECK(executedPlain == static_cast(kExpected)); + auto const [lowest, highest] = std::minmax_element(cells.begin(), cells.end()); + CHECK(*lowest == kExpected); + CHECK(*highest == kExpected); + // FIFO per key is part of the contract, and an orphaned strand breaks it + // without any two tasks having to overlap. + CHECK(outOfOrder.load() == 0); + REQUIRE(maxInFlight.load() == 1); + } +} + // Regression test for ThreadPoolExecutor(0): a zero-worker pool used to accept // tasks that could never run, hanging every post() forever. The constructor now // clamps the worker count to at least 1, so a pool built with 0 is still usable. From dd9cb74e0553fb675ff5bd0c5cbb101b6e676cd7 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 16:33:35 +0200 Subject: [PATCH 2/3] ci: record and check the Catch2 the clang-tidy job analyses against (fixes #666, refs #667) `apt-get install -y catch2` pins nothing, and which Catch2 is on the include path decides whether a `readability-function-cognitive-complexity` finding on a `TEST_CASE` body reaches the clang-tidy-diff job at all. So a workstation can run the same clang-tidy over the same diff with the same job flags and exit 0 where CI exits 1 -- silently, reporting nothing rather than reporting less (#666). Nothing in the tree stated which Catch2 the job had, except nine hand-written copies of a sentence that nothing checked. `scripts/check_catch2_pin.sh` has two halves, and only the second one is new in kind: A. Textual, modelled on `scripts/check_ci_clang_pin.sh` (#457) down to the rule-B rewording trap and the anti-vacuity failure: every `CI pins catch2 ` in the tree must name `CATCH2_VERSION` from ci.yml, and any other line naming a Catch2 version beside a CI reference is rejected as a phrasing the gate cannot check. B. Behavioural: the Catch2 headers actually present are read and compared against that pin. Under `--strict`, which is how the clang-tidy job runs it, a moved runner package -- or no Catch2 at all -- fails the job before it measures anything. Without `--strict`, a workstation run prints the divergence and says in as many words that its clang-tidy-diff result is not the measurement CI makes. `scripts/test_check_catch2_pin.sh` drives both halves from the wrong side as well as the right one: 13 cases, every drift the gate claims to catch reintroduced one at a time into a scratch git tree or a synthetic include directory, each required to fail *for the stated reason*, plus four false-positive mirrors. Run here: ok: the unmodified tree passes against the pinned Catch2 ok: caught: a doc asserting CI pins catch2 3.5.3 while ci.yml pins 3.4.0 ok: caught: ci.yml bumped to 3.5.3 while the nine copies still say 3.4.0 ok: caught: a CI Catch2 claim in an unrecognised phrasing, even with the right version ok: caught: a CI Catch2 claim in an unrecognised phrasing with the wrong version ok: caught: every canonical assertion removed from the tree ok: caught: ci.yml with no CATCH2_VERSION to read ok: accepted: a Catch2 version named with no CI reference on the line ok: accepted: a historical record carrying the documented marker ok: caught: --strict against a runner whose Catch2 package has moved ok: caught: --strict with no Catch2 installed at all ok: accepted: a workstation whose Catch2 differs is warned, not failed ok: accepted: a workstation whose Catch2 matches the pin is not warned ok: all catch2-pin checker self-tests passed The version is 3.4.0, which is what the nine `.clang-tidy` copies already said -- #667 is wrong and is not "fixed" here. ubuntu-24.04's package is 3.4.0-1build1 (Launchpad, noble Release pocket, no -updates publication), and the artifact line both tickets read as 3.5.x is v3.4.0's text exactly: v3.4.0 catch_test_registry.hpp:121 INTERNAL_CATCH_TESTCASE2( INTERNAL_CATCH_UNIQUE_NAME( dummyFunction ) ) v3.5.3 catch_test_registry.hpp:121 INTERNAL_CATCH_TESTCASE2( INTERNAL_CATCH_UNIQUE_NAME( dummyFunction ), __VA_ARGS__ ) The artifact has no `, __VA_ARGS__`. Both tickets rest on "3.4.0 has no `CATCH_CONFIG_EXPERIMENTAL_STATIC_ANALYSIS_SUPPORT`, so it cannot produce `dummyFunctionNN`"; v3.4.0 has it, at `catch_test_registry.hpp:77/91/124`. The nine copies are therefore reworded rather than corrected: same fact, in the canonical phrasing the gate reads, so the number is checked instead of asserted. What this does not do, because it matters that the line is drawn: it does not make a local clang-tidy-diff agree with CI's. Only the job not depending on the runner's Catch2 does that -- #666's first closing condition, which needs a CMake change in a file this branch does not own, and is filed separately rather than folded in. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/ci.yml | 35 +++++ examples/bank/tests/.clang-tidy | 9 +- examples/bookmarks/tests/.clang-tidy | 9 +- examples/common/testkit/.clang-tidy | 9 +- examples/crm/tests/.clang-tidy | 9 +- examples/kanban/tests/.clang-tidy | 9 +- examples/ledger/tests/.clang-tidy | 9 +- examples/lims/tests/.clang-tidy | 9 +- examples/pastebin/tests/.clang-tidy | 9 +- examples/polls/tests/.clang-tidy | 9 +- scripts/check_catch2_pin.sh | 218 ++++++++++++++++++++++++++ scripts/test_check_catch2_pin.sh | 226 +++++++++++++++++++++++++++ 12 files changed, 533 insertions(+), 27 deletions(-) create mode 100755 scripts/check_catch2_pin.sh create mode 100755 scripts/test_check_catch2_pin.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54112f4a7..3be12e6c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,27 @@ env: # the cache itself -- jobs still read each other's entries. FASTCACHE_PREFETCH_GROUP: "${{ github.run_id }}-${{ github.job }}" CLANG_VERSION: "22" + # The Catch2 the clang-tidy-diff job analyses against. Not a version this + # workflow installs -- `apt-get install -y catch2` takes whatever + # ubuntu-24.04 ships -- but a record of what that has been measured to be, + # which the "Assert the Catch2 this job analyses against" step below reads + # back off the runner and fails on if it has moved. + # + # It is worth recording because it is load-bearing and invisible. Which + # Catch2 is on the include path decides whether + # readability-function-cognitive-complexity findings on a TEST_CASE body + # reach this job at all, so a workstation with a different one can run the + # same clang-tidy over the same diff with the same flags and exit 0 where + # this job exits 1 -- silently, reporting nothing rather than reporting less + # (morph#666). An unpinned package that decides a gate's outcome and moves + # without notice is the shape this repository keeps getting caught by. + # + # ubuntu-24.04's package is 3.4.0-1build1 (Launchpad, noble Release pocket), + # and the clang-tidy-report artifact of run 35581623269 prints + # `/usr/include/catch2/internal/catch_test_registry.hpp:121` with + # `INTERNAL_CATCH_TESTCASE2( INTERNAL_CATCH_UNIQUE_NAME( dummyFunction ) )` + # and no `, __VA_ARGS__` -- v3.4.0's text exactly, and not v3.5.3's. + CATCH2_VERSION: "3.4.0" # MORPH_BUILD_FORMS_QML needs Qt 6.5+; ubuntu-24.04 apt still ships 6.4.2. QT_VERSION: "6.8.1" @@ -2193,6 +2214,20 @@ jobs: - name: Self-test the clang-tidy suppression-scope checker run: bash scripts/test_check_tidy_suppression_scope.sh clang-tidy-${{ env.CLANG_VERSION }} + # The Catch2 on this runner's include path decides whether a + # readability-function-cognitive-complexity finding on a TEST_CASE body + # reaches this job, and `apt-get install -y catch2` above pins nothing. + # This step reads the version out of the headers the step above just + # installed and fails if it is not the one CATCH2_VERSION records, so a + # move in the runner image is a red job rather than a quiet change of + # what this gate measures (morph#666). Its self-test runs first, for the + # same reason the suppression-scope checker's does. + - name: Self-test the catch2-pin checker + run: bash scripts/test_check_catch2_pin.sh + + - name: Assert the Catch2 this job analyses against + run: bash scripts/check_catch2_pin.sh . --strict + # Catches morph#632's bug class: tests/.clang-tidy's thirteen # suppressions are argued as Catch2 and raw-syscall idiom, which is true # of test sources and says nothing about include/morph/** -- yet diff --git a/examples/bank/tests/.clang-tidy b/examples/bank/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/bank/tests/.clang-tidy +++ b/examples/bank/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/bookmarks/tests/.clang-tidy b/examples/bookmarks/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/bookmarks/tests/.clang-tidy +++ b/examples/bookmarks/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/common/testkit/.clang-tidy b/examples/common/testkit/.clang-tidy index 759365510..21f86b275 100644 --- a/examples/common/testkit/.clang-tidy +++ b/examples/common/testkit/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/crm/tests/.clang-tidy b/examples/crm/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/crm/tests/.clang-tidy +++ b/examples/crm/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/kanban/tests/.clang-tidy b/examples/kanban/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/kanban/tests/.clang-tidy +++ b/examples/kanban/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/ledger/tests/.clang-tidy b/examples/ledger/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/ledger/tests/.clang-tidy +++ b/examples/ledger/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/lims/tests/.clang-tidy b/examples/lims/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/lims/tests/.clang-tidy +++ b/examples/lims/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/pastebin/tests/.clang-tidy b/examples/pastebin/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/pastebin/tests/.clang-tidy +++ b/examples/pastebin/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/polls/tests/.clang-tidy b/examples/polls/tests/.clang-tidy index 1bbb5028c..d7ad0d3f1 100644 --- a/examples/polls/tests/.clang-tidy +++ b/examples/polls/tests/.clang-tidy @@ -11,9 +11,12 @@ # logical operator -- would defeat the expression decomposition that makes a # failing assertion print its two operands. Catch2 says so itself; the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. The clang-tidy-diff job installs ubuntu-24.04's catch2, -# 3.4.0, which predates that comment -- which is why the finding reaches CI -# here and not on a workstation with a current Catch2. +# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which +# predates that comment -- which is why the finding reaches CI here and not on +# a workstation with a current Catch2. That number is checked rather than +# asserted: scripts/check_catch2_pin.sh reads the runner's installed version +# back off the include path and fails this line and the clang-tidy-diff job +# together if the package moves (morph#666). # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/scripts/check_catch2_pin.sh b/scripts/check_catch2_pin.sh new file mode 100755 index 000000000..b1a8197d4 --- /dev/null +++ b/scripts/check_catch2_pin.sh @@ -0,0 +1,218 @@ +#!/usr/bin/env bash +# Usage: bash scripts/check_catch2_pin.sh [REPO_ROOT] [--strict] +# +# Two halves, both about the same fact: which Catch2 the clang-tidy-diff job +# analyses against. +# +# A. Textual. Every line in the tree asserting `CI pins catch2 ` must name +# the version .github/workflows/ci.yml records in CATCH2_VERSION, and any +# other line naming a Catch2 version beside a CI reference is rejected as +# a phrasing this gate cannot check. +# B. Behavioural. The Catch2 headers actually installed here are read and +# compared against that same pin. Under `--strict` (how the CI job runs +# it) a mismatch, or no Catch2 at all, fails. Without it -- a workstation +# run -- a mismatch prints a divergence notice instead, because a +# workstation is not required to carry the runner's package, only to know +# that it does not. +# +# Why this gate exists (morph#666): a local `clang-tidy-diff` over the same +# diff, with the same clang-tidy version and the same job flags, can exit 0 on +# a diff the CI job fails -- silently, reporting nothing rather than reporting +# less. The variable is the Catch2 on the include path: +# `readability-function-cognitive-complexity` computes the *same* score under +# both, and what differs is whether `ClangTidyDiagnosticConsumer` classifies +# the finding as user code, which depends on which notes a given Catch2 +# release's `TEST_CASE` expansion produces and where it puts them. That is how +# morph#656's branch shipped a NOLINT reason asserting a neighbouring TEST_CASE +# "scores under the threshold" while it scored 87 against a threshold of 25: +# the local gate agreed with it. +# +# What this gate does and does not do, stated plainly, because the distinction +# is the whole point of the ticket: +# +# * It makes CI's own Catch2 a *decision* rather than an accident. `apt-get +# install -y catch2` is unpinned; if the runner image's package moves, the +# clang-tidy job's measurement changes with nothing anywhere saying so. +# Half B under `--strict` turns that silent move into a failed job. +# * It does not make a local clang-tidy-diff agree with CI's. Nothing short +# of the job not depending on the runner's Catch2 at all does that -- +# morph#666's first closing condition, filed separately as a follow-up +# because it needs a CMake change in a file this change does not own. +# What half B does on a workstation is report, every time it is run, that +# the two measurements are not the same one. +# +# Requires git and grep; compiles nothing. +set -euo pipefail + +repo_root="" +strict=0 +for arg in "$@"; do + case "$arg" in + --strict) strict=1 ;; + *) repo_root="$arg" ;; + esac +done +if [ -z "$repo_root" ]; then + repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +fi +readonly repo_root strict + +readonly ci_workflow=".github/workflows/ci.yml" +readonly self="scripts/check_catch2_pin.sh" +readonly self_test="scripts/test_check_catch2_pin.sh" + +# `catch2 3.4.0`, `Catch2-3.4.0`, `Catch2 v3.8.1`. Not `Catch2.git` (no +# separator), and not a bare `3.4.0` with no product name on the line. +readonly catch2_version_re='[Cc]atch2[ -]v?[0-9]+\.[0-9]+(\.[0-9]+)?' +readonly ci_ref_re='(\bCI\b|ci\.yml|CATCH2_VERSION)' +readonly canonical_re='CI pins catch2 [0-9]+\.[0-9]+\.[0-9]+' +readonly historical_marker='catch2-pin: historical' + +failures=0 +canonical_sites=0 + +note() { printf 'ok: %s\n' "$*"; } +fail() { printf 'error: %s\n' "$*" >&2; failures=$((failures + 1)); } + +cd "$repo_root" + +# -- The source of truth ----------------------------------------------------- +if [ ! -f "$ci_workflow" ]; then + printf 'error: %s not found under %s\n' "$ci_workflow" "$repo_root" >&2 + exit 1 +fi + +pinned="$(sed -nE 's/^[[:space:]]*CATCH2_VERSION:[[:space:]]*"?([0-9]+\.[0-9]+\.[0-9]+)"?[[:space:]]*$/\1/p' \ + "$ci_workflow" | head -n 1)" + +if [ -z "$pinned" ]; then + printf 'error: no `CATCH2_VERSION: ""` found in %s -- this gate reads\n' \ + "$ci_workflow" >&2 + printf ' its expected value from there and cannot check anything without it\n' >&2 + exit 1 +fi + +note "${ci_workflow} pins catch2 ${pinned}" + +# -- A. The textual half ----------------------------------------------------- +mapfile -t files < <(git ls-files \ + | grep -vFx "$ci_workflow" \ + | grep -vFx "$self" \ + | grep -vFx "$self_test" \ + | { grep -v '^$' || true; }) + +if [ "${#files[@]}" -eq 0 ]; then + printf 'error: git ls-files returned nothing under %s\n' "$repo_root" >&2 + exit 1 +fi + +while IFS= read -r hit; do + [ -n "$hit" ] || continue + location="${hit%%:*}" + rest="${hit#*:}" + lineno="${rest%%:*}" + text="${rest#*:}" + + case "$text" in + *"$historical_marker"*) + note "${location}:${lineno}: marked historical, not checked" + continue + ;; + esac + + if printf '%s' "$text" | grep -qE "$canonical_re"; then + while IFS= read -r stated; do + canonical_sites=$((canonical_sites + 1)) + if [ "$stated" = "$pinned" ]; then + note "${location}:${lineno}: states catch2 ${stated}" + else + fail "${location}:${lineno}: states 'CI pins catch2 ${stated}', but ${ci_workflow} pins catch2 ${pinned}: + ${text}" + fi + done < <(printf '%s' "$text" | grep -oE "$canonical_re" \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+') + continue + fi + + fail "${location}:${lineno}: names a Catch2 version beside a CI reference in a + phrasing this gate cannot check. Write it as 'CI pins catch2 ${pinned}', or + append the marker '${historical_marker}' if it is a dated record rather + than a claim about the pin now: + ${text}" +done < <(grep -nHIE "$catch2_version_re" -- "${files[@]}" 2>/dev/null \ + | grep -E "$ci_ref_re" || true) + +if [ "$canonical_sites" -eq 0 ]; then + fail "no 'CI pins catch2 ' assertion found anywhere in the tree. Either + the documentation stopped saying which Catch2 the clang-tidy job analyses + against, or it was reworded out of the shape this gate reads -- both leave + the gate checking nothing while still exiting 0, so it fails instead." +fi + +# -- B. The behavioural half ------------------------------------------------- +# Read the version out of the headers that are actually on this machine's +# include path, the same ones clang-tidy would expand TEST_CASE from. +# +# MORPH_CATCH2_INCLUDE_DIR, when set, *replaces* the default search rather than +# preceding it: the self-test needs a run in which no Catch2 is found, and a +# fallback to /usr/include would make that case pass or fail depending on what +# the machine running the self-test happens to have installed. +installed="" +installed_dir="" +if [ -n "${MORPH_CATCH2_INCLUDE_DIR:-}" ]; then + search_prefixes="${MORPH_CATCH2_INCLUDE_DIR}" +else + search_prefixes="/usr/include /usr/local/include" +fi +for prefix in $search_prefixes; do + header="${prefix}/catch2/catch_version_macros.hpp" + [ -f "$header" ] || continue + major="$(sed -nE 's/^#define CATCH_VERSION_MAJOR ([0-9]+).*$/\1/p' "$header" | head -n 1)" + minor="$(sed -nE 's/^#define CATCH_VERSION_MINOR ([0-9]+).*$/\1/p' "$header" | head -n 1)" + patch="$(sed -nE 's/^#define CATCH_VERSION_PATCH ([0-9]+).*$/\1/p' "$header" | head -n 1)" + if [ -n "$major" ] && [ -n "$minor" ] && [ -n "$patch" ]; then + installed="${major}.${minor}.${patch}" + installed_dir="$prefix" + break + fi +done + +if [ -z "$installed" ]; then + if [ "$strict" -eq 1 ]; then + fail "no Catch2 headers found on this machine, but --strict says this run *is* + the clang-tidy job's own environment. The job installs catch2 from apt + before this step; if that stopped happening, the measurement below this + step is no longer the one the pin describes." + else + note "no Catch2 headers found here -- nothing to compare against the pin" + fi +elif [ "$installed" = "$pinned" ]; then + note "installed Catch2 ${installed} (${installed_dir}) matches the pin" +elif [ "$strict" -eq 1 ]; then + fail "installed Catch2 is ${installed} (${installed_dir}) but ${ci_workflow} pins + catch2 ${pinned}. The runner image's package moved. Every clang-tidy-diff + result from this job is now a measurement against ${installed}, and the + nine examples/*/tests/.clang-tidy comments that explain why a local run + disagrees with CI describe ${pinned}. Update CATCH2_VERSION and those + comments together, having checked that the suppressions they argue for + still apply to ${installed}." +else + printf '\n' >&2 + printf 'WARNING: this machine'"'"'s Catch2 is %s (%s); CI pins catch2 %s.\n' \ + "$installed" "$installed_dir" "$pinned" >&2 + printf ' A local clang-tidy-diff run is therefore NOT the measurement the\n' >&2 + printf ' clang-tidy-diff job makes. For checks whose evidence lives inside\n' >&2 + printf ' Catch2 macro expansions -- readability-function-cognitive-complexity\n' >&2 + printf ' on a TEST_CASE body is the known one -- it can exit 0 on a diff CI\n' >&2 + printf ' fails, reporting nothing rather than reporting less (morph#666).\n' >&2 + printf ' A green local run is not evidence for those checks. It is still\n' >&2 + printf ' evidence for every check whose finding lands on a line you wrote.\n' >&2 + printf '\n' >&2 +fi + +if [ "$failures" -ne 0 ]; then + printf '\n%s catch2-pin check(s) failed\n' "$failures" >&2 + exit 1 +fi + +note "all ${canonical_sites} catch2-pin assertion(s) agree with ${ci_workflow}" diff --git a/scripts/test_check_catch2_pin.sh b/scripts/test_check_catch2_pin.sh new file mode 100755 index 000000000..d1f35fefe --- /dev/null +++ b/scripts/test_check_catch2_pin.sh @@ -0,0 +1,226 @@ +#!/usr/bin/env bash +# Usage: bash scripts/test_check_catch2_pin.sh +# +# Self-test for scripts/check_catch2_pin.sh, the gate that keeps the Catch2 the +# clang-tidy-diff job analyses against a recorded, checked fact rather than +# whatever apt shipped that morning. +# +# A lint gate nobody tests reports green whether or not it still detects +# anything, and this one is doubly exposed: the tree it guards is correct +# today, and the runner's package is the pinned one today, so the gate passes +# today whether or not either half is looking at anything. Both halves are +# therefore driven from the wrong side as well as the right one -- every drift +# the gate claims to catch is reintroduced into a scratch copy of the tree, or +# into a synthetic include directory, and must be caught for the stated reason +# rather than merely with a nonzero exit. +# +# One mutation at a time: applied together, a single detection would mask +# every other. +# +# The checker enumerates the tree with `git ls-files`, so each case runs +# against a throwaway git repository holding a copy of this one's tracked +# files rather than against a directory argument. +set -euo pipefail + +readonly repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly checker="scripts/check_catch2_pin.sh" + +failures=0 + +note() { printf 'ok: %s\n' "$*"; } +fail() { printf 'error: %s\n' "$*" >&2; failures=$((failures + 1)); } + +scratch="$(mktemp -d)" +trap 'rm -rf "$scratch"' EXIT + +readonly pristine="${scratch}/pristine" +mkdir -p "$pristine" +while IFS= read -r -d '' tracked; do + mkdir -p "${pristine}/$(dirname "$tracked")" + cp "${repo_root}/${tracked}" "${pristine}/${tracked}" +done < <(cd "$repo_root" && git ls-files -z) +git -C "$pristine" init -q +git -C "$pristine" add -A + +make_tree() { + local dest="$1" + rm -rf "$dest" + mkdir -p "$dest" + cp -R "${pristine}/." "$dest" +} + +# `sed -i` is not portable between GNU and BSD sed; edit through a temp file. +edit() { + local file="$1"; shift + sed "$@" "$file" > "${file}.new" + mv "${file}.new" "$file" +} + +# A synthetic Catch2 include tree carrying exactly the three version macros the +# checker reads. This is what lets the behavioural half be driven from a +# version the machine running the self-test does not have installed. +make_catch2() { + local dir="$1" version="$2" + local major="${version%%.*}" rest="${version#*.}" + local minor="${rest%%.*}" patch="${rest#*.}" + rm -rf "$dir" + mkdir -p "${dir}/catch2" + { + printf '#define CATCH_VERSION_MAJOR %s\n' "$major" + printf '#define CATCH_VERSION_MINOR %s\n' "$minor" + printf '#define CATCH_VERSION_PATCH %s\n' "$patch" + } > "${dir}/catch2/catch_version_macros.hpp" +} + +# An include directory with no Catch2 in it at all. +readonly empty_include="${scratch}/no-catch2" +mkdir -p "$empty_include" + +# `$expected` is a substring the resulting diagnostic must contain; without it +# a mutation that broke the tree in some unrelated way -- a mangled sed, a file +# the mutator emptied -- would count as a detection, and this self-test would +# report a gate that no longer detects anything as fully working. +expect_caught() { + local description="$1" mutator="$2" expected="$3" extra_args="${4:-}" catch2_dir="${5:-}" + local tree="${scratch}/case" output + make_tree "$tree" + if ! ( cd "$tree" && eval "$mutator" ); then + fail "mutator failed to apply: ${description}" + return + fi + if output="$( cd "$tree" && MORPH_CATCH2_INCLUDE_DIR="$catch2_dir" \ + bash "$checker" . $extra_args 2>&1 )"; then + fail "NOT caught: ${description} -- the gate passed a tree it should reject" + printf '%s\n' "$output" >&2 + return + fi + if printf '%s' "$output" | grep -qF "$expected"; then + note "caught: ${description}" + else + fail "caught for the WRONG reason: ${description} -- no diagnostic containing '${expected}':" + printf '%s\n' "$output" >&2 + fi +} + +# The mirror, for the false positives this gate must not manufacture. A rule +# that rejected every mention of a Catch2 version anywhere would "catch" every +# case above while being useless. +expect_accepted() { + local description="$1" mutator="$2" expected="${3:-}" extra_args="${4:-}" catch2_dir="${5:-}" + local tree="${scratch}/case" output + make_tree "$tree" + if ! ( cd "$tree" && eval "$mutator" ); then + fail "mutator failed to apply: ${description}" + return + fi + if ! output="$( cd "$tree" && MORPH_CATCH2_INCLUDE_DIR="$catch2_dir" \ + bash "$checker" . $extra_args 2>&1 )"; then + fail "FALSE POSITIVE: ${description} -- the gate rejected a tree it should accept:" + printf '%s\n' "$output" >&2 + return + fi + if [ -z "$expected" ] || printf '%s' "$output" | grep -qF "$expected"; then + note "accepted: ${description}" + else + fail "accepted but SILENT: ${description} -- no output containing '${expected}':" + printf '%s\n' "$output" >&2 + fi +} + +readonly pinned_catch2="${scratch}/catch2-3.4.0" +readonly moved_catch2="${scratch}/catch2-3.5.3" +make_catch2 "$pinned_catch2" 3.4.0 +make_catch2 "$moved_catch2" 3.5.3 + +# -- The unmodified tree must pass ------------------------------------------- +make_tree "${scratch}/clean" +if output="$( cd "${scratch}/clean" && MORPH_CATCH2_INCLUDE_DIR="$pinned_catch2" \ + bash "$checker" . --strict 2>&1 )"; then + note "the unmodified tree passes against the pinned Catch2" +else + fail "the unmodified tree was rejected by the gate:" + printf '%s\n' "$output" >&2 +fi + +# -- A. The textual half ----------------------------------------------------- +expect_caught "a doc asserting CI pins catch2 3.5.3 while ci.yml pins 3.4.0" \ + "printf '%s\n' 'The lint leg reproduces because CI pins catch2 3.5.3 there.' \ + >> docs/spec/testing_charter.md" \ + "states 'CI pins catch2 3.5.3', but .github/workflows/ci.yml pins catch2 3.4.0" + +# The direction morph#666 will actually take: the runner image moves, someone +# updates CATCH2_VERSION, and the nine .clang-tidy copies stay where they are. +expect_caught "ci.yml bumped to 3.5.3 while the nine copies still say 3.4.0" \ + "edit .github/workflows/ci.yml -e 's/^ CATCH2_VERSION: \"3.4.0\"/ CATCH2_VERSION: \"3.5.3\"/'" \ + "but .github/workflows/ci.yml pins catch2 3.5.3" + +# Rule B: the rewording that would defeat rule A alone. The nine copies said +# the same sentence nine times; a tenth site wording it differently is exactly +# how the number went unchecked in the first place. +expect_caught "a CI Catch2 claim in an unrecognised phrasing, even with the right version" \ + "printf '%s\n' 'The CI lint job installs catch2 3.4.0 from apt.' \ + >> docs/spec/testing_charter.md" \ + "phrasing this gate cannot check" + +expect_caught "a CI Catch2 claim in an unrecognised phrasing with the wrong version" \ + "printf '%s\n' 'Measured against Catch2 3.16.0 while CI has catch2-3.5.3.' \ + >> docs/spec/testing_charter.md" \ + "phrasing this gate cannot check" + +expect_caught "every canonical assertion removed from the tree" \ + "for f in \$(git grep -lF 'CI pins catch2' -- . ':!scripts/check_catch2_pin.sh' \ + ':!scripts/test_check_catch2_pin.sh'); do + edit \"\$f\" -e 's/CI pins catch2 [0-9.]*/the pinned Catch2/g' + done" \ + "no 'CI pins catch2 ' assertion found anywhere in the tree" + +expect_caught "ci.yml with no CATCH2_VERSION to read" \ + "edit .github/workflows/ci.yml -e 's/^ CATCH2_VERSION: \"3.4.0\"/ UNRELATED_CATCH2: \"3.4.0\"/'" \ + 'no `CATCH2_VERSION: ""` found' + +# A Catch2 version discussed without invoking CI in the same breath is not this +# gate's business; rejecting it would make the gate unsatisfiable for any +# document recording a local measurement or a FetchContent tag. +expect_accepted "a Catch2 version named with no CI reference on the line" \ + "printf '%s\n' 'Reproduced against Catch2 3.16.0 on this workstation.' \ + >> docs/spec/testing_charter.md" + +expect_accepted "a historical record carrying the documented marker" \ + "printf '%s\n' 'Before noble, CI had catch2 2.13.10 (catch2-pin: historical).' \ + >> docs/spec/testing_charter.md" + +# -- B. The behavioural half ------------------------------------------------- +# The whole reason this gate is not just another prose checker: the runner's +# package moving under an unpinned `apt-get install -y catch2` must fail the +# job rather than quietly change what it measures. +expect_caught "--strict against a runner whose Catch2 package has moved" \ + "true" \ + "installed Catch2 is 3.5.3" \ + "--strict" "$moved_catch2" + +expect_caught "--strict with no Catch2 installed at all" \ + "true" \ + "no Catch2 headers found on this machine" \ + "--strict" "$empty_include" + +# Without --strict -- a workstation run -- a divergence is reported rather than +# failed, because a workstation is not required to carry the runner's package. +# But it must be *reported*: silence here is the defect morph#666 is about. +expect_accepted "a workstation whose Catch2 differs is warned, not failed" \ + "true" \ + "A local clang-tidy-diff run is therefore NOT the measurement" \ + "" "$moved_catch2" + +# And a workstation that does match must not be warned, or the notice becomes +# noise that gets filtered out. +expect_accepted "a workstation whose Catch2 matches the pin is not warned" \ + "true" \ + "matches the pin" \ + "" "$pinned_catch2" + +if [ "$failures" -ne 0 ]; then + printf '\n%s self-test check(s) failed\n' "$failures" >&2 + exit 1 +fi + +note "all catch2-pin checker self-tests passed" From f619158825d27db64ffc23f8bcba2659e33439bf Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 16:45:31 +0200 Subject: [PATCH 3/3] tests: use the ranges overload the clang-tidy gate asks for `clang-tidy-diff` reported one finding on the new race case: tests/test_strand_race.cpp:289:40: error: use a ranges version of this algorithm [modernize-use-ranges,-warnings-as-errors] `std::minmax_element(cells.begin(), cells.end())` becomes `std::ranges::minmax_element(cells)`. `` was already included, and the result is an aggregate of two iterators rather than a pair, so the structured binding and both dereferences are unchanged -- verified by compiling the same shape standalone under clang 22.1.8 `-std=c++23 -Werror`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- tests/test_strand_race.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_strand_race.cpp b/tests/test_strand_race.cpp index d1f64a3e4..ace091859 100644 --- a/tests/test_strand_race.cpp +++ b/tests/test_strand_race.cpp @@ -286,7 +286,7 @@ TEST_CASE("StrandExecutor keeps one strand per key when a post races the drain", // Lost updates to the plain state: the sanitizer-free reading of the // same defect the TSan legs see as a data race on it. CHECK(executedPlain == static_cast(kExpected)); - auto const [lowest, highest] = std::minmax_element(cells.begin(), cells.end()); + auto const [lowest, highest] = std::ranges::minmax_element(cells); CHECK(*lowest == kExpected); CHECK(*highest == kExpected); // FIFO per key is part of the contract, and an orphaned strand breaks it