Merge pull request #15 from benpatterson/benp/jshint-violations

Benp/jshint violations

diff_cover/tests/test_violations_reporter.py

@@ -9,7 +9,8 @@
 import six
 from diff_cover.violations_reporter import XmlCoverageReporter, Violation, \
     Pep8QualityReporter, PyflakesQualityReporter, PylintQualityReporter, \
-    QualityReporterError, Flake8QualityReporter
+    QualityReporterError, Flake8QualityReporter, JsHintQualityReporter, \
+    BaseQualityReporter
 from diff_cover.tests.helpers import unittest
 
 
@@ -976,7 +977,191 @@ def test_quality_pregenerated_report_continuation_char(self):
         self.assertEqual(violations, [Violation(2, u"W1401: Invalid char '\ufffd'")])
 
 
+class JsHintQualityReporterTest(unittest.TestCase):
+    """
+    JsHintQualityReporter tests. Assumes JsHint is not available as a python library,
+    but is available on the commandline.
+    """
+
+    def setUp(self):
+        # Mock patch the installation of jshint
+        self._mock_command_simple = patch.object(JsHintQualityReporter, '_run_command_simple').start()
+        self._mock_command_simple.return_value = 0
+        # Mock patch the jshint results
+        self._mock_communicate = patch.object(subprocess, 'Popen').start()
+        self.subproc_mock = MagicMock()
+        self.subproc_mock.returncode = 0
+
+    def tearDown(self):
+        """
+        Undo all patches
+        """
+        patch.stopall()
+
+    def test_quality(self):
+        """
+        Test basic scenarios, including special characters that would appear in JavaScript and mixed quotation marks
+        """
+
+        # Patch the output of `jshint`
+        return_string = '\n' + dedent("""
+                ../test_file.js: line 3, col 9, Missing "use strict" statement.
+                ../test_file.js: line 10, col 17, '$hi' is defined but never used.
+            """).strip() + '\n'
+        self.subproc_mock.communicate.return_value = (
+            (return_string.encode('utf-8'), b''))
+        self._mock_communicate.return_value = self.subproc_mock
+
+        # Parse the report
+        quality = JsHintQualityReporter('jshint', [])
+
+        # Expect that the name is set
+        self.assertEqual(quality.name(), 'jshint')
+
+        # Measured_lines is undefined for
+        # a quality reporter since all lines are measured
+        self.assertEqual(quality.measured_lines('../blah.js'), None)
+
+        # Expect that we get the right violations
+        expected_violations = [
+            Violation(3, 'Missing "use strict" statement.'),
+            Violation(10, "'$hi' is defined but never used."),
+        ]
+
+        self.assertEqual(expected_violations, quality.violations('../test_file.js'))
+
+    def test_no_quality_issues_newline(self):
+
+        # Patch the output of `jshint`
+        self.subproc_mock.communicate.return_value = (b'\n', b'')
+        self._mock_communicate.return_value = self.subproc_mock
+
+        # Parse the report
+        quality = JsHintQualityReporter('jshint', [])
+        self.assertEqual([], quality.violations('test-file.js'))
+
+    def test_no_quality_issues_emptystring(self):
+
+        # Patch the output of `jshint`
+        self.subproc_mock.communicate.return_value = (b'', b'')
+        self._mock_communicate.return_value = self.subproc_mock
+
+        # Parse the report
+        quality = JsHintQualityReporter('jshint', [])
+        self.assertEqual([], quality.violations('file1.js'))
+
+    def test_quality_error(self):
+
+        # Override the subprocess return code to a failure
+        self.subproc_mock.returncode = 1
+
+        # Patch the output of `jshint`
+        self.subproc_mock.communicate.return_value = (b"", 'whoops Ƕئ'.encode('utf-8'))
+        self._mock_communicate.return_value = self.subproc_mock
+
+        # Parse the report
+        quality = JsHintQualityReporter('jshint', [])
+
+        # Expect that the name is set
+        self.assertEqual(quality.name(), 'jshint')
+        with self.assertRaises(QualityReporterError) as ex:
+            quality.violations('file1.js')
+        self.assertEqual(six.text_type(ex.exception), 'whoops Ƕئ')
+
+    def test_no_such_file(self):
+        quality = JsHintQualityReporter('jshint', [])
+
+        # Expect that we get no results
+        result = quality.violations('')
+        self.assertEqual(result, [])
+
+    def test_no_js_file(self):
+        quality = JsHintQualityReporter('jshint', [])
+        file_paths = ['file1.py', 'subdir/file2.java']
+        # Expect that we get no results because no JS files
+        for path in file_paths:
+            result = quality.violations(path)
+            self.assertEqual(result, [])
+
+    def test_quality_pregenerated_report(self):
+
+        # When the user provides us with a pre-generated jshint report
+        # then use that instead of calling jshint directly.
+        jshint_reports = [
+            BytesIO(('\n' + dedent("""
+                path/to/file.js: line 3, col 9, Missing "use strict" statement.
+                path/to/file.js: line 10, col 130, Line is too long.
+                another/file.js: line 1, col 1, 'require' is not defined.
+            """).strip() + '\n').encode('utf-8')),
+
+            BytesIO(('\n' + dedent(u"""
+                path/to/file.js: line 12, col 14, \u9134\u1912
+                path/to/file.js: line 10, col 17, '$hi' is defined but never used.
+            """).strip() + '\n').encode('utf-8')),
+        ]
+
+        # Parse the report
+        quality = JsHintQualityReporter('jshint', jshint_reports)
+
+        # Measured_lines is undefined for
+        # a quality reporter since all lines are measured
+        self.assertEqual(quality.measured_lines('path/to/file.js'), None)
+
+        # Expect that we get the right violations
+        expected_violations = [
+            Violation(3, u'Missing "use strict" statement.'),
+            Violation(10, u"Line is too long."),
+            Violation(10, u"'$hi' is defined but never used."),
+            Violation(12, u"\u9134\u1912")
+        ]
+
+        # We're not guaranteed that the violations are returned
+        # in any particular order.
+        actual_violations = quality.violations('path/to/file.js')
+
+        self.assertEqual(len(actual_violations), len(expected_violations))
+        for expected in expected_violations:
+            self.assertIn(expected, actual_violations)
+
+    def test_not_installed(self):
+        """
+        If jshint is not available via commandline, it should raise an EnvironmentError
+        """
+        self._mock_command_simple = patch.object(JsHintQualityReporter, '_run_command_simple').start()
+        self._mock_command_simple.return_value = 1
+        with self.assertRaises(EnvironmentError):
+            JsHintQualityReporter('jshint', [])
+
+
+class SimpleCommandTestCase(unittest.TestCase):
+    """
+    Tests that the exit code detected by the method is passed as the return value of the method.
+    """
+
+    def setUp(self):
+        self._mock_communicate = patch.object(subprocess, 'Popen').start()
+        self.subproc_mock = MagicMock()
+
+    def test_run_simple_failure(self):
+        # command_simple should fail
+        self.subproc_mock.returncode = 127
+        self._mock_communicate.return_value = self.subproc_mock
+        # Create an implementation of BaseQualityReporter and explicitly call _run_command_simple
+        bad_command = BaseQualityReporter('collections', [])._run_command_simple('foo')  # pylint: disable=protected-access
+        self.assertEquals(bad_command, 127)
+
+    def test_run_simple_success(self):
+        self.subproc_mock.returncode = 0
+        self._mock_communicate.return_value = self.subproc_mock
+        # Create an implementation of BaseQualityReporter and explicitly call _run_command_simple
+        good_command = BaseQualityReporter('collections', [])._run_command_simple('foo')  # pylint: disable=protected-access
+        self.assertEquals(good_command, 0)
+
+
 class SubprocessErrorTestCase(unittest.TestCase):
