Method coverage conflates methods with the same name

[G-Rath]

I've been doing my periodic check-in w/ turbo_tests2 and simplecov v1 rcs - really loving the progress I'm seeing!

I have found an odd behaviour with method coverage not reporting some methods as covered when used with turbo_tests2, but they are with rspec - while I'm not sure in what gem that issue lies, in trying to reproduce it in my reproduction repository I believe I have found a bug in simplecov that might be the actual issue.

In a nutshell it seems that something in the method coverage code is mixing up methods with the same name i.e. G-Rath/simplecov-wat@cc71b05 adds methods add3 and substract3 in a new file (math/math3.rb) which are fully covered by spec/math/math3_spec.rb, while also adding a different version of add3 to math/add.rb which is not covered.

Running with turbo_tests2 reports both line and method coverage as below 100%:

Coverage report generated for rspec-1, rspec-2, rspec-3, rspec-4 to coverage/index.html
Line coverage: 25 / 26 (96.15%)
Branch coverage: 24 / 24 (100.00%)
Method coverage: 5 / 6 (83.33%)
Line coverage (96.15%) is below the expected minimum coverage (100.00%).
  Lowest-coverage files (line):
     87.50%  math/add.rb
    100.00%  math/divide.rb
    100.00%  math/math3.rb
    100.00%  math/subtract.rb
Method coverage (83.33%) is below the expected minimum coverage (100.00%).
  Lowest-coverage files (method):
     50.00%  math/add.rb
    100.00%  math/divide.rb
    100.00%  math/math3.rb
    100.00%  math/subtract.rb
SimpleCov failed with exit 2 due to a coverage related error

However, running with rspec reports only line coverage as below 100%, and that there are only 5 methods instead of 6:

Coverage report generated for RSpec to coverage/index.html
Line coverage: 25 / 26 (96.15%)
Branch coverage: 24 / 24 (100.00%)
Method coverage: 5 / 5 (100.00%)
Line coverage (96.15%) is below the expected minimum coverage (100.00%).
  Lowest-coverage files (line):
     87.50%  math/add.rb
    100.00%  math/divide.rb
    100.00%  math/math3.rb
    100.00%  math/subtract.rb
SimpleCov failed with exit 2 due to a coverage related error

[sferik]

Thanks, this a great bug report. After digging in, I believe the root cause is in Ruby's Coverage library rather than in SimpleCov.

Unlike line and branch coverage, method coverage has no per-file storage. When Coverage.result is called, CRuby reconstructs it by walking the live object heap for method entries (method_coverage_i via rb_objspace_each_objects in ext/coverage/coverage.c). Your second def add3 makes the first one garbage, and it was never called, so it has no execution counter either. Once GC collects the old method entry, the heap walk cannot find it and the definition is silently forgotten. It does not show up as uncovered, it just vanishes from the results.

This reproduces with Coverage alone, in a single file:

# target.rb
def foo
  1
end

def foo
  2
end

foo

# run.rb
require "coverage"
Coverage.start(methods: true)
require_relative "./target"
GC.start if ENV["FORCE_GC"]
pp Coverage.result.find { |f, _| f.end_with?("target.rb") }.last[:methods]

$ ruby run.rb
{[Object, :foo, 5, 0, 7, 3] => 1, [Object, :foo, 1, 0, 3, 3] => 0}

$ FORCE_GC=1 ruby run.rb
{[Object, :foo, 5, 0, 7, 3] => 1}

Whether the shadowed def survives depends only on whether GC has run before Coverage.result. That explains why a real suite (plenty of allocation after your app files load) triggers it consistently while a tiny script is flaky, and it explains the runner difference: with turbo_tests2, each worker process loads only its own files, so no redefinition happens within any single process and both add3 entries survive to be merged into the 6 methods you saw. With rspec, everything loads in one process, the first add3 is dropped, and SimpleCov only ever sees 5 methods, so it reports 5/5 instead of 5/6.

I verified this on every Ruby from 3.2.11 through 4.0.5.

So I think the next step is a bug report against the Coverage standard library, since it is discarding a tracked def on redefinition. Let me know if you're willing to file it or if you'd prefer me to.

On the SimpleCov side, there may be something we can do by cross-checking loaded files against our Prism-based static method extractor to restore methods Coverage dropped, but that is a larger change and I would rather not paper over a Ruby bug until we understand whether it is getting fixed upstream.

Thanks again! This was a great catch and a genuinely interesting one to chase down.

[G-Rath]

woah, that is an interesting bug! if you're happy to file the issue I think I'd prefer that just because you know so much more about it 😅

[sferik]

Filed upstream: https://bugs.ruby-lang.org/issues/22179

I’ll leave this issue open to track whatever happens upstream and to consider whether SimpleCov should compensate in the meantime.

[sferik]

Update on that issue from @mame:

Thanks for the report and the very precise analysis. I think your diagnosis is exactly right.

After discussing with @ko1, we decided to fix it as follows:

1. Record the set of methods when a method is added.
2. At Coverage.result time, stop using each_object, and report method coverage by aggregating that set together with the counter.

I've prepared a draft PR. However, @ko1 is planning to implement Ractor-local GC soon, and this change could interfere with it, so he asked to hold off on merging for about a month.

ruby/ruby#17729

[sferik]

A minimal reproduction already exists above that triggers the bug with the Coverage library. That is the root cause and a fix is already underway so a SimpleCov fixture keyed to turbo_tests2 would not test the right thing. The runner difference was only a symptom of GC timing across processes.