Skip to content

Add merge finalization ownership config#1216

Merged
sferik merged 5 commits into
simplecov-ruby:mainfrom
kettle-dev:fix/finalize-merge-ownership
Jun 26, 2026
Merged

Add merge finalization ownership config#1216
sferik merged 5 commits into
simplecov-ruby:mainfrom
kettle-dev:fix/finalize-merge-ownership

Conversation

@pboling

@pboling pboling commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds SimpleCov.finalize_merge so parallel workers can store mergeable resultsets without also owning final report finalization.

Fixes #1215.

Root cause

use_merging true currently covers two responsibilities: storing resultsets and finalizing the merged report. In setups like the repro, each worker writes to an isolated coverage_dir and a later SimpleCov.collate cleanup step owns the final report. The selected final worker was polling only its own shard .resultset.json, so it waited for sibling workers that were intentionally writing different files.

Changes

  • Add SimpleCov.finalize_merge with explicit true/false support.
  • Keep the default true, but infer false with a configuration warning for recognized multi-worker parallel runs with merging enabled and an explicit custom coverage destination.
  • Gate worker wait/merge/format/threshold/.last_run.json finalization when finalize_merge false.
  • Preserve SimpleCov.collate as a finalizer that writes .last_run.json for the collated result.
  • Document merge finalization ownership using the v1 coverage configuration API and add changelog coverage.

Validation

  • Running RuboCop
Inspecting 196 files
....................................................................................................................................................................................................

196 files inspected, no offenses detected
  • rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
Randomized with seed 20626
......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................*....................*.................................................................................................................................................

Non-app warnings written to tmp/warnings.txt

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) SimpleCov::StaticCoverageExtractor.real_source_positions when Prism is not available returns nil so the eval_generated filter is a no-op
     # Prism is available; the no-Prism path can't be exercised on this Ruby
     # ./spec/static_coverage_extractor_spec.rb:190

  2) SimpleCov::StaticCoverageExtractor.call when Prism is not available returns nil so callers fall back to empty hashes
     # Prism is available; the no-Prism path can't be exercised on this Ruby
     # ./spec/static_coverage_extractor_spec.rb:18

Finished in 11.75 seconds (files took 0.54538 seconds to load)
1149 examples, 0 failures, 2 pending

Randomized with seed 20626

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 26, 2026 01:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an explicit configuration switch to decouple “storing mergeable worker resultsets” from “owning final merge/report finalization,” addressing the parallel-worker hang described in #1215 when workers write to sharded coverage destinations and a separate SimpleCov.collate step performs the final merge/report.

Changes:

  • Adds SimpleCov.finalize_merge (defaulting to true) and merge_finalization_owner? to control whether a process performs wait/merge/format/threshold checks/.last_run.json finalization.
  • Infers finalize_merge false (with a one-time warning) for recognized multi-worker parallel runs with merging enabled and an explicitly customized coverage destination.
  • Updates result processing and exit handling so SimpleCov.collate always finalizes correctly, while non-owning workers only store their shard resultsets and exit without waiting or formatting; adds/updates specs and documentation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/simplecov_spec.rb Adds coverage for merge_finalization_owner? gating, collate finalization behavior, and worker behavior when finalization is disabled.
spec/configuration_spec.rb Adds tests for finalize_merge default/explicit values and inference + warning behavior.
README.md Documents “merge finalization ownership” and recommended worker/collate setup.
lib/simplecov/result_processing.rb Implements collate “finalizer context” flag and gates worker wait/merge on finalize_merge?.
lib/simplecov/exit_handling.rb Updates readiness-to-process logic to respect merge-finalization ownership and collate flows.
lib/simplecov/configuration/merging.rb Adds finalize_merge, inference logic, warning emission, and merge_finalization_owner?.
lib/simplecov/configuration.rb Prevents formatting in at_exit when the process does not own merge finalization; tracks explicit coverage_dir changes for inference.
CHANGELOG.md Notes the new option and the behavior change/fix for sharded parallel workers + external collation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pboling and others added 3 commits June 25, 2026 20:13
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@sferik sferik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit finalize_merge API is clean and I’m 👍 on that part. I have some reservations about the auto-inference, but I’m willing to put it out in an RC and see what comes back.

My only nit is that the call to merging_enabled_for_inference? could be replaced with merging, at which point I believe merging_enabled_for_inference? could be deleted since it’s no longer called anywhere.

Comment thread lib/simplecov/configuration/merging.rb Outdated
@sferik sferik merged commit 01a034f into simplecov-ruby:main Jun 26, 2026
10 checks passed
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.

Parallel workers with sharded coverage_dir wait before external collate

3 participants