Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.2, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Jacoco (http://www.jacoco.org/) is a much cleaner and simpler to use code coverage tool than clover and additionally doesn't require having a third party license since it is open source. It also has nice Jenkins integration tools that make it incredibly easy to see what is and isn't tested. We should convert the Lucene and Solr builds to use Jacoco instead of Clover.

      1. LUCENE-5439.patch
        14 kB
        Robert Muir
      2. LUCENE-5439.patch
        14 kB
        Robert Muir
      3. LUCENE-5439.patch
        7 kB
        Robert Muir

        Activity

        Hide
        Robert Muir added a comment -

        I don't know how i feel about this. Clover's reports are far superior though from everything ive seen. We would lose a lot of capabilities: like being able to see individual test contributions. I'm not ready to give that up.

        personally I use both. I look at the batch reports from jenkins but I use jacoco (eclemma) in my IDE because... i dont want code coverage completely taking over my IDE.

        Maybe we should wait until jacoco matures and catches up feature-wise so we don't lose things?

        Show
        Robert Muir added a comment - I don't know how i feel about this. Clover's reports are far superior though from everything ive seen. We would lose a lot of capabilities: like being able to see individual test contributions. I'm not ready to give that up. personally I use both. I look at the batch reports from jenkins but I use jacoco (eclemma) in my IDE because... i dont want code coverage completely taking over my IDE. Maybe we should wait until jacoco matures and catches up feature-wise so we don't lose things?
        Hide
        Grant Ingersoll added a comment -

        Fair enough. I'll probably put up a patch, but leave in the Clover stuff and then we can compare side-by-side. I like that Jacoco doesn't require you to pre-instrument your classes, but I admit I am not up on Clover's latest capabilities to know whether it can or not.

        I forget the Clover licensing terms. Can anyone use it or only committers? If it is the latter, I think we should add Jacoco too, since then anyone can run it. If it is the former, then we can either leave alone or use side-by-side.

        Show
        Grant Ingersoll added a comment - Fair enough. I'll probably put up a patch, but leave in the Clover stuff and then we can compare side-by-side. I like that Jacoco doesn't require you to pre-instrument your classes, but I admit I am not up on Clover's latest capabilities to know whether it can or not. I forget the Clover licensing terms. Can anyone use it or only committers? If it is the latter, I think we should add Jacoco too, since then anyone can run it. If it is the former, then we can either leave alone or use side-by-side.
        Hide
        Robert Muir added a comment -

        I think anyone can use it, but I think we should add jacoco either way? I was just speaking to "switch" (as in removing clover).

        Show
        Robert Muir added a comment - I think anyone can use it, but I think we should add jacoco either way? I was just speaking to "switch" (as in removing clover).
        Hide
        Uwe Schindler added a comment -

        I forget the Clover licensing terms. Can anyone use it or only committers? If it is the latter, I think we should add Jacoco too, since then anyone can run it. If it is the former, then we can either leave alone or use side-by-side.

        Anyone can use Clover. The License is not personalized, it enables use for code in the org.apache Java Package name. The license is shipped in SVN checkout, in Source Artifacts it is excluded.

        In my opinion we should have both options: Why remove Clover? - it is very useful!!!

        Show
        Uwe Schindler added a comment - I forget the Clover licensing terms. Can anyone use it or only committers? If it is the latter, I think we should add Jacoco too, since then anyone can run it. If it is the former, then we can either leave alone or use side-by-side. Anyone can use Clover. The License is not personalized, it enables use for code in the org.apache Java Package name. The license is shipped in SVN checkout, in Source Artifacts it is excluded. In my opinion we should have both options: Why remove Clover? - it is very useful!!!
        Hide
        Robert Muir added a comment -

        I retitled the issue, not to remove clover but add jacoco as an option. I hope you don't mind if i assign, I have a working patch.

        Show
        Robert Muir added a comment - I retitled the issue, not to remove clover but add jacoco as an option. I hope you don't mind if i assign, I have a working patch.
        Hide
        Robert Muir added a comment -

        Here is an initial patch that adds 'ant jacoco' (its per-module).

        The logic is fairly contained and not nearly as invasive on the build as clover. The jacoco task just downloads the jacoco ant tasks from ivy and sets up JVM parameters to junit4 slaves so that each slave writes its own coverage database. When producing reports we just merge coverage databases across all the slaves.

        Today the clover build takes many hours and this is a nice alternative. For example 'ant jacoco' on lucene-core is quite fast (on my machine, about as fast as running tests normally).

          ...
           [junit4] JVM J0:     1.24 ..   146.46 =   145.22s
           [junit4] JVM J1:     1.46 ..   146.30 =   144.85s
           [junit4] JVM J2:     1.71 ..   146.37 =   144.65s
           [junit4] JVM J3:     1.46 ..   146.49 =   145.03s
           [junit4] Execution time total: 2 minutes 26 seconds
           [junit4] Tests summary: 414 suites, 3438 tests, 154 ignored (144 assumptions)
             [echo] 5 slowest tests:
        [junit4:tophints]  14.63s | org.apache.lucene.index.TestDocValuesFormat
        [junit4:tophints]  14.31s | org.apache.lucene.index.TestNormsFormat
        [junit4:tophints]  11.80s | org.apache.lucene.index.TestTermVectorsFormat
        [junit4:tophints]  11.76s | org.apache.lucene.codecs.compressing.TestCompressingTermVectorsFormat
        [junit4:tophints]  11.16s | org.apache.lucene.codecs.lucene50.TestLucene50DocValuesFormat
        
        -check-totals:
        
        -post-jacoco:
           [delete] Deleting directory /home/rmuir/workspace/trunk-iw/lucene/build/jacoco/core
            [mkdir] Created dir: /home/rmuir/workspace/trunk-iw/lucene/build/jacoco/core
        [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J0/jacoco.db
        [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J1/jacoco.db
        [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J2/jacoco.db
        [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J3/jacoco.db
        [jacoco:report] Writing bundle 'lucene-core-6.0.0-SNAPSHOT JaCoCo coverage report' with 1371 classes
        
        jacoco:
        
        BUILD SUCCESSFUL
        Total time: 2 minutes 30 seconds
        

        You can see this coverage report for lucene-core (only from lucene-core's tests) here:
        http://people.apache.org/~rmuir/jacoco-core/

        I will now look at adding some top-level build tasks that merge coverage dbs across all modules for an aggregate view similar to what clover does, too. But i really like having the per-module option at least for now.

        Show
        Robert Muir added a comment - Here is an initial patch that adds 'ant jacoco' (its per-module). The logic is fairly contained and not nearly as invasive on the build as clover. The jacoco task just downloads the jacoco ant tasks from ivy and sets up JVM parameters to junit4 slaves so that each slave writes its own coverage database. When producing reports we just merge coverage databases across all the slaves. Today the clover build takes many hours and this is a nice alternative. For example 'ant jacoco' on lucene-core is quite fast (on my machine, about as fast as running tests normally). ... [junit4] JVM J0: 1.24 .. 146.46 = 145.22s [junit4] JVM J1: 1.46 .. 146.30 = 144.85s [junit4] JVM J2: 1.71 .. 146.37 = 144.65s [junit4] JVM J3: 1.46 .. 146.49 = 145.03s [junit4] Execution time total: 2 minutes 26 seconds [junit4] Tests summary: 414 suites, 3438 tests, 154 ignored (144 assumptions) [echo] 5 slowest tests: [junit4:tophints] 14.63s | org.apache.lucene.index.TestDocValuesFormat [junit4:tophints] 14.31s | org.apache.lucene.index.TestNormsFormat [junit4:tophints] 11.80s | org.apache.lucene.index.TestTermVectorsFormat [junit4:tophints] 11.76s | org.apache.lucene.codecs.compressing.TestCompressingTermVectorsFormat [junit4:tophints] 11.16s | org.apache.lucene.codecs.lucene50.TestLucene50DocValuesFormat -check-totals: -post-jacoco: [delete] Deleting directory /home/rmuir/workspace/trunk-iw/lucene/build/jacoco/core [mkdir] Created dir: /home/rmuir/workspace/trunk-iw/lucene/build/jacoco/core [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J0/jacoco.db [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J1/jacoco.db [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J2/jacoco.db [jacoco:report] Loading execution data file /home/rmuir/workspace/trunk-iw/lucene/build/core/test/J3/jacoco.db [jacoco:report] Writing bundle 'lucene-core-6.0.0-SNAPSHOT JaCoCo coverage report' with 1371 classes jacoco: BUILD SUCCESSFUL Total time: 2 minutes 30 seconds You can see this coverage report for lucene-core (only from lucene-core's tests) here: http://people.apache.org/~rmuir/jacoco-core/ I will now look at adding some top-level build tasks that merge coverage dbs across all modules for an aggregate view similar to what clover does, too. But i really like having the per-module option at least for now.
        Hide
        Robert Muir added a comment -

        updated patch. I added top-level "jacoco" targets for lucene and solr that run coverage+generate report for each module, and then additionally create "lucene-all" and "solr-all" aggregated reports.

        The time to run "ant jacoco" for lucene/ for me is 9 minutes, essentially the same amount of time it takes to run "ant test".

        I think its ready.

        Show
        Robert Muir added a comment - updated patch. I added top-level "jacoco" targets for lucene and solr that run coverage+generate report for each module, and then additionally create "lucene-all" and "solr-all" aggregated reports. The time to run "ant jacoco" for lucene/ for me is 9 minutes, essentially the same amount of time it takes to run "ant test". I think its ready.
        Hide
        Robert Muir added a comment -

        Slight tweak to the top-level targets.

        Show
        Robert Muir added a comment - Slight tweak to the top-level targets.
        Hide
        Adrien Grand added a comment -

        +1 being able to create coverage reports on my local machine in a reasonable time will be super useful!

        Show
        Adrien Grand added a comment - +1 being able to create coverage reports on my local machine in a reasonable time will be super useful!
        Hide
        Uwe Schindler added a comment -

        Hi Robert,
        very cool. I had the same idea for clover a while back to have separate databases per thread, but i gave up, because clover has no merging functionality (at least I did not understand how to use it). Also the setup of clover in lucene was to have a one-for-all clover database...

        I checked you patch, very minimal invasive, cool! Just a few test policy files and the "hack" with relative paths for the database. I just wonder why the "clover" target is sometimes included as dependency to the new tasks, but maybe thats just copypaste.

        A further imprvement could be to have a root-level report with lucene+solr for Jenkins. Should be easy. Maybe make the report generator a macro in common-build.

        I still like the clover reports more (they have much nicer report output and functionality), but jacoco is fast and - as Grant said - does not need to istrument classes at compile time. So you can quickly switch between jacoco and non-jacoco without recompiling (for Clover you had to "ant clean" first).

        +1 to start with this. Maybe we can remove the invasive clover at some time - but not under the scope of that issue!

        Show
        Uwe Schindler added a comment - Hi Robert, very cool. I had the same idea for clover a while back to have separate databases per thread, but i gave up, because clover has no merging functionality (at least I did not understand how to use it). Also the setup of clover in lucene was to have a one-for-all clover database... I checked you patch, very minimal invasive, cool! Just a few test policy files and the "hack" with relative paths for the database. I just wonder why the "clover" target is sometimes included as dependency to the new tasks, but maybe thats just copypaste. A further imprvement could be to have a root-level report with lucene+solr for Jenkins. Should be easy. Maybe make the report generator a macro in common-build. I still like the clover reports more (they have much nicer report output and functionality), but jacoco is fast and - as Grant said - does not need to istrument classes at compile time. So you can quickly switch between jacoco and non-jacoco without recompiling (for Clover you had to "ant clean" first). +1 to start with this. Maybe we can remove the invasive clover at some time - but not under the scope of that issue!
        Hide
        Uwe Schindler added a comment -

        +1 being able to create coverage reports on my local machine in a reasonable time will be super useful!

        This was possible with Clover, too. Just took longer

        Show
        Uwe Schindler added a comment - +1 being able to create coverage reports on my local machine in a reasonable time will be super useful! This was possible with Clover, too. Just took longer
        Hide
        Robert Muir added a comment -

        I just wonder why the "clover" target is sometimes included as dependency to the new tasks, but maybe thats just copypaste.

        No copypaste. Because tests may fail if you do not. this target initializes stuff even for the case where clover is not used! So it must be called. Seems like clover integration should be cleaned up a bit here.

        A further imprvement could be to have a root-level report with lucene+solr for Jenkins. Should be easy. Maybe make the report generator a macro in common-build.

        I don't want this. I am tired of not having a code coverage report in jenkins for lucene for months, and its because solr tests fail! Nobody wants to make solr tests reliable, so fine, i will make code coverage separate so that we have it again for lucene. That is why i worked on this issue, so that lucene will have code coverage again.

        Show
        Robert Muir added a comment - I just wonder why the "clover" target is sometimes included as dependency to the new tasks, but maybe thats just copypaste. No copypaste. Because tests may fail if you do not. this target initializes stuff even for the case where clover is not used! So it must be called. Seems like clover integration should be cleaned up a bit here. A further imprvement could be to have a root-level report with lucene+solr for Jenkins. Should be easy. Maybe make the report generator a macro in common-build. I don't want this. I am tired of not having a code coverage report in jenkins for lucene for months, and its because solr tests fail! Nobody wants to make solr tests reliable, so fine, i will make code coverage separate so that we have it again for lucene. That is why i worked on this issue, so that lucene will have code coverage again.
        Hide
        ASF subversion and git services added a comment -

        Commit 1672298 from Robert Muir in branch 'dev/trunk'
        [ https://svn.apache.org/r1672298 ]

        LUCENE-5439: add jacoco coverage

        Show
        ASF subversion and git services added a comment - Commit 1672298 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1672298 ] LUCENE-5439 : add jacoco coverage
        Hide
        ASF subversion and git services added a comment -

        Commit 1672300 from Robert Muir in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1672300 ]

        LUCENE-5439: add jacoco coverage

        Show
        ASF subversion and git services added a comment - Commit 1672300 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1672300 ] LUCENE-5439 : add jacoco coverage
        Hide
        Uwe Schindler added a comment - - edited

        I don't want this. I am tired of not having a code coverage report in jenkins for lucene for months, and its because solr tests fail!

        The code coverage is calculated in any case, because jenkins does not fail on broken tests (it passes some Dtests.dontfail like property, see root clover job.

        The problem why code coverage was "red" for several month was another issue: Clover OOMed because the -Xmx of Jenkins's ANT call was to small If solr tests fail, Jenkins just turns "yellow", but still produces coverage. Look at the console output... I just did not notice and forgot to fix! Sorry.

        Show
        Uwe Schindler added a comment - - edited I don't want this. I am tired of not having a code coverage report in jenkins for lucene for months, and its because solr tests fail! The code coverage is calculated in any case, because jenkins does not fail on broken tests (it passes some Dtests.dontfail like property, see root clover job. The problem why code coverage was "red" for several month was another issue: Clover OOMed because the -Xmx of Jenkins's ANT call was to small If solr tests fail, Jenkins just turns "yellow", but still produces coverage. Look at the console output... I just did not notice and forgot to fix! Sorry.
        Hide
        Uwe Schindler added a comment -

        This is what root's build.xml contains:

              <!-- The idea behind Clover is to determine test coverage, so be immune to failing tests: -->
              <param name="tests.haltonfailure" value="false"/>
        

        If we do the same there is no problem to also generate a combined coverage report for Jenkins. Users can still generate reports as they like for sub-modules. I will open another issue about that.

        Show
        Uwe Schindler added a comment - This is what root's build.xml contains: <!-- The idea behind Clover is to determine test coverage, so be immune to failing tests: --> <param name="tests.haltonfailure" value="false"/> If we do the same there is no problem to also generate a combined coverage report for Jenkins. Users can still generate reports as they like for sub-modules. I will open another issue about that.
        Hide
        Robert Muir added a comment -

        Sorry, I think i've explained my position. Feel free to open another issue, but personally my vote will be -1.

        I want to keep lucene code coverage working by itself and not influenced by unreliable solr tests.

        Show
        Robert Muir added a comment - Sorry, I think i've explained my position. Feel free to open another issue, but personally my vote will be -1. I want to keep lucene code coverage working by itself and not influenced by unreliable solr tests.
        Hide
        Uwe Schindler added a comment -

        I want to keep lucene code coverage working by itself and not influenced by unreliable solr tests.

        You can still calculate the coverage and there is no influence at all. Its just for Jenkins.

        Show
        Uwe Schindler added a comment - I want to keep lucene code coverage working by itself and not influenced by unreliable solr tests. You can still calculate the coverage and there is no influence at all. Its just for Jenkins.
        Hide
        Robert Muir added a comment -

        I also don't think combined coverage reports are useful. I only added one for each project here because its similar to what clover did: but these -all summaries do not highlight the source code so you cannot do much with them (you need to go to e.g. lucene-core report for that)

        Each module should test itself.

        Show
        Robert Muir added a comment - I also don't think combined coverage reports are useful. I only added one for each project here because its similar to what clover did: but these -all summaries do not highlight the source code so you cannot do much with them (you need to go to e.g. lucene-core report for that) Each module should test itself.
        Hide
        Uwe Schindler added a comment -

        I agree for jacoco. In Clover you have much more possibilities, so its also quite nice that you have combined coverage. Because when looking at block join module you see which calls are coming from Solr. As this is impossible with Jacoco, I agree.

        I also agree that we should keep Clover, it is much more fancy report-wise and records much more information (like call counts, which are useful to find hotspots) or from where the call is coming from. If it runs like 6 hours nightly on jenkins, who cares? But we get more results. For quickly testing coverage locally, a simple and fast ant task is much more useful.

        Show
        Uwe Schindler added a comment - I agree for jacoco. In Clover you have much more possibilities, so its also quite nice that you have combined coverage. Because when looking at block join module you see which calls are coming from Solr. As this is impossible with Jacoco, I agree. I also agree that we should keep Clover, it is much more fancy report-wise and records much more information (like call counts, which are useful to find hotspots) or from where the call is coming from. If it runs like 6 hours nightly on jenkins, who cares? But we get more results. For quickly testing coverage locally, a simple and fast ant task is much more useful.
        Hide
        Robert Muir added a comment -

        I agree, the clover report is better here. I would love to see it functioning again. But in the meantime I wanted something, to ensure we fix test coverage for the span queries.

        Show
        Robert Muir added a comment - I agree, the clover report is better here. I would love to see it functioning again. But in the meantime I wanted something, to ensure we fix test coverage for the span queries.
        Hide
        Uwe Schindler added a comment - - edited

        I would love to see it functioning again.

        The 5.x one works. But Trunk builds were disabled, because FreeBSD crushes Java 8. The migration of Lucene.zones to Ubuntu is on the go, I am just waiting for INFRA to activate it (see INFRA-9096).

        Show
        Uwe Schindler added a comment - - edited I would love to see it functioning again. The 5.x one works. But Trunk builds were disabled, because FreeBSD crushes Java 8. The migration of Lucene.zones to Ubuntu is on the go, I am just waiting for INFRA to activate it (see INFRA-9096 ).
        Hide
        Anshum Gupta added a comment -

        Bulk close for 5.2.0.

        Show
        Anshum Gupta added a comment - Bulk close for 5.2.0.

          People

          • Assignee:
            Robert Muir
            Reporter:
            Grant Ingersoll
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development