zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mfenes <...@git.apache.org>
Subject [GitHub] zookeeper pull request #443: ZOOKEEPER-2955: Enable Clover code coverage rep...
Date Tue, 23 Jan 2018 10:22:17 GMT
Github user mfenes commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/443#discussion_r163198966
  
    --- Diff: build.xml ---
    @@ -124,6 +160,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
         <property name="ivy.javacc.lib" value="${build.dir}/javacc/lib"/>
         <property name="ivy.releaseaudit.lib" value="${build.dir}/releaseaudit/lib"/>
         <property name="ivy.owasp.lib" value="${build.dir}/owasp/lib"/>
    +    <property name="ivy.coverage.lib" value="${test.java.build.dir}/lib"/>
    --- End diff --
    
    I did see the naming inconsistency, but on the other hand code coverage measurement belongs
to testing, and everything related to testing is placed under build/test. Not all the tools
have their own subdirectory under the build directory (like javacc, releaseaudit, owasp),
e.g. JUnit does not have. If JUnit does not have its own subdirectory under build/, then why
should OpenClover have. If the reason to put OpenClover under an own build/coverage directory
instead of putting it into build/test/lib is packaging, i.e. not delivering the clover jar
in releases, then basically the answer is that a ZK compiled with Clover should not be released.
I also verified that ant tar does not include build/test/lib jars. If ant targets tar, jar,
compile, test-core-java etc. are run without the -Drun.clover=true parameter, then the clover
jar is not even retrieved by ivy, so it won't be included, everything will run as before.
Another reason is, that originally in current code everything related
  to Clover (db, reports) is configured to be under build/test/clover. If Clover db and reports
are under build/test/clover, then why should the clover jar be put in build/coverage/lib and
not in build/test/lib? All-in-all, these were the reasons behind my decision to put Clover
under build/test/lib instead of build/coverage/lib. However, I agree that this is a naming
inconsistency and I can see the reason that everything which has its own ivy configuration
should have its own directory under build, so I can change the location of Clover if you prefer
having Clover under build/coverage/lib.


---

Mime
View raw message