Derby
  1. Derby
  2. DERBY-5817

Add support for the JaCoCo code coverage tool

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.10.1.1
    • Fix Version/s: 10.10.1.1
    • Component/s: Miscellaneous, Test
    • Labels:
      None

      Description

      Add support for the JaCoCo code coverage tool ([1]).
      While we are currently relying on EMMA for code coverage, the tool isn't being maintained. It works nicely for the time being, but having additional support for a tool that's being actively developed and maintained would be nice.

      [1] http://www.eclemma.org/jacoco/

      1. derby-5817-1a-jacoco_support.diff
        10 kB
        Kristian Waagan
      2. derby-5817-1b-jacoco_support.diff
        13 kB
        Kristian Waagan
      3. derby-5817-1c-jacoco_support.diff
        16 kB
        Kristian Waagan

        Activity

        Kristian Waagan created issue -
        Hide
        Kristian Waagan added a comment -

        Attaching patch 1a, which adds support for JaCoCo.

        The change in build.xml enables JaCoCo, and the change in BaseTestCase ensure that JaCoCo will also be enabled in spawned Java processes (as for EMMA).

        There are two new top-level targets, based on junit-all and junit-single:
        jacoco-all
        jacoco-single

        You have to download JaCoCo and place the two jar files jacocoant.jar and jacocoagent.jar in tools/java. The targets above will complain if these files are missing.

        When running jacoco-all I'm still seeing one error, which appears to be a problem with ASM, which is used by JaCoCo. I'll see if I can find any more information on this issue. The error doesn't cause the run to be aborted, and the coverage data appears ok.
        The report ends up in junit_

        {timestamp}

        /coverage-report.

        Patch ready for review.

        Show
        Kristian Waagan added a comment - Attaching patch 1a, which adds support for JaCoCo. The change in build.xml enables JaCoCo, and the change in BaseTestCase ensure that JaCoCo will also be enabled in spawned Java processes (as for EMMA). There are two new top-level targets, based on junit-all and junit-single: jacoco-all jacoco-single You have to download JaCoCo and place the two jar files jacocoant.jar and jacocoagent.jar in tools/java. The targets above will complain if these files are missing. When running jacoco-all I'm still seeing one error, which appears to be a problem with ASM, which is used by JaCoCo. I'll see if I can find any more information on this issue. The error doesn't cause the run to be aborted, and the coverage data appears ok. The report ends up in junit_ {timestamp} /coverage-report. Patch ready for review.
        Kristian Waagan made changes -
        Field Original Value New Value
        Attachment derby-5817-1a-jacoco_support.diff [ 12532058 ]
        Hide
        Kristian Waagan added a comment -

        Attaching patch 1b, which also adds a derbyall target.

        Needs some additional testing, but feel free to provide feedback.
        Not quite sure what to do about junit-pptesting yet, since it requires the classes-directory to be present. I'll probably move it away from junit-all and run it explicitly.

        With this patch the coverage increased with about 2 percent. The compatibility test is still not included (I want rewrite it first ).

        Show
        Kristian Waagan added a comment - Attaching patch 1b, which also adds a derbyall target. Needs some additional testing, but feel free to provide feedback. Not quite sure what to do about junit-pptesting yet, since it requires the classes-directory to be present. I'll probably move it away from junit-all and run it explicitly. With this patch the coverage increased with about 2 percent. The compatibility test is still not included (I want rewrite it first ).
        Kristian Waagan made changes -
        Attachment derby-5817-1b-jacoco_support.diff [ 12532192 ]
        Hide
        Kristian Waagan added a comment -

        Attaching patch 1c, which further improves the JaCoCo support in build.xml.

        I refactored some code related to getting the svn version, because I'm adding the version to the report title.

        I updated the report at people.apache.org/~kristwaa/jacoco/ with the results produced by patch 1c and the target jacoco-complete.

        I think the EMMA results include coverage data from runs with several different JVM versions. I have not added support for that, but assuming the merged.exec file is kept from each run a separate merge/report target can be used.

        Patch ready for review.

        Show
        Kristian Waagan added a comment - Attaching patch 1c, which further improves the JaCoCo support in build.xml. I refactored some code related to getting the svn version, because I'm adding the version to the report title. I updated the report at people.apache.org/~kristwaa/jacoco/ with the results produced by patch 1c and the target jacoco-complete. I think the EMMA results include coverage data from runs with several different JVM versions. I have not added support for that, but assuming the merged.exec file is kept from each run a separate merge/report target can be used. Patch ready for review.
        Kristian Waagan made changes -
        Attachment derby-5817-1c-jacoco_support.diff [ 12532398 ]
        Kristian Waagan made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Kristian Waagan added a comment -

        Committed patch 1c to trunk with revision 1352502.

        This concludes the initial work on adding support for JaCoCo. Further improvements can be tracked under their own JIRAs.

        Show
        Kristian Waagan added a comment - Committed patch 1c to trunk with revision 1352502. This concludes the initial work on adding support for JaCoCo. Further improvements can be tracked under their own JIRAs.
        Kristian Waagan made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Fix Version/s 10.10.0.0 [ 12321550 ]
        Resolution Fixed [ 1 ]
        Hide
        Kristian Waagan added a comment -

        I was suspecting that JaCoCo was slower than EMMA, but the results from a new test disagrees.

        Here's what I saw from runs on a Windows Vista machine:
        o ant junit-all: ~107 minutes
        o ant emma-all: ~135 minutes
        o ant jacoco-junit: ~134 minutes

        The times for EMMA and JaCoCo are comparable, but there are some differences (EMMAs instrumentation step, the JaCoCo target doesn't build a report - use ant jacoco-complete for that).
        I did see some warnings where files couldn't be deleted immediately, so it appears we still have issues in the tests where resources aren't closed timely or we otherwise do concurrent access on files.

        Closing the issue.

        Show
        Kristian Waagan added a comment - I was suspecting that JaCoCo was slower than EMMA, but the results from a new test disagrees. Here's what I saw from runs on a Windows Vista machine: o ant junit-all: ~107 minutes o ant emma-all: ~135 minutes o ant jacoco-junit: ~134 minutes The times for EMMA and JaCoCo are comparable, but there are some differences (EMMAs instrumentation step, the JaCoCo target doesn't build a report - use ant jacoco-complete for that). I did see some warnings where files couldn't be deleted immediately, so it appears we still have issues in the tests where resources aren't closed timely or we otherwise do concurrent access on files. Closing the issue.
        Kristian Waagan made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Gavin made changes -
        Workflow jira [ 12673316 ] Default workflow, editable Closed status [ 12802326 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        4d 3h 21m 1 Kristian Waagan 18/Jun/12 11:26
        In Progress In Progress Resolved Resolved
        3d 20m 1 Kristian Waagan 21/Jun/12 11:46
        Resolved Resolved Closed Closed
        230d 8h 54m 1 Kristian Waagan 06/Feb/13 20:41

          People

          • Assignee:
            Kristian Waagan
            Reporter:
            Kristian Waagan
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development