quickfixj icon indicating copy to clipboard operation
quickfixj copied to clipboard

Published 20 hours ago • verified publisher icon quickfix-j

Integration with JaCoCo and Coveralls

Open the-thing opened this issue 5 years ago • 11 comments

I was able to integrate with JaCoCo code coverage and Coveralls plugins. The configuration excludes auto generated code so it covers only quickfixj-core logic (as it should). The build takes 30 seconds more on Travis - no impact on local builds as the goals are not executed.

Since the project is public there are no tokens required etc. The only post requisite is to login to Coveralls and add quickfix-j/quickfixj repo via QuickFIX/J organization from one of the members. Coverage status badge already points to it.

At the moment coverage is 78% which will make the badge red (> 80% orange, > 90% green), but I will be happy to improve the coverage later on.

Please have a look at my branch view in Coveralls. https://coveralls.io/github/the-thing/quickfixj

There are also other code coverage projects such as Code Climate, but they do more than that. Coveralls seems to the most feasible for pure code coverage.

the-thing avatar Jun 24 '20 13:06 the-thing

Thanks for the PR. I've authorized Coverall for this repo.

chrjohn avatar Jun 24 '20 14:06 chrjohn

Cool.

I'm not able to see it on Coveralls though. Is this the right link? https://coveralls.io/github/quickfix-j/quickfixj?branch=master

the-thing avatar Jun 24 '20 14:06 the-thing

@the-thing , link should work now. Had to add it manually on Coveralls.

chrjohn avatar Jun 26 '20 12:06 chrjohn

Seems to wok. Uploaded successfully to Coveralls. https://coveralls.io/github/quickfix-j/quickfixj

There is a problem with coverage for quickfix.field.converter classes. It seems that JaCoCo is not able to detect the common inclusion/exclusion for quickfix.field.* and quickfix.field.converter.*. I'm getting the same reports locally and I think this is because of JaCoCo plugin filtering being different.

We can either exclude fields and converters from the report or include them both. The way to fix it would be to move out converters to a different package e.g. quickfix.converter (package changes + pom exclusion pattern change). However, this will be a breaking change if somebody is using converters in their code directly.

the-thing avatar Jun 26 '20 13:06 the-thing

Local coverage after moving converters. coverage with converters

the-thing avatar Jun 26 '20 13:06 the-thing

@the-thing :+1: for quickfix.converter package. The change should be OK for 3.0.0 milestone. However, we probably need to decouple this from the Coveralls change.

chrjohn avatar Jun 26 '20 13:06 chrjohn

Ok. Leaving this PR as is.

the-thing avatar Jun 26 '20 13:06 the-thing

Sorry for the long delay, still busy with other stuff. :-( Will hopefully be able to get back to this soon...

chrjohn avatar Aug 17 '20 23:08 chrjohn

No problem, I was on holidays myself. I'm actually more keen on merging https://github.com/quickfix-j/quickfixj/pull/305.

the-thing avatar Aug 26 '20 09:08 the-thing

@the-thing I see that you did this change for Travis. Would it be much work to integrate this into the Java CI jobs?

chrjohn avatar May 02 '21 22:05 chrjohn

Not sure, but I will have a look sometime this week. Yes, this seems to be obsolete change atm.

the-thing avatar May 03 '21 07:05 the-thing