diff --git a/CHANGELOG b/CHANGELOG index 84f4238d..0c1ed30e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +10/25/2018 v1.0.5 + + * Add support for jacoco xml + 7/10/2018 v1.0.4 * Fix issue where whitespace was not trimmed pulling source file from cobertura causing a missed match diff --git a/README.rst b/README.rst index ac924cce..0c387a90 100644 --- a/README.rst +++ b/README.rst @@ -2,8 +2,8 @@ diff-cover |build-status| |coverage-status| |docs-status| ========================================================= Automatically find diff lines that need test coverage. -Also finds diff lines that have violations (according to tools such as pycodestyle, -Pyflakes, Flake8, or Pylint). +Also finds diff lines that have violations (according to tools such +as pycodestyle, pyflakes, flake8, or pylint). This is used as a code quality metric during code reviews. Overview @@ -22,18 +22,21 @@ for lines in the diff. Currently, ``diff-cover`` requires that: - You are using ``git`` for version control. -- Your test runner generates coverage reports in Cobertura XML format. +- Your test runner generates coverage reports in Cobertura, Clover + or JaCoCo XML format. -Cobertura XML coverage reports can be generated with many coverage tools, +Supported XML coverage reports can be generated with many coverage tools, including: - Cobertura__ (Java) - Clover__ (Java) +- JaCoCo__ (Java) - coverage.py__ (Python) - JSCover__ (JavaScript) __ http://cobertura.sourceforge.net/ __ http://openclover.org/ +__ https://www.jacoco.org/ __ http://nedbatchelder.com/code/coverage/ __ http://tntim96.github.io/JSCover/ @@ -67,7 +70,7 @@ Getting Started 1. Set the current working directory to a ``git`` repository. -2. Run your test suite under coverage and generate a Cobertura XML report. +2. Run your test suite under coverage and generate a [Cobertura, Clover or JaCoCo] XML report. For example, if you are using `nosetests`__ and `coverage.py`__: .. code:: bash diff --git a/diff_cover/tests/test_java_violations_reporter.py b/diff_cover/tests/test_java_violations_reporter.py index 017690bd..89d31b78 100644 --- a/diff_cover/tests/test_java_violations_reporter.py +++ b/diff_cover/tests/test_java_violations_reporter.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import os import xml.etree.cElementTree as etree from textwrap import dedent import unittest @@ -13,7 +14,7 @@ from diff_cover.command_runner import CommandError from diff_cover.violationsreporters.base import QualityReporter from diff_cover.violationsreporters.java_violations_reporter import ( - CloverXmlCoverageReporter, Violation, checkstyle_driver, + JacocoXmlCoverageReporter, CloverXmlCoverageReporter, Violation, checkstyle_driver, CheckstyleXmlDriver, FindbugsXmlDriver) @@ -30,7 +31,6 @@ def _setup_patch(return_value, status_code=0): mocked_subprocess.Popen.return_value = mocked_process return mocked_process - class CloverXmlCoverageReporterTest(unittest.TestCase): MANY_VIOLATIONS = {Violation(3, None), Violation(7, None), @@ -269,6 +269,247 @@ def _coverage_xml( line.set('type', 'stmt') return root +class JacocoXmlCoverageReporterTest(unittest.TestCase): + + MANY_VIOLATIONS = set([Violation(3, None), Violation(7, None), + Violation(11, None), Violation(13, None)]) + FEW_MEASURED = set([2, 3, 5, 7, 11, 13]) + + FEW_VIOLATIONS = set([Violation(3, None), Violation(11, None)]) + MANY_MEASURED = set([2, 3, 5, 7, 11, 13, 17]) + + ONE_VIOLATION = set([Violation(11, None)]) + VERY_MANY_MEASURED = set([2, 3, 5, 7, 11, 13, 17, 23, 24, 25, 26, 26, 27]) + + def setUp(self): + # Paths generated by git_path are always the given argument + _git_path_mock = patch('diff_cover.violationsreporters.java_violations_reporter.GitPathTool').start() + _git_path_mock.relative_path = lambda path: path + _git_path_mock.absolute_path = lambda path: path + + def tearDown(self): + patch.stopall() + + def test_violations(self): + + # Construct the XML report + file_paths = ['file1.java', 'subdir/file2.java'] + violations = self.MANY_VIOLATIONS + measured = self.FEW_MEASURED + xml = self._coverage_xml(file_paths, violations, measured) + + # Parse the report + coverage = JacocoXmlCoverageReporter([xml]) + + # Expect that the name is set + self.assertEqual(coverage.name(), "XML") + + # By construction, each file has the same set + # of covered/uncovered lines + self.assertEqual(violations, coverage.violations('file1.java')) + self.assertEqual(measured, coverage.measured_lines('file1.java')) + + # Try getting a smaller range + result = coverage.violations('subdir/file2.java') + self.assertEqual(result, violations) + + # Once more on the first file (for caching) + result = coverage.violations('file1.java') + self.assertEqual(result, violations) + + def test_two_inputs_first_violate(self): + + # Construct the XML report + file_paths = ['file1.java'] + + violations1 = self.MANY_VIOLATIONS + violations2 = self.FEW_VIOLATIONS + + measured1 = self.FEW_MEASURED + measured2 = self.MANY_MEASURED + + xml = self._coverage_xml(file_paths, violations1, measured1) + xml2 = self._coverage_xml(file_paths, violations2, measured2) + + # Parse the report + coverage = JacocoXmlCoverageReporter([xml, xml2]) + + # By construction, each file has the same set + # of covered/uncovered lines + self.assertEqual( + violations1 & violations2, + coverage.violations('file1.java') + ) + + self.assertEqual( + measured1 | measured2, + coverage.measured_lines('file1.java') + ) + + def test_two_inputs_second_violate(self): + + # Construct the XML report + file_paths = ['file1.java'] + + violations1 = self.MANY_VIOLATIONS + violations2 = self.FEW_VIOLATIONS + + measured1 = self.FEW_MEASURED + measured2 = self.MANY_MEASURED + + xml = self._coverage_xml(file_paths, violations1, measured1) + xml2 = self._coverage_xml(file_paths, violations2, measured2) + + # Parse the report + coverage = JacocoXmlCoverageReporter([xml2, xml]) + + # By construction, each file has the same set + # of covered/uncovered lines + self.assertEqual( + violations1 & violations2, + coverage.violations('file1.java') + ) + + self.assertEqual( + measured1 | measured2, + coverage.measured_lines('file1.java') + ) + + def test_three_inputs(self): + + # Construct the XML report + file_paths = ['file1.java'] + + violations1 = self.MANY_VIOLATIONS + violations2 = self.FEW_VIOLATIONS + violations3 = self.ONE_VIOLATION + + measured1 = self.FEW_MEASURED + measured2 = self.MANY_MEASURED + measured3 = self.VERY_MANY_MEASURED + + xml = self._coverage_xml(file_paths, violations1, measured1) + xml2 = self._coverage_xml(file_paths, violations2, measured2) + xml3 = self._coverage_xml(file_paths, violations3, measured3) + + # Parse the report + coverage = JacocoXmlCoverageReporter([xml2, xml, xml3]) + + # By construction, each file has the same set + # of covered/uncovered lines + self.assertEqual( + violations1 & violations2 & violations3, + coverage.violations('file1.java') + ) + + self.assertEqual( + measured1 | measured2 | measured3, + coverage.measured_lines('file1.java') + ) + + def test_different_files_in_inputs(self): + + # Construct the XML report + xml_roots = [ + self._coverage_xml(['file.java'], self.MANY_VIOLATIONS, self.FEW_MEASURED), + self._coverage_xml(['other_file.java'], self.FEW_VIOLATIONS, self.MANY_MEASURED) + ] + + # Parse the report + coverage = JacocoXmlCoverageReporter(xml_roots) + + self.assertEqual(self.MANY_VIOLATIONS, coverage.violations('file.java')) + self.assertEqual(self.FEW_VIOLATIONS, coverage.violations('other_file.java')) + + def test_empty_violations(self): + """ + Test that an empty violations report is handled properly + """ + + # Construct the XML report + file_paths = ['file1.java'] + + violations1 = self.MANY_VIOLATIONS + violations2 = set() + + measured1 = self.FEW_MEASURED + measured2 = self.MANY_MEASURED + + xml = self._coverage_xml(file_paths, violations1, measured1) + xml2 = self._coverage_xml(file_paths, violations2, measured2) + + # Parse the report + coverage = JacocoXmlCoverageReporter([xml2, xml]) + + # By construction, each file has the same set + # of covered/uncovered lines + self.assertEqual( + violations1 & violations2, + coverage.violations('file1.java') + ) + + self.assertEqual( + measured1 | measured2, + coverage.measured_lines('file1.java') + ) + + def test_no_such_file(self): + + # Construct the XML report with no source files + xml = self._coverage_xml([], [], []) + + # Parse the report + coverage = JacocoXmlCoverageReporter(xml) + + # Expect that we get no results + result = coverage.violations('file.java') + self.assertEqual(result, set([])) + + def _coverage_xml( + self, + file_paths, + violations, + measured + ): + """ + Build an XML tree with source files specified by `file_paths`. + Each source fill will have the same set of covered and + uncovered lines. + + `file_paths` is a list of path strings + `line_dict` is a dictionary with keys that are line numbers + and values that are True/False indicating whether the line + is covered + + This leaves out some attributes of the Cobertura format, + but includes all the elements. + """ + root = etree.Element('report') + root.set('name', 'diff-cover') + sessioninfo = etree.SubElement(root, 'sessioninfo') + sessioninfo.set('id', 'C13WQ1WFHTEE-83e2bc9b') + + + violation_lines = set(violation.line for violation in violations) + + for path in file_paths: + + package = etree.SubElement(root, 'package') + package.set('name', os.path.dirname(path)) + src_node = etree.SubElement(package, 'sourcefile') + src_node.set('name', os.path.basename(path)) + + # Create a node for each line in measured + for line_num in measured: + is_covered = line_num not in violation_lines + line = etree.SubElement(src_node, 'line') + + hits = 1 if is_covered else 0 + line.set('ci', str(hits)) + line.set('nr', str(line_num)) + return root + + class CheckstyleQualityReporterTest(unittest.TestCase): """Tests for checkstyle quality violations.""" diff --git a/diff_cover/tool.py b/diff_cover/tool.py index 2fad132c..db8f718c 100644 --- a/diff_cover/tool.py +++ b/diff_cover/tool.py @@ -26,7 +26,7 @@ jshint_driver, eslint_driver, pydocstyle_driver, pycodestyle_driver) from diff_cover.violationsreporters.java_violations_reporter import ( - CloverXmlCoverageReporter, + CloverXmlCoverageReporter, JacocoXmlCoverageReporter, CheckstyleXmlDriver, checkstyle_driver, FindbugsXmlDriver) QUALITY_DRIVERS = { @@ -53,6 +53,7 @@ IGNORE_STAGED_HELP = "Ignores staged changes" IGNORE_UNSTAGED_HELP = "Ignores unstaged changes" EXCLUDE_HELP = "Exclude files, more patterns supported" +SRC_ROOTS_HELP = "List of source directories (only for jacoco coverage reports)" LOGGER = logging.getLogger(__name__) @@ -136,6 +137,15 @@ def parse_coverage_args(argv): help=EXCLUDE_HELP ) + parser.add_argument( + '--src-roots', + metavar='DIRECTORY', + type=str, + nargs='+', + default=['src/main/java', 'src/test/java'], + help=SRC_ROOTS_HELP + ) + return vars(parser.parse_args(argv)) @@ -240,7 +250,7 @@ def parse_quality_args(argv): def generate_coverage_report(coverage_xml, compare_branch, html_report=None, css_file=None, ignore_staged=False, ignore_unstaged=False, - exclude=None): + exclude=None, src_roots=None): """ Generate the diff coverage report, using kwargs from `parse_args()`. """ @@ -251,14 +261,17 @@ def generate_coverage_report(coverage_xml, compare_branch, xml_roots = [cElementTree.parse(xml_root) for xml_root in coverage_xml] clover_xml_roots = [clover_xml for clover_xml in xml_roots if clover_xml.findall('.[@clover]')] cobertura_xml_roots = [cobertura_xml for cobertura_xml in xml_roots if cobertura_xml.findall('.[@line-rate]')] - if clover_xml_roots and cobertura_xml_roots: + jacoco_xml_roots = [jacoco_xml for jacoco_xml in xml_roots if jacoco_xml.findall('.[@name]')] + + if (clover_xml_roots and cobertura_xml_roots) or (clover_xml_roots and jacoco_xml_roots) or (cobertura_xml_roots and jacoco_xml_roots): raise TypeError("Can't handle mixed coverage reports") elif clover_xml_roots: coverage = CloverXmlCoverageReporter(clover_xml_roots) - elif cobertura_xml_roots: + elif jacoco_xml_roots: + coverage = JacocoXmlCoverageReporter(jacoco_xml_roots, src_roots) + else: coverage = XmlCoverageReporter(cobertura_xml_roots) - # Build a report generator if html_report is not None: css_url = css_file @@ -346,6 +359,7 @@ def main(argv=None, directory=None): ignore_staged=arg_dict['ignore_staged'], ignore_unstaged=arg_dict['ignore_unstaged'], exclude=arg_dict['exclude'], + src_roots=arg_dict['src_roots'], ) if percent_covered >= fail_under: diff --git a/diff_cover/violationsreporters/java_violations_reporter.py b/diff_cover/violationsreporters/java_violations_reporter.py index f8a56ff3..ef7ca6e6 100644 --- a/diff_cover/violationsreporters/java_violations_reporter.py +++ b/diff_cover/violationsreporters/java_violations_reporter.py @@ -3,6 +3,7 @@ """ from __future__ import unicode_literals +import os from collections import defaultdict import itertools @@ -118,6 +119,132 @@ def measured_lines(self, src_path): return self._info_cache[src_path][1] +class JacocoXmlCoverageReporter(BaseViolationReporter): + """ + Query information from a Jacoco XML coverage report. + """ + + def __init__(self, xml_roots, src_roots=None): + """ + Load the Jacoco XML coverage report represented + by the cElementTree with root element `xml_root`. + """ + super(JacocoXmlCoverageReporter, self).__init__("XML") + self._xml_roots = xml_roots + + # Create a dict to cache violations dict results + # Keys are source file paths, values are output of `violations()` + self._info_cache = defaultdict(list) + + if src_roots: + self._src_roots = src_roots + else: + self._src_roots = [''] + + def _measured_source_path_matches(self, package_name, file_name, src_path): + # find src_path in any of the source roots + for root in self._src_roots: + if GitPathTool.relative_path(os.path.join(root, package_name, file_name)) == src_path: + return True + + def _get_src_path_line_nodes(self, xml_document, src_path): + """ + Return a list of nodes containing line information for `src_path` + in `xml_document`. + + If file is not present in `xml_document`, return None + """ + + files = [] + packages = [pkg for pkg in xml_document.findall(".//package")] + for pkg in packages: + _files = [_file + for _file in pkg.findall('sourcefile') + if self._measured_source_path_matches(pkg.get('name'), _file.get('name'), src_path) + or [] + ] + files.extend(_files) + + if not files: + return None + lines = [file_tree.findall('./line') + for file_tree in files] + return [elem for elem in itertools.chain(*lines)] + + def _cache_file(self, src_path): + """ + Load the data from `self._xml_roots` + for `src_path`, if it hasn't been already. + """ + # If we have not yet loaded this source file + if src_path not in self._info_cache: + # We only want to keep violations that show up in each xml source. + # Thus, each time, we take the intersection. However, to do this + # we must treat the first time as a special case and just add all + # the violations from the first xml report. + violations = None + + # A line is measured if it is measured in any of the reports, so + # we take set union each time and can just start with the empty set + measured = set() + + # Loop through the files that contain the xml roots + for xml_document in self._xml_roots: + # https://github.com/jacoco/jacoco/blob/master/org.jacoco.report/src/org/jacoco/report/xml/report.dtd + _number = 'nr' + _hits = 'ci' + line_nodes = self._get_src_path_line_nodes(xml_document, + src_path) + + if line_nodes is None: + continue + + # First case, need to define violations initially + if violations is None: + violations = set( + Violation(int(line.get(_number)), None) + for line in line_nodes + if int(line.get(_hits, 0)) == 0) + + # If we already have a violations set, + # take the intersection of the new + # violations set and its old self + else: + violations = violations & set( + Violation(int(line.get(_number)), None) + for line in line_nodes + if int(line.get(_hits, 0)) == 0 + ) + + # Measured is the union of itself and the new measured + measured = measured | set( + int(line.get(_number)) for line in line_nodes + ) + + # If we don't have any information about the source file, + # don't report any violations + if violations is None: + violations = set() + self._info_cache[src_path] = (violations, measured) + + def violations(self, src_path): + """ + See base class comments. + """ + + self._cache_file(src_path) + + # Yield all lines not covered + return self._info_cache[src_path][0] + + def measured_lines(self, src_path): + """ + See base class docstring. + """ + self._cache_file(src_path) + return self._info_cache[src_path][1] + + """ Report checkstyle violations. diff --git a/requirements/test-requirements.txt b/requirements/test-requirements.txt index 01dc197a..929fb92b 100644 --- a/requirements/test-requirements.txt +++ b/requirements/test-requirements.txt @@ -2,7 +2,7 @@ mock nose coverage -pycodestyle<2.4.0 +pycodestyle>=2.4.0 flake8 pyflakes pylint