diff --git a/.github/workflows/regression_template.yml b/.github/workflows/regression_template.yml index 62a28e937..7eea8f432 100644 --- a/.github/workflows/regression_template.yml +++ b/.github/workflows/regression_template.yml @@ -29,8 +29,13 @@ on: default: false required: false type: boolean + # The merged report, not one configuration's. Every configuration is + # instrumented now (TX_COVERAGE below) and coverage.sh --merge unions + # them; default_build_coverage was one build of five, and the code the + # other four select was absent from its denominator rather than uncovered + # in it. coverage_name: - default: 'default_build_coverage' + default: 'merged' required: false type: string skip_deploy: @@ -79,12 +84,20 @@ jobs: timeout-minutes: 10 run: ${{ inputs.install_script }} + # TX_COVERAGE instruments every build configuration rather than only the one + # whose name ends in _coverage. It has to be set for the build as well as the + # test: the build is where -fprofile-arcs is decided, and the test run is + # where the reports are collected and merged. - name: Build timeout-minutes: 15 + env: + TX_COVERAGE: ${{ inputs.skip_coverage && 'OFF' || 'ON' }} run: ${{ inputs.build_script }} - name: Test timeout-minutes: 60 + env: + TX_COVERAGE: ${{ inputs.skip_coverage && 'OFF' || 'ON' }} run: ${{ inputs.test_script }} - name: Publish Test Results @@ -163,12 +176,32 @@ jobs: if: ${{ !cancelled() && (!inputs.skip_coverage) }} run: echo "coverage_report=coverage_report-$(date +%s)" >> $GITHUB_OUTPUT + # per_configuration is excluded deliberately. deploy_code_coverage downloads + # every coverage_report-* artifact with merge-multiple, so whatever is in + # here lands on the published site -- and each suite's per-configuration + # directories carry the same names, so ThreadX's would overwrite SMP's. + # The top level therefore holds only the suite directory and the merged XML, + # which is the shape the deploy already expects. - name: Upload Code Coverage Artifacts uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: ${{ !cancelled() && (inputs.skip_deploy && !inputs.skip_coverage) }} with: name: ${{ steps.artifact.outputs.coverage_report }} - path: ${{ inputs.cmake_path }}/coverage_report + path: | + ${{ inputs.cmake_path }}/coverage_report + !${{ inputs.cmake_path }}/coverage_report/per_configuration/** + retention-days: 1 + + # The per-configuration reports, kept separately so they are downloadable + # when the merged number moves and the question is which configuration moved + # it. The name deliberately does not match coverage_report-*, so the deploy + # job's pattern does not pick it up and it never reaches the published site. + - name: Upload Per-Configuration Coverage + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: ${{ !cancelled() && (!inputs.skip_coverage) }} + with: + name: coverage_detail ${{ inputs.result_affix }} + path: ${{ inputs.cmake_path }}/coverage_report/per_configuration retention-days: 1 - name: Upload Code Coverage Pages diff --git a/scripts/cmake_bootstrap.sh b/scripts/cmake_bootstrap.sh index faa43133f..eb9bee1b4 100755 --- a/scripts/cmake_bootstrap.sh +++ b/scripts/cmake_bootstrap.sh @@ -57,7 +57,7 @@ function generate() { echo "Compiler changed since build/$build was configured. Reconfiguring from scratch." rm -rf build/$build fi - cmake -Bbuild/$build -GNinja -DBUILD_SHARED_LIBS=ON -DCMAKE_TOOLCHAIN_FILE=$(dirname $(realpath $0))/../cmake/linux.cmake -DCMAKE_BUILD_TYPE=$build . + cmake -Bbuild/$build -GNinja -DBUILD_SHARED_LIBS=ON -DCMAKE_TOOLCHAIN_FILE=$(dirname $(realpath $0))/../cmake/linux.cmake -DCMAKE_BUILD_TYPE=$build -DTX_COVERAGE=${TX_COVERAGE:-OFF} . } function build() { @@ -84,11 +84,14 @@ function test() { fi # 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. + # test, and every configuration after it would go untested -- and, before + # collection moved out of this function, uncovered as well. 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. Collection + # itself now happens in collect_all_coverage, after every configuration has + # been tested, and it does not stop at the first failure either. 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 @@ -96,9 +99,72 @@ function test() { # 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 + return $status +} + +# Coverage is collected for any configuration that was instrumented, which is +# what TX_COVERAGE decides. The build-type match is kept for the one +# configuration instrumented by its name, so building a single configuration by +# hand behaves exactly as it did before. +# +# Three of the test trees -- freertos, posix and tx/cmake/riscv -- have no +# coverage.sh at all, and this script serves them too. Say so rather than +# failing, so that TX_COVERAGE=ON is harmless anywhere it does not apply. +function collect_coverage() { + if [[ $1 = *"_coverage" ]] || [ "${TX_COVERAGE:-OFF}" = "ON" ]; then + if [ ! -x ./coverage.sh ]; then + echo "No coverage.sh in $(pwd); skipping coverage for $1." + return 0 + fi ./coverage.sh $1 fi +} + +# Collection runs after every configuration has been tested, and strictly one at +# a time. +# +# It used to sit inside test(), which was safe only while a single configuration +# was instrumented. gcov writes its intermediate .gcov files into the directory +# gcovr is rooted at, and coverage.sh roots every configuration at the repository +# root so that the report can name files the way the repository does. Five +# concurrent gcovr processes therefore share one scratch directory and delete +# each other's output. The symptom is a gcovr SanityCheckError naming a .gcov +# file that "doesn't exist but no error from GCOV detected", and it cost the +# report for three of the five configurations while all 480 tests still passed -- +# a green suite with most of its coverage missing. +# +# Measured both ways: two gcovr invocations rooted at the repository root fail +# when run concurrently and both succeed when run in sequence. CI happens to be +# safe already, because test_tx.sh sets CTEST_PARALLEL_LEVEL=1 and takes the +# serial branch below, but that is a coincidence of one caller rather than a +# property of this script. +# +# Collection deliberately does not stop at the first failure, for the reason +# given in test(): the configurations that did produce data should still report. +# +# The trade this makes, recorded rather than discovered later: a configuration +# that hangs long enough to hit the job's own timeout now costs the reports for +# the configurations that already finished, where collecting inside test() would +# have kept them. That is judged the smaller risk -- a per-test timeout is a +# ctest failure and the loop carries on, the job timeout sits at 60 minutes +# against a suite that takes 5 to 25, and the alternative loses most of the +# coverage on every parallel run instead of on a hang. +function collect_all_coverage() { + local item status=0 + for item in $builds; do + collect_coverage $item || status=$? + done + + # Union the per-configuration reports. Only the union is a coverage figure: + # each configuration compiles a different set of TX_ feature macros, so a + # line one of them compiles out is absent from its denominator rather than + # uncovered in it, and an average of five percentages means nothing. + # + # Done here rather than as a separate workflow step so a local run produces + # the same merged report CI reads. + if [ "${TX_COVERAGE:-OFF}" = "ON" ] && [ -x ./coverage.sh ]; then + ./coverage.sh --merge || status=$? + fi return $status } @@ -155,6 +221,11 @@ elif [ "$command" == "test" ]; then for p in $pids; do wait $p || exit_code=$? done + # A coverage failure turns the run red, but must not overwrite a test + # failure's status with its own. + coverage_status=0 + collect_all_coverage || coverage_status=$? + [ $exit_code -ne 0 ] || exit_code=$coverage_status exit $exit_code else # Run builds in serial. The status is collected the same way the parallel @@ -168,6 +239,11 @@ elif [ "$command" == "test" ]; then echo "Testing $item" test $item $parallel_jobs || exit_code=$? done + # A coverage failure turns the run red, but must not overwrite a test + # failure's status with its own. + coverage_status=0 + collect_all_coverage || coverage_status=$? + [ $exit_code -ne 0 ] || exit_code=$coverage_status exit $exit_code fi elif [ "$command" == "build_libs" ]; then diff --git a/test/smp/cmake/CMakeLists.txt b/test/smp/cmake/CMakeLists.txt index e2ca8627c..3a0d9638a 100644 --- a/test/smp/cmake/CMakeLists.txt +++ b/test/smp/cmake/CMakeLists.txt @@ -64,7 +64,21 @@ add_subdirectory(regression) add_subdirectory(samples) # Coverage -if(CMAKE_BUILD_TYPE MATCHES ".*_coverage") +# +# The gate here used to be the build type alone, and only one of the five +# configurations -- default_build_coverage -- has a name matching "*_coverage". +# So four configurations built and ran every test and their coverage was thrown +# away. That is not redundancy discarded: each configuration selects a different +# set of TX_ feature macros, so the code the other four compile is *absent from +# the denominator* rather than reported as uncovered. A figure taken from one of +# them is the executable-line count of one configuration, not of the kernel. +# +# TX_COVERAGE instruments a build regardless of its name. It defaults to OFF and +# the build-type match is kept, so configuring a single configuration by hand +# behaves exactly as it did before; the regression run turns it on for all five +# and merges the results. +option(TX_COVERAGE "Instrument this build for coverage regardless of build type" OFF) +if(TX_COVERAGE OR CMAKE_BUILD_TYPE MATCHES ".*_coverage") if(NOT MSVC) target_compile_options(threadx_smp PRIVATE -fprofile-arcs -ftest-coverage) target_link_options(threadx_smp PRIVATE -fprofile-arcs -ftest-coverage) diff --git a/test/smp/cmake/coverage.sh b/test/smp/cmake/coverage.sh index abcf7cceb..88de4f04a 100755 --- a/test/smp/cmake/coverage.sh +++ b/test/smp/cmake/coverage.sh @@ -14,7 +14,6 @@ set -e cd $(dirname $0) -mkdir -p coverage_report/$1 # gcov reads a data format tied to the compiler that produced it, so gcov has to match # the gcc that built the objects. Since cmake/linux.cmake began honouring CC, taking @@ -41,6 +40,57 @@ if ! command -v "$GCOV" >/dev/null 2>&1; then exit 1 fi +# The repository root, needed by every mode below. It has to be absolute: see the +# note on the per-configuration paths further down. +repo_root=$(cd ../../.. && pwd) +filter=$repo_root/common_smp/src + +# --merge unions the per-configuration reports into the one number that means +# something. Each configuration writes an intermediate JSON beside its XML, and +# this pass adds them all together. +# +# Reporting five separate percentages instead would invite a reader to average +# them, and an average is not a coverage figure -- a line covered only by +# trace_build is covered, and only the union says so. +# +# Do not be alarmed when the merged percentage is lower than the single +# configuration this used to report. That is the point: the denominator now +# includes code the old report never counted at all, because it was compiled out +# of the only instrumented build. +if [ "$1" = "--merge" ]; then + shopt -s nullglob + tracefiles=(coverage_report/per_configuration/*.json) + shopt -u nullglob + if [ ${#tracefiles[@]} -eq 0 ]; then + echo "coverage.sh --merge: no JSON in coverage_report/per_configuration/." >&2 + echo "Run the suites with TX_COVERAGE=ON first." >&2 + exit 1 + fi + + add_args=() + for t in "${tracefiles[@]}"; do + add_args+=(--add-tracefile "$t") + done + + mkdir -p coverage_report/merged + gcovr -r "$repo_root" "${add_args[@]}" --xml-pretty --output coverage_report/merged.xml + gcovr -r "$repo_root" "${add_args[@]}" --html --html-details --output coverage_report/merged/index.html + + if ! grep -q "&2 + exit 1 + fi + + # Named, not just counted. coverage_report/ is not cleaned between runs, so a + # tracefile left by an earlier run of a different set of configurations would + # otherwise be merged in without anything saying so. + echo "coverage.sh --merge: SMP, ${#tracefiles[@]} configuration(s):" + for t in "${tracefiles[@]}"; do + echo " $(basename "$t" .json)" + done + exit 0 +fi + # gcovr is given three paths below, and each of them has to be absolute, for a # different reason. # @@ -82,17 +132,26 @@ objdir=$PWD/build/$1/threadx_smp/CMakeFiles/threadx_smp.dir$repo_root/common_smp # architecture ports are validated functionally rather than structurally, and # linux/gnu is a development host port that nothing ships on. Written down # because a filter argument on its own is not a decision the next reader can see. -filter=$repo_root/common_smp/src -gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --xml-pretty --output coverage_report/$1.xml -gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --html --html-details --output coverage_report/$1/index.html +# Per-configuration output is kept in a subdirectory of its own, and the merged +# report sits alongside it at the top. That is not tidiness: the Pages deploy +# uploads coverage_report wholesale and merges the ThreadX and SMP artifacts +# into one tree, and every configuration directory has the same name in both +# suites. Left at the top level, default_build_coverage/ from one suite would +# overwrite the other's on the published site. Nested here, the top level still +# holds exactly the one suite directory the deploy expects. +mkdir -p coverage_report/per_configuration/$1 +gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" \ + --json coverage_report/per_configuration/$1.json \ + --xml-pretty --output coverage_report/per_configuration/$1.xml +gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --html --html-details --output coverage_report/per_configuration/$1/index.html # An empty report is not an error as far as gcovr is concerned: it warns and # exits 0. Worse, it advertises line-rate="1.0" alongside lines-valid="0", so # every downstream consumer reads "no data at all" as "100% covered". A coverage # threshold cannot catch that, because an empty report passes any threshold. So # the assertion belongs here, next to the paths that would cause it. -if ! grep -q "&2 echo "Expected gcda files under $objdir." >&2 exit 1 diff --git a/test/tx/cmake/CMakeLists.txt b/test/tx/cmake/CMakeLists.txt index 97b3615bd..b2f0ab1fe 100644 --- a/test/tx/cmake/CMakeLists.txt +++ b/test/tx/cmake/CMakeLists.txt @@ -62,7 +62,21 @@ add_subdirectory(regression) add_subdirectory(samples) # Coverage -if(CMAKE_BUILD_TYPE MATCHES ".*_coverage") +# +# The gate here used to be the build type alone, and only one of the five +# configurations -- default_build_coverage -- has a name matching "*_coverage". +# So four configurations built and ran every test and their coverage was thrown +# away. That is not redundancy discarded: each configuration selects a different +# set of TX_ feature macros, so the code the other four compile is *absent from +# the denominator* rather than reported as uncovered. A figure taken from one of +# them is the executable-line count of one configuration, not of the kernel. +# +# TX_COVERAGE instruments a build regardless of its name. It defaults to OFF and +# the build-type match is kept, so configuring a single configuration by hand +# behaves exactly as it did before; the regression run turns it on for all five +# and merges the results. +option(TX_COVERAGE "Instrument this build for coverage regardless of build type" OFF) +if(TX_COVERAGE OR CMAKE_BUILD_TYPE MATCHES ".*_coverage") if(NOT MSVC) target_compile_options(threadx PRIVATE -fprofile-arcs -ftest-coverage) target_link_options(threadx PRIVATE -fprofile-arcs -ftest-coverage) diff --git a/test/tx/cmake/coverage.sh b/test/tx/cmake/coverage.sh index aabf039f7..32d466b73 100755 --- a/test/tx/cmake/coverage.sh +++ b/test/tx/cmake/coverage.sh @@ -14,7 +14,6 @@ set -e cd $(dirname $0) -mkdir -p coverage_report/$1 # gcov reads a data format tied to the compiler that produced it, so gcov has to match # the gcc that built the objects. Since cmake/linux.cmake began honouring CC, taking @@ -41,6 +40,57 @@ if ! command -v "$GCOV" >/dev/null 2>&1; then exit 1 fi +# The repository root, needed by every mode below. It has to be absolute: see the +# note on the per-configuration paths further down. +repo_root=$(cd ../../.. && pwd) +filter=$repo_root/common/src + +# --merge unions the per-configuration reports into the one number that means +# something. Each configuration writes an intermediate JSON beside its XML, and +# this pass adds them all together. +# +# Reporting five separate percentages instead would invite a reader to average +# them, and an average is not a coverage figure -- a line covered only by +# trace_build is covered, and only the union says so. +# +# Do not be alarmed when the merged percentage is lower than the single +# configuration this used to report. That is the point: the denominator now +# includes code the old report never counted at all, because it was compiled out +# of the only instrumented build. +if [ "$1" = "--merge" ]; then + shopt -s nullglob + tracefiles=(coverage_report/per_configuration/*.json) + shopt -u nullglob + if [ ${#tracefiles[@]} -eq 0 ]; then + echo "coverage.sh --merge: no JSON in coverage_report/per_configuration/." >&2 + echo "Run the suites with TX_COVERAGE=ON first." >&2 + exit 1 + fi + + add_args=() + for t in "${tracefiles[@]}"; do + add_args+=(--add-tracefile "$t") + done + + mkdir -p coverage_report/merged + gcovr -r "$repo_root" "${add_args[@]}" --xml-pretty --output coverage_report/merged.xml + gcovr -r "$repo_root" "${add_args[@]}" --html --html-details --output coverage_report/merged/index.html + + if ! grep -q "&2 + exit 1 + fi + + # Named, not just counted. coverage_report/ is not cleaned between runs, so a + # tracefile left by an earlier run of a different set of configurations would + # otherwise be merged in without anything saying so. + echo "coverage.sh --merge: ThreadX, ${#tracefiles[@]} configuration(s):" + for t in "${tracefiles[@]}"; do + echo " $(basename "$t" .json)" + done + exit 0 +fi + # gcovr is given three paths below, and each of them has to be absolute, for a # different reason. # @@ -55,8 +105,8 @@ fi # # Both -r and -f must be absolute. "-r ../../.. -f common/src" produces a report # containing zero files and exits 0, which is the worst failure mode available -# here: a green run carrying an empty report. Measured, not assumed. -repo_root=$(cd ../../.. && pwd) +# here: a green run carrying an empty report. Measured, not assumed. repo_root +# and filter are set above, before the merge mode, because it needs them too. # This is what actually scopes the report to one build configuration, and it is # the positional search path -- not --object-directory, which used to be here @@ -79,17 +129,26 @@ objdir=$PWD/build/$1/threadx/CMakeFiles/threadx.dir/common/src # ports are validated functionally rather than structurally, and linux/gnu is a # development host port that nothing ships on. Written down because a filter # argument on its own is not a decision the next reader can see. -filter=$repo_root/common/src -gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --xml-pretty --output coverage_report/$1.xml -gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --html --html-details --output coverage_report/$1/index.html +# Per-configuration output is kept in a subdirectory of its own, and the merged +# report sits alongside it at the top. That is not tidiness: the Pages deploy +# uploads coverage_report wholesale and merges the ThreadX and SMP artifacts +# into one tree, and every configuration directory has the same name in both +# suites. Left at the top level, default_build_coverage/ from one suite would +# overwrite the other's on the published site. Nested here, the top level still +# holds exactly the one suite directory the deploy expects. +mkdir -p coverage_report/per_configuration/$1 +gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" \ + --json coverage_report/per_configuration/$1.json \ + --xml-pretty --output coverage_report/per_configuration/$1.xml +gcovr --gcov-executable "$GCOV" -r "$repo_root" -f "$filter" "$objdir" --html --html-details --output coverage_report/per_configuration/$1/index.html # An empty report is not an error as far as gcovr is concerned: it warns and # exits 0. Worse, it advertises line-rate="1.0" alongside lines-valid="0", so # every downstream consumer reads "no data at all" as "100% covered". A coverage # threshold cannot catch that, because an empty report passes any threshold. So # the assertion belongs here, next to the paths that would cause it. -if ! grep -q "&2 echo "Expected gcda files under $objdir." >&2 exit 1