+    """
+    Error in subprocess call(s)
+    """
     def setUp(self):
         # when you create a new subprocess.Popen() object and call .communicate()
         # on it, raise an OSError

diff_cover/tool.py

@@ -13,7 +13,7 @@
 from diff_cover.violations_reporter import (
     XmlCoverageReporter, Pep8QualityReporter,
     PyflakesQualityReporter, PylintQualityReporter,
-    Flake8QualityReporter
+    Flake8QualityReporter, JsHintQualityReporter
 )
 from diff_cover.report_generator import (
     HtmlReportGenerator, StringReportGenerator,
@@ -35,6 +35,7 @@
     'pyflakes': PyflakesQualityReporter,
     'pylint': PylintQualityReporter,
     'flake8': Flake8QualityReporter,
+    'jshint': JsHintQualityReporter,
 }
 
 
@@ -283,7 +284,7 @@ def main(argv=None, directory=None):
                     LOGGER.error("Failure. Quality is below {0}%.".format(fail_under))
                     return 1
 
-            except ImportError:
+            except (ImportError, EnvironmentError):
                 LOGGER.error(
                     "Quality tool not installed: '{0}'".format(tool)
                 )

diff_cover/violations_reporter.py

@@ -222,6 +222,7 @@ class BaseQualityReporter(BaseViolationReporter):
     (provided by subclasses).
     """
     COMMAND = ''
+    DISCOVERY_COMMAND = ''
     OPTIONS = []
 
     # Encoding of the stdout from the command
@@ -256,9 +257,7 @@ def __init__(self, name, input_reports, user_options=None):
         """
         super(BaseQualityReporter, self).__init__(name)
         # Test if the tool requested is installed
