Enforced a coverage floor on the merged report - #667
Merged
Conversation
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 eclipse-threadx#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 eclipse-threadx#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 -- eclipse-threadx#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 eclipse-threadx#664 put in each suite's coverage.sh, not this one. Also corrected two stale filenames in the deploy job's comment: since eclipse-threadx#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 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.
The coverage summary has always reported a percentage and never been able to fail.
irongut/CodeCoverageSummarytakesthresholdsandfail_below_min; neither was set, so coverage could fall from 99.97% to anything at all and every check stayed green — against anAGENTS.mdthat asks for 100% test coverage. A stated requirement measured with a gauge that has no failure mode.Both are set now, through a new
coverage_thresholdsinput onregression_template.yml, because the two suites do not sit at the same figure.Figures from the
devpush run of #666, run32967526931, read off the uploaded artifacts.What was probed before picking those numbers
The action pins its own repository to a SHA but runs
docker://ghcr.io/irongut/codecoveragesummary:v1.3.0— a mutable tag — so reading the source at that SHA is not proof of what executes. All of the below was measured on a runner against the two realmerged.xmlfiles above, plus one edited to 4502/4503 to stand in for the flap.The floor compares the line rate and nothing else. This was the open question, and it mattered: branch coverage is ~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 if 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 change was meant to carry — is rejected withSystem.ArgumentException - Threshold parameter set incorrectly., and the step fails whether or notfail_below_minis set. So the choice is 99 or 100, with nothing between them.100 would fail on a race.
tx_thread_system_resume.c:529is reached by timing rather than by construction and flaps between runs of the same green tree — it is the one line #666 could not reach deliberately, and two local runs of the same tree gave 100.00% and 99.98%. 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 again.A shared floor would not have worked. SMP at 98.76% fails a floor of 99 and clears 98. #666 closed the uncovered lines in
common/srconly;common_smp/srcstill carries the same gaps intx_block_allocate.c,tx_byte_allocate.c,tx_thread_create.candtx_trace_object_register.c, plus a family of its own in the*_delete.cteardown paths andtx_byte_pool_search.c— 64 lines across 11 files. Closing those and raising the SMP floor is follow-up work, not this PR.One limit recorded rather than fixed
An empty report reads as 100%. gcovr writes
line-rate="1.0"besidelines-valid="0"when it finds no data, and the action printsLine Rate = 100% (0 / 0)and passes any floor — probed. No threshold can catch that, so the floor does not defend the failure mode it most looks like it defends. The check for it is the emptiness assertion #664 added to each suite'scoverage.sh. Said so in a comment next to the step, because the next reader will otherwise assume the floor covers it.Also in here
Two stale filenames in
deploy_code_coverage's comment: since #665 each coverage artifact carriesmerged.xml, notdefault_build_coverage.xml, and the per-suite directory is renamed frommerged. Verified against the artifacts of the run above. Folded in rather than spending a PR on it.Verification
The floor is exercised by this PR's own
txandsmpruns — both jobs read a real merged report against a real threshold, so a wrong number here shows up as a red check rather than as a surprise ondev.