mirror of
https://github.com/eclipse-threadx/threadx.git
synced 2026-10-06 06:59:08 +08:00
Instrumented every build configuration and merged their coverage (#665)
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 #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 #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>
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user