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

Fixes NPE when coverage.xml has multiple <source> element #1

Merged
merged 2 commits into from
Oct 26, 2013

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Oct 25, 2013

Coverage report can have multiple source folders

    <sources>
        <source>src/main/java</source>
        <source>src/main/groovy</source>
    </sources>

Aside of that Gradle's Cobertura plugin can generate an additional wrong source folder.

So this PR

  • find the first matching source in any of the source directories
  • makes this plugin more lenient if the actual source file cannot be found

Also I'd like to thank Adrien Lecharpentier (@alecharp) who helped me to coin this bug

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 899ba2a on bric3:fixes_multiple_source_element into 5bd222c on kt3k:master.

@kt3k
Copy link
Owner

kt3k commented Oct 26, 2013

Thank you for the suggestion!

This seems the right treatment of <source> entries.

Thanks for improvement 😄

kt3k added a commit that referenced this pull request Oct 26, 2013
Fixes NPE when coverage.xml has multiple <source> element
@kt3k kt3k merged commit a242d6d into kt3k:master Oct 26, 2013
@bric3
Copy link
Contributor Author

bric3 commented Oct 26, 2013

You're welcome 😸
Thx for crafting the first usable code

@kt3k
Copy link
Owner

kt3k commented Oct 29, 2013

deployed v0.1.5, including this fix.

@bric3 bric3 deleted the fixes_multiple_source_element branch October 29, 2013 11:31
@bric3
Copy link
Contributor Author

bric3 commented Oct 29, 2013

Cool thanks.

I've put your plugin at use there : https://github.com/mockito/mockito
By the way, it would be great to see this plugin on mavenCentral()

Cheers.

@kt3k
Copy link
Owner

kt3k commented Oct 30, 2013

@bric3

I'm very impressed with seeing mockito using my plugin! 😄

By they way, it would be great to see this plugin on mavenCentral()

I'll try.

@bric3
Copy link
Contributor Author

bric3 commented Oct 30, 2013

You're welcome 😸

Thanks for the initial implementation.

Cheers

@kt3k
Copy link
Owner

kt3k commented Nov 5, 2013

@bric3
Copy link
Contributor Author

bric3 commented Nov 5, 2013

That's great, thanks a lot :)

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.

3 participants