Undefined method each for Nil in result_merger

[JanStevens]

Hello,

Using latest ruby/simplecov/jenkins.
On our jenkins build system we sometimes see the following error returning when all specs passed and simplecov starts building its report:

Coverage report generated for  to /var/lib/jenkins/jobs/SpaceCII/workspace/coverage. 16478 / 19835 LOC (83.08%) covered.
/var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov/result_merger.rb:47:in `results': undefined method `each' for nil:NilClass (NoMethodError)
    from /var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov/result_merger.rb:69:in `merged_result'
    from /var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov.rb:61:in `result'
    from /var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov/configuration.rb:159:in `block in at_exit'
    from /var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov/defaults.rb:60:in `call'
    from /var/lib/jenkins/jobs/SpaceCI/workspace/vendor/bundle/ruby/2.2.0/gems/simplecov-0.10.0/lib/simplecov/defaults.rb:60:in `block in <top (required)>'

Looking at the code I see that resultset can return nil when the stored_data returns nil. For some reason the data is not written yet to the file so simplecov fails. I find this failing a bit ugly. Would it be more preferred to always return a Hash, even when the reading of JSON data fails?

In result_merger.rb

# Loads the cached resultset from JSON and returns it as a Hash
      def resultset
        if stored_data
          begin
            JSON.parse(stored_data) || {}
          rescue
            {}
          end
        else
          {}
        end
      end

This would prevent simplecov to generate an error but wont generate any reporting. Willing to make a PR to fix this issue. I believe a race condition might exists, for large projects the system is not complete done writing to the file but simplecov already starts reading it, resulting in an empty file. Don't know how to solve that issue

Regards

[xaviershay]

for large projects the system is not complete done writing to the file but simplecov already starts reading it, resulting in an empty file. Don't know how to solve that issue

If this is indeed the issue, the correct way to do it is to write to temp file and then do an atomic move. I'll have a poke around to see if this is what is happening.

[xaviershay]

I found this unsafe write, but it's possible this doesn't run concurrently anywhere:

module SimpleCov
  module LastRun
      def write(json)
        File.open(last_run_path, "w+") do |f|
          f.puts JSON.pretty_generate(json)
        end
      end
    end
  end
end

The write that would be causing your issue (result merging) appears to be protected by a write lock.

[xaviershay]

irb(main):008:0> JSON.parse(nil)
TypeError: no implicit conversion of nil into String
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `initialize'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `new'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from (irb):8
    from /Users/xavier/.rubies/cruby-2.2.2/bin/irb:11:in `<main>'
irb(main):009:0> JSON.parse("nil")
JSON::ParserError: 757: unexpected token at 'nil'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from (irb):9
    from /Users/xavier/.rubies/cruby-2.2.2/bin/irb:11:in `<main>'
irb(main):010:0> JSON.parse("null")
JSON::ParserError: 757: unexpected token at 'null'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from (irb):10
    from /Users/xavier/.rubies/cruby-2.2.2/bin/irb:11:in `<main>'
irb(main):011:0> JSON.parse("")
JSON::ParserError: A JSON text must at least contain two octets!
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `initialize'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `new'
    from /Users/xavier/.gem/ruby/2.2.2/gems/json-1.8.3/lib/json/common.rb:155:in `parse'
    from (irb):11
    from /Users/xavier/.rubies/cruby-2.2.2/bin/irb:11:in `<main>'

How can you make this call return nil? Without using quirks_mode, I can't see a way to do it. Any empty or nil value will raise instead. Given I can't come up with a repro, I'm not convinced that just putting in a guard here will fix.

Given you only see this sometimes, it must be some kind of race condition. I just don't know what :/

[xaviershay]

Link to v0.10.0 code: https://github.com/colszowka/simplecov/blob/v0.10.0/lib/simplecov/result_merger.rb#L21

[JanStevens]

@xaviershay thats quite easy, when using the OJ gem for example you get the following

[9] pry(main)> JSON.parse(nil)
=> nil
[10] pry(main)> JSON.parse("nil")
Oj::ParseError: not a number or other value at line 1, column 1 [parse.c:422]
from (pry):10:in `parse'
[11] pry(main)> JSON.parse("null")
Oj::ParseError: unexpected non-document value
from (pry):11:in `parse'
[12] pry(main)> JSON.parse("")
Oj::ParseError: unexpected non-document value
from (pry):12:in `parse'

So when oj gem receives nil it will return nil. I think thats the root cause of my problems. On our CI setup we see it a couple of times a day (I think we have 20-50 builds a day). For me thats quite a high percentage and its especially frustrating if all specs pass but simplecov fails.

We do use a parallel_test setup so it's quite possible that there are race conditions.

Thanks for looking into it!

[xaviershay]

ah didn't not know about OJ. Thanks, that helps!

[JanStevens]

Anyway can we move on with this? Our CI is still failing daily because sometimes we parse nil

[xaviershay]

I still can't repro. If your PR fixes your builds for you, I'd recommend running off that branch until we get a fix in master. That should at least unblock you. (You can set this up in your Gemfile.) I'm not prepared to merge a speculative fix without a repro or test coverage though.

Looking into OJ, I still can't repro:

irb(main):009:0> Oj.load("")
=> nil
irb(main):010:0> Oj.mimic_JSON()
=> JSON
irb(main):011:0> JSON.parse("")
Oj::ParseError: unexpected non-document value
    from (irb):11:in `parse'
    from (irb):11
    from /Users/xavier/.rubies/ruby-2.2.2/bin/irb:11:in `<main>'

Oj.load does indeed return nil, but when set up as a JSON replacement it correct matches the original JSON behaviour.

What version of oj and ruby are you using? How are you setting it up? Are you changing compat modes? Can you post code showing a nil return value from a JSON.parse?