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

ZOOKEEPER-2266: Integrate JaCoCo Coverage Library #1451

Closed
wants to merge 0 commits into from

Conversation

nkalmar
Copy link
Contributor

@nkalmar nkalmar commented Sep 7, 2020

No description provided.

@nkalmar
Copy link
Contributor Author

nkalmar commented Sep 7, 2020

We've seen demand for code coverage, and some apache projects are already using jacoco.
I found an old jira to integrate jacoco with ant.

I know this doesn't really tell us how good are testing is (specially that for many tests we spin up a whole zk server), but there is a demand to see this in reports, so.. here it is.

I did several tests (all test, one test, jacoco on/off and see generated sources - should be only in zk-server), a good starting point:
'mvn clean install -Dtest=WatcherTest -DfailIfNoTests=false -Dcodecoverage'

By default code coverage is off, enable it by adding -Dcodecoverage to mvn args.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some experience with JaCoCo.
For an OpenSource project like Zookeeper we should integrate it with services like CodeCov or Coveralls.io (I am using Coveralls for instance)

Such tool will be useful for requiring a quality gate on pull requests and in order to increase code coverage in general.

@nkalmar
Copy link
Contributor Author

nkalmar commented Sep 22, 2020

Hi @eolivelli , I tried to integrate coveralls according to the readme.

I enabled Apache/ZooKeeper on github in coveralls.io.
Now, if I understand this, I should just add
after_success:

  • mvn clean cobertura:cobertura coveralls:report

to travis.yml? Is that it? (I already added the maven plugin dependency, it doesn't run in local as it needs the repokey)

@eolivelli
Copy link
Contributor

@nkalmar Travis is not running tests, so it will be useless.
We have to add it to Jenkins builds.
It will also require to share the secret TOKEN on Jenkins, because in Travis is it simpler because there is an automatic integration .

Unfortunately the test suite it too big for Travis (or at least we have too many output pauses)
if we fix the Travis build, by running at least Java tests, we can go the way you are suggesting

cc @anmolnar

@hanm
Copy link
Contributor

hanm commented Oct 1, 2020

we already have clover for code coverage (in ant days) and seems maven build support clover too. do we plan to deprecate clover in favor of JaCoCo?

in any case, it's good to have more testing related addition for ZK as part of CI pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants