ci: three of the four sanitizer sweep steps still enumerate ctest without their Test step's environment

[Yaraslaut]

Found while fixing #690, and deliberately not folded into that fix: the same trap is still armed in three other jobs.

The shape of it

check_sanitizer_instrumentation.sh begins with ctest --show-only=json-v1. For any suite registered with DISCOVERY_MODE PRE_TEST, listing the tests runs the test binary (<binary> --list-tests), inside the sweep step, not inside the build. If that binary cannot start, Catch2's CatchAddTests.cmake raises message(FATAL_ERROR ...), and ctest exits nonzero having printed no JSON at all — the whole listing, not just that target's share of it. The sweep then correctly refuses to pass having examined nothing, and the job is red for a reason that has nothing to do with instrumentation. That is #690 exactly.

The sweep step therefore has an implicit requirement: it must run in the environment its own Test step declares. Four jobs run the sweep, and after #690 only one of them does:

job	sweep step env	its Test step env
bank-sanitizers (ci.yml:1212, on this branch)	QT_QPA_PLATFORM: offscreen (added by the #690 fix)	QT_QPA_PLATFORM: offscreen
linux-sanitizers (ci.yml:567)	none	QT_QPA_PLATFORM not set either
kanban-tsan (ci.yml:960)	none	QT_QPA_PLATFORM: offscreen
ladder-asan (ci.yml:1961, on this branch)	none	QT_QPA_PLATFORM: offscreen

kanban-tsan and ladder-asan both declare QT_QPA_PLATFORM: offscreen on their Build and Test steps and omit it on the sweep step sitting between them. They are green today only because of a fact nobody wrote down: every Qt-linked suite they build uses DISCOVERY_MODE POST_BUILD (cmake/morph_add_rung.cmake:532, examples/common/CMakeLists.txt:307, tests/qt/CMakeLists.txt:91), so their enumeration already happened during the Build step, under that step's offscreen. The PRE_TEST suites they build (morph_tests at tests/CMakeLists.txt:222, morph_concepts_tests, morph_vetted_hmac_*) link no Qt.

Changing one word — POST_BUILD to PRE_TEST on any Qt-linked rung suite — turns two green jobs red at a step that will report only "ctest listed no tests".

A second, sharper edge of the same thing

catch_discover_tests(... PROPERTIES ENVIRONMENT "QT_QPA_PLATFORM=offscreen") reads exactly like "this suite is headless-safe". It is not. PROPERTIES are applied to the tests Catch2 registers; the run that discovers what those tests are happens before that, and CatchAddTests.cmake's execute_process() forwards only DL_PATHS and DL_FRAMEWORK_PATHS into it — never ENVIRONMENT. examples/bank/CMakeLists.txt:402 carries that exact combination, and it is what made #690 look impossible for two sessions. The #690 fix adds a comment there saying so; no other call site in the tree pairs PRE_TEST with an ENVIRONMENT property today, but nothing stops one.

Verification status

Mechanism reproduced; these three jobs' exposure inferred from reading, not reproduced.

Reproduced (local, 24a470c, clang 22.1.8, Catch2 3.16.0, Qt 6.11.2): on the real clang-ubsan bank configure, headless, the sweep fails with ctest --show-only=json-v1 exiting 8 and zero bytes of stdout, and succeeds with 9 ctest binaries all carry __ubsan_ symbols when QT_QPA_PLATFORM=offscreen is set. Full output is in the #690 fix's commit message.

Also reproduced on a two-target fixture (one Catch2 binary owning a QGuiApplication, one plain, both PRE_TEST): one failed discovery removes all three tests from the listing, not two — which is the part that makes the symptom so uninformative.

Not verified: I did not build the linux-sanitizers, kanban-tsan or ladder-asan configures. Their safety is inferred from grepping every catch_discover_tests call in the tree for DISCOVERY_MODE and checking which targets link Qt. No currently-failing job is claimed here — this is a latent hazard, not an outage.

What would resolve it

Either of, not both:

1. Give the three remaining sweep steps the same env: block their Test step has. Three lines, inert where nothing needs a platform.
2. Or state the requirement once, in scripts/check_sanitizer_instrumentation.sh's header, that a caller whose tree contains a Qt-linked PRE_TEST suite must invoke it under the Test step's environment — and leave the workflows alone.

(1) is cheaper and does not depend on anyone reading a header. Deliberately not doing it as part of #690: that fix is about one measured failure, and three unmeasured jobs do not belong in it.

What would change the verdict

Close as invalid if PRE_TEST discovery turns out not to run under ctest --show-only in the Catch2 version CI resolves (v3.8.1 via morph_cache_dep, or the distro catch2 the bank job installs from apt), rather than the 3.16.0 this was measured against. The execute_process + FATAL_ERROR path in CatchAddTests.cmake is long-standing, so I do not expect that, but I did not check 3.8.1's copy.

Related

• ci: the bank-sanitizers job has never passed — ctest lists no tests, and the checker hides why #690 — the same defect, measured, on bank-sanitizers.
• ci: check_sanitizer_instrumentation.sh refuses a single-target build tree, so it cannot verify the narrow case it is most needed for #675 — the floor that correctly refuses; it is the messenger in both.

[Yaraslaut]

Triage: valid — verified on 38dcba18, and the exposure is exactly two jobs

Enumerated every sweep step from the parsed workflow, against whether its job builds Qt at all:

  linux-sanitizers       sweep-step QT_QPA_PLATFORM=NO    builds Qt=no     <- safe, structurally
  kanban-tsan            sweep-step QT_QPA_PLATFORM=NO    builds Qt=yes    <- exposed
  bank-sanitizers        sweep-step QT_QPA_PLATFORM=yes   builds Qt=yes    <- fixed by #692
  ladder-sanitizers      sweep-step QT_QPA_PLATFORM=NO    builds Qt=yes    <- exposed

And the reason the two exposed ones are green is the one this issue gives: every Qt-linked suite they build uses POST_BUILD (cmake/morph_add_rung.cmake:532, examples/common/CMakeLists.txt:307), so enumeration happens during Build, where QT_QPA_PLATFORM=offscreen is set. Nothing about the sweep step protects them.

triage: valid, bug, area: ci.

Why this is worth fixing now rather than when it fires

The failure mode is already measured, because #690 was it: a headless --list-tests abort takes out the entire ctest listing, not just the offending suite, and surfaces as ctest listed no tests with no further detail. That cost three sessions across two days — two of which, mine included, concluded "cannot reproduce locally" without noticing the runner's one distinguishing property was DISPLAY.

One DISCOVERY_MODE word changed in morph_add_rung.cmake or examples/common/CMakeLists.txt turns two green jobs red with that same uninformative message. The cost of the fix is one env: block per step; the cost of not fixing it is the same hunt, and the person doing it will not have #690 in front of them.

linux-sanitizers needs nothing — it configures neither MORPH_BUILD_QT nor any Qt suite, so adding the variable there would be inert. Say so rather than adding it for symmetry; an unnecessary line invites the next reader to wonder what it protects against.

The trap recorded here is the durable part

catch_discover_tests(... PROPERTIES ENVIRONMENT ...) reads like "headless-safe", but CatchAddTests.cmake forwards only DL_PATHS/DL_FRAMEWORK_PATHS into the discovery run.

That is exactly the shape of thing that makes a second occurrence likely: the CMake looks like it already handles it. #692 added a comment at examples/bank/CMakeLists.txt saying so; whoever fixes this should consider whether that comment belongs everywhere PROPERTIES ENVIRONMENT appears, or whether a gate is better than three comments.

Verification status: the four-job table above is measured. The claim that kanban-tsan and ladder-sanitizers would fail if their suites moved to PRE_TEST is inferred from #690's mechanism, not reproduced — the filer says so, and I have not reproduced it either.

What would change the verdict

Close as wontfix if the considered answer is that POST_BUILD is a deliberate invariant for Qt-linked suites and should be enforced directly — a gate saying "a Qt-linked target must not use PRE_TEST" would make the env blocks unnecessary and is arguably the better fix. That is a design choice, and it belongs to whoever owns the ladder's test registration.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW