coveralls-maven-plugin icon indicating copy to clipboard operation
coveralls-maven-plugin copied to clipboard

Adding branch coverage support

Open nickmerwin opened this issue 8 years ago • 3 comments

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!

nickmerwin avatar Mar 09 '17 20:03 nickmerwin

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:

  • Tests
  • Fix method Source.merge(Source)
  • IMO, the ugly access to branches raw array in CoverageTracingLogger.log(Log) must be fixed

The limitations:

  • There is NO block number in JaCoCo report
  • There is NO branch number in JaCoCo report

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.

andrioli avatar Mar 11 '17 18:03 andrioli

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.

trautonen avatar Mar 12 '17 20:03 trautonen

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.

andrioli avatar Mar 14 '17 14:03 andrioli