From bf878a976ce5b66cc65e3ee147df72a1ef6e9301 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 00:11:09 -0400 Subject: [PATCH 1/6] Fix CI concurrency group so push and pull_request share one group github.event.pull_request.number is always set on a pull_request event, so the old ||-fallback to github.ref never actually triggered there -- push and pull_request runs for the same commit landed in different groups and neither cancelled the other. head_ref || ref_name resolves to the same bare branch name for both event types. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed23a721..3fe5f5d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,11 +50,14 @@ on: # Cancel any in-progress run for the same unit of work when a new event arrives. # A push to a branch that also has an open PR fires both the push and pull_request -# triggers for the very same commit; grouping by PR number when one exists (falling -# back to the ref otherwise) means those two runs share one group instead of two, -# so the second one to start cancels the first rather than both running to completion. +# triggers for the very same commit; github.head_ref carries the bare source branch +# name for a pull_request event and is empty for a push, while github.ref_name carries +# that same bare branch name for a push (and an unrelated merge-ref name for a +# pull_request, which is why it's only ever reached as the fallback here) -- so +# `head_ref || ref_name` resolves to the identical string for both event types on the +# same branch, landing both runs in one group so the second to start cancels the first. concurrency: - group: ci-${{ github.event.pull_request.number || github.ref }} + group: ci-${{ github.head_ref || github.ref_name }} cancel-in-progress: true From 61c63c8995c4fa825dd2a15e9ad75ef9472f5a02 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 00:30:08 -0400 Subject: [PATCH 2/6] Add combined unit+system test coverage reporting to CI Unit tests run in-process, so SimpleCov started in spec_helper.rb captures them directly. System tests never touch lib/ceedling in-process at all -- every example shells out to a real `bundle exec ruby -S ceedling ...` subprocess via SystemContext, running against a throwaway Bundler environment that strips the outer process's env clean. Capturing that requires starting SimpleCov inside each child process instead, injected via RUBYOPT (spec/support/system/ simplecov_boot.rb) after Bundler's env-stripping, with root pointed back at this repo since the child's own CWD is an ephemeral deployed project directory. Both sides accumulate into one shared coverage/.resultset.json, differentiated by SimpleCov's own command_name, and `rake coverage:report` merges and formats it once after both suites finish. Everything is gated behind CEEDLING_TEST_COVERAGE, whose value (not just its presence) matters: spec_helper.rb is also loaded by the system suite's own outer rspec process, which barely touches lib/ceedling directly, so distinguishing 'units' from 'system' keeps that process from claiming (and silently overwriting) the real unit-test resultset entry. Only enabled on CI's tests-linux Ruby-3.3 leg, reusing that job's existing tool installs rather than adding a separate job. simplecov is a Gemfile-only dependency (never in ceedling.gemspec, matching the existing diff-lcs precedent), and the throwaway system-test Gemfile only gains it, pinned to the same version constraint, when coverage mode is on. Co-Authored-By: Claude Sonnet 5 --- .github/actions/run-system-tests/action.yml | 5 +++ .github/workflows/ci.yml | 25 +++++++++++ .gitignore | 3 ++ .simplecov | 26 ++++++++++++ Gemfile | 5 +++ Gemfile.lock | 8 ++++ Rakefile | 23 +++++++++++ spec/support/spec_helper.rb | 19 +++++++++ spec/support/system/simplecov_boot.rb | 37 +++++++++++++++++ spec/support/system/system_context.rb | 46 +++++++++++++++------ 10 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 .simplecov create mode 100644 spec/support/system/simplecov_boot.rb diff --git a/.github/actions/run-system-tests/action.yml b/.github/actions/run-system-tests/action.yml index 8fba6575..491a0fb2 100644 --- a/.github/actions/run-system-tests/action.yml +++ b/.github/actions/run-system-tests/action.yml @@ -21,6 +21,10 @@ inputs: ruby: description: 'Ruby version (used in failure artifact name)' required: true + coverage: + description: "Set to 'system' to instrument each system-test child process with coverage (see spec/support/system/simplecov_boot.rb) -- the literal value matters, not just truthiness, so this same run's own outer rspec process doesn't also claim the unit suite's SimpleCov resultset entry" + required: false + default: 'false' runs: using: "composite" @@ -41,6 +45,7 @@ runs: shell: bash env: CI_RSPEC_PROGRESS_FORMAT: true + CEEDLING_TEST_COVERAGE: ${{ inputs.coverage }} run: rake specs:system:debug # Upload system test failure logs and temp project directories on test job failure. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3fe5f5d7..ff4280d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,9 +111,18 @@ jobs: # Run unit tests immediately after gem install — before the slower external tool # installation steps — so unit failures surface as quickly as possible. # Unit tests have no dependency on gdb, valgrind, cppcheck, gcovr, or ReportGenerator. + # + # Coverage instrumentation (CEEDLING_TEST_COVERAGE) only runs on one leg of this + # matrix -- Ruby 3.3, matching the other Linux-only singleton jobs' pinned version + # -- rather than a separate job, so the combined unit+system report reuses the + # tool installs this job already pays for instead of duplicating them. The value + # ('units' here, 'system' on the Run System Tests step below) tells spec_helper.rb + # which suite is instrumenting -- see spec_helper.rb's own comment for why a bare + # on/off flag isn't enough. - name: Run Unit Tests env: CI_RSPEC_PROGRESS_FORMAT: true + CEEDLING_TEST_COVERAGE: ${{ matrix.ruby == '3.3' && 'units' || 'false' }} run: rake specs:units # Install gdb for backtrace feature testing @@ -174,6 +183,22 @@ jobs: uses: ./.github/actions/run-system-tests with: ruby: ${{ matrix.ruby }} + coverage: ${{ matrix.ruby == '3.3' && 'system' || 'false' }} + + # Merges the resultset both prior steps accumulated (see Run Unit Tests above) + # into one HTML report and uploads it, downloadable from this run's own Actions + # summary page. Ruby-3.3-only, same leg the two steps above instrumented. + - name: Generate Combined Coverage Report + if: matrix.ruby == '3.3' + run: rake coverage:report + + - name: Upload Combined Coverage Report + if: matrix.ruby == '3.3' + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage/ + if-no-files-found: error # Common plugin test steps - name: Run Plugin Tests diff --git a/.gitignore b/.gitignore index b974028e..875027c0 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,9 @@ systest.pass.* systest.fail.* systests/ +# Coverage report output (CEEDLING_TEST_COVERAGE test runs) +/coverage/ + # Generated documentation site builds site-web/ site-local/ diff --git a/.simplecov b/.simplecov new file mode 100644 index 00000000..990e66dd --- /dev/null +++ b/.simplecov @@ -0,0 +1,26 @@ +# ========================================================================= +# Ceedling - Test-Centered Build System for C +# ThrowTheSwitch.org +# Copyright (c) 2010-26 Mike Karlesky, Mark VanderVoord, & Greg Williams +# SPDX-License-Identifier: MIT +# ========================================================================= + +# Shared SimpleCov configuration. The unit-test suite picks this up through +# SimpleCov's own upward-directory-search autoload (its CWD is this repo). The +# system-test suite's own subprocess boot (spec/support/system/simplecov_boot.rb) +# loads this file explicitly by absolute path instead, since that subprocess's CWD +# is a throwaway deployed project directory, not this repo, when it starts -- the +# autoload search would walk straight past it and find nothing. +# +# track_files (rather than only reporting files a run actually happened to require) +# means a file no test ever touches still shows up at 0% instead of being silently +# absent from the total -- lib/ and bin/ are what the gem actually ships and what +# both test suites exercise; vendor/ is CMock/Unity/CException's own separately +# tested source, not this repo's. +SimpleCov.start do + track_files 'lib/**/*.rb' + track_files 'bin/**/*.rb' + + add_filter '/spec/' + add_filter '/vendor/' +end diff --git a/Gemfile b/Gemfile index 804218e8..a351fa9e 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,11 @@ gem "require_all" # ceedling.gemspec, so it is never a hard requirement for an installed release gem. gem "diff-lcs", "~> 1.5" +# Dev-only: code coverage for CI's combined unit+system test coverage report. +# require: false since it's only ever loaded when CEEDLING_TEST_COVERAGE is set -- +# deliberately NOT declared in ceedling.gemspec, same as diff-lcs above. +gem "simplecov", "~> 0.22", require: false + # Ceedling dependencies gem "diy", "~> 1.1" gem "constructor", "~> 2" diff --git a/Gemfile.lock b/Gemfile.lock index cf49a0ff..0cad1ee6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,6 +7,7 @@ GEM diff-lcs (1.6.2) diy (1.1.2) constructor (>= 1.0.0) + docile (1.4.1) erb (2.2.0) parallel (1.28.0) rake (13.2.1) @@ -25,6 +26,12 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.7) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.13.2) + simplecov_json_formatter (0.1.4) thor (1.5.0) unicode-display_width (3.2.0) unicode-emoji (~> 4.1) @@ -51,6 +58,7 @@ DEPENDENCIES require_all rr rspec (~> 3.8) + simplecov (~> 0.22) thor (~> 1.3) unicode-display_width (~> 3.1) diff --git a/Rakefile b/Rakefile index 36372450..6734f8b6 100644 --- a/Rakefile +++ b/Rakefile @@ -43,6 +43,29 @@ task 'specs:system:debug' do Rake::Task['specs:system'].invoke end +# Formats the combined unit+system coverage resultset that CEEDLING_TEST_COVERAGE +# test runs accumulate into one shared coverage/.resultset.json (see .simplecov and +# spec/support/system/simplecov_boot.rb) into a single HTML report. Run after both +# `specs:units` and `specs:system`/`specs:system:debug` have completed with that env +# var set -- this task only reads back what they already wrote, it doesn't run any +# specs itself. `require 'simplecov'` here briefly starts SimpleCov for this task's +# own process too (via .simplecov's own autoload) -- overriding at_exit the same way +# spec_helper.rb/simplecov_boot.rb do keeps that from redundantly re-merging and +# reformatting a second time after the explicit format below already ran. +desc "Merge and format the combined unit+system SimpleCov coverage report" +task 'coverage:report' do + require 'simplecov' + SimpleCov.at_exit { SimpleCov.result } + + result = SimpleCov::ResultMerger.merged_result + raise "No coverage data found in #{SimpleCov.coverage_path} -- " \ + "run specs:units and specs:system with CEEDLING_TEST_COVERAGE set first" if result.nil? + + SimpleCov::Formatter::HTMLFormatter.new.format(result) + puts "Combined coverage: #{result.covered_percent.round(2)}% " \ + "(#{result.covered_lines}/#{result.total_lines} lines)" +end + # Individual unit specs Dir['spec/units/**/*_spec.rb'].each do |p| base = File.basename(p,'.*').gsub('_spec','') diff --git a/spec/support/spec_helper.rb b/spec/support/spec_helper.rb index 6195b9b7..4feec45d 100644 --- a/spec/support/spec_helper.rb +++ b/spec/support/spec_helper.rb @@ -5,6 +5,25 @@ # SPDX-License-Identifier: MIT # ========================================================================= +# Coverage instrumentation must start before any Ceedling code below is required, or +# Ruby's Coverage module never sees those files' lines at all. Only active when +# CEEDLING_TEST_COVERAGE is exactly 'units' -- this file is also required by the +# system-test suite's own outer rspec process (spec_system_helper.rb requires it too), +# which barely touches lib/ceedling directly itself (the real work happens in each +# system-test child subprocess, instrumented separately by simplecov_boot.rb); a bare +# truthy check here would make that process ALSO claim SimpleCov's "units" resultset +# entry and silently overwrite the real unit-test coverage with its own near-empty +# coverage, since SimpleCov's own multi-process merging replaces rather than +# accumulates same-named entries. Picks up the shared .simplecov config (repo root) +# via SimpleCov's own upward-directory-search autoload. The final HTML report is +# generated once, explicitly, by `rake coverage:report` after both the unit and +# system suites finish, rather than by this process's own exit. +if ENV['CEEDLING_TEST_COVERAGE'] == 'units' + require 'simplecov' + SimpleCov.command_name 'units' + SimpleCov.at_exit { SimpleCov.result } +end + require 'require_all' require 'constructor' diff --git a/spec/support/system/simplecov_boot.rb b/spec/support/system/simplecov_boot.rb new file mode 100644 index 00000000..318822f0 --- /dev/null +++ b/spec/support/system/simplecov_boot.rb @@ -0,0 +1,37 @@ +# ========================================================================= +# Ceedling - Test-Centered Build System for C +# ThrowTheSwitch.org +# Copyright (c) 2010-26 Mike Karlesky, Mark VanderVoord, & Greg Williams +# SPDX-License-Identifier: MIT +# ========================================================================= + +# Injected into each system-test child process's RUBYOPT (see SystemContext#with_context) +# so coverage instrumentation starts before that process's own `require`s run -- Ruby's +# Coverage module only sees lines loaded after it starts. RUBYOPT applies to every +# subprocess spawned while coverage mode is on, not just the one build/appcmd call +# actually being measured, so this re-checks its own gate rather than assuming it's +# only ever loaded when wanted. +if ENV['CEEDLING_TEST_COVERAGE_ROOT'] + require 'simplecov' + + # This process's own CWD is a throwaway deployed project directory, not this repo, + # so the require above's own upward-directory-search for .simplecov walks straight + # past the real one and finds nothing. Point root back at the real repo and load + # the shared config explicitly now that root is correct. + SimpleCov.root(ENV['CEEDLING_TEST_COVERAGE_ROOT']) + load File.join(ENV['CEEDLING_TEST_COVERAGE_ROOT'], '.simplecov') + + # SimpleCov's resultset stores one entry per command_name, and a new result under + # an already-used name *replaces* rather than accumulates with the previous one -- + # a shared name across the many sequential child processes a full system-test run + # spawns would silently keep only the last one's coverage. PID alone isn't enough + # (a long run can cycle through enough child processes, each itself spawning + # further subprocesses for compiles/links/etc., to plausibly repeat a PID); + # PID plus a microsecond timestamp is. + SimpleCov.command_name "system-#{Process.pid}-#{Time.now.strftime('%Y%m%d%H%M%S%6N')}" + # Each of potentially hundreds of these child processes across a full system-test + # run only needs to merge its own coverage into the shared resultset -- the + # formatted HTML report is generated once, explicitly, by `rake coverage:report` + # after both suites finish, not redundantly by every child process's own exit. + SimpleCov.at_exit { SimpleCov.result } +end diff --git a/spec/support/system/system_context.rb b/spec/support/system/system_context.rb index 0fa546bc..ce58b911 100644 --- a/spec/support/system/system_context.rb +++ b/spec/support/system/system_context.rb @@ -24,6 +24,7 @@ class VerificationFailed < RuntimeError; end # Eliminates redundant `bundle install` runs (one per describe group → one per suite). @@shared_gem_dir = nil @@shared_gem = nil + @@git_repo = nil def self.setup_shared_gem! return if @@shared_gem_dir @@ -32,19 +33,24 @@ def self.setup_shared_gem! shared_gem = GemDirLayout.new(shared_dir) git_repo = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..')) - File.write( - File.join(shared_dir, 'Gemfile'), - [ - %Q{source "http://rubygems.org/"}, - %Q{gem "rake"}, - %Q{gem "constructor"}, - %Q{gem "diy"}, - %Q{gem "thor"}, - %Q{gem "deep_merge"}, - %Q{gem "unicode-display_width"}, - %Q{gem "ceedling", :path => '#{git_repo}'} - ].join("\n") - ) + gemfile_lines = [ + %Q{source "http://rubygems.org/"}, + %Q{gem "rake"}, + %Q{gem "constructor"}, + %Q{gem "diy"}, + %Q{gem "thor"}, + %Q{gem "deep_merge"}, + %Q{gem "unicode-display_width"}, + %Q{gem "ceedling", :path => '#{git_repo}'} + ] + # This machine-generated Gemfile is never committed -- adding simplecov here only + # when coverage mode is on keeps every other run's deployed environment identical + # to what a real installed gem's own dependencies actually are. Pinned to the same + # constraint as the main Gemfile so both processes run the identical SimpleCov + # version -- an unpinned resolve here could otherwise drift to whatever's newest on + # rubygems.org independent of the main Gemfile.lock. + gemfile_lines << %Q{gem "simplecov", "~> 0.22"} if ENV['CEEDLING_TEST_COVERAGE'] == 'system' + File.write( File.join(shared_dir, 'Gemfile'), gemfile_lines.join("\n") ) Dir.chdir(shared_dir) do begin @@ -78,12 +84,14 @@ def self.setup_shared_gem! @@shared_gem_dir = shared_dir @@shared_gem = shared_gem + @@git_repo = git_repo end def self.cleanup_shared_gem! FileUtils.rm_rf(@@shared_gem_dir) if @@shared_gem_dir @@shared_gem_dir = nil @@shared_gem = nil + @@git_repo = nil end def initialize @@ -149,6 +157,18 @@ def with_context ENV['RUBYLIB'] = @gem.lib ENV['RUBYPATH'] = @gem.bin + # RUBYOPT survives Bundler's env-stripping above only because it's set here, + # inside this block, rather than by whatever called into this method -- forces + # spec/support/system/simplecov_boot.rb to load before the child process's own + # requires run, the same way a coverage tool would hook any externally invoked + # CLI. CEEDLING_TEST_COVERAGE_ROOT tells that boot script where this repo (and + # its shared .simplecov config) actually live, since the child's own CWD is + # this ephemeral deployed project directory, not the repo. + if ENV['CEEDLING_TEST_COVERAGE'] == 'system' + ENV['RUBYOPT'] = "-r#{File.join(File.dirname(__FILE__), 'simplecov_boot.rb')}" + ENV['CEEDLING_TEST_COVERAGE_ROOT'] = @@git_repo + end + ENV['LANG'] = 'en_US.UTF-8' ENV['LANGUAGE'] = 'en_US.UTF-8' ENV['LC_ALL'] = 'en_US.UTF-8' From 9654e3ab5e0842c746f2e65118d8542b01214fb2 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 08:29:35 -0400 Subject: [PATCH 3/6] Split coverage report into units/system/combined and fix track_files bugs Adds `bin`, `lib`, and `plugins` group tabs to every report via add_group, and generates units-only and system-only reports alongside the existing combined one from the same resultset data (Rakefile coverage:report task). Also fixes two pre-existing bugs in the coverage instrumentation from the prior commit: track_files was being called multiple times and only the last call took effect (SimpleCov overwrites rather than accumulates), so lib/ tracking was silently dropped; consolidated into one brace-glob call. Separately, the system-test child processes' backfill-untouched-files glob resolves against the process's actual working directory rather than SimpleCov.root, so it silently found nothing while CWD stayed in the throwaway deployed test project -- simplecov_boot.rb now chdirs into the repo for that call. Co-Authored-By: Claude Sonnet 5 --- .simplecov | 23 ++++++++--- Rakefile | 58 +++++++++++++++++++-------- spec/support/system/simplecov_boot.rb | 13 +++++- 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/.simplecov b/.simplecov index 990e66dd..75ffcbce 100644 --- a/.simplecov +++ b/.simplecov @@ -14,13 +14,26 @@ # # track_files (rather than only reporting files a run actually happened to require) # means a file no test ever touches still shows up at 0% instead of being silently -# absent from the total -- lib/ and bin/ are what the gem actually ships and what -# both test suites exercise; vendor/ is CMock/Unity/CException's own separately -# tested source, not this repo's. +# absent from the total -- lib/, bin/, and plugins/ are what the gem actually ships +# and what both test suites exercise; vendor/ is CMock/Unity/CException's own +# separately tested source, not this repo's. The /spec/ and /vendor/ filters below +# also cover the handful of plugins with their own nested spec/vendor code (e.g. +# plugins/fff/spec/, plugins/fff/vendor/fff/), not just this repo's own top-level +# spec/ and vendor/. +# +# One track_files call with a brace-glob covering all three trees, not three +# separate calls -- track_files stores a single glob rather than accumulating one, +# so each call replaces whatever the previous call set rather than adding to it. +# +# Grouped into the same three trees as a report's own tabs, so a report reads as +# "how well is each shipped piece covered" rather than one flat file list. SimpleCov.start do - track_files 'lib/**/*.rb' - track_files 'bin/**/*.rb' + track_files '{lib,bin,plugins}/**/*.rb' add_filter '/spec/' add_filter '/vendor/' + + add_group 'bin', '/bin/' + add_group 'lib', '/lib/' + add_group 'plugins', '/plugins/' end diff --git a/Rakefile b/Rakefile index 6734f8b6..56ac1bb1 100644 --- a/Rakefile +++ b/Rakefile @@ -43,27 +43,53 @@ task 'specs:system:debug' do Rake::Task['specs:system'].invoke end -# Formats the combined unit+system coverage resultset that CEEDLING_TEST_COVERAGE -# test runs accumulate into one shared coverage/.resultset.json (see .simplecov and -# spec/support/system/simplecov_boot.rb) into a single HTML report. Run after both -# `specs:units` and `specs:system`/`specs:system:debug` have completed with that env -# var set -- this task only reads back what they already wrote, it doesn't run any -# specs itself. `require 'simplecov'` here briefly starts SimpleCov for this task's -# own process too (via .simplecov's own autoload) -- overriding at_exit the same way -# spec_helper.rb/simplecov_boot.rb do keeps that from redundantly re-merging and -# reformatting a second time after the explicit format below already ran. -desc "Merge and format the combined unit+system SimpleCov coverage report" +# Formats three HTML reports -- units alone, system alone, and both combined -- +# from the one shared coverage/.resultset.json that CEEDLING_TEST_COVERAGE test +# runs accumulate into (see .simplecov and spec/support/system/simplecov_boot.rb). +# Run after both `specs:units` and `specs:system`/`specs:system:debug` have +# completed with that env var set -- this task only reads back what they already +# wrote, it doesn't run any specs itself. Each report is just a different subset +# of the same resultset hash, built with the identical merge machinery +# SimpleCov's own merged_result uses internally, one subset per report: +# units -- the lone entry named exactly "units" (spec_helper.rb) +# system -- every entry named "system--" (simplecov_boot.rb, +# one per system-test child process) +# combined -- every entry, regardless of name +# +# `require 'simplecov'` here briefly starts SimpleCov for this task's own process +# too (via .simplecov's own autoload) -- overriding at_exit to a no-op keeps that +# process's own trivial self-coverage from being written anywhere at all, since +# this task reformats coverage_dir multiple times over its own run and has +# nothing of its own worth preserving. +desc "Merge and format units-only, system-only, and combined SimpleCov coverage reports" task 'coverage:report' do require 'simplecov' - SimpleCov.at_exit { SimpleCov.result } + SimpleCov.at_exit { } - result = SimpleCov::ResultMerger.merged_result + resultset = SimpleCov::ResultMerger.read_resultset raise "No coverage data found in #{SimpleCov.coverage_path} -- " \ - "run specs:units and specs:system with CEEDLING_TEST_COVERAGE set first" if result.nil? + "run specs:units and specs:system with CEEDLING_TEST_COVERAGE set first" if resultset.empty? + + reports = { + 'units' => resultset.select { |name, _| name == 'units' }, + 'system' => resultset.select { |name, _| name.start_with?('system-') }, + 'combined' => resultset + } + + reports.each do |label, subset| + if subset.empty? + puts "Skipping #{label} report -- no matching coverage data (run with CEEDLING_TEST_COVERAGE=#{label == 'combined' ? 'units/system' : label} first)" + next + end + + command_names, coverage = SimpleCov::ResultMerger.merge_valid_results(subset) + result = SimpleCov::ResultMerger.create_result(command_names, coverage) - SimpleCov::Formatter::HTMLFormatter.new.format(result) - puts "Combined coverage: #{result.covered_percent.round(2)}% " \ - "(#{result.covered_lines}/#{result.total_lines} lines)" + SimpleCov.coverage_dir(File.join('coverage', label)) + SimpleCov::Formatter::HTMLFormatter.new.format(result) + puts "#{label.capitalize} coverage: #{result.covered_percent.round(2)}% " \ + "(#{result.covered_lines}/#{result.total_lines} lines)" + end end # Individual unit specs diff --git a/spec/support/system/simplecov_boot.rb b/spec/support/system/simplecov_boot.rb index 318822f0..f310f9f3 100644 --- a/spec/support/system/simplecov_boot.rb +++ b/spec/support/system/simplecov_boot.rb @@ -33,5 +33,16 @@ # run only needs to merge its own coverage into the shared resultset -- the # formatted HTML report is generated once, explicitly, by `rake coverage:report` # after both suites finish, not redundantly by every child process's own exit. - SimpleCov.at_exit { SimpleCov.result } + # + # `.simplecov`'s own track_files glob (backfilling files this particular process + # never happened to require, so they show as 0% instead of silently vanishing + # from the total) resolves relative to the process's actual working directory at + # the moment coverage is finalized, not SimpleCov.root -- SimpleCov.root only + # governs how already-covered files get reported, not where that glob itself + # looks. Left alone, that resolves against this process's own throwaway deployed + # project directory (still the real CWD here, unrelated to the repo), backfilling + # nothing. Chdir'ing into the repo just for this one call is enough to make the + # glob agree with SimpleCov.root above without disturbing the rest of this + # process's own work, which depends on staying in its own deployed directory. + SimpleCov.at_exit { Dir.chdir(ENV['CEEDLING_TEST_COVERAGE_ROOT']) { SimpleCov.result } } end From c7c17c29ebeff752dce5911372039f6a081f70f3 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 09:46:46 -0400 Subject: [PATCH 4/6] Fix O(n^2) coverage resultset blowup causing the Linux/Ruby-3.3 CI job to run far longer than the other legs Every system-test child process shared one coverage/.resultset.json. SimpleCov's own store_result/merged_result pair reads, re-merges, and rewrites that entire shared file on every single process's exit -- confirmed in the installed simplecov gem source -- so each new process paid a cost proportional to every process that ran before it, and the gap between the coverage job and the other CI legs widened the longer a system-test run went. Each system-test child process now gets its own small resultset file under coverage/raw/ instead of sharing one growing file, so each process's own write stays O(1). coverage:report merges all of those files together at the end via SimpleCov::ResultMerger.merge_results, the same one-file-at-a-time approach SimpleCov's own docs recommend for large multi-process CI setups. Co-Authored-By: Claude Sonnet 5 --- Rakefile | 51 ++++++++++++++++----------- spec/support/system/simplecov_boot.rb | 33 ++++++++++------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/Rakefile b/Rakefile index 56ac1bb1..4f4c592d 100644 --- a/Rakefile +++ b/Rakefile @@ -44,17 +44,24 @@ task 'specs:system:debug' do end # Formats three HTML reports -- units alone, system alone, and both combined -- -# from the one shared coverage/.resultset.json that CEEDLING_TEST_COVERAGE test -# runs accumulate into (see .simplecov and spec/support/system/simplecov_boot.rb). -# Run after both `specs:units` and `specs:system`/`specs:system:debug` have -# completed with that env var set -- this task only reads back what they already -# wrote, it doesn't run any specs itself. Each report is just a different subset -# of the same resultset hash, built with the identical merge machinery -# SimpleCov's own merged_result uses internally, one subset per report: -# units -- the lone entry named exactly "units" (spec_helper.rb) -# system -- every entry named "system--" (simplecov_boot.rb, -# one per system-test child process) -# combined -- every entry, regardless of name +# from the raw coverage data that CEEDLING_TEST_COVERAGE test runs accumulate +# (see .simplecov and spec/support/system/simplecov_boot.rb). Run after both +# `specs:units` and `specs:system`/`specs:system:debug` have completed with that +# env var set -- this task only reads back what they already wrote, it doesn't +# run any specs itself. Sources, one per report: +# units -- coverage/.resultset.json, written directly by the one unit-test +# process (spec_helper.rb) +# system -- coverage/raw/system--/.resultset.json, one small +# file per system-test child process (simplecov_boot.rb). Each +# process gets its own file rather than all sharing one growing +# coverage/.resultset.json -- sharing one file means every single +# process's exit re-reads, re-merges, and rewrites the *entire* +# accumulated file so far, a cost that grows with every process +# that ran before it and compounds across a full system-test run. +# merge_results below reads each small file once, the same +# approach SimpleCov itself recommends for "big CI setups" with +# many result files. +# combined -- the units file plus every system file # # `require 'simplecov'` here briefly starts SimpleCov for this task's own process # too (via .simplecov's own autoload) -- overriding at_exit to a no-op keeps that @@ -66,24 +73,26 @@ task 'coverage:report' do require 'simplecov' SimpleCov.at_exit { } - resultset = SimpleCov::ResultMerger.read_resultset - raise "No coverage data found in #{SimpleCov.coverage_path} -- " \ - "run specs:units and specs:system with CEEDLING_TEST_COVERAGE set first" if resultset.empty? + units_file = File.join('coverage', '.resultset.json') + system_files = Dir[File.join('coverage', 'raw', 'system-*', '.resultset.json')] + + raise "No coverage data found under coverage/ -- " \ + "run specs:units and specs:system with CEEDLING_TEST_COVERAGE set first" \ + if !File.exist?(units_file) && system_files.empty? reports = { - 'units' => resultset.select { |name, _| name == 'units' }, - 'system' => resultset.select { |name, _| name.start_with?('system-') }, - 'combined' => resultset + 'units' => File.exist?(units_file) ? [units_file] : [], + 'system' => system_files, + 'combined' => (File.exist?(units_file) ? [units_file] : []) + system_files } - reports.each do |label, subset| - if subset.empty? + reports.each do |label, files| + if files.empty? puts "Skipping #{label} report -- no matching coverage data (run with CEEDLING_TEST_COVERAGE=#{label == 'combined' ? 'units/system' : label} first)" next end - command_names, coverage = SimpleCov::ResultMerger.merge_valid_results(subset) - result = SimpleCov::ResultMerger.create_result(command_names, coverage) + result = SimpleCov::ResultMerger.merge_results(*files) SimpleCov.coverage_dir(File.join('coverage', label)) SimpleCov::Formatter::HTMLFormatter.new.format(result) diff --git a/spec/support/system/simplecov_boot.rb b/spec/support/system/simplecov_boot.rb index f310f9f3..1cf8040a 100644 --- a/spec/support/system/simplecov_boot.rb +++ b/spec/support/system/simplecov_boot.rb @@ -21,19 +21,28 @@ SimpleCov.root(ENV['CEEDLING_TEST_COVERAGE_ROOT']) load File.join(ENV['CEEDLING_TEST_COVERAGE_ROOT'], '.simplecov') - # SimpleCov's resultset stores one entry per command_name, and a new result under - # an already-used name *replaces* rather than accumulates with the previous one -- - # a shared name across the many sequential child processes a full system-test run - # spawns would silently keep only the last one's coverage. PID alone isn't enough - # (a long run can cycle through enough child processes, each itself spawning - # further subprocesses for compiles/links/etc., to plausibly repeat a PID); - # PID plus a microsecond timestamp is. - SimpleCov.command_name "system-#{Process.pid}-#{Time.now.strftime('%Y%m%d%H%M%S%6N')}" + # A unique name per process, still useful for identifying which process a given + # raw result file came from. PID alone isn't enough (a long run can cycle through + # enough child processes, each itself spawning further subprocesses for + # compiles/links/etc., to plausibly repeat a PID); PID plus a microsecond + # timestamp is. + name = "system-#{Process.pid}-#{Time.now.strftime('%Y%m%d%H%M%S%6N')}" + SimpleCov.command_name name + # Each of potentially hundreds of these child processes across a full system-test - # run only needs to merge its own coverage into the shared resultset -- the - # formatted HTML report is generated once, explicitly, by `rake coverage:report` - # after both suites finish, not redundantly by every child process's own exit. - # + # run gets its own raw resultset file under coverage/raw/ rather than sharing one + # -- `rake coverage:report` merges them all afterward. Sharing a single + # coverage/.resultset.json here would mean every process's exit re-reads, + # re-merges, and rewrites the *entire* accumulated file (SimpleCov::ResultMerger + # locks it, reads it whole, and rewrites it whole on every store, then reads and + # merges it whole *again* on the same call for a return value nothing here even + # uses) -- cost growing with every process that ran before it, compounding into a + # full system-test run taking dramatically longer than it should the more + # processes pile up. A dedicated directory per process makes each one's own + # store trivial (a brand-new, single-entry file), turning that growing cost back + # into a flat one. + SimpleCov.coverage_dir(File.join('coverage', 'raw', name)) + # `.simplecov`'s own track_files glob (backfilling files this particular process # never happened to require, so they show as 0% instead of silently vanishing # from the total) resolves relative to the process's actual working directory at From 3bf4f3f6603acf45e9f5e058fcd5453cef1c7035 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 10:26:53 -0400 Subject: [PATCH 5/6] Fix coverage:report crash from SimpleCov's default merge_timeout dropping stale results coverage:report runs after both specs:units and specs:system finish, reading back result files those already-finished steps wrote. SimpleCov's default merge_results call filters out any file older than its 10-minute merge_timeout -- meant for live in-process merges, not this after-the-fact read-back -- so a system-test suite running longer than 10 minutes left the early units file past the cutoff by the time this task ran, dropping it entirely and crashing HTMLFormatter#format on a nil result. Passing ignore_timeout: true matches what SimpleCov.collate's own API defaults to for this same after-the-fact-merge use case. Co-Authored-By: Claude Sonnet 5 --- Rakefile | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 4f4c592d..e5b70411 100644 --- a/Rakefile +++ b/Rakefile @@ -92,7 +92,17 @@ task 'coverage:report' do next end - result = SimpleCov::ResultMerger.merge_results(*files) + # ignore_timeout: SimpleCov's default 10-minute merge_timeout exists to keep + # live, in-process merges from combining coverage across unrelated runs -- it + # doesn't apply here, where every file being merged is one CI step's already- + # finished output, deliberately read back after the fact (the same reasoning + # SimpleCov.collate's own API uses this same override for). Without it, a + # system-test suite that legitimately runs longer than 10 minutes leaves the + # early "units" file (and any early "system" files) older than the cutoff by + # the time this task runs last, so they'd get silently dropped -- for units + # alone (only ever one file) that's a full wipeout, nil coverage, and a crash + # in HTMLFormatter#format on a nil result. + result = SimpleCov::ResultMerger.merge_results(*files, ignore_timeout: true) SimpleCov.coverage_dir(File.join('coverage', label)) SimpleCov::Formatter::HTMLFormatter.new.format(result) From aebdf3f68884364a8e1c020c71e3ede299d702f2 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 21 Aug 2026 12:53:22 -0400 Subject: [PATCH 6/6] Fix three CI workflow warnings: Node 20 deprecation, Homebrew tap trust, cppcheck cache permissions Node 20 deprecation: actions/checkout@v4, actions/cache@v4, and actions/upload-artifact@v4 (plus actions/download-artifact@v4, the same family though not itself named in the warning) still resolve to releases whose action.yml declares Node 20, which GitHub Actions runners now force onto Node 24 anyway. Bumped every occurrence to each action's latest major (checkout v7, cache v6, upload-artifact v7, download-artifact v8), all of which declare Node 24 directly -- confirmed against each action's actual released action.yml, not assumed. Homebrew aws/tap trust warning (macOS job): aws/tap is pre-tapped on GitHub's hosted macOS runner image and unrelated to anything this repo uses; newer Homebrew warns about every untrusted tap present on any brew install, not just ones involved in that install. Untapped before installing cppcheck rather than trusting it or disabling tap-trust checks outright. cppcheck cache restore failure (Linux job): actions/cache's restore step runs unprivileged and needs write access to recreate cached paths on a fresh runner VM. /usr/local/bin is writable on this runner image but /usr/local/share is not, so extracting the cached cppcheck subtree there failed with "Cannot mkdir: Permission denied" on every cache hit. Added a step to pre-create and chown the destination before the cache-restore step runs, every run, since each job starts on a fresh ephemeral VM. Co-Authored-By: Claude Sonnet 5 --- .github/actions/run-system-tests/action.yml | 7 ++- .github/actions/setup/action.yml | 5 +- .github/workflows/_generate-docs.yml | 10 ++-- .github/workflows/_publish-gem.yml | 10 ++-- .github/workflows/ci.yml | 55 +++++++++++++++------ 5 files changed, 63 insertions(+), 24 deletions(-) diff --git a/.github/actions/run-system-tests/action.yml b/.github/actions/run-system-tests/action.yml index 491a0fb2..b74f682b 100644 --- a/.github/actions/run-system-tests/action.yml +++ b/.github/actions/run-system-tests/action.yml @@ -32,8 +32,11 @@ runs: # Download HTML docs bundle built by generate-docs job. # Required by system tests that exercise Ceedling documentation features. # Unit tests (run as a separate job step before this action) do not need the docs bundle. + # Pinned to the latest major (not v4) for Node 24 runner support -- + # GitHub Actions runners now force Node 24 regardless, and v4 still + # targets the deprecated Node 20. Same reasoning below for upload-artifact. - name: Download HTML Documentation Bundle - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: gem-docs-site-local path: site-local/ @@ -53,7 +56,7 @@ runs: # before uploading — the systests/ directory contains only failure data. - name: Upload system test failure artifacts if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: systest-failures-${{ runner.os }}-ruby-${{ inputs.ruby }} path: systests/ diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml index 28933bfb..5cfcd82d 100644 --- a/.github/actions/setup/action.yml +++ b/.github/actions/setup/action.yml @@ -19,7 +19,10 @@ inputs: runs: using: "composite" steps: - - uses: actions/cache@v4 + # Pinned to the latest major (not v4) for Node 24 runner support -- + # GitHub Actions runners now force Node 24 regardless, and v4 still + # targets the deprecated Node 20. + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-${{ inputs.ruby }}-${{ hashFiles('**/Gemfile.lock') }} diff --git a/.github/workflows/_generate-docs.yml b/.github/workflows/_generate-docs.yml index 7c208585..e921b63f 100644 --- a/.github/workflows/_generate-docs.yml +++ b/.github/workflows/_generate-docs.yml @@ -33,7 +33,11 @@ jobs: steps: # Use a cache for our tools to speed up builds # No matrix here; Ruby version is hardcoded to match the cache key format used by test jobs - - uses: actions/cache@v4 + # + # cache/checkout/upload-artifact below are pinned to their latest majors (not v4) for + # Node 24 runner support -- GitHub Actions runners now force Node 24 regardless, and v4 + # still targets the deprecated Node 20. + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-3.3-${{ hashFiles('**/Gemfile.lock') }} @@ -41,7 +45,7 @@ jobs: bundle-use-ruby-${{ runner.os }}-3.3- - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -68,7 +72,7 @@ jobs: rake docs:build:local --trace - name: Upload HTML Documentation Bundle - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: gem-docs-site-local path: site-local/ diff --git a/.github/workflows/_publish-gem.yml b/.github/workflows/_publish-gem.yml index 82a843c3..d4c32461 100644 --- a/.github/workflows/_publish-gem.yml +++ b/.github/workflows/_publish-gem.yml @@ -54,7 +54,11 @@ jobs: steps: # Use a cache for our tools to speed up builds # No matrix here; Ruby version is hardcoded to match the cache key format used by test jobs - - uses: actions/cache@v4 + # + # cache/checkout/download-artifact below are pinned to their latest majors (not v4) for + # Node 24 runner support -- GitHub Actions runners now force Node 24 regardless, and v4 + # still targets the deprecated Node 20. + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-3.3-${{ hashFiles('**/Gemfile.lock') }} @@ -62,7 +66,7 @@ jobs: bundle-use-ruby-${{ runner.os }}-3.3- - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -122,7 +126,7 @@ jobs: # Download HTML docs bundle built by _generate-docs.yml # Artifacts are shared within the same workflow run by run_id - name: Download HTML Documentation Bundle - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: gem-docs-site-local path: site-local/ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff4280d2..c6355ec8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,8 +83,12 @@ jobs: ruby: ['3.0', '3.1', '3.2', '3.3', '3.4', '3.5'] steps: # Checks out repository under $GITHUB_WORKSPACE — must come first to enable local action access + # + # checkout/cache/upload-artifact/download-artifact throughout this workflow are pinned + # to their latest majors (not v4) for Node 24 runner support -- GitHub Actions runners + # now force Node 24 regardless, and v4 still targets the deprecated Node 20. - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -140,9 +144,23 @@ jobs: # Build cppcheck from source at a pinned version so all plugin options # (including SARIF, which requires 2.16+) are available for testing. # The installed binary is cached by version to avoid rebuilding on every run. + # + # actions/cache's restore step runs unprivileged and needs write access to recreate + # cached paths on a fresh runner VM. /usr/local/bin is already writable on this runner + # image, but /usr/local/share is not -- extracting the cached cppcheck subtree into it + # fails with "Cannot mkdir: Permission denied" otherwise. This has to run before every + # restore, on every run, since each job starts on a fresh ephemeral VM with the base + # image's permissions reset -- nothing from a prior run's chown carries forward. Saving + # the cache itself only ever needed read access (root-owned files are world-readable), + # so this doesn't change how or where cppcheck ultimately gets installed below. + - name: Ensure cppcheck Cache Destination Is Writable + run: | + sudo mkdir -p /usr/local/share/cppcheck + sudo chown -R "$(id -u):$(id -g)" /usr/local/share/cppcheck + - name: Cache cppcheck binary and data files id: cache-cppcheck - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | /usr/local/bin/cppcheck @@ -194,7 +212,7 @@ jobs: - name: Upload Combined Coverage Report if: matrix.ruby == '3.3' - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: coverage-report path: coverage/ @@ -218,7 +236,7 @@ jobs: steps: # Checks out repository under $GITHUB_WORKSPACE — must come first to enable local action access - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -300,7 +318,7 @@ jobs: steps: # Checks out repository under $GITHUB_WORKSPACE — must come first to enable local action access - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -333,8 +351,15 @@ jobs: # valgrind is unavailable on macOS — valgrind specs already skip via @valgrind_available guard # Install cppcheck via Homebrew + # + # aws/tap is pre-tapped on GitHub's hosted macOS runner image (unrelated to anything + # this repo uses) -- newer Homebrew warns about every untrusted tap present on any + # `brew install`, not just ones involved in that install, so it's untapped here rather + # than trusted or having tap-trust checks disabled outright. `|| true` in case a future + # runner image doesn't have it pre-tapped. - name: "Install cppcheck for Tests of Ceedling Plugin: Cppcheck" run: | + brew untap aws/tap || true brew install cppcheck # --break-system-packages required on macOS 14+ (PEP 668 prevents pip from installing @@ -374,7 +399,7 @@ jobs: name: "Linux Test Suite (ja_JP.UTF-8 Locale)" runs-on: ubuntu-latest steps: - - uses: actions/cache@v4 + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-3.3-${{ hashFiles('**/Gemfile.lock') }} @@ -382,7 +407,7 @@ jobs: bundle-use-ruby-${{ runner.os }}-3.3- - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -427,7 +452,7 @@ jobs: # Upload system test failure logs and temp project directories on test job failure - name: Upload system test failure artifacts if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: systest-failures-locale-ruby-3.3 path: systests/ @@ -445,7 +470,7 @@ jobs: name: "Linux Encoding Stress Test (C/POSIX)" runs-on: ubuntu-latest steps: - - uses: actions/cache@v4 + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-3.3-${{ hashFiles('**/Gemfile.lock') }} @@ -453,7 +478,7 @@ jobs: bundle-use-ruby-${{ runner.os }}-3.3- - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -499,7 +524,7 @@ jobs: # Upload system test failure logs and temp project directories on test job failure - name: Upload system test failure artifacts if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: systest-failures-encoding-stress-ruby-3.3 path: systests/ @@ -522,7 +547,7 @@ jobs: steps: # Use a cache for our tools to speed up builds # No matrix here; Ruby version is hardcoded to match the cache key format used by test jobs - - uses: actions/cache@v4 + - uses: actions/cache@v6 with: path: vendor/bundle key: bundle-use-ruby-${{ runner.os }}-3.3-${{ hashFiles('**/Gemfile.lock') }} @@ -530,7 +555,7 @@ jobs: bundle-use-ruby-${{ runner.os }}-3.3- - name: Checkout Latest Repo - uses: actions/checkout@v4 + uses: actions/checkout@v7 with: submodules: recursive @@ -545,7 +570,7 @@ jobs: # Download HTML docs bundle built by generate-docs job to validate docs ingestion in gem build - name: Download HTML Documentation Bundle - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: gem-docs-site-local path: site-local/ @@ -556,7 +581,7 @@ jobs: # Upload the built gem as a workflow artifact (downloadable from the Actions UI) - name: Upload Built Gem Artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: ceedling-gem path: ceedling-*.gem