Enforced a coverage floor on the merged report (#667)
cortex_m / Cortex M0 build (push) Canceled after 0s
cortex_m / Cortex M3 build (push) Canceled after 0s
cortex_m / Cortex M4 build (push) Canceled after 0s
cortex_m / Cortex M7 build (push) Canceled after 0s
regression_test / tx (push) Canceled after 0s
regression_test / smp (push) Canceled after 0s
regression_test / freertos (push) Canceled after 0s
regression_test / deploy (push) Canceled after 0s
regression_template / run_tests (push) Canceled after 0s
regression_template / deploy_code_coverage (push) Canceled after 0s

The coverage summary reported a percentage and could not fail. Coverage
could fall from 99.97% to anything at all and every check stayed green,
against an AGENTS.md that asks for 100% test coverage -- a stated
requirement measured with a gauge that had no failure mode.

CodeCoverageSummary already takes thresholds and fail_below_min; neither
was set. Both are now, through a new coverage_thresholds input on the
template, because the two suites do not sit at the same figure: ThreadX
99, SMP 98.

Three things were probed against the pinned action on a runner before
picking those numbers, using the real merged.xml files from the dev push
run of #666.

The floor compares the line rate and nothing else. That mattered because
branch coverage is around 78% in both suites while line coverage is
98.8-100%, so a floor aimed at the line figure would have been an
immediate red wall had it tested branches or the lower of the two. The
ThreadX report at 100.00% lines and 77.67% branches clears a floor of 99.

The thresholds are whole numbers. '99.9 100' -- the value this was meant
to be -- is rejected with 'System.ArgumentException - Threshold parameter
set incorrectly.', and the step fails whether or not fail_below_min is
set. So the choice is 99 or 100 with nothing between.

100 would fail on a race. tx_thread_system_resume.c:529 is reached by
timing rather than by construction and flaps between runs of the same
green tree, which is why #666 left it; 4502/4503 fails a floor of 100 and
clears one of 99. A coverage gate that goes red on a coin toss is how
coverage gates get switched off.

SMP is 5114/5178 lines, 98.76%, with 64 uncovered lines across 11 files
of common_smp/src -- #666 closed the equivalent gaps in common/src only.
A shared floor of 99 would have failed that job on every run while
ThreadX passed.

One limit is recorded in the file rather than fixed: an empty report
reads as 100%. gcovr writes line-rate="1.0" beside lines-valid="0" when
it finds no data, and the action prints 'Line Rate = 100% (0 / 0)' and
passes any floor. The check for that is the emptiness assertion #664 put
in each suite's coverage.sh, not this one.

Also corrected two stale filenames in the deploy job's comment: since
#665 each coverage artifact carries merged.xml, not
default_build_coverage.xml. Verified on the runner.

Assisted-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Frédéric Desbiens
2026-08-26 09:13:35 -04:00
committed by GitHub
parent f89d65f041
commit 147754cc86
2 changed files with 69 additions and 6 deletions
+49 -6
View File
@@ -38,6 +38,33 @@ on:
default: 'merged'
required: false
type: string
# The lower and upper threshold percentages handed to
# CodeCoverageSummary, lower first. The lower one is a hard floor: the
# step below sets fail_below_min, so a run whose merged line rate falls
# under it turns the job red.
#
# Both must be whole numbers. Probed against the pinned action on a
# runner, 26 Aug 2026: '99.9 100' is rejected with
# 'System.ArgumentException - Threshold parameter set incorrectly.' and
# the step fails whether or not fail_below_min is set, because the action
# parses each half with an integer parse. So a floor can be 99 or 100 and
# nothing between, and the intended 99.9 is not expressible.
#
# The floor is compared against the LINE rate only -- not the branch
# rate, and not the lower of the two. Probed on the same run: the
# ThreadX report at 100.00% lines and 77.67% branches passes a floor of
# 99. Worth knowing, because branch coverage is around 78% in both
# suites while AGENTS.md asks for 100%, and a floor set from the
# headline line figure says nothing about it.
#
# Each suite sets its own in regression_test.yml, because they do not
# sit at the same figure. The default here is the action's own, low
# enough to be no floor at all in practice -- a caller that collects
# coverage is expected to name its number.
coverage_thresholds:
default: '50 75'
required: false
type: string
skip_deploy:
default: false
required: false
@@ -128,6 +155,18 @@ jobs:
# 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.
#
# fail_below_min turns the summary into a gate. Until now coverage could
# fall from any figure to any other and no check went red, against an
# AGENTS.md that asks for 100% -- a stated requirement measured with a
# gauge that could not fail.
#
# One thing the floor does not defend, and it is the likeliest way for
# coverage to break: an empty report reads as 100%. gcovr writes
# line-rate="1.0" beside lines-valid="0" when it finds no data, and the
# action reports 'Line Rate = 100% (0 / 0)' and passes any floor --
# probed on a runner, 26 Aug 2026. The check that catches that is the
# emptiness assertion in each suite's coverage.sh, not this one.
- name: Generate Code Coverage Results Summary
if: ${{ !cancelled() && (!inputs.skip_coverage) }}
uses: irongut/CodeCoverageSummary@51cc3a756ddcd398d447c044c02cb6aa83fdae95 # v1.3.0
@@ -137,6 +176,8 @@ jobs:
badge: true
hide_complexity: true
output: file
thresholds: ${{ inputs.coverage_thresholds }}
fail_below_min: true
- name: Write Code Coverage Summary
if: ${{ !cancelled() && (!inputs.skip_coverage) }}
@@ -239,13 +280,15 @@ jobs:
# merge-multiple puts the contents of both coverage artifacts directly
# into coverage_report rather than under a directory named for each
# artifact. Each one holds a single directory named for its suite,
# ThreadX or SMP, renamed from default_build_coverage by the "Prepare
# Coverage GitHub Pages" step, so the merge yields exactly the two
# suite directories the deploy expects, and the artifact name -- with
# its timestamp -- stops appearing in the published path at all.
# ThreadX or SMP, renamed from merged by the "Prepare Coverage GitHub
# Pages" step, so the merge yields exactly the two suite directories the
# deploy expects, and the artifact name -- with its timestamp -- stops
# appearing in the published path at all.
#
# The two artifacts also each carry a default_build_coverage.xml, and
# the merge means one overwrites the other. That file is consumed by
# The two artifacts also each carry a merged.xml -- it was
# default_build_coverage.xml until #665 instrumented every configuration
# and unioned them -- and the merge means one overwrites the other.
# Verified on the runner: both hold merged.xml. That file is consumed by
# CodeCoverageSummary back in run_tests and is not read here, so this is
# untidy rather than wrong.
- uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+20
View File
@@ -37,6 +37,17 @@ jobs:
cmake_path: ./test/tx/cmake
result_affix: ThreadX
skip_deploy: true
# 99 is the tightest floor available below this suite's figure. The
# merged report reads 4503/4503 lines -- 100.00% -- on the dev push run
# of #666, and two local runs of the same green tree came in at 100.00%
# and 99.98%: tx_thread_system_resume.c:529 is reached by timing rather
# than by construction and flaps between runs, and #666 could not reach
# it deliberately. So a floor of 100 is a check that fails on a race --
# probed against the pinned action, 4502/4503 fails a floor of 100 and
# clears one of 99 -- and the thresholds are integers, so there is
# nothing between the two. Ratchet this to 100 if 529 is ever covered
# by construction.
coverage_thresholds: '99 100'
smp:
permissions:
contents: read
@@ -52,6 +63,15 @@ jobs:
cmake_path: ./test/smp/cmake
result_affix: SMP
skip_deploy: true
# Lower than ThreadX's, because this suite is: the merged report reads
# 5114/5178 lines -- 98.76% -- on the same run, with 64 uncovered lines
# across 11 files of common_smp/src. #666 closed the equivalent gaps in
# common/src only, so common_smp still carries them, plus a family of
# its own in the *_delete.c teardown paths and tx_byte_pool_search.c.
# A shared floor of 99 would therefore red-wall this job on every run
# while ThreadX passed -- probed against the pinned action. Raise this
# towards 99 as those lines are covered.
coverage_thresholds: '98 100'
freertos:
permissions:
contents: read