Coverage doesn't cover begin

[okuramasafumi]

Environment Information

Provide at least:

• JRuby version: jruby-9.4.6.0
• Operating system and platform: ubuntu-latest and macOs on GitHub Actions

Expected Behavior

• Coverage library will cover begin clause

Actual Behavior

• Coverage library doesn't cover begin clause
• See https://app.codecov.io/gh/okuramasafumi/alba/commit/5abda13479a0d3ac25758c8b4aa1f4848828286c/blob/lib/alba.rb

Additional info

I noticed that after adding jruby to GitHub Actions matrix, the coverage percentage slightly get down. It seems coverage library doesn't cover begin clause.
CRuby does cover, so I believe JRuby could behave the same.

[enebo]

This may be difficult to solve as we transform syntax to instructions and if we see no instructions executing logic we remove contiguous line instructions since we figured nothing could execute on that line. Trace instrs perhaps we can change though.

[headius]

@enebo Yeah I suppose we'd have to add instructions indicating begin entry among other things.

@okuramasafumi FYI we generally don't trace everything unless you pass --debug or -Xdebug.fullTrace to JRuby.

[headius]

Oops, clicked wrong button...

[okuramasafumi]

I'm just curious, is adding coverage done with this line? https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/interpreter/InterpreterEngine.java#L418

[okuramasafumi]

After some research, it seems to me that begin is just a Label in JRuby.

jruby/core/src/main/java/org/jruby/ir/builder/IRBuilder.java

Line 2528 in cd2d0a0

Label rBeginLabel = getNewLabel();

And we don't add coverage for a label?

jruby/core/src/main/java/org/jruby/ir/interpreter/InterpreterEngine.java

Lines 373 to 374 in cd2d0a0

	case LABEL:
	break;

If I'm correct, we can try to add coverage for labels. Probably I'm wrong so I appreciate any help.

[okuramasafumi]

@Anebo @headius

Here's a minimal repro code for the difference in coverage.

# cov_result.rb
require 'coverage'
Coverage.start(lines: true)
require_relative 'cov_body'
p Coverage.result

# cov_body.rb
begin
  a = 42
end

The results:

ruby --version
# jruby 9.4.6.0 (3.1.4) 2024-02-20 576fab2c51 OpenJDK 64-Bit Server VM 21.0.2 on 21.0.2 +jit [arm64-darwin]

ruby cov_result.rb
# {"/Users/okuramasafumi/src/Playground/Ruby/cov_body.rb"=>{:lines=>[0, 1, nil]}}

ruby --version
# ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [arm64-darwin23]

ruby cov_result.rb
# {"/Users/okuramasafumi/src/Playground/Ruby/cov_body.rb"=>{:lines=>[nil, 1, nil]}}

[enebo]

@okuramasafumi begin does happen to have a start label but many other things do as well so I am unsure if this will not add more coverage lines than CRuby. That fix is interesting but I think we just need to add an explicit line for begin in IRBuilder.

Note: As an aside, labels only get seen during interpretation. Once prepared for compilation they are used for constructing our graph. I do think your solution could be used even for compilation but only if we added a line number instr at that point when constructing a graph. Still IRBuilder may be the simpler fix since we can just target begin.

[enebo]

I just realized I misunderstood this issue. CRuby does not cover the line as much as mark it nil which does not count it as part of coverage statistics. We add it as a line which needs coverage and we do not emit a line instr so we never "cover" that line. We should NOT be marking that line as needing coverage. This problem is likely in the parser itself.

[okuramasafumi]

@enebo Thank you for the fix! I think I'd like to add a test case for this issue so that we can be sure it's fixed. What do you think?

[enebo]

@okuramasafumi yeah that would be great! It can be a regression test for us or even better would be ruby/spec contribution under ruby/library/coverage. That is up to you though.

[okuramasafumi]

@enebo Ah yes ruby/spec should include that behavior. I'm not familiar with JRuby's testing strategy, but should I add test/spec to both test directory and ruby/spec?

[enebo]

@okuramasafumi ruby/spec will occasionally get synced with our project tree so submitting there will get copied into us at some future point (usuallly about a month or so).

[enebo]

@okuramasafumi but I should have completely answered that by saying you should only contribute to ruby/spec if you are ok with adding a spec there.