A red CI job reports only its first failing test *step*, hiding the rest (the step-level twin of #616)

[Yaraslaut]

#616 removed stopOnFailure from the test presets so a red leg names every failing test rather than the first one ctest reached. The same defect exists one level up, at the step level, and #616 does not touch it: GitHub Actions skips a job's remaining steps once one fails, so a job with two or more test steps still reports only the first one that went red.

Found while checking, for #616, whether any stop-on-failure equivalent lived outside CMakePresets.json. It does not — there is no --stop-on-failure, no Catch2 --abort, and every ci.yml matrix already sets fail-fast: false — but this is the same failure mode by a different mechanism, and it is worth recording rather than leaving for whoever next reads a half-reported job.

Where it bites

Three jobs in .github/workflows/ci.yml run more than one independent test activity in sequence, with nothing marking the later ones if: always():

• ladder-sanitizers (job at line 1430). Test (offscreen Qt platform, ladder tests only, stress excluded) (ctest --preset clang-asan -L ladder -LE stress, line 1577), then Test (the Qt transport suites, which no sanitizer leg ran) (line 1602). These are disjoint test sets. A ladder failure means the Qt transport suites — the only place any leg runs them under a sanitizer, per that step's own comment — are never run, and the log cannot distinguish "the Qt suites are fine" from "the Qt suites were not attempted".
• ladder-tests (job at line 981). Test (offscreen Qt platform, ladder tests only, stress excluded) (line 1295), then Run the scenario corpus against the built servers (line 1328). A ctest failure skips the wire-level scenario corpus entirely.
• linux-all-features (job at line 1689). Test (offscreen Qt platform) (line 1850), then Verify the fuzz replay covered the committed reproducers (line 1858). The second step is a guard against the fuzz replay silently matching nothing — a guard that is itself skipped whenever the test step it follows goes red.

linux-coverage (line 559) has the same shape between its ctest step and the coverage checks after it, though there the later steps genuinely depend on the earlier one's profile output, so it is the weakest of the four.

Verification status

Inferred from reading ci.yml at 1e5d0ade (this is the shape of the file at a020e69c plus #616's preset change, which does not touch ci.yml), not reproduced by making a leg fail in CI. What is verified:

• The step lists above are real: the steps named exist at those lines, in that order, and none of the later ones carries if: always() or continue-on-error.
• The matrix-level equivalent is not a problem — all four strategy: blocks in ci.yml set fail-fast: false:

$ grep -n "strategy:" -A1 .github/workflows/ci.yml
103:    strategy:
104-      fail-fast: false
--
230:    strategy:
231-      fail-fast: false
--
401:    strategy:
402-      fail-fast: false
--
1701:    strategy:
1702-      fail-fast: false

What is not verified: I did not push a branch that fails ladder-sanitizers' first test step to observe the second being skipped. That claim rests on GitHub Actions' documented default step behaviour, not on an observed run in this repository.

What would change the verdict

• Close it if a run is produced in which a job's first test step fails and a later test step in the same job still executes and reports — that would mean the default does not apply here as described.
• Close it as not worth fixing if someone argues the ordering is deliberate: there is a real counter-argument, that running the Qt transport suites after the ladder suites have already gone red costs sanitizer-leg minutes for a job that will fail regardless. ci: every ctest preset sets stopOnFailure, so a leg with six failures reports one — and which one is a race #616's own reasoning cuts the other way (the slowest leg in the matrix, Valgrind, already reports in full), but that reasoning was argued for ctest cases within one step and has not been argued for whole steps. This is the decision the fix needs, and it should be made before the if: always() sprinkling, not after.
• Re-open if it is fixed and a later job grows a second test step without if: always().

Not folded into #616

.github/workflows/ci.yml is queued to be touched by #605 next sweep, and #616's change needed no workflow edit at all, so #617 leaves that file untouched on purpose.

Related

• ci: every ctest preset sets stopOnFailure, so a leg with six failures reports one — and which one is a race #616 / cmake: let a red leg name every failing test, not just the first (fixes #616) #617 — the ctest-case-level version of this, fixed in CMakePresets.json.
• docs/spec/testing_charter.md is where the resulting policy ("a red leg reports everything it was able to measure") would belong; that file is currently held by PR ci: audit mutation_survivors.json's line citations instead of hoping (fixes #608) #614.

[Yaraslaut]

Triage: valid — and the decision this ticket asks for, made

This issue correctly refuses to prescribe a fix before the decision is made:

This is the decision the fix needs, and it should be made before the if: always() sprinkling, not after.

Making it here, with reasoning, so the lane that takes this does not have to.

Premise — confirmed by inspection, same standard as the filer's

I re-checked the two mechanical claims on master @ a020e69c. All four strategy: blocks set fail-fast: false, so the matrix level is genuinely not a problem, and the three named jobs do run disjoint test activities in sequence with no condition on the later steps. Like the filer, I did not push a failing branch to observe a step being skipped — that claim rests on GitHub Actions' documented default, not on an observed run here. Recorded as inferred, not reproduced.

The decision: yes, run the later test steps — with one exception and one correction

Run them. The cost argument the issue fairly raises — that continuing after a red costs sanitizer-leg minutes on a job that will fail regardless — loses on the arithmetic, and more clearly at step level than at case level:

• The extra minutes are spent once, on a run already going to be red.
• Discovering the hidden failure on the next push costs a full 26–74 minute cycle, including every green leg that had nothing to do with it.
• These steps are disjoint test sets, which is the material difference from ci: every ctest preset sets stopOnFailure, so a leg with six failures reports one — and which one is a race #616. Within one ctest step, six failures are often one cause. Here, ladder-sanitizers' second step is — by its own comment — the only place any leg runs the Qt transport suites under a sanitizer. Skipping it does not hide a duplicate symptom; it leaves an entire test surface unmeasured, with nothing in the log distinguishing "fine" from "never attempted."

That last point is the one that settles it. A job that silently stops measuring a surface, while its log looks like a complete run, is the exact shape AGENTS.md's "verify rather than assert" section is about.

The exception: linux-coverage. The issue already calls it "the weakest of the four" and it should be left alone. Its later steps consume the earlier step's profile output; running a coverage check against truncated .profraw data produces a number, and a number nobody should trust is worse than no number. Forcing those steps to run would manufacture the very failure mode this repository keeps hitting. Three jobs change, not four — and the reason gets written into the workflow next to the steps that were deliberately not changed, so the asymmetry reads as a decision rather than an oversight.

The correction: use if: '!cancelled()', not if: always(). always() also runs the step when the run was cancelled, which is not wanted and is actively harmful here — this repository's own triage guidance warns that "a leg that stopped within seconds of its siblings is a cancellation, not a defect." Forcing test steps to execute during teardown of a cancelled run produces exactly that confusing artefact. !cancelled() runs on success or failure and stops on cancellation, which is the intent.

The job still fails. Neither condition changes the job's conclusion — a red step keeps the job red. Worth stating in the PR body so this does not read as weakening a gate.

Scope

.github/workflows/ci.yml only:

• ladder-sanitizers — if: '!cancelled()' on Test (the Qt transport suites, which no sanitizer leg ran).
• ladder-tests — same on Run the scenario corpus against the built servers.
• linux-all-features — same on Verify the fuzz replay covered the committed reproducers.
• linux-coverage — unchanged, with a comment recording why.

Acceptance, and the trap in it

A green run proves nothing, exactly as in #616. Deliberately fail the first test step of one of the three jobs on a throwaway branch, and show the later step still ran and still reported — then show the job is still red. Without that pair, this is a config edit nobody measured.

If that demonstration is genuinely impractical to run in CI, say so plainly and mark the change as inferred rather than verified. That is a legitimate outcome and much better than an unmeasured claim of success.

Contention

.github/workflows/ci.yml is also wanted by #605. The two edits are disjoint (an option list in one job's configure step, versus step conditions in three other jobs), so they belong in one lane, one branch, one CI cycle — not two PRs racing for the same file.

Where the policy belongs

docs/spec/testing_charter.md, as the issue says — currently held by PR #614. Add the row once that lands; the sentence to add is roughly "a red leg reports everything it was able to measure, except where a later step consumes an earlier one's output." The exception is the part worth writing down, because it is the part someone will otherwise "fix" later.

Labels: triage: valid, plus the bug / area: ci already applied.