JRuby (please use github issues at http://bugs.jruby.org)
  1. JRuby (please use github issues at http://bugs.jruby.org)
  2. JRUBY-6106

JRuby 1.9 coverage library reports different results than ruby 1.9

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.6.4
    • Fix Version/s: JRuby 1.7.0.RC1
    • Component/s: Ruby 1.9.2
    • Labels:
      None
    • Environment:
      Ubuntu 11.04, jruby 1.6.4 (ruby-1.9.2-p136) (2011-08-23 17ea768) (OpenJDK Client VM 1.6.0_22) [linux-i386-java], ruby 1.9.2p0 (2010-08-18 revision 29036) [i686-linux]
    • Number of attachments :
      1

      Description

      The coverage array returned from jruby and ruby differ. Ruby marks valid code lines not covered as 0, whereas Jruby marks these same lines as nil, thus making code coverage tools not being able to distinguish between valid non-covered lines of code, and non-code lines. As you can see from the test file attached, and the output, ruby marks one line with a 0, and jruby does not. Jruby also misses the last two blank lines, but I don't see that as an issue for getting proper code coverage tools working.

      ruby1.9.2 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result"
      {"/home/ray/Perforce/ray_ray-desktop/Atria/test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, 0, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1, nil, nil]}
      
      jruby --debug --1.9 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result"
      {"/home/ray/Perforce/ray_ray-desktop/ThirdParty/jruby/jruby-1.6.4/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb"=>[nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1], "/home/ray/Perforce/ray_ray-desktop/Atria/./test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, nil, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1], "-e"=>[2]}
      

        Activity

        Hide
        Charles Oliver Nutter added a comment - - edited

        Well the initial experiment seems to be a success!

        system ~/projects/jruby $ jruby --debug --1.9 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result"
        {"/Users/headius/projects/jruby/./test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, 0, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1]}
        
        system ~/projects/jruby $ ruby-1.9.3 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result"
        {"/Users/headius/projects/jruby/test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, 0, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1, nil, nil]}
        

        The changes I made:

        • In ParserSupport.newline_node, call ParserConfiguration.coverLine with the appropriate line number
        • In ParserConfiguration.coverLine, maintain a growing Integer[] as lines are encountered. Zero the line being covered.
        • In Parser, at the end of parse associate the filename with that Integer[] only if coverage is enabled.
        • In CoverageData's LINE event hook, only update lines for files that already have an associated Integer[].

        This should basically mimic MRI's behavior, modulo any newline nodes we're missing.

        There are a few places outside newline_node where we create newlines. I did not add coverage calls there, because I'm unsure if they're intended to be visitable lines or if they're outside of normal executable code.

        I'm going to push these changes to JRuby master. I have no idea what's available for testing coverage beyond the one-off cases folks like you have provided, so I'd appreciate help finding tests for this.

        Show
        Charles Oliver Nutter added a comment - - edited Well the initial experiment seems to be a success! system ~/projects/jruby $ jruby --debug --1.9 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result" {"/Users/headius/projects/jruby/./test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, 0, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1]} system ~/projects/jruby $ ruby-1.9.3 -e "require 'coverage'; Coverage.start; require './test_cov.rb'; p Coverage.result" {"/Users/headius/projects/jruby/test_cov.rb"=>[nil, nil, 1, 1, 1, nil, nil, nil, nil, nil, 1, nil, 0, nil, nil, nil, 1, nil, nil, nil, nil, nil, nil, nil, nil, nil, 1, 1, nil, nil]} The changes I made: In ParserSupport.newline_node, call ParserConfiguration.coverLine with the appropriate line number In ParserConfiguration.coverLine, maintain a growing Integer[] as lines are encountered. Zero the line being covered. In Parser, at the end of parse associate the filename with that Integer[] only if coverage is enabled. In CoverageData's LINE event hook, only update lines for files that already have an associated Integer[]. This should basically mimic MRI's behavior, modulo any newline nodes we're missing. There are a few places outside newline_node where we create newlines. I did not add coverage calls there, because I'm unsure if they're intended to be visitable lines or if they're outside of normal executable code. I'm going to push these changes to JRuby master. I have no idea what's available for testing coverage beyond the one-off cases folks like you have provided, so I'd appreciate help finding tests for this.
        Hide
        Charles Oliver Nutter added a comment -

        FWIW, I'm not sure if the missing lines at the end of the file are important or not. JRuby's parser does not get a count of lines at any point during parsing, and if a line is not parsed as coverable, we will never grow the array to that point. In this case, the last two lines of the file are blank and so the parser never has anything to do there. They won't execute as code, and so they're not in the array.

        I would be surprised if this affected coverage tools, but that remains to be seen.

        Show
        Charles Oliver Nutter added a comment - FWIW, I'm not sure if the missing lines at the end of the file are important or not. JRuby's parser does not get a count of lines at any point during parsing, and if a line is not parsed as coverable, we will never grow the array to that point. In this case, the last two lines of the file are blank and so the parser never has anything to do there. They won't execute as code, and so they're not in the array. I would be surprised if this affected coverage tools, but that remains to be seen.
        Hide
        Charles Oliver Nutter added a comment -
        commit 8c0e98c239c0ce29762b0522d7f16ac929f767c5
        Author: Charles Oliver Nutter <headius@headius.com>
        Date:   Wed Aug 22 17:01:06 2012 -0500
        
            Fix JRUBY-6106
            
            JRuby 1.9 coverage library reports different results than ruby 1.9
            
            I have a basic understanding now of how MRI handles 'coverage'.
            
            Coverage support is turned on by loading the 'coverage' extension.
            This sets a global "rb_coverages" to non-nil, as a Hash.
            
            At parse time, files parsed are added to the coverages hash as
            having 'nil' for all lines. Files parsed before the 'coverage' ext
            is loaded are not added to this hash. This is the first behavioral
            difference in JRuby, since we're just adding lines to coverage for
            any file+line encountered, regardless of whether the file was
            loaded before or after 'coverage' ext loaded. See parse.y,
            "coverage" function, called from yycompile0 to set up the array
            for that file.
            
            At compile time (after parse) code lines get reset to zeros
            whenever that line would potentially trigger a "line" event in
            trace hooks. This is in compile.c in the ADD_TRACE macro.
            Essentially, compile.c adds the tracing logic as it goes only to
            the lines that should trigger a line event, and at the same time
            zeros out that line in the coverages array.
            
            The determination of a "code line" happens in compile.c in the
            iseq_compile_each function, right at the top, where it determines
            the line is a newline.
            
                if (node->flags & NODE_FL_NEWLINE) {
            	ADD_TRACE(ret, nd_line(node), RUBY_EVENT_LINE);
                }
            
            So for every actual NEWLINE node in the AST, the according line in
            the coverages array is reset to zero.
            
            Assuming our set of newline nodes matches MRI's (questionable), it
            should basically mean we just need to do the same zeroing of the
            according line when we construct a NewlineNode in our parser.
            
            The changes I made:
            
            * In ParserSupport.newline_node, call
            ParserConfiguration.coverLine with the appropriate line number
            
            * In ParserConfiguration.coverLine, maintain a growing Integer[]
            as lines are encountered. Zero the line being covered.
            
            * In Parser, at the end of parse associate the filename with that
            Integer[] only if coverage is enabled.
            
            * In CoverageData's LINE event hook, only update lines for files
            that already have an associated Integer[].
            
            This should basically mimic MRI's behavior, modulo any newline
            nodes we're missing (which is an open question).
            
            There are a few places outside newline_node where we create
            newlines. I did not add coverage calls there, because I'm unsure
            if they're intended to be visitable lines or if they're outside of
            normal executable code.
        
        :100644 100644 56e6158... 8833289... M	src/org/jruby/ext/coverage/CoverageData.java
        :100644 100644 376c236... 2195419... M	src/org/jruby/parser/Parser.java
        :100644 100644 f7fbd0b... 3db299c... M	src/org/jruby/parser/ParserConfiguration.java
        :100644 100644 68cd481... 30d3835... M	src/org/jruby/parser/ParserSupport.java
        
        Show
        Charles Oliver Nutter added a comment - commit 8c0e98c239c0ce29762b0522d7f16ac929f767c5 Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Aug 22 17:01:06 2012 -0500 Fix JRUBY-6106 JRuby 1.9 coverage library reports different results than ruby 1.9 I have a basic understanding now of how MRI handles 'coverage'. Coverage support is turned on by loading the 'coverage' extension. This sets a global "rb_coverages" to non-nil, as a Hash. At parse time, files parsed are added to the coverages hash as having 'nil' for all lines. Files parsed before the 'coverage' ext is loaded are not added to this hash. This is the first behavioral difference in JRuby, since we're just adding lines to coverage for any file+line encountered, regardless of whether the file was loaded before or after 'coverage' ext loaded. See parse.y, "coverage" function, called from yycompile0 to set up the array for that file. At compile time (after parse) code lines get reset to zeros whenever that line would potentially trigger a "line" event in trace hooks. This is in compile.c in the ADD_TRACE macro. Essentially, compile.c adds the tracing logic as it goes only to the lines that should trigger a line event, and at the same time zeros out that line in the coverages array. The determination of a "code line" happens in compile.c in the iseq_compile_each function, right at the top, where it determines the line is a newline. if (node->flags & NODE_FL_NEWLINE) { ADD_TRACE(ret, nd_line(node), RUBY_EVENT_LINE); } So for every actual NEWLINE node in the AST, the according line in the coverages array is reset to zero. Assuming our set of newline nodes matches MRI's (questionable), it should basically mean we just need to do the same zeroing of the according line when we construct a NewlineNode in our parser. The changes I made: * In ParserSupport.newline_node, call ParserConfiguration.coverLine with the appropriate line number * In ParserConfiguration.coverLine, maintain a growing Integer[] as lines are encountered. Zero the line being covered. * In Parser, at the end of parse associate the filename with that Integer[] only if coverage is enabled. * In CoverageData's LINE event hook, only update lines for files that already have an associated Integer[]. This should basically mimic MRI's behavior, modulo any newline nodes we're missing (which is an open question). There are a few places outside newline_node where we create newlines. I did not add coverage calls there, because I'm unsure if they're intended to be visitable lines or if they're outside of normal executable code. :100644 100644 56e6158... 8833289... M src/org/jruby/ext/coverage/CoverageData.java :100644 100644 376c236... 2195419... M src/org/jruby/parser/Parser.java :100644 100644 f7fbd0b... 3db299c... M src/org/jruby/parser/ParserConfiguration.java :100644 100644 68cd481... 30d3835... M src/org/jruby/parser/ParserSupport.java
        Hide
        Peter Robinson added a comment -

        Thanks for doing this fix. Much appreciated. When do you think this will be available in a released version of JRuby?

        Show
        Peter Robinson added a comment - Thanks for doing this fix. Much appreciated. When do you think this will be available in a released version of JRuby?
        Hide
        Tobias Pfeiffer added a comment - - edited

        Hi all!
        At first big thanks to Charles for tackling all this and to the JRuby dev team in general for well.. JRuby <3<3<3 you guys!

        For reference, when I'm talking about jruby-head I'm talking about jruby with the latest commit being this one:
        "commit ff42564963ac0d5b8b6471f23fd3441fb2db6fba
        Author: Charles Oliver Nutter <headius@headius.com>
        Date: Wed Aug 22 18:42:34 2012 -0500

        Fix JRUBY-6827 (...)"
        All of this on my LinudMint Debian Edition (based on Debian Testing) - 64 bit.

        Good news first: it seems to kind of work. I created a minimal example where it seems to work after an adjustment to rspec-expectations. You can check it out at github - the fix is described there and the error output is documented. The error is an index out of bounds exception when trying to load the 'pp' library - no idea how that is triggered. And yes it only occurs when using jruby-head and simplecov.

        Which gets us to the bad part, namely that error. I created a branch of my shoes4 fork where I use jruby-head and simplecov again. There the error can be seen again in this gist - again rspec-expectations. I tried to fix it there as well but then the error goes on and occurs in pry (another require of 'pp'). Uncommenting that one however leads to everything not working anymore even without simplecov. However if one keeps going the next error come for pry when trying to require 'tmpdir'. The stacktrace can be seen in this gist.

        On a related note, jruby-head (without simplecov) seems to make 68 tests fail (all for the same reason apparently) while everything is green on jruby-1.6.7.2 but that's rather the topic for a separate issue.

        I hope that I provided enough information, if some kind of information or an example or something is missing please tell me

        Cheers,
        Tobi

        edit: Oh and as far as I know rcov doesn't work with 1.9 at least it was like that like half a year ago

        Show
        Tobias Pfeiffer added a comment - - edited Hi all! At first big thanks to Charles for tackling all this and to the JRuby dev team in general for well.. JRuby <3<3<3 you guys! For reference, when I'm talking about jruby-head I'm talking about jruby with the latest commit being this one: "commit ff42564963ac0d5b8b6471f23fd3441fb2db6fba Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Aug 22 18:42:34 2012 -0500 Fix JRUBY-6827 (...)" All of this on my LinudMint Debian Edition (based on Debian Testing) - 64 bit. Good news first: it seems to kind of work. I created a minimal example where it seems to work after an adjustment to rspec-expectations. You can check it out at github - the fix is described there and the error output is documented. The error is an index out of bounds exception when trying to load the 'pp' library - no idea how that is triggered. And yes it only occurs when using jruby-head and simplecov. Which gets us to the bad part, namely that error. I created a branch of my shoes4 fork where I use jruby-head and simplecov again. There the error can be seen again in this gist - again rspec-expectations. I tried to fix it there as well but then the error goes on and occurs in pry (another require of 'pp'). Uncommenting that one however leads to everything not working anymore even without simplecov. However if one keeps going the next error come for pry when trying to require 'tmpdir'. The stacktrace can be seen in this gist . On a related note, jruby-head (without simplecov) seems to make 68 tests fail (all for the same reason apparently) while everything is green on jruby-1.6.7.2 but that's rather the topic for a separate issue. I hope that I provided enough information, if some kind of information or an example or something is missing please tell me Cheers, Tobi edit: Oh and as far as I know rcov doesn't work with 1.9 at least it was like that like half a year ago

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Raymond
          • Votes:
            12 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: