Skip to content

Instrumented every build configuration and merged their coverage - #665

Merged
fdesbiens merged 1 commit into
eclipse-threadx:devfrom
fdesbiens:feature/coverage-all-configurations
Aug 26, 2026
Merged

Instrumented every build configuration and merged their coverage#665
fdesbiens merged 1 commit into
eclipse-threadx:devfrom
fdesbiens:feature/coverage-all-configurations

Conversation

@fdesbiens

@fdesbiens fdesbiens commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Only default_build_coverage carried -fprofile-arcs, because the gate was the build type and it is the only one of five whose name ends in _coverage. The other four build and run every test and their coverage was discarded.

That is not redundancy thrown away. Each configuration selects a different set of TX_ feature macros, so the code the other four compile is absent from the denominator rather than uncovered in it — a figure taken from one of them is the executable-line count of one configuration, not of the kernel.

What this does

TX_COVERAGE instruments a build regardless of its name, defaulting to OFF so a single configuration built by hand behaves exactly as before. coverage.sh gains a --merge mode that unions the per-configuration JSON tracefiles, and cmake_bootstrap.sh runs it after the test loop so a local run produces the same merged report CI reads.

Expect the percentage to drop, and expect that to be correct

Measured on the ThreadX suite, all 480 tests passing:

configuration lines-valid lines-covered
default_build_coverage 3827 3827
disable_notify_callbacks_build 3767 3766
stack_checking_build 3857 3856
stack_checking_rand_fill_build 3862 3861
trace_build 4123 4108
merged 4503 4487

The denominator grows by 676 lines — 17.7% — and the reported figure moves from 99.97% to 99.64%. The second number is the honest one; the denominator now includes code the old report never counted at all.

The union also contains a file the old report did not contain at all: tx_thread_stack_error_handler.c compiles only under TX_ENABLE_STACK_CHECKING, so it was not listed at 0%, it was simply absent. 177 files becomes 178.

Coverage collection is serialised, and that is load-bearing

Collection moved out of test() and now runs after the test loop, one configuration at a time.

gcov writes its intermediate .gcov files into the directory gcovr is rooted at, and coverage.sh roots every configuration at the repository root so filenames come out repo-relative. Five concurrent gcovr processes therefore share one scratch directory and delete each other's output. The first full run of this change passed all 480 tests and produced no report for three of the five configurations — a green suite with most of its coverage silently missing.

Measured both ways: two gcovr rooted at the repository root fail concurrently and both succeed in sequence. CI would not have caught it, because test_tx.sh sets CTEST_PARALLEL_LEVEL=1 and takes the serial branch — a coincidence of one caller rather than a property of the script.

The SMP question, and why it is not a blocker

An earlier run of this branch saw trace_build fail threadx_smp_time_slice_test and then hang, which raised the question of whether -fprofile-arcs perturbs a timing-sensitive test. It does not. Sixteen runs settle it.

The decisive one: threadx_smp_time_slice_test failed ERROR #31 — twice in a row under --repeat until-pass:2 — on an uninstrumented build, in the exact shape CI runs, while three instrumented runs of that shape passed 5/5.

In the CI shape, CTEST_PARALLEL_LEVEL=1 run.sh test all:

runs result wall clock
TX_COVERAGE=OFF (= dev today) 3 2 green, one ERROR #31 310 s
TX_COVERAGE=ON (this PR) 3 3 green, 5/5 each 325–329 s

So the test is a pre-existing flake on dev, and instrumenting all five costs about 5% of the suite's wall clock.

Separately, and in both instrumented and uninstrumented builds, run.sh's parallel branch — what you get typing run.sh test all with no CTEST_PARALLEL_LEVEL — hangs under its own load, four times in twelve runs. Several SMP tests create 1024 ThreadX threads by construction and the Linux port backs each with a pthread, so five configurations at once put on the order of 5000 threads on the machine. CI sets CTEST_PARALLEL_LEVEL=1 and does not take that branch. Not introduced here, and not addressed here.

Reviewing

coverage_report/per_configuration/ exists because of the Pages deploy: it merges the ThreadX and SMP artifacts into one tree and every configuration directory has the same name in both suites, so left at the top level one suite's would overwrite the other's on the published site.

