search

trautonen / coveralls-maven-plugin

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

Adding branch coverage support #107

Open nickmerwin opened 7 years ago

nickmerwin commented 7 years ago

Hi @trautonen, we're rolling out branch coverage support for Coveralls.io and have landed it in two other integration libraries:

Node: nickmerwin/node-coveralls@d571dac

Python: coveralls-clients/coveralls-python#145

There are more details about the new branches parameter here: https://coveralls.zendesk.com/hc/en-us/articles/201350799-API-Reference

I wanted to get it on your radar now in case you or another maintainer has spare time to add this support to goveralls.

Thank you!

andrioli commented 7 years ago

Hi @trautonen.

FYI, I just created a POC that adds branch coverage support for JaCoCo in Coveralls. You can check my code here https://github.com/trautonen/coveralls-maven-plugin/compare/master...andrioli:feature/branch-support-for-jacoco. The code isn't ready. But is working.

What is missing:

The limitations:

In both cases I'm sending 0 as placeholder (node-coveralls does the same for block number).

WDYT? If you agree with the implementation I can create a PR.

trautonen commented 7 years ago

Yea, I've quickly checked the existing implementations and I guess your implementation is at least close what we can do in this maven plugin.

I was thinking if it made sense to misuse the Coveralls branch coverage details for JaCoCo. Like use only single entry which has 0 hits if it's partially or totally missed and 1 for no misses. Then use block and branch numbers to show how many branches were covered and how many missed. I think this would be visually better in Coveralls, but totally misuses the API.

Would be great to get some feedback how we should use the Coveralls API when the branches are not identifiable with the coverage tool.

But I think your implementation is fine for pull request. I'll do some polishing if it needs anything.

andrioli commented 7 years ago

Great. This week I'll fix all missing things I listed in the above comment and create a PR.

About the misuse of the API: Will look great in the Coveralls interface. But we take a risk. What happen if they change how branches are displayed? IDK if is a good idea.