Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ryan-williams opened this issue Aug 24, 2016 · 7 comments
Closed

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

ryan-williams opened this issue Aug 24, 2016 · 7 comments

Comments

@ryan-williams
Copy link

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
Copy link
Author

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:

  • Why would changing the line number of a line in A.scala (e.g. inserting a blank line at the top of the file) change the line number that the error is reporting in B.scala? Feels like an off-by-one array-indexing error or something could be happening.
  • The problem manifests identically when coveralls-maven-plugin is consuming the output of cobertura:cobertura or jacoco:report, which makes it seem possible that the bug is in this plugin.

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
Copy link
Owner

@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
Copy link
Author

ryan-williams commented Aug 24, 2016

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
Copy link
Owner

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
Copy link
Author

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

@andrioli
Copy link
Contributor

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants