Working implementation of jacoco. (#4978)

jtrobec · Stu Hood · commit a6437101d975 · 2017-10-23T11:03:24.000-07:00
### Problem Jacoco is now available as an option, but the existing implementation is a no-op stub. ### Solution Filled out the implementation of the jacoco code coverage engine. ### Result Specifying jacoco as the code coverage processor now creates jacoco reports for junit tests. See the attachment for an example of the jacoco output against pants java code: [coverage.zip](https://github.com/pantsbuild/pants/files/1385892/coverage.zip) One place where I could use advice: what's the best way to handle working with the snapshot? I'm using the snapshot because it was the only available version of jacoco that included the client tools. I could write a wrapper around the reporting classes, but it seemed best to avoid that if there was already a cli I could use, and there is...just not in any of the release versions. It's not clear when the next release will be (https://groups.google.com/forum/#!topic/jacoco/gd8xD30TDNo), so my feeling is that moving forward with the current snapshot build (as long as it seems to work) is the best bet. However, maybe pants has some space where I could push the current snapshot so we can guarantee a stable version until the next jacoco release hits maven central? Please let me know what seems best.
diff --git a/build-support/ivy/ivysettings.xml b/build-support/ivy/ivysettings.xml
@@ -13,6 +13,9 @@ Licensed under the Apache License, Version 2.0 (see LICENSE).
             value="${m2.repo.relpath}/[artifact](-[classifier])-[revision].[ext]"/>
   <property name="m2.repo.dir" value="${user.home}/.m2/repository" override="false"/>
   <property name="kythe.artifact" value="[organization]-[revision]/[artifact].[ext]"/>
+  <!-- for retrieving jacoco snapshot, remove when a version containing cli is released -->
+  <!-- see https://github.com/pantsbuild/pants/issues/5010 -->
+  <property name="sonatype.nexus.snapshots.url" value="https://oss.sonatype.org/content/repositories/snapshots/" />
 
   <!-- This dance to set ANDROID_HOME seems more complex than it need be, but an absolute path is
        needed whether or not the ANDROID_HOME env var is set and just using the following does
@@ -69,6 +72,10 @@ Licensed under the Apache License, Version 2.0 (see LICENSE).
       <url name="benjyw/binhost">
         <artifact pattern="https://github.com/benjyw/binhost/raw/master/[organisation]/${kythe.artifact}" />
       </url>
+
+      <!-- for retrieving jacoco snapshot, remove when a version containing cli is released -->
+      <!-- see https://github.com/pantsbuild/pants/issues/5010 -->
+      <ibiblio name="sonatype-nexus-snapshots" m2compatible="true" root="${sonatype.nexus.snapshots.url} "/>
     </chain>
   </resolvers>
 </ivysettings>
diff --git a/src/python/pants/backend/jvm/tasks/coverage/cobertura.py b/src/python/pants/backend/jvm/tasks/coverage/cobertura.py
@@ -93,8 +93,8 @@ def __init__(self, settings, targets, execute_java_for_targets):
     self._settings = settings
     options = settings.options
     self._context = settings.context
-    self._coverage = options.coverage
     self._coverage_datafile = os.path.join(settings.coverage_dir, 'cobertura.ser')
+    self._coverage_force = options.coverage_force
     touch(self._coverage_datafile)
     self._rootdirs = defaultdict(OrderedSet)
     self._include_classes = options.coverage_cobertura_include_classes
@@ -184,7 +184,7 @@ def instrument(self):
         args += ["--listOfFilesToInstrument", tmp_file.name]
 
         main = 'net.sourceforge.cobertura.instrument.InstrumentMain'
-        self._context.log.debug(
+        self._settings.log.debug(
           "executing cobertura instrumentation with the following args: {}".format(args))
         result = self._execute_java(classpath=cobertura_cp,
                                     main=main,
@@ -208,40 +208,45 @@ def classpath_prepend(self):
   def extra_jvm_options(self):
     return ['-Dnet.sourceforge.cobertura.datafile=' + self._coverage_datafile]
 
-  def report(self, execution_failed_exception=None):
+  def should_report(self, execution_failed_exception=None):
     if self._nothing_to_instrument:
-      self._context.log.warn('Nothing found to instrument, skipping report...')
-      return
+      self._settings.log.warn('Nothing found to instrument, skipping report...')
+      return False
     if execution_failed_exception:
-      self._context.log.warn('Test failed: {0}'.format(execution_failed_exception))
+      self._settings.log.warn('Test failed: {0}'.format(execution_failed_exception))
       if self._settings.coverage_force:
-        self._context.log.warn('Generating report even though tests failed.')
+        self._settings.log.warn('Generating report even though tests failed.')
+        return True
       else:
-        return
-    cobertura_cp = self._settings.tool_classpath('cobertura-report')
-    source_roots = {t.target_base for t in self._targets if Cobertura.is_coverage_target(t)}
-    for report_format in ['xml', 'html']:
-      report_dir = os.path.join(self._settings.coverage_dir, report_format)
-      safe_mkdir(report_dir, clean=True)
-      args = list(source_roots)
-      args += [
-        '--datafile',
-        self._coverage_datafile,
-        '--destination',
-        report_dir,
-        '--format',
-        report_format,
-      ]
-      main = 'net.sourceforge.cobertura.reporting.ReportMain'
-      result = self._execute_java(classpath=cobertura_cp,
-                                  main=main,
-                                  jvm_options=self._settings.coverage_jvm_options,
-                                  args=args,
-                                  workunit_factory=self._context.new_workunit,
-                                  workunit_name='cobertura-report-' + report_format)
-      if result != 0:
-        raise TaskError("java {0} ... exited non-zero ({1})"
-                        " 'failed to report'".format(main, result))
+        return False
+    return True
+
+  def report(self, execution_failed_exception=None):
+    if self.should_report(execution_failed_exception):
+      cobertura_cp = self._settings.tool_classpath('cobertura-report')
+      source_roots = {t.target_base for t in self._targets if Cobertura.is_coverage_target(t)}
+      for report_format in ['xml', 'html']:
+        report_dir = os.path.join(self._settings.coverage_dir, report_format)
+        safe_mkdir(report_dir, clean=True)
+        args = list(source_roots)
+        args += [
+          '--datafile',
+          self._coverage_datafile,
+          '--destination',
+          report_dir,
+          '--format',
+          report_format,
+        ]
+        main = 'net.sourceforge.cobertura.reporting.ReportMain'
+        result = self._execute_java(classpath=cobertura_cp,
+                                    main=main,
+                                    jvm_options=self._settings.coverage_jvm_options,
+                                    args=args,
+                                    workunit_factory=self._context.new_workunit,
+                                    workunit_name='cobertura-report-' + report_format)
+        if result != 0:
+          raise TaskError("java {0} ... exited non-zero ({1})"
+                          " 'failed to report'".format(main, result))
 
   def maybe_open_report(self):
     if self._settings.coverage_open:
diff --git a/src/python/pants/backend/jvm/tasks/coverage/jacoco.py b/src/python/pants/backend/jvm/tasks/coverage/jacoco.py
@@ -6,19 +6,54 @@
                         unicode_literals, with_statement)
 
 import functools
+import os
 
+from pants.backend.jvm.subsystems.jvm_tool_mixin import JvmToolMixin
 from pants.backend.jvm.tasks.coverage.engine import CoverageEngine
+from pants.base.exceptions import TaskError
+from pants.java.jar.jar_dependency import JarDependency
 from pants.subsystem.subsystem import Subsystem
+from pants.util import desktop
+from pants.util.dirutil import safe_delete, safe_mkdir
 
 
 class Jacoco(CoverageEngine):
   """Class to run coverage tests with Jacoco."""
 
-  class Factory(Subsystem):
+  class Factory(Subsystem, JvmToolMixin):
     options_scope = 'jacoco'
 
     @classmethod
-    def create(cls, settings, targets, execute_java_for_targets):
+    def register_options(cls, register):
+      super(Jacoco.Factory, cls).register_options(register)
+
+      # We need to inject the jacoco agent at test runtime
+      cls.register_jvm_tool(register,
+                        'jacoco-agent',
+                        classpath=[
+                          JarDependency(
+                            org='org.jacoco',
+                            name='org.jacoco.agent',
+                            # TODO(jtrobec): get off of snapshat once jacoco release with cli is available
+                            # see https://github.com/pantsbuild/pants/issues/5010
+                            rev='0.7.10-SNAPSHOT',
+                            classifier='runtime',
+                            intransitive=True)
+                        ])
+
+      # We'll use the jacoco-cli to generate reports
+      cls.register_jvm_tool(register,
+                        'jacoco-cli',
+                        classpath=[
+                          JarDependency(
+                            org='org.jacoco',
+                            name='org.jacoco.cli',
+                            # TODO(jtrobec): get off of snapshat once jacoco release with cli is available
+                            # see https://github.com/pantsbuild/pants/issues/5010
+                            rev='0.7.10-SNAPSHOT')
+                        ])
+
+    def create(self, settings, targets, execute_java_for_targets):
       """
       :param settings: Generic code coverage settings.
       :type settings: :class:`CodeCoverageSettings`
@@ -30,9 +65,11 @@ def create(cls, settings, targets, execute_java_for_targets):
                                        `pants.java.util.execute_java`.
       """
 
-      return Jacoco(settings, targets, execute_java_for_targets)
+      agent_path = self.tool_jar_from_products(settings.context.products, 'jacoco-agent', scope='jacoco')
+      cli_path = self.tool_classpath_from_products(settings.context.products, 'jacoco-cli', scope='jacoco')
+      return Jacoco(settings, agent_path, cli_path, targets, execute_java_for_targets)
 
-  def __init__(self, settings, targets, execute_java_for_targets):
+  def __init__(self, settings, agent_path, cli_path, targets, execute_java_for_targets):
     """
     :param settings: Generic code coverage settings.
     :type settings: :class:`CodeCoverageSettings`
@@ -44,12 +81,20 @@ def __init__(self, settings, targets, execute_java_for_targets):
                                      `pants.java.util.execute_java`.
     """
     self._settings = settings
+    options = settings.options
+    self._context = settings.context
     self._targets = targets
+    self._coverage_targets = {t for t in targets if Jacoco.is_coverage_target(t)}
+    self._agent_path = agent_path
+    self._cli_path = cli_path
     self._execute_java = functools.partial(execute_java_for_targets, targets)
+    self._coverage_force = options.coverage_force
+    self._coverage_datafile = os.path.join(settings.coverage_dir, 'jacoco.exec')
+    self._coverage_report_dir = os.path.join(settings.coverage_dir, 'reports')
 
   def instrument(self):
-    # jacoco does runtime instrumentation, so this is a noop
-    pass
+    # jacoco does runtime instrumentation, so this only does clean-up of existing run
+    safe_delete(self._coverage_datafile)
 
   @property
   def classpath_append(self):
@@ -61,13 +106,71 @@ def classpath_prepend(self):
 
   @property
   def extra_jvm_options(self):
-    # TODO(jtrobec): implement code coverage using jacoco
-    return []
+    agent_option = '-javaagent:{agent}=destfile={destfile}'.format(agent=self._agent_path,
+                                                                   destfile=self._coverage_datafile)
+    return [agent_option]
+
+  @staticmethod
+  def is_coverage_target(tgt):
+    return (tgt.is_java or tgt.is_scala) and not tgt.is_test and not tgt.is_synthetic
 
   def report(self, execution_failed_exception=None):
-    # TODO(jtrobec): implement code coverage using jacoco
-    pass
+    if execution_failed_exception:
+      self._settings.log.warn('Test failed: {0}'.format(execution_failed_exception))
+      if self._coverage_force:
+        self._settings.log.warn('Generating report even though tests failed, because the coverage-force flag is set.')
+      else:
+        return
+
+    safe_mkdir(self._coverage_report_dir, clean=True)
+    for report_format in ['xml', 'csv', 'html']:
+      target_path = os.path.join(self._coverage_report_dir, report_format)
+      args = ['report', self._coverage_datafile] + self._get_target_classpaths() + self._get_source_roots() + [
+        '--{report_format}={target_path}'.format(report_format=report_format,
+                                                 target_path=target_path)
+      ]
+      main = 'net.sourceforge.cobertura.reporting.ReportMain'
+      result = self._execute_java(classpath=self._cli_path,
+                                  main='org.jacoco.cli.internal.Main',
+                                  jvm_options=self._settings.coverage_jvm_options,
+                                  args=args,
+                                  workunit_factory=self._context.new_workunit,
+                                  workunit_name='jacoco-report-' + report_format)
+      if result != 0:
+        raise TaskError("java {0} ... exited non-zero ({1})"
+                        " 'failed to report'".format(main, result))
+
+  def _get_target_classpaths(self):
+    runtime_classpath = self._context.products.get_data('runtime_classpath')
+
+    target_paths = []
+    for target in self._coverage_targets:
+      paths = runtime_classpath.get_for_target(target)
+      for (name, path) in paths:
+        target_paths.append(path)
+
+    return self._make_multiple_arg('--classfiles', target_paths)
+
+  def _get_source_roots(self):
+    source_roots = {t.target_base for t in self._coverage_targets}
+    return self._make_multiple_arg('--sourcefiles', source_roots)
+
+  def _make_multiple_arg(self, arg_name, arg_list):
+    """Jacoco cli allows the specification of multiple values for certain args by repeating the argument
+    with a new value. E.g. --classfiles a.class --classfiles b.class, etc. This method creates a list of
+    strings interleaved with the arg name to satisfy that format.
+    """
+    unique_args = list(set(arg_list))
+
+    args = [(arg_name, f) for f in unique_args]
+    flattened = list(sum(args, ()))
+
+    return flattened
 
   def maybe_open_report(self):
-    # TODO(jtrobec): implement code coverage using jacoco
-    pass
+    if self._settings.coverage_open:
+      report_file_path = os.path.join(self._settings.coverage_dir, 'reports/html', 'index.html')
+      try:
+        desktop.ui_open(report_file_path)
+      except desktop.OpenError as e:
+        raise TaskError(e)
diff --git a/tests/python/pants_test/backend/jvm/tasks/coverage/test_cobertura.py b/tests/python/pants_test/backend/jvm/tasks/coverage/test_cobertura.py
@@ -40,6 +40,12 @@ def debug(self, string):
     """
     pass
 
+  def warn(self, string):
+    """
+    :API: public
+    """
+    pass
+
 
 class MockSystemCalls(object):
   """
@@ -85,6 +91,7 @@ def setUp(self):
 
     self.pants_workdir = 'workdir'
     self.conf = 'default'
+    self.factory = Cobertura.Factory("test_scope", [])
 
     self.jar_lib = self.make_target(spec='3rdparty/jvm/org/example:foo',
                                     target_type=JarLibrary,
@@ -195,3 +202,40 @@ def test_target_annotation_processor(self):
     self.assertEquals(len(syscalls.copy2_calls), 0,
                       'Should be 0 call for the single annotation target.')
     self._assert_target_copytree(syscalls, '/anno/target/dir', '/coverage/classes/foo.foo-anno/0')
+
+  def _get_fake_execute_java(self):
+    def _fake_execute_java(classpath, main, jvm_options, args, workunit_factory, workunit_name):
+      # at some point we could add assertions here for expected paramerter values
+      pass
+    return _fake_execute_java
+
+  def test_coverage_forced(self):
+    """
+    :API: public
+    """
+    options = attrdict(coverage=True, coverage_force=True, coverage_jvm_options=[])
+
+    syscalls = MockSystemCalls()
+    settings = self.get_settings(options, self.pants_workdir, fake_log(), syscalls)
+    cobertura = self.factory.create(settings, [self.binary_target], self._get_fake_execute_java())
+
+    self.assertEquals(cobertura.should_report(), False, 'Without instrumentation step, there should be nothing to instrument or report')
+
+    # simulate an instrument step with results
+    cobertura._nothing_to_instrument = False
+
+    self.assertEquals(cobertura.should_report(), True, 'Should do reporting when there is something to instrument')
+
+    exception = Exception("uh oh, test failed")
+
+    self.assertEquals(cobertura.should_report(exception), True, 'We\'ve forced coverage, so should report.')
+
+    no_force_options = attrdict(coverage=True, coverage_force=False, coverage_jvm_options=[])
+    no_force_settings = self.get_settings(no_force_options, self.pants_workdir, fake_log(), syscalls)
+    no_force_cobertura = self.factory.create(no_force_settings, [self.binary_target], self._get_fake_execute_java())
+
+    no_force_cobertura._nothing_to_instrument = False
+    self.assertEquals(no_force_cobertura.should_report(exception), False, 'Don\'t report after a failure if coverage isn\'t forced.')
+
+
+
diff --git a/tests/python/pants_test/backend/jvm/tasks/test_junit_run_integration.py b/tests/python/pants_test/backend/jvm/tasks/test_junit_run_integration.py
@@ -56,6 +56,27 @@ def test_junit_run_with_cobertura_coverage_succeeds(self):
       with codecs.open(cucumber_src_html, 'r', encoding='utf8') as src:
         self.assertIn('String pleasantry()', src.read())
 
+  def test_junit_run_with_jacoco_coverage_succeeds(self):
+    with self.pants_results(['clean-all',
+                             'test.junit',
+                             'testprojects/tests/java/org/pantsbuild/testproject/unicode::',
+                             '--test-junit-coverage-processor=jacoco',
+                             '--test-junit-coverage']) as results:
+      self.assert_success(results)
+      # validate that the expected coverage file exists, and it reflects 100% line rate coverage
+      coverage_xml = os.path.join(results.workdir, 'test/junit/coverage/reports/xml')
+      self.assertTrue(os.path.isfile(coverage_xml))
+      with codecs.open(coverage_xml, 'r', encoding='utf8') as xml:
+        self.assertIn('<class name="org/pantsbuild/testproject/unicode/cucumber/CucumberAnnotatedExample"><method name="&lt;init&gt;" desc="()V" line="13"><counter type="INSTRUCTION" missed="0" covered="3"/>', xml.read())
+      # validate that the html report was able to find sources for annotation
+      cucumber_src_html = os.path.join(
+          results.workdir,
+          'test/junit/coverage/reports/html/'
+          'org.pantsbuild.testproject.unicode.cucumber/CucumberAnnotatedExample.html')
+      self.assertTrue(os.path.isfile(cucumber_src_html))
+      with codecs.open(cucumber_src_html, 'r', encoding='utf8') as src:
+        self.assertIn('class="el_method">pleasantry()</a>', src.read())
+
   def test_junit_run_against_invalid_class_fails(self):
     pants_run = self.run_pants(['clean-all',
                                 'test.junit',
