Kept the coverage report from the runs that most need one - #659
Merged
fdesbiens merged 1 commit intoAug 24, 2026
Merged
Conversation
A failing test threw away coverage that had already been collected, and the run whose behaviour changed is exactly the run whose coverage is worth reading. Measured on the failing run of 2026-08-18: it uploaded test_reports for all three suites and no coverage_report artifact at all. Two causes, and the workflow one is the smaller of them. cmake_bootstrap.sh runs under set -e, so a failing ctest aborted test() before ./coverage.sh was reached. The gcda files exist by that point, so nothing was missing except the step that reads them. ctest's status is now captured and returned at the end, and the summary grep is allowed to fail rather than being the thing that stops the coverage behind it. The serial branch of the test dispatch collected no status either, so under set -e the first failing configuration stopped the remaining four from being tested at all -- and their coverage from being collected. That was cheap while the suites ran in parallel, because the parallel branch already collects exit codes from its background jobs. Moving to serial execution in eclipse-threadx#643 quietly made one failure cost the other four configurations. The serial branch now collects status the same way the parallel branch does. With those fixed the report exists, so the workflow steps that publish it no longer skip on failure. They are guarded with !cancelled() rather than always(), so a cancelled run still stops promptly, which is the idiom deploy_code_coverage already uses. The ${{ }} wrapping is required and not decoration: a bare ! opens a YAML tag, and the file will not parse without it. Verified locally by replacing one test binary with a stub that exits 1: before the failing run of 2026-08-18 produced no coverage_report artifact after run.sh test default_build_coverage exits 8, and produces coverage_report/default_build_coverage.xml with 177 files and 3804 of 3827 lines after run.sh test all exits 8, and all five configurations run rather than stopping at the first The failure still fails. Only the reporting around it changed. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A failing test threw away coverage that had already been collected — and the run whose behaviour changed is exactly the run whose coverage is worth reading.
Measured on the failing run of 18 August (
32186061073), which uploaded:No
coverage_report-*artifact at all. Compare a passing run, which uploads two.Two causes, and the workflow one is the smaller
scripts/cmake_bootstrap.shruns underset -e, so a failingctestabortedtest()before./coverage.shwas ever reached. The.gcdafiles exist by that point — nothing was missing except the step that reads them.ctest's status is now captured and returned at the end, and the summarygrepis allowed to fail rather than being the thing that stops the coverage behind it.The serial branch of the test dispatch collected no status either. Under
set -e, the first failing configuration stopped the remaining four from being tested at all, and their coverage from being collected. That was cheap while the suites ran in parallel, because the parallel branch already collects exit codes from its background jobs — but #643 moved them to serial, which quietly made one failure cost the other four configurations. The serial branch now collects status the same way the parallel branch does.Only with both fixed does the report exist, so the workflow steps that publish it can stop skipping on failure. Adding
if:guards alone would have changed nothing.Verified locally
By replacing one test binary with a stub that exits 1:
coverage_reportartifactrun.sh test default_build_coverageexits 8, and producescoverage_report/default_build_coverage.xml— 177 files, 3804/3827 linesrun.sh test allexits 8, and all five configurations run rather than stopping at the firstThe failure still fails. Only the reporting around it changed.
Two details worth a reviewer's eye
!cancelled()rather thanalways()— so a cancelled run still stops promptly. It is the idiomdeploy_code_coveragealready uses at the job level.The
${{ }}wrapping is required, not decoration. A bare!at the start of a YAML scalar opens a tag, and the file does not parse without it:I hit that on the first attempt; it is worth knowing before anyone adds another negated condition here.
Where this sits
First of the coverage changes, and deliberately first: it has no effect on any coverage number, and the changes that follow — bumping
gcovroff the 20184.1pin, fixing the report's absolute paths, and instrumenting all five build configurations instead of one — all move numbers and will produce failing runs while they are iterated on. Doing this one last would mean working blind through exactly the part where the report matters most.