Project

General

Profile

Actions

Bug #22203

open

Coverage (eval: true): branch entries for re-eval'd code are nondeterministically duplicated

Bug #22203: Coverage (eval: true): branch entries for re-eval'd code are nondeterministically duplicated

Added by sferik (Erik Berlin) 5 days ago. Updated 1 day ago.

Status:
Open
Target version:
-
[ruby-core:126115]

Description

When the same code is eval'd more than once against the same file path under Coverage.start(branches: true, eval: true), the branch section of Coverage.result sometimes contains one condition entry with accumulated arm counts, and sometimes contains multiple condition entries at identical source coordinates (differing only in their id element), each carrying only the executions of one compile.

Which of the two shapes you get varies from run to run of the same program and is influenced by GC. On Ruby 3.2 through 3.4 the entries were almost always merged. On Ruby 4.0 they are almost always duplicated, so programs that rely on the long-standing de facto behavior changed silently.

This is related to but distinct from #19857, which asked for line counts to accumulate across evals and was rejected as a design decision. My concern is that the shape of the branch result is nondeterministic and that lines and branches disagree with each other in a single run.

Reproduction

require "coverage"

Coverage.start(lines: true, branches: true, eval: true)

src = "if $flag\n  :a\nelse\n  :b\nend\n"

$flag = true;  eval(src, binding, "fake.rb", 1)
$flag = false; eval(src, binding, "fake.rb", 1)

result = Coverage.result["fake.rb"]
puts "conditions: #{result[:branches].size}"
result[:branches].each { |cond, arms| puts "  #{cond.inspect} => #{arms.values.inspect}" }
puts "lines: #{result[:lines].inspect}"

Adding GC.start between the two evals makes the duplication reproduce on every run on 4.0.6.

I'd expect that either the second compile reuses the existing branch entries for that path, so there is one condition whose arm counts cover both evals (the behavior Ruby 3.2 through 3.4 exhibit) or every compile appends fresh entries, always.

I can work with either semantics, but can’t rely on a structure that depends on GC timing. In the current state, SimpleCov and similar tools can’t distinguish one branch, observed twice from two branches, each half-observed.

Here are examples of the two different shapes I'm seeing across runs of identical code:

Merged (one condition, arms accumulated across both evals):

conditions: 1
  [:if, 0, 1, 0, 5, 3] => [1, 1]

Duplicated (two conditions at identical coordinates, complementary counts):

conditions: 2
  [:if, 0, 1, 0, 5, 3] => [1, 0]
  [:if, 3, 1, 0, 5, 3] => [0, 1]

Observed rates for 10 runs each, "gc" meaning GC.start inserted between the two evals:

ruby 3.2.11:  plain 0/10 duplicated,  gc 1/10 duplicated
ruby 3.3.12:  plain 0/10 duplicated,  gc 0/10 duplicated
ruby 3.4.10:  plain 0/10 duplicated,  gc 1/10 duplicated
ruby 4.0.6:   plain 9/10 duplicated,  gc 10/10 duplicated

So the failure to reuse existing entries has existed at low frequency since eval coverage was introduced (it can be provoked with GC pressure on 3.2 through 3.4), and Ruby 4.0 inverted the common case.

Additionally, lines and branches disagree within a single run. Eval'ing the same code twice down the same arm gives:

lines:    [1, 1, nil, 0, nil]   # line counts reflect only the last compile (per #19857 design)
branches: [:then] => 2          # branch counts accumulated across both compiles

The branch counter says the arm ran twice while line coverage says its line ran once. Whichever semantics is intended, the two criteria should agree.

Updated by sophiathedev (Nguyen Thang) 1 day ago · Edited Actions #1 [ruby-core:126123]

Hi @sferik (Erik Berlin),

I've opened a Pull Request to fix this issue by using a stable, location-based key for branch coverage instead of dynamic AST addresses.

Could you and the core team please take a look? Any feedback would be greatly appreciated!

https://github.com/ruby/ruby/pull/17979

P/S: I've updated a notes for this comment, please help me if everyone interested. https://github.com/ruby/ruby/pull/17979#issuecomment-5020693137

Updated by nobu (Nobuyoshi Nakada) 1 day ago Actions #2

  • Assignee set to mame (Yusuke Endoh)
Actions

Also available in: PDF Atom