From dd1982972869950949373ab5b5dab92264b5651e Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:19:52 +0300 Subject: [PATCH 1/4] tests(outbox): cover the null-sink warning path in relay() Every existing test paired a null sink with a non-empty drainOutbox(), which would dereference the null sink -- real UB, not exercisable. This pairs the null sink with an empty drainOutbox() result instead, keeping relay() on its early-return path (which runs after logIfAnyDepNull() but before any sink use), so the null-sink warning fires without ever calling through the null pointer. The surrounding comment is updated to reflect exactly what's now covered vs. what's still tracked by LASTRADA-Software/morph#95 (the sink dereference itself, on a non-empty drain, which still needs the fault-injection seam that issue requests). --- tests/test_outbox.cpp | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/tests/test_outbox.cpp b/tests/test_outbox.cpp index 103a7768..833bd4ed 100644 --- a/tests/test_outbox.cpp +++ b/tests/test_outbox.cpp @@ -361,7 +361,37 @@ TEST_CASE("OutboxRelay::relay(): a null drainOutbox is logged, not rejected, at REQUIRE(sink->entries().empty()); // drainOutbox() threw before anything reached the sink } -// A null `sink` is deliberately NOT exercised into relay()'s body here: unlike +TEST_CASE("OutboxRelay::relay(): a null sink is logged, not rejected, at call time", "[outbox][relay]") { + // Unlike the null-drainOutbox/markRelayed cases above, relay() must not + // actually dereference the null sink (sink->append() on a null + // shared_ptr is a null-pointer virtual dispatch -- real UB, + // not a catchable exception -- see the comment below). Pairing the null + // sink with an empty drainOutbox() keeps relay() on its early-return path + // (`if (rows.empty()) return {};`), which runs *after* + // logIfAnyDepNull() but *before* any sink use -- so the null-sink warning + // fires without ever calling through the null pointer. + std::vector logged; + morph::log::ScopedLoggerOverride guard{ + [&](morph::log::LogLevel, std::string_view msg) { logged.emplace_back(msg); }, + morph::log::LogLevel::debug, + }; + + OutboxRelay relay; + relay.drainOutbox = [] { return std::vector{}; }; + relay.markRelayed = [](std::span) {}; + // relay.sink left null on purpose. + + auto result = relay.relay(); + REQUIRE(result.relayed == 0); + + bool sawWarning = std::any_of(logged.begin(), logged.end(), [](const std::string& line) { + return line.find("null sink") != std::string::npos; + }); + REQUIRE(sawWarning); +} + +// A null `sink` reaching relay()'s *sink-using* path (i.e. with drainOutbox() +// returning rows) is deliberately NOT exercised here: unlike // `drainOutbox`/`markRelayed` (null std::function -> catchable // std::bad_function_call), `sink` is a std::shared_ptr, so // `sink->append(row)` on a null sink is a null-pointer virtual dispatch -- @@ -373,10 +403,10 @@ TEST_CASE("OutboxRelay::relay(): a null drainOutbox is logged, not rejected, at // unit test would need a signal/guard-page harness like // test_bridge_lifetime.cpp's POSIX-only hasSubscribers() case, which is far // more machinery than this one branch warrants and still would not run on -// Windows/MSVC, where this suite also builds. The logIfAnyDepNull() call -// itself (the actual branch under test) is still reached and its warning -// still fires -- only the subsequent crash is left unexercised. Tracked as -// LASTRADA-Software/morph#95. +// Windows/MSVC, where this suite also builds. The logIfAnyDepNull() call's +// null-sink warning line itself is now covered by the empty-drainOutbox test +// above -- only the subsequent crash on a non-empty drain is left +// unexercised. Tracked as LASTRADA-Software/morph#95. TEST_CASE("OutboxRelay + FileActionLog: re-relay after a simulated process restart dedups via the sink", "[outbox][relay][file_action_log]") { From f1ecf26645b9f29b7a8247572da599598e417134 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:19:57 +0300 Subject: [PATCH 2/4] tests(logger): cover logFormat's suppressed-below-threshold path Every existing test called the plain std::string_view overloads (going through detail::log directly); none exercised the std::format_string template overload (logFormat/logWarn(fmt, args...)) at a suppressed level, so its own early-return-before-formatting branch never ran. --- tests/test_logger.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_logger.cpp b/tests/test_logger.cpp index ef27437d..3205c195 100644 --- a/tests/test_logger.cpp +++ b/tests/test_logger.cpp @@ -167,6 +167,27 @@ TEST_CASE("log(level, msg) respects threshold and delegates to sink", "[logger]" REQUIRE(last == "emitted"); } +// ── logFormat (format-string level helpers) ─────────────────────────────────── + +TEST_CASE("morph::log::logWarn(fmt, args...): suppressed below threshold without formatting or invoking the sink", + "[logger]") { + LogGuard guard; + int callCount = 0; + morph::log::setLogger([&](morph::log::LogLevel, std::string_view) { ++callCount; }); + morph::log::setLogLevel(morph::log::LogLevel::error); + + // Below the "error" threshold: logFormat's own level check must reject + // this before std::format runs or the sink is touched. + morph::log::logWarn("value={}", 42); + REQUIRE(callCount == 0); + + morph::log::setLogLevel(morph::log::LogLevel::warn); + std::string last; + morph::log::setLogger([&](morph::log::LogLevel, std::string_view msg) { last = std::string{msg}; }); + morph::log::logWarn("value={}", 42); + REQUIRE(last == "value=42"); +} + // ── Thread safety ───────────────────────────────────────────────────────────── TEST_CASE("concurrent log calls are thread-safe", "[logger]") { From 14105298daab580135109bbc6c35386518303821 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:20:03 +0300 Subject: [PATCH 3/4] docs: explain 3 unreachable default: arms in closed-enum-to-string helpers FieldSlot::fieldSlotName, GroupKind::groupKindName, and ReconnectOutcome::reconnectOutcomeName each have a default: arm that only exists to satisfy the compiler's return-on-every-path check for an out-of-range static_cast -- every enumerator is already handled explicitly. Mirrors forms.hpp's identical ruleKindName pattern. --- include/morph/forms/i18n.hpp | 6 ++++++ include/morph/forms/layout.hpp | 6 ++++++ include/morph/offline/reconnect_coordinator.hpp | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/include/morph/forms/i18n.hpp b/include/morph/forms/i18n.hpp index f6f57fd8..78233252 100644 --- a/include/morph/forms/i18n.hpp +++ b/include/morph/forms/i18n.hpp @@ -50,6 +50,12 @@ enum class FieldSlot : std::uint8_t { case FieldSlot::Placeholder: return "placeholder"; default: + // Unreachable through any real code path: FieldSlot is a closed + // enum and every enumerator is already handled explicitly above. + // This arm only exists to satisfy the compiler that the function + // returns on every enum value, including one manufactured by an + // out-of-range `static_cast` -- mirrors ruleKindName's identical + // default: arm in forms.hpp. return "label"; } } diff --git a/include/morph/forms/layout.hpp b/include/morph/forms/layout.hpp index 5efc85dd..c718c220 100644 --- a/include/morph/forms/layout.hpp +++ b/include/morph/forms/layout.hpp @@ -51,6 +51,12 @@ enum class GroupKind : std::uint8_t { case GroupKind::Accordion: return "accordion"; default: + // Unreachable through any real code path: GroupKind is a closed + // enum and every enumerator is already handled explicitly above. + // This arm only exists to satisfy the compiler that the function + // returns on every enum value, including one manufactured by an + // out-of-range `static_cast` -- mirrors ruleKindName's identical + // default: arm in forms.hpp. return "section"; } } diff --git a/include/morph/offline/reconnect_coordinator.hpp b/include/morph/offline/reconnect_coordinator.hpp index 4e087e0e..090619e1 100644 --- a/include/morph/offline/reconnect_coordinator.hpp +++ b/include/morph/offline/reconnect_coordinator.hpp @@ -33,6 +33,12 @@ constexpr std::string_view reconnectOutcomeName(ReconnectOutcome outcome) noexce case ReconnectOutcome::Aborted: return "Aborted"; default: + // Unreachable through any real code path: ReconnectOutcome is a + // closed enum and every enumerator is already handled explicitly + // above. This arm only exists to satisfy the compiler that the + // function returns on every enum value, including one + // manufactured by an out-of-range `static_cast` -- mirrors + // ruleKindName's identical default: arm in forms.hpp. return "?"; } } From 5b792b6bde0e842299b2a9ec0babf254a85ded0f Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:28:08 +0300 Subject: [PATCH 4/4] ci(coverage): exclude examples/common/testkit's own test files from measurement examples/common/testkit/ mixes real, reusable test-support code (backend_rig.hpp, db_fixture.hpp, strand_interleaver.hpp, ...) with actual Catch2 test files (test_event_poller.cpp, test_presenter.cpp, test_fault_proxy.cpp, ...) in the same directory. Unlike include/morph and examples/pastebin's SOURCES entries, which contain no test files at all, `SOURCES+=(examples/common)` swept both kinds in indiscriminately -- a TEST_CASE body's own untaken assertion/lambda branches (a REQUIRE's fail arm, a "must not run" callback proving itself unreachable, the untested half of a dispatch closure a sibling test exercises instead) were being measured as if they were product code. This is the exact "phantom uncovered branch" pattern this session repeatedly had to hand-verify file by file across several PRs (test_event_poller.cpp, test_presenter.cpp, test_fault_proxy.cpp, test_backend_rig.cpp, and others) before concluding each one was tooling noise, not a real gap -- this fix addresses the actual root cause once, for the whole category, instead of continuing to verify each test file individually as its turn comes up in a coverage sweep. Adds -ignore-filename-regex='.*/testkit/test_[^/]+\.cpp$' to all four llvm-cov invocations (show/report/export-lcov/export-json). Verified directly against the real llvm-cov binary this repo's CI uses (a local Windows build of the same clang/llvm-cov version): the flag correctly excludes every test_*.cpp under testkit/ while leaving every real testkit helper (backend_rig.hpp/.cpp, db_fixture.hpp, etc.) and examples/common/gui/*.hpp measured exactly as before. Confirmed scripts/aggregate_lcov_branches.py's own output is byte-identical before/after this change (it only reads whatever coverage.sh already filtered out for it -- no change to that script itself). --- scripts/coverage.sh | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 023080c1..601441c9 100644 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -49,8 +49,8 @@ fi # the same reason. AUTOMOC's generated # mocs_compilation.cpp lives under $OUT (the build tree), never under a # source-tree path named here, so moc output is excluded automatically — -# no separate exclusion mechanism needed. Test files, demo src/, system -# headers and fetched dependencies are excluded the same way. +# no separate exclusion mechanism needed. Demo src/, system headers and +# fetched dependencies are excluded the same way. SOURCES=(include/morph) if [ -x "$LADDER_TEST_EXE" ]; then SOURCES+=(examples/common) @@ -66,6 +66,20 @@ if [ -x "$PASTEBIN_TEST_EXE" ]; then SOURCES+=(examples/pastebin/include examples/pastebin/src examples/pastebin/gui_lib) fi +# examples/common/testkit/ mixes real, reusable test-support headers/.cpp +# (backend_rig.hpp, db_fixture.hpp, strand_interleaver.hpp, ...) with actual +# Catch2 test files (test_event_poller.cpp, test_presenter.cpp, ...) in the +# same directory — unlike include/morph and examples/pastebin's SOURCES +# entries above, which contain no test files at all. `SOURCES+=(examples/ +# common)` swept both in indiscriminately: a TEST_CASE body's own untaken +# assertion/lambda branches (a REQUIRE's fail arm, a "must not run" callback +# proving itself unreachable) were being measured as if they were product +# code, manufacturing the exact "phantom uncovered branch" noise this +# project has repeatedly had to hand-verify file by file. Test files +# genuinely are not part of what examples/IMPLEMENTATION.md rule 5's 100% +# bar means to hold to that standard — only the real testkit/GUI code is. +IGNORE_REGEX='.*/testkit/test_[^/]+\.cpp$' + PROFILES=$(find "$OUT" -name "*.profraw" 2>/dev/null | tr '\n' ' ') if [ -z "$PROFILES" ]; then echo "ERROR: No .profraw files found in $OUT." >&2 @@ -79,6 +93,7 @@ mkdir -p "$REPORT_DIR" ${LLVM_COV} show "$TEST_EXE" \ "${OBJECT_ARGS[@]}" \ -instr-profile="$MERGED" \ + -ignore-filename-regex="$IGNORE_REGEX" \ -format=html \ -output-dir="$REPORT_DIR" \ "${SOURCES[@]}" @@ -88,11 +103,13 @@ echo "Coverage report: $REPORT_DIR/index.html" ${LLVM_COV} report "$TEST_EXE" \ "${OBJECT_ARGS[@]}" \ -instr-profile="$MERGED" \ + -ignore-filename-regex="$IGNORE_REGEX" \ "${SOURCES[@]}" ${LLVM_COV} export "$TEST_EXE" \ "${OBJECT_ARGS[@]}" \ -instr-profile="$MERGED" \ + -ignore-filename-regex="$IGNORE_REGEX" \ -format=lcov \ "${SOURCES[@]}" \ > "$OUT/coverage.lcov.raw" @@ -106,6 +123,7 @@ ${LLVM_COV} export "$TEST_EXE" \ ${LLVM_COV} export "$TEST_EXE" \ "${OBJECT_ARGS[@]}" \ -instr-profile="$MERGED" \ + -ignore-filename-regex="$IGNORE_REGEX" \ "${SOURCES[@]}" \ > "$OUT/coverage.json"