trautonen / coveralls-maven-plugin

Maven plugin for submitting Java code coverage reports to Coveralls web service.
MIT License
312 stars 123 forks source link

Line number N is greater than the source file X size #98

Closed ryan-williams closed 8 years ago

ryan-williams commented 8 years ago

I'm seeing this error, already discussed on #73 and #82, and have made as small a repro case as I can in this repo: https://github.com/ryan-williams/coverage-test. I'm using version 4.2.0 of this plugin and jacoco 0.7.7.201606060606.

case class A is declared on A.scala:7, and for some reason that line number (7) is being looked up in B.scala, which is only 6 lines long.

Here you can see it having failed on Travis (the build succeeded, but if you look at the logs for after_success command you can see the coveralls:report failure.

ryan-williams commented 8 years ago

I got something working in the end; I guess most of my problem was trying to use Jacoco / Cobertura directly on Scala code.

I was able to make things work by instead using Scoverage to output a Cobertura-formatted report.

It still feels like the failure I was seeing points to a bug somewhere in either this plugin or jacoco and cobertura:

For future travelers, here is a coverage profile that correctly pipes scoverage's report-generation to this plugin's coveralls-uploading (when -Pcoverage is used or env.TRAVIS is true), and here is TravisCI config for replacing mvn test with mvn scoverage:report (which runs tests and generates a coverage report) and uploading that report to Coveralls afterward.

trautonen commented 8 years ago

@ryan-williams thanks a lot for very thorough testing. I ran your sample project and looked the jacoco report file which is parsed by the plugin. The relevant parts look like this:

        <sourcefile name="B.scala">
            <line nr="3" mi="116" ci="0" mb="16" cb="0"/>
            <line nr="4" mi="7" ci="0" mb="0" cb="0"/>
            <line nr="7" mi="5" ci="0" mb="0" cb="0"/>
            <counter type="INSTRUCTION" missed="139" covered="0"/>
            <counter type="BRANCH" missed="16" covered="0"/>
            <counter type="LINE" missed="3" covered="0"/>
            <counter type="COMPLEXITY" missed="29" covered="0"/>
            <counter type="METHOD" missed="21" covered="0"/>
            <counter type="CLASS" missed="2" covered="0"/>
        </sourcefile>
        <sourcefile name="A.scala">
            <line nr="7" mi="122" ci="0" mb="16" cb="0"/>
            <counter type="INSTRUCTION" missed="133" covered="0"/>
            <counter type="BRANCH" missed="16" covered="0"/>
            <counter type="LINE" missed="1" covered="0"/>
            <counter type="COMPLEXITY" missed="28" covered="0"/>
            <counter type="METHOD" missed="20" covered="0"/>
            <counter type="CLASS" missed="2" covered="0"/>
        </sourcefile>

By looking that I guess jacoco still does not handle the scala source files correctly and reports invalid lines from source file B.scala. The files are not mixed because clearly B's report has 3 relevant lines and A's report has 1 relevant source code line.

Why B is incorrect? I have no idea...

ryan-williams commented 8 years ago

Yea, when I dug a bit into the jacoco and cobertura inputs to this plugin, I did see that they appeared to both output things talking about B.scala:7, which they shouldn't have been, meaning it as least partly not this plugin's fault, most likely.

The really strange thing is that that line number, or at least the line number reported in the error msg from this plugin, changes based on what line number in A.scala the case class A happens on. That seems like it must be a bug somewhere in the stack here, even granting that these Java-coverage tools are being mis-applied to Scala code.

trautonen commented 8 years ago

Yea, that's a good finding. I also checked that out too and it seems that the case class line number in A changes the jacoco output, and when i moved the case class to line 4 in the file, I got this.

        <sourcefile name="B.scala">
            <line nr="3" mi="116" ci="0" mb="16" cb="0"/>
            <line nr="4" mi="12" ci="0" mb="0" cb="0"/>
            <counter type="INSTRUCTION" missed="139" covered="0"/>
            <counter type="BRANCH" missed="16" covered="0"/>
            <counter type="LINE" missed="2" covered="0"/>
            <counter type="COMPLEXITY" missed="29" covered="0"/>
            <counter type="METHOD" missed="21" covered="0"/>
            <counter type="CLASS" missed="2" covered="0"/>
        </sourcefile>
        <sourcefile name="A.scala">
            <line nr="4" mi="122" ci="0" mb="16" cb="0"/>
            <counter type="INSTRUCTION" missed="133" covered="0"/>
            <counter type="BRANCH" missed="16" covered="0"/>
            <counter type="LINE" missed="1" covered="0"/>
            <counter type="COMPLEXITY" missed="28" covered="0"/>
            <counter type="METHOD" missed="20" covered="0"/>
            <counter type="CLASS" missed="2" covered="0"/>
        </sourcefile>

I did not change B.scala at all and it's totally different in the report. I guess the conclusion is that the report generation is just totally broken with Java reporting tools and we should stick to scoverage only.

Your correct way to use scoverage should be documented in the readme and I'll try to put that together at some point soon. Your help on this one has been great!

ryan-williams commented 8 years ago

Sounds great, thanks for following up and for making this!

andrioli commented 8 years ago

Hey guys, just a further comment

I want to point that the scalac compiler is generating all this mess. Compile the code with mvn compile and use javap to verify the LineNumberTable generated by scalac, there you can find the lines reported by JaCoCo.

Remember that JaCoCo works directly with the JVM bytecode and will map the bytecode instruction to lines in the sourcefile looking in the LineNumberTable.

trautonen commented 8 years ago

@andrioli fair enough. Let's not blame the tools just because they are used in a context they are not intended to work.