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
Edited by 🤖 GitLab Bot 🤖