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

Don't generate line 0 in profdata_coverage_file.rb from line with error #387

Closed

Conversation

lukaskukacka
Copy link

@lukaskukacka lukaskukacka commented May 3, 2018

I was able to reproduce issue #290 by having error in coverage processed using profdata_coverage_file.rb

Problem description

When passing Cobertura XML format generated using slather to sonar-scanner, it crashes on error like this:

java.lang.IllegalStateException: Measure with line 0 for file '/somepath/sonar-scanner/SomeClass.swift' must be > 0

It makes sense since there is no "line 0" in source code (text file).

In my case there was line number 0 generated in Cobertura XML output like this:

<class name="SomeClass" filename="SomeModule/SomeClass.swift" line-rate="0.9746835443037974" branch-rate="1.0000000000000000" complexity="0.0">
    <methods/>
    <lines>
        <line number="30" branch="false" hits="16200"/>
        <line number="0" branch="false" hits="30"/>

I found it is because the source being passed to ProfdataCoverageFile contains error in following format:

...
30|  16.2k|    private let someVar: [String] = SomeClass.sharedInstance.someProperty.compactMap { $0[0] }
------------------
| _T010SomeModule9SomeClassC7someVar33_B9BF662D291A982BAD48D03250F9A280LLSaySSGvpfiSSSgSScfU_:
|   30|  16.2k|    private let someVar: [String] = SomeClass.sharedInstance.someProperty.compactMap { $0[0] }
------------------
| Unexecuted instantiation: _T010SomeModule9SomeClassCACycfC
------------------
| Unexecuted instantiation: _T010SomeModule9SomeClassCACSgSo7NSCoderC5coder_tcfC
------------------
31|       |
32|       |    /**
...

Implementation

This change just filters out these lines from source_code_lines and it fixes the issue in my case.

It is implemented only in case of self.line_numbers_first because this case is easy and fast to detect and handle.
Detecting the issue if self.line_numbers_first == false would require rather complex regex due to duplication of valid source line in the error listing.

Solution

This change prevents the 0 line from appearing in XML coverage report.

Tests

Not having good enough knowledge of slather and ruby, I didn't find simple / elegant solution to adjust tests for this case (tests are using line_numbers_first = false, while my fix is for true case).
Sorry about that.

Notes

I will appreciate any suggestion or better solution for this problem.
However I would appreciate some fix for this problem to be released in slather so I don't have to use fork or some XML text manipulation in production to workaround this problem.

Thanks to everyone for your time to review!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 95.694% when pulling e39fb89 on lukaskukacka:ProfdataIgnoreErrorLines into 036f147 on SlatherOrg:master.

chrishulton pushed a commit to codeclimate/test-reporter that referenced this pull request May 14, 2018
This prevents the test reporter from crashing when an invalid line
number is detected in the cobertuna.xml coverage file.

While this does not address the actual issue causing invalid line
numbers, it does allow the test reporter to ignore the invalid lines.

Related: SlatherOrg/slather#387
Addresses: #320
chrishulton pushed a commit to codeclimate/test-reporter that referenced this pull request May 15, 2018
This prevents the test reporter from crashing when an invalid line
number is detected in the cobertuna.xml coverage file.

While this does not address the actual issue causing invalid line
numbers, it does allow the test reporter to ignore the invalid lines.

Related: SlatherOrg/slather#387
Addresses: #320
@ButkiewiczP
Copy link
Contributor

Is there any movement on this PR?

@ksuther
Copy link
Contributor

ksuther commented Nov 11, 2018

Have you tried this PR to see if it fixes the problem you're running in to? I haven't tried this myself, but I can merge it if anyone who is running into the problem can confirm it fixes it.

@mRs-
Copy link

mRs- commented Nov 21, 2018

for me the issue is done. It's already working.

@kevinvandenbreemen
Copy link

kevinvandenbreemen commented Apr 14, 2020

Hey there,

In answer to question posed by @ksuther :

Have you tried this PR to see if it fixes the problem you're running in to? I haven't tried this myself, but I can merge it if anyone who is running into the problem can confirm it fixes it.

The iOS team I'm on at Pelmorex Corp has been having issues with SONAR throwing an IllegalArgumentException on account of coverage values being generated for line 0.

We have tried a version of Slather with this particular pull request (not the updated version from March) applied, and it has addressed the problem.

Additional info:
Slather Version: v2.4.7
Sonar version used was 7.1
XCode Version: 11.4

Branch with fix here:
https://github.com/kevinvandenbreemen/slather/tree/latestReleaseInt

@ksuther
Copy link
Contributor

ksuther commented Apr 22, 2020

Merged the newer version from #449. Thank you for submitting this!

@ksuther ksuther closed this Apr 22, 2020
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

Successfully merging this pull request may close these issues.

6 participants