-        # Assumes in can be imported with the same name.
-        # This applies to all our tools so far
-        __import__(self._name)
+        self._confirm_installed(name)
         self._info_cache = defaultdict(list)
         self.user_options = user_options
 
@@ -314,6 +313,13 @@ def _update_cache(self, violations_dict):
         for src_path, violations in six.iteritems(violations_dict):
             self._info_cache[src_path].extend(violations)
 
+    def _confirm_installed(self, name):
+        """
+        Assumes it can be imported with the same name.
+        This applies to all python tools so far
+        """
+        __import__(name)
+
     def _run_command(self, src_path):
         """
         Run the quality command and return its output as a unicode string.
@@ -322,7 +328,6 @@ def _run_command(self, src_path):
         encoding = sys.getfilesystemencoding()
         user_options = [self.user_options] if self.user_options else []
         command = [self.COMMAND] + self.OPTIONS + user_options + [src_path.encode(encoding)]
-
         try:
             process = subprocess.Popen(
                 command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
@@ -339,6 +344,17 @@ def _run_command(self, src_path):
 
         return stdout.strip().decode(self.STDOUT_ENCODING, 'replace')
 
+    def _run_command_simple(self, command):
+        """
+        Returns command's exit code.
+        """
+        process = subprocess.Popen(
+            command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True
+        )
+        process.communicate()
+        exit_code = process.returncode
+        return exit_code
+
     def _parse_output(self, output, src_path=None):
         """
         Parse the output of this reporter
@@ -505,6 +521,26 @@ def _parse_output(self, output, src_path=None):
         return violations_dict
 
 
+class JsHintQualityReporter(BaseQualityReporter):
+    """
+    Report JSHint violations.
+    """
+    COMMAND = 'jshint'
+    # The following command can confirm jshint is installed
+    DISCOVERY_COMMAND = 'jshint -v'
+    EXTENSIONS = ['js']
+    VIOLATION_REGEX = re.compile(r'^([^:]+): line (\d+), col \d+, (.*)$')
+
+    def _confirm_installed(self, name):
+        """
+        Override base method. Confirm the tool is installed by running this command and
+        getting exit 0. Otherwise, raise an Environment Error.
+        """
+        if self._run_command_simple(self.DISCOVERY_COMMAND) == 0:
+            return
+        raise EnvironmentError
+
+
 class QualityReporterError(Exception):
     """
     A quality reporter command produced an error.