diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 547d822a5..53bfac2b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1140,8 +1140,12 @@ jobs: -DCMAKE_CXX_COMPILER_LAUNCHER=sccache # QT_QPA_PLATFORM=offscreen at build time as well as test time: Catch2's - # catch_discover_tests() runs each Qt-linked test binary once during the - # build to enumerate its cases — see ladder-tests' own Build step. + # catch_discover_tests() runs each POST_BUILD-discovered Qt-linked test + # binary once during the build to enumerate its cases — see ladder-tests' + # own Build step. In this configure that is morph_qt_tests + # (tests/qt/CMakeLists.txt:91); it is *not* bank's three suites, which + # are PRE_TEST and therefore enumerate at ctest time instead. The step + # below is where that matters (morph#690). - name: Build env: QT_QPA_PLATFORM: offscreen @@ -1159,7 +1163,52 @@ jobs: # # naming bank_tests, bank_gui_tests and bank_gui_qml_tests. With the # blocks in place the same sweep reports 9 of 9. + # + # QT_QPA_PLATFORM=offscreen, and this is the step that cannot do without + # it (morph#690 — this job was red on every run from the day it landed, + # here). The sweep's first act is `ctest --show-only=json-v1`, and ctest + # is exactly where bank's PRE_TEST discovery runs: listing the tests + # *executes* `bank_gui_qml_tests --list-tests`, whose main constructs a + # QGuiApplication (examples/common/testkit/testkit_main.cpp) before + # Catch2 ever sees the flag. With no display and no QT_QPA_PLATFORM that + # aborts; Catch2's CatchAddTests.cmake raises a message(FATAL_ERROR) on a + # nonzero discovery, and ctest then exits 8 having printed no JSON at all + # — not bank's entries missing, the entire listing, every other suite + # included. The sweep correctly refused to pass having examined nothing + # (morph#675's floor), so the symptom was the guard and the cause was + # this missing line. The Test step below already declares the same value, + # which is the point: this sweep's subject is the binaries that step will + # run, so it has to enumerate them in that step's environment. + # + # Reproduced locally against this very configure (clang 22.1.8, Catch2 + # 3.16.0, Qt 6.11.2 — the runner's versions differ, the code path does + # not), by emulating the runner's one distinguishing property, a headless + # session: + # + # $ env -u DISPLAY -u WAYLAND_DISPLAY \ + # bash scripts/check_sanitizer_instrumentation.sh \ + # build/clang-ubsan ubsan + # ::error::check_sanitizer_instrumentation: ctest listed no tests in + # build/clang-ubsan -- this check would pass having examined nothing + # `ctest --show-only=json-v1` exited 8 and wrote 0 bytes of stdout. + # | CMake Error at .../CatchAddTests.cmake:307 (message): + # | Error listing tests from executable + # | '.../examples/bank/bank_gui_qml_tests': + # | Result: Subprocess aborted + # + # $ env -u DISPLAY -u WAYLAND_DISPLAY QT_QPA_PLATFORM=offscreen \ + # bash scripts/check_sanitizer_instrumentation.sh \ + # build/clang-ubsan ubsan + # check_sanitizer_instrumentation: 9 ctest binaries all carry + # __ubsan_ symbols (0 allowlisted). + # + # Same tree, same build, one environment variable apart. That is also + # why the two earlier attempts to reproduce this job's failure locally + # could not: a workstation has a display, so the same command enumerates + # all nine there whether or not the variable is set. - name: Every ctest binary is instrumented + env: + QT_QPA_PLATFORM: offscreen run: bash scripts/check_sanitizer_instrumentation.sh build/clang-ubsan ubsan # -L bank, not the whole suite: this configure also builds morph_tests diff --git a/examples/bank/CMakeLists.txt b/examples/bank/CMakeLists.txt index f69d4f751..cb15341df 100644 --- a/examples/bank/CMakeLists.txt +++ b/examples/bank/CMakeLists.txt @@ -381,6 +381,24 @@ if(MORPH_BUILD_TESTS) # never looks at a pixel -- it reads property values off the items # the engine created. The same variable the ladder's own GUI legs # set for ctest (examples/TESTING.md). + # + # It does not cover the *discovery* run, and that gap cost this + # repository a permanently red CI leg (morph#690). PROPERTIES are + # set on the tests Catch2 registers; the run that finds out what + # those tests are happens first, and under DISCOVERY_MODE PRE_TEST + # it happens inside ctest rather than inside the build. Catch2's + # CatchAddTests.cmake execute_process() forwards only DL_PATHS and + # DL_FRAMEWORK_PATHS into that run -- never the ENVIRONMENT + # property -- so ` --list-tests` starts under whatever + # environment invoked ctest. This main constructs a QGuiApplication + # before Catch2 parses the flag (examples/common/testkit/ + # testkit_main.cpp), so with no display and no QT_QPA_PLATFORM it + # aborts, CatchAddTests raises message(FATAL_ERROR), and *every* + # suite in the tree vanishes from `ctest --show-only` -- the whole + # listing fails, not this target's share of it. Anything that + # enumerates this build tree headless therefore has to set + # QT_QPA_PLATFORM itself; ci.yml's bank-ubsan sweep step carries + # the worked example and the measurement. catch_discover_tests(bank_gui_qml_tests DISCOVERY_MODE PRE_TEST PROPERTIES diff --git a/scripts/check_sanitizer_instrumentation.sh b/scripts/check_sanitizer_instrumentation.sh index f95d57499..1dce6a687 100755 --- a/scripts/check_sanitizer_instrumentation.sh +++ b/scripts/check_sanitizer_instrumentation.sh @@ -126,14 +126,46 @@ build_dir="${target}" # so its documented exemption at CMakeLists.txt:582 does not reach here.) allowlist=() +# stderr goes to a file rather than /dev/null, and the exit status is kept. +# +# `2>/dev/null` made the two ways this list can come back empty +# indistinguishable: "ctest enumerated the tree and it registers no tests" and +# "ctest itself failed before printing any JSON". morph#690 was the second one, +# and it cost three CI runs and two local sessions to name, because the +# sentence that named it was being discarded one pipe away from the error +# message. What ctest actually wrote, reproduced locally against a +# DISCOVERY_MODE PRE_TEST suite whose binary aborts at listing time: +# +# CMake Error at .../CatchAddTests.cmake:307 (message): +# Error listing tests from executable '.../gui_tests': +# Result: Subprocess aborted +# +# with an exit status of 8 and an entirely empty stdout -- one suite's failed +# discovery takes the whole listing down, not just its own entries. The stream +# still has to be kept off stdout (it is not JSON and jq would choke on it), +# so it is captured and printed only on the path that needs it. +ctest_stderr="$(mktemp)" +ctest_stdout="$(mktemp)" +trap 'rm -f "${ctest_stderr}" "${ctest_stdout}"' EXIT + +ctest_status=0 +ctest --test-dir "${build_dir}" --show-only=json-v1 \ + >"${ctest_stdout}" 2>"${ctest_stderr}" || ctest_status=$? + mapfile -t commands < <( - ctest --test-dir "${build_dir}" --show-only=json-v1 2>/dev/null \ - | jq -r '.tests[]?.command[0]? // empty' \ + jq -r '.tests[]?.command[0]? // empty' <"${ctest_stdout}" 2>/dev/null \ | sort -u ) if [ "${#commands[@]}" -eq 0 ]; then echo "::error::check_sanitizer_instrumentation: ctest listed no tests in ${build_dir} -- this check would pass having examined nothing" + echo "check_sanitizer_instrumentation: \`ctest --show-only=json-v1\` exited ${ctest_status} and wrote $(wc -c <"${ctest_stdout}") bytes of stdout." + if [ -s "${ctest_stderr}" ]; then + echo "check_sanitizer_instrumentation: its stderr follows -- a non-empty stderr here means the listing *failed*, not that the tree registers no tests:" + sed 's/^/ | /' "${ctest_stderr}" + else + echo "check_sanitizer_instrumentation: it wrote nothing to stderr, so this is a build tree that genuinely registers no tests rather than a listing that failed." + fi exit 1 fi diff --git a/scripts/test_check_sanitizer_instrumentation.sh b/scripts/test_check_sanitizer_instrumentation.sh index 6389b0e48..8afad412c 100755 --- a/scripts/test_check_sanitizer_instrumentation.sh +++ b/scripts/test_check_sanitizer_instrumentation.sh @@ -185,8 +185,38 @@ if output="$(run_checker "$dir" ubsan 2>&1)"; then elif ! mentions 'listed no tests' "$output"; then fail "the empty test list was rejected, but not for being empty:" printf '%s\n' "$output" >&2 +elif ! mentions 'genuinely registers no tests' "$output"; then + fail "the empty test list was rejected, but the message does not say ctest succeeded -- it reads the same as a listing that failed, which is morph#690:" + printf '%s\n' "$output" >&2 +else + note "ok: an empty ctest test list is rejected rather than passing vacuously, and named as empty rather than broken" +fi + +# ── 3b. sweep: ctest *fails* to list -> the reason is printed (morph#690) ─── +# The distinction case 3 cannot make on its own, and the one that cost three CI +# runs: "ctest enumerated a tree with no tests" and "ctest died before printing +# any JSON" both arrive here as an empty list. On the bank-ubsan leg it was the +# second -- one DISCOVERY_MODE PRE_TEST suite's binary aborted at listing time, +# ctest exited 8 with an empty stdout and a CMake FATAL_ERROR on stderr, and +# the checker's `2>/dev/null` threw that sentence away. +# +# The fixture reproduces the shape rather than the cause: a CTestTestfile.cmake +# that fails while being read makes ctest exit nonzero with nothing on stdout, +# which is exactly the state the checker has to tell apart. Asserting on the +# fixture's own marker string, not on ctest's wording, is what makes this a +# test of the pass-through rather than of ctest. +dir="$(case_dir sweep_listing_failed)" +printf 'message(FATAL_ERROR "morph690_fixture_marker: listing deliberately failed")\n' \ + > "${dir}/CTestTestfile.cmake" + +if output="$(run_checker "$dir" ubsan 2>&1)"; then + fail "a ctest listing that failed outright was reported as clean:" + printf '%s\n' "$output" >&2 +elif ! mentions 'morph690_fixture_marker' "$output"; then + fail "the failed listing was rejected, but ctest's own reason was discarded -- the caller is left with 'listed no tests' and no cause, which is morph#690:" + printf '%s\n' "$output" >&2 else - note "ok: an empty ctest test list is rejected rather than passing vacuously" + note "ok: a ctest listing that failed prints the reason it failed" fi # ── 4. narrow: an instrumented binary -> pass, reporting the count ──────────