The one thing no PR has exercised before is the !.../per_configuration/** exclusion in the artifact upload — worth checking on this run that the uploaded coverage_report-* artifact holds only the suite directory and merged.xml.

Only default_build_coverage carried -fprofile-arcs, because the gate was the
build type and it is the only one of five whose name ends in _coverage. The
other four build and run all their tests and their coverage was discarded. That
is not redundancy thrown away: each configuration selects a different set of TX_
feature macros, so the code the other four compile is absent from the
denominator rather than uncovered in it.

TX_COVERAGE instruments a build regardless of its name, defaulting to OFF so a
single configuration built by hand behaves as before. coverage.sh gains a
--merge mode that unions the per-configuration JSON tracefiles, and
cmake_bootstrap.sh runs it after the test loop so a local run produces the same
merged report CI reads. The template sets TX_COVERAGE for build and test, and
coverage_name moves to the merged report.

Measured on the ThreadX suite, all 480 tests passing:

  default_build_coverage           3827 valid   3827 covered
  disable_notify_callbacks_build   3767         3766
  stack_checking_build             3857         3856
  stack_checking_rand_fill_build   3862         3861
  trace_build                      4123         4108
  merged                           4503         4487

The denominator grows by 676 lines, 17.7%, and the figure moves from 99.97% to
99.64%. The second one is honest, and the drop is the point rather than a
regression: the denominator now includes code the old report never counted. The
union also contains a file the old report did not contain at all --
tx_thread_stack_error_handler.c compiles only under TX_ENABLE_STACK_CHECKING, so
it was not listed at 0%, it was simply absent. 177 files becomes 178.

Coverage collection moved out of test() and now runs after the test loop, one
configuration at a time. gcov writes its intermediate gcov files into the
directory gcovr is rooted at, and coverage.sh roots every configuration at the
repository root so filenames come out repo-relative. Five concurrent gcovr
processes therefore share one scratch directory and delete each other's output:
the first full run of this change passed all 480 tests and produced no report
for three of the five configurations. Measured both ways -- two gcovr rooted at
the repository root fail concurrently and succeed in sequence. CI would not
have caught it, because test_tx.sh sets CTEST_PARALLEL_LEVEL=1 and takes the
serial branch.

Per-configuration output moved under coverage_report/per_configuration/ and is
excluded from the Pages artifact. The deploy job merges the ThreadX and SMP
artifacts into one tree and every configuration directory has the same name in
both, so left at the top level one suite's would overwrite the other's on the
published site.

On the SMP suite, an earlier run of this change saw trace_build fail
threadx_smp_time_slice_test and then hang, which raised the question of whether
-fprofile-arcs perturbs a timing-sensitive test. It does not. Sixteen runs
settle it, and the decisive one is that threadx_smp_time_slice_test failed
ERROR eclipse-threadx#31 -- twice in a row under --repeat until-pass:2 -- on an uninstrumented
build, in the exact shape CI runs, while three instrumented runs of that shape
passed 5 of 5. In the CI shape, CTEST_PARALLEL_LEVEL=1 run.sh test all:

  TX_COVERAGE=OFF   3 runs   2 green, one ERROR eclipse-threadx#31        310 s
  TX_COVERAGE=ON    3 runs   3 green, 5/5 each             325-329 s

So the test is a pre-existing flake on dev and instrumenting all five costs
about 5% of the suite's wall clock. Separately, and also in both instrumented
and uninstrumented builds, run.sh's parallel branch -- what a developer gets
typing run.sh test all with no CTEST_PARALLEL_LEVEL -- hangs under its own load,
four times in twelve runs. Several SMP tests create 1024 ThreadX threads by
construction and the Linux port backs each with a pthread, so five
configurations at once put on the order of 5000 threads on the machine. CI sets
CTEST_PARALLEL_LEVEL=1 and does not take that branch.

Assisted-by: Claude Opus 5 <noreply@anthropic.com>
@fdesbiens
fdesbiens merged commit 3e85bbd into eclipse-threadx:dev Aug 26, 2026
8 checks passed
@fdesbiens
fdesbiens deleted the feature/coverage-all-configurations branch August 26, 2026 12:13
fdesbiens added a commit that referenced this pull request Aug 26, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant