Coverage not reporting for methods returning nil
Problem
Given a method that just returns nil, rspec:coverage returns no data for that line. This in turn cause rspec:undercoverage to fail.
179: def summarize_new_merge_request_disabled_reason(_merge_request) hits: n/a
180: nil hits: n/a
181: end hits: n/a
-- example failure at https://gitlab.com/gitlab-org/gitlab/-/jobs/11791490163
Note that this is not the same as hits: 0.
Examining gitlab.lcov from the rspec:coverage job, we see lines 180 not having any coverage data:
SF:./app/helpers/merge_requests_helper.rb
#...
DA:170,1948
DA:173,186
DA:174,0
DA:185,186
DA:187,868
DA:219,186
Cause
RubyVM bytecode for def some_method; nil; end shows putnil is optimized - the method definition itself marks the return value, no instruction executes on line 2.
So: Line 2 is evaluated at method call time conceptually, but generates no executable instruction that Coverage can track.
This can be tested with a simple script, line 2 has no coverage data:
$ cat test_nil.rb
def method_nil
nil
end
def method_redundant
'hello'
:hello
'result'
end
def method_uncalled
'hello'
'uncalled'
end
method_nil
method_redundant
(arm64) tkuah--20211125-NXMMF:gitlab tkuah$ ruby test_coverage.rb
{"/Users/tkuah/code/gdk/gitlab/test_nil.rb"=>{:lines=>[1, nil, nil, nil, 1, nil, nil, 1, nil, nil, 1, nil, 0, nil, nil, 1, 1, nil]}}
test_coverage.rb
require 'coverage'
Coverage.start(lines: true)
require_relative 'test_nil'
p Coverage.result
You can also see the compile bytecode Ruby compiles too which skips line 2:
$ irb
irb(main):001> require_relative 'test_nil'
=> true
irb(main):002> puts RubyVM::InstructionSequence.of(method(:method_nil)).disasm
== disasm: #<ISeq:method_nil@/Users/tkuah/code/gdk/gitlab/test_nil.rb:1 (1,0)-(3,3)>
0000 putnil ( 1)[Ca]
0001 leave ( 3)[Re]
=> nil
Compare this to a method which returns a string
irb(main):004> puts RubyVM::InstructionSequence.of(method(:method_redundant)).disasm
== disasm: #<ISeq:method_redundant@/Users/tkuah/code/gdk/gitlab/test_nil.rb:5 (5,0)-(9,3)>
0000 putstring "result" ( 8)[LiCa]
0002 leave ( 9)[Re]
=> nil
Solution
Refactor your method to be like this instead:
def summarize_new_merge_request_disabled_reason(_merge_request); end