diff options
| author | Frédéric Desbiens <[email protected]> | 2026-08-24 16:36:18 -0400 |
|---|---|---|
| committer | GitHub <[email protected]> | 2026-08-24 16:36:18 -0400 |
| commit | 7959aef3bc39c7ffc49fa47e00575aa6c697c273 (patch) | |
| tree | ac62598f7b5725b82b0491241fbac0a5e585ea82 | |
| parent | adc6469b91f36892288a61ff9e46bf483ce486bf (diff) | |
Kept the coverage report from the runs that most need one (#659)
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
#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 <[email protected]>
| -rw-r--r-- | .github/workflows/regression_template.yml | 23 | ||||
| -rwxr-xr-x | scripts/cmake_bootstrap.sh | 26 |
2 files changed, 37 insertions, 12 deletions
diff --git a/.github/workflows/regression_template.yml b/.github/workflows/regression_template.yml index 7f9f496f..27666ae7 100644 --- a/.github/workflows/regression_template.yml +++ b/.github/workflows/regression_template.yml @@ -104,8 +104,15 @@ jobs: - name: Configure GitHub Pages uses: actions/[email protected] + # The coverage steps below run even when a test failed. cmake_bootstrap.sh + # now produces the report in that case, and the run whose behaviour changed + # is the one whose coverage is worth reading; previously a single flaky test + # suppressed the report for the whole run. !cancelled() rather than always(), + # so a cancelled run still stops promptly -- the same idiom the + # deploy_code_coverage job below already uses. The ${{ }} is required: a bare + # ! opens a YAML tag, and the expression will not parse without it. - name: Generate Code Coverage Results Summary - if: (!inputs.skip_coverage) + if: ${{ !cancelled() && (!inputs.skip_coverage) }} uses: irongut/[email protected] with: filename: ${{ inputs.cmake_path }}/coverage_report/${{ inputs.coverage_name }}.xml @@ -115,13 +122,13 @@ jobs: output: file - name: Write Code Coverage Summary - if: (!inputs.skip_coverage) + if: ${{ !cancelled() && (!inputs.skip_coverage) }} run: | echo "## Coverage Report ${{ inputs.result_affix }}" >> $GITHUB_STEP_SUMMARY cat code-coverage-results.md >> $GITHUB_STEP_SUMMARY - name: Create CheckRun for Code Coverage - if: ((github.event_name == 'push') || (github.event_name == 'workflow_dispatch') || (github.event.pull_request.head.repo.full_name == github.repository)) && (!inputs.skip_coverage) + if: ${{ !cancelled() && ((github.event_name == 'push') || (github.event_name == 'workflow_dispatch') || (github.event.pull_request.head.repo.full_name == github.repository)) && (!inputs.skip_coverage) }} uses: LouisBrunner/[email protected] with: token: ${{ secrets.GITHUB_TOKEN }} @@ -132,7 +139,7 @@ jobs: output_text_description_file: code-coverage-results.md - name: Add Code Coverage PR Comment - if: ((github.event_name == 'push') || (github.event.pull_request.head.repo.full_name == github.repository)) && (!inputs.skip_coverage) + if: ${{ !cancelled() && ((github.event_name == 'push') || (github.event.pull_request.head.repo.full_name == github.repository)) && (!inputs.skip_coverage) }} uses: marocchino/[email protected] with: header: Code Coverage ${{ inputs.result_affix }} @@ -140,7 +147,7 @@ jobs: # Add sudo to move coverage folder created by root user - name: Prepare Coverage GitHub Pages - if: (!inputs.skip_coverage) + if: ${{ !cancelled() && (!inputs.skip_coverage) }} run: >- if [ "${{ inputs.result_affix }}" != "" ] && ${{ inputs.skip_deploy }}; then sudo mv ${{ inputs.cmake_path }}/coverage_report/${{ inputs.coverage_name }} \ @@ -149,12 +156,12 @@ jobs: - name: Coverage Report name id: artifact - if: (!inputs.skip_coverage) + if: ${{ !cancelled() && (!inputs.skip_coverage) }} run: echo "coverage_report=coverage_report-$(date +%s)" >> $GITHUB_OUTPUT - name: Upload Code Coverage Artifacts uses: actions/[email protected] - if: (inputs.skip_deploy && !inputs.skip_coverage) + if: ${{ !cancelled() && (inputs.skip_deploy && !inputs.skip_coverage) }} with: name: ${{ steps.artifact.outputs.coverage_report }} path: ${{ inputs.cmake_path }}/coverage_report @@ -162,7 +169,7 @@ jobs: - name: Upload Code Coverage Pages uses: actions/[email protected] - if: (!inputs.skip_deploy && !inputs.skip_coverage) + if: ${{ !cancelled() && (!inputs.skip_deploy && !inputs.skip_coverage) }} with: path: ${{ inputs.cmake_path }}/coverage_report/${{ inputs.coverage_name }} diff --git a/scripts/cmake_bootstrap.sh b/scripts/cmake_bootstrap.sh index 014f4423..faa43133 100755 --- a/scripts/cmake_bootstrap.sh +++ b/scripts/cmake_bootstrap.sh @@ -82,13 +82,24 @@ function test() { else repeat_fail=${CTEST_REPEAT_FAIL} fi - ctest $parallel --timeout 1000 -O $1.txt -T test --no-compress-output --test-output-size-passed 4194304 --test-output-size-failed 4194304 --output-on-failure --repeat until-pass:${repeat_fail} --output-junit $1.xml + # ctest's status is captured rather than allowed to abort the function, and + # returned at the end. set -e would otherwise stop here on the first failing + # test, and everything below would be skipped -- including coverage.sh. The + # gcda files exist by that point, so a failing run threw away coverage it had + # already collected, and the run whose behaviour changed is exactly the one + # whose coverage is worth reading. Measured: the failing run of 2026-08-18 + # produced test_reports artifacts and no coverage_report artifact at all. + local status=0 + ctest $parallel --timeout 1000 -O $1.txt -T test --no-compress-output --test-output-size-passed 4194304 --test-output-size-failed 4194304 --output-on-failure --repeat until-pass:${repeat_fail} --output-junit $1.xml || status=$? popd - grep -E "^(\s*[0-9]+|Total)" build/$1/$1.txt >build/$1.txt + # Tolerated because this is a summary for humans, and a ctest that died early + # enough to leave no matching line must not be what stops the coverage below. + grep -E "^(\s*[0-9]+|Total)" build/$1/$1.txt >build/$1.txt || true sed -i "s/\x1B\[[0-9;]*[JKmsu]//g" build/$1.txt if [[ $1 = *"_coverage" ]]; then ./coverage.sh $1 fi + return $status } cd $(dirname $0) @@ -146,11 +157,18 @@ elif [ "$command" == "test" ]; then done exit $exit_code else - # Run builds in serial + # Run builds in serial. The status is collected the same way the parallel + # branch above collects it, so one failing configuration no longer stops + # the remaining ones from being tested. That mattered more after the + # suites moved to serial execution: a failure in the first configuration + # meant the other four never ran, and their coverage was never collected + # either. + exit_code=0 for item in $builds; do echo "Testing $item" - test $item $parallel_jobs + test $item $parallel_jobs || exit_code=$? done + exit $exit_code fi elif [ "$command" == "build_libs" ]; then build_libs |
