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

Adding branch coverage support #107

Open
nickmerwin opened this issue Mar 9, 2017 · 3 comments
Open

Adding branch coverage support #107

nickmerwin opened this issue Mar 9, 2017 · 3 comments
Labels
Milestone

Comments

@nickmerwin
Copy link

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: TheKevJames/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
Copy link
Contributor

andrioli commented Mar 11, 2017

Hi @trautonen.

FYI, I just created a POC that adds branch coverage support for JaCoCo in Coveralls. You can check my code here 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.

@trautonen
Copy link
Owner

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

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.

@trautonen trautonen added this to the v4.4.0 milestone Mar 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants