fix: merge coverage records collected across Ruby 3 and Ruby 4 - #7
Merged
Conversation
| item.nil? ? nil : item + other.execution_count[index] | ||
| other_item = other.execution_count[index] | ||
| if item.nil? && other_item.nil? then nil | ||
| elsif item.nil? || other_item.nil? then other_item |
There was a problem hiding this comment.
question: Should we merge the data using the same logic that last_executed_at is using?
For example, if other_item is nil but item is 1 (somehow the unused code is now used), should it be item instead of other_item?
Collaborator
Author
There was a problem hiding this comment.
Thanks for the comment, I've made the change 👍
choznerol
reviewed
Jul 15, 2026
| record | ||
| else | ||
| record + found | ||
| found + record |
Contributor
There was a problem hiding this comment.
thought(non-blocking): Unrelated to this PR, but I found it challenging to keep track of existing v.s. current v.s. record v.s. found at the same time.
WDYT about:
-existing
+existing_records
-found
+existing_record
-current
+current_records
-record
+current_recordThis way, instead of:
- record + found
+ found + recordthe PR diff would be:
- current_record + existing_record
+ existing_record + current_record
Collaborator
Author
There was a problem hiding this comment.
I've renamed variables. Let me know if it's clearer to you 🙏
Ruby 4 changed which lines `Coverage` measures: lines that Ruby 3 reported as nil (not measurable) can now carry an execution count, e.g. a literal `nil` statement. To reproduce: ``` λ cat main.rb def sum(a, b) a + b nil end sum(1, 2) λ docker run --rm -v .:/app -w /app -e RUBYLIB=lib ruby:3.3.10-trixie bash -c 'ruby -r cover_rage main.rb && bin/cover_rage -f json > cover_rage.json && paste <(ruby -r json -e "puts JSON.parse(ARGF.read)[0][%q(execution_count)].map(&:inspect)" cover_rage.json) <(ruby -r json -e "puts JSON.parse(ARGF.read)[0][%q(source)]" cover_rage.json)' sh 11 def sum(a, b) 11 a + b nil nil nil end nil 11 sum(1, 2) λ docker run --rm -v .:/app -w /app -e RUBYLIB=lib ruby:4.0.5-trixie bash -c 'ruby -r cover_rage main.rb && bin/cover_rage -f json > cover_rage.json && paste <(ruby -r json -e "puts JSON.parse(ARGF.read)[0][%q(execution_count)].map(&:inspect)" cover_rage.json) <(ruby -r json -e "puts JSON.parse(ARGF.read)[0][%q(source)]" cover_rage.json)' sh 12 def sum(a, b) 12 a + b 1 nil nil end nil 12 sum(1, 2) ``` As a result, records collected under different Ruby versions disagree on which entries of `execution_count` are nil, and merging them raised TypeError (`Integer + nil`). Resolve the mismatch in favor of the incoming record: when exactly one side is nil, `Record#+` takes the incoming record's entry — for both `execution_count` and `last_executed_at` — and the merge order in `Record.merge` is flipped so the freshly collected record wins (`existing_record + current_record`). Also rename variables in `Record.merge` for clarity: `existing` / `current` / `found` / `record` become `existing_records` / `current_records` / `existing_record` / `current_record`.
tonytonyjan
force-pushed
the
fix/ruby-4-coverage-merge
branch
from
July 16, 2026 03:48
02dea16 to
e176a6b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ruby 4 changed which lines
Coveragemeasures: lines that Ruby 3 reported asnil(not measurable) can now carry an execution count, e.g. a literalnilstatement. As a result, records collected under different Ruby versions disagree on which entries ofexecution_countarenil, and merging them raisedTypeError(Integer + nil).Reproduction
Ruby 3.3.10 reports the
nilline as not measurable:Ruby 4.0.5 counts it:
Fix
Resolve the mismatch in favor of the incoming record:
Record#+now falls back toother's entry when exactly one side isnil.Record.mergeis flipped fromrecord + foundtofound + recordso the freshly collected record wins.Tests
Three tests added to
test/test_record.rb, mutation-checked so that reverting either half of the fix independently makes them fail:test_sum_uses_other_when_exactly_one_execution_count_is_nil— pins thenil-fallback inRecord#+(raisedTypeErrorbefore the fix).test_merge_when_current_record_measures_more_lines— a stored Ruby-3-shaped record merged with a fresh Ruby-4-shaped one keeps the new line's count.test_merge_when_current_record_measures_fewer_lines— the reverse direction, pinning thefound + recordorder flip.Full suite passes:
TEST_REDIS_URL=redis://localhost:6379 bundle exec rake test→ 28 runs, 33 assertions, 0 failures, 0 errors.🤖 Generated with Claude Code