Use jshint in paver, include in builds.
JShint will be executed with paver run_jshint, which will use a defined set of directories (likewise defined are directories to ignore). A limit can be imposed on the total number of violations. Note that this change does NOT include adding jshint to diff-quality or `paver run_quality`.
This commit is contained in:
2
.jshintignore
Normal file
2
.jshintignore
Normal file
@@ -0,0 +1,2 @@
|
||||
common/static/js/vendor
|
||||
lms/static/js/vendor
|
||||
@@ -494,10 +494,10 @@ To view test coverage:
|
||||
3. Reports are located in the ``reports`` folder. The command generates
|
||||
HTML and XML (Cobertura format) reports.
|
||||
|
||||
Code Style Quality
|
||||
Python Code Style Quality
|
||||
------------------
|
||||
|
||||
To view code style quality (including pep8 and pylint violations)::
|
||||
To view Python code style quality (including pep8 and pylint violations)::
|
||||
|
||||
paver run_quality
|
||||
|
||||
@@ -530,6 +530,20 @@ More specific options are below.
|
||||
|
||||
``system`` is an optional argument here. It defaults to
|
||||
``cms,lms,common``.
|
||||
|
||||
|
||||
JavaScript Code Style Quality
|
||||
------------------
|
||||
|
||||
To view JavaScript code style quality::
|
||||
|
||||
paver run_jshint
|
||||
|
||||
- This command also comes with a ``--limit`` switch, for example::
|
||||
|
||||
paver run_jshint --limit=700
|
||||
|
||||
|
||||
|
||||
Testing using queue servers
|
||||
---------------------------
|
||||
|
||||
50
pavelib/paver_tests/test_jshint.py
Normal file
50
pavelib/paver_tests/test_jshint.py
Normal file
@@ -0,0 +1,50 @@
|
||||
"""
|
||||
Tests for paver quality tasks
|
||||
"""
|
||||
import unittest
|
||||
from mock import patch
|
||||
|
||||
import pavelib.quality
|
||||
from paver.easy import BuildFailure
|
||||
|
||||
|
||||
class TestPaverJsHint(unittest.TestCase):
|
||||
"""
|
||||
For testing run_jshint
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super(TestPaverJsHint, self).setUp()
|
||||
|
||||
# Mock the paver @needs decorator
|
||||
self._mock_paver_needs = patch.object(pavelib.quality.run_jshint, 'needs').start()
|
||||
self._mock_paver_needs.return_value = 0
|
||||
|
||||
# Mock shell commands
|
||||
patcher = patch('pavelib.quality.sh')
|
||||
self._mock_paver_sh = patcher.start()
|
||||
|
||||
# Cleanup mocks
|
||||
self.addCleanup(patcher.stop)
|
||||
self.addCleanup(self._mock_paver_needs.stop)
|
||||
|
||||
@patch.object(pavelib.quality, '_write_metric')
|
||||
@patch.object(pavelib.quality, '_prepare_report_dir')
|
||||
@patch.object(pavelib.quality, '_get_count_from_last_line')
|
||||
def test_jshint_violation_number_not_found(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument
|
||||
"""
|
||||
run_jshint encounters an error parsing the jshint output log
|
||||
"""
|
||||
mock_count.return_value = None
|
||||
with self.assertRaises(BuildFailure):
|
||||
pavelib.quality.run_jshint("")
|
||||
|
||||
@patch.object(pavelib.quality, '_write_metric')
|
||||
@patch.object(pavelib.quality, '_prepare_report_dir')
|
||||
@patch.object(pavelib.quality, '_get_count_from_last_line')
|
||||
def test_jshint_vanilla(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument
|
||||
"""
|
||||
jshint finds violations, but a limit was not set
|
||||
"""
|
||||
mock_count.return_value = 1
|
||||
pavelib.quality.run_jshint("")
|
||||
@@ -2,6 +2,7 @@
|
||||
Tests for paver quality tasks
|
||||
"""
|
||||
import os
|
||||
from path import path # pylint: disable=no-name-in-module
|
||||
import tempfile
|
||||
import unittest
|
||||
from mock import patch, MagicMock, mock_open
|
||||
@@ -56,6 +57,70 @@ class TestPaverQualityViolations(unittest.TestCase):
|
||||
self.assertEqual(num, 2)
|
||||
|
||||
|
||||
class TestPaverJsHintViolationsCounts(unittest.TestCase):
|
||||
"""
|
||||
For testing run_jshint
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super(TestPaverJsHintViolationsCounts, self).setUp()
|
||||
|
||||
# Mock the paver @needs decorator
|
||||
self._mock_paver_needs = patch.object(pavelib.quality.run_quality, 'needs').start()
|
||||
self._mock_paver_needs.return_value = 0
|
||||
|
||||
# Temporary file infrastructure
|
||||
self.f = tempfile.NamedTemporaryFile(delete=False)
|
||||
self.f.close()
|
||||
|
||||
# Cleanup various mocks and tempfiles
|
||||
self.addCleanup(self._mock_paver_needs.stop)
|
||||
self.addCleanup(os.remove, self.f.name)
|
||||
|
||||
def test_get_violations_count(self):
|
||||
with open(self.f.name, 'w') as f:
|
||||
f.write("3000 violations found")
|
||||
actual_count = pavelib.quality._get_count_from_last_line(self.f.name) # pylint: disable=protected-access
|
||||
self.assertEqual(actual_count, 3000)
|
||||
|
||||
def test_get_violations_no_number_found(self):
|
||||
with open(self.f.name, 'w') as f:
|
||||
f.write("Not expected string regex")
|
||||
actual_count = pavelib.quality._get_count_from_last_line(self.f.name) # pylint: disable=protected-access
|
||||
self.assertEqual(actual_count, None)
|
||||
|
||||
def test_get_violations_count_truncated_report(self):
|
||||
"""
|
||||
A truncated report (i.e. last line is just a violation)
|
||||
"""
|
||||
with open(self.f.name, 'w') as f:
|
||||
f.write("foo/bar/js/fizzbuzz.js: line 45, col 59, Missing semicolon.")
|
||||
actual_count = pavelib.quality._get_count_from_last_line(self.f.name) # pylint: disable=protected-access
|
||||
self.assertEqual(actual_count, None)
|
||||
|
||||
|
||||
class TestPrepareReportDir(unittest.TestCase):
|
||||
"""
|
||||
Tests the report directory preparation
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super(TestPrepareReportDir, self).setUp()
|
||||
self.test_dir = tempfile.mkdtemp()
|
||||
self.test_file = tempfile.NamedTemporaryFile(delete=False, dir=self.test_dir)
|
||||
self.addCleanup(os.removedirs, self.test_dir)
|
||||
|
||||
def test_report_dir_with_files(self):
|
||||
self.assertTrue(os.path.exists(self.test_file.name))
|
||||
pavelib.quality._prepare_report_dir(path(self.test_dir)) # pylint: disable=protected-access
|
||||
self.assertFalse(os.path.exists(self.test_file.name))
|
||||
|
||||
def test_report_dir_without_files(self):
|
||||
os.remove(self.test_file.name)
|
||||
pavelib.quality._prepare_report_dir(path(self.test_dir)) # pylint: disable=protected-access
|
||||
self.assertEqual(os.listdir(path(self.test_dir)), [])
|
||||
|
||||
|
||||
class TestPaverRunQuality(unittest.TestCase):
|
||||
"""
|
||||
For testing the paver run_quality task
|
||||
|
||||
@@ -240,6 +240,86 @@ def run_complexity():
|
||||
print "ERROR: Unable to calculate python-only code-complexity."
|
||||
|
||||
|
||||
@task
|
||||
@needs('pavelib.prereqs.install_node_prereqs')
|
||||
@cmdopts([
|
||||
("limit=", "l", "limit for number of acceptable violations"),
|
||||
])
|
||||
def run_jshint(options):
|
||||
"""
|
||||
Runs jshint on static asset directories
|
||||
"""
|
||||
|
||||
violations_limit = int(getattr(options, 'limit', -1))
|
||||
|
||||
jshint_report_dir = (Env.REPORT_DIR / "jshint")
|
||||
jshint_report = jshint_report_dir / "jshint.report"
|
||||
_prepare_report_dir(jshint_report_dir)
|
||||
|
||||
jshint_directories = ["common/static/js", "cms/static/js", "lms/static/js"]
|
||||
|
||||
sh(
|
||||
"jshint {list} --config .jshintrc >> {jshint_report}".format(
|
||||
list=(" ".join(jshint_directories)), jshint_report=jshint_report
|
||||
),
|
||||
ignore_error=True
|
||||
)
|
||||
num_violations = _get_count_from_last_line(jshint_report)
|
||||
|
||||
if not num_violations:
|
||||
raise BuildFailure("Error in calculating total number of violations.")
|
||||
|
||||
# Record the metric
|
||||
_write_metric(str(num_violations), (Env.METRICS_DIR / "jshint"))
|
||||
|
||||
# Fail if number of violations is greater than the limit
|
||||
if num_violations > violations_limit > -1:
|
||||
raise Exception(
|
||||
"JSHint Failed. Too many violations ({count}).\nThe limit is {violations_limit}.".format(
|
||||
count=num_violations, violations_limit=violations_limit
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _write_metric(metric, filename):
|
||||
"""
|
||||
Write a given metric to a given file
|
||||
Used for things like reports/metrics/jshint, which will simply tell you the number of
|
||||
jshint violations found
|
||||
"""
|
||||
with open(filename, "w") as metric_file:
|
||||
metric_file.write(metric)
|
||||
|
||||
|
||||
def _prepare_report_dir(dir_name):
|
||||
"""
|
||||
Sets a given directory to a created, but empty state
|
||||
"""
|
||||
dir_name.rmtree_p()
|
||||
dir_name.mkdir_p()
|
||||
|
||||
|
||||
def _get_last_report_line(filename):
|
||||
"""
|
||||
Returns the last line of a given file. Used for getting output from quality output files.
|
||||
"""
|
||||
with open(filename, 'r') as report_file:
|
||||
lines = report_file.readlines()
|
||||
return lines[len(lines) - 1]
|
||||
|
||||
|
||||
def _get_count_from_last_line(filename):
|
||||
"""
|
||||
This will return the number in a line that looks something like "3000 errors found". It is returning
|
||||
the digits only (as an integer).
|
||||
"""
|
||||
last_line = _get_last_report_line(filename)
|
||||
try:
|
||||
return int(re.search(r'^\d+', last_line).group(0))
|
||||
except AttributeError:
|
||||
return None
|
||||
|
||||
|
||||
@task
|
||||
@needs('pavelib.prereqs.install_python_prereqs')
|
||||
@cmdopts([
|
||||
|
||||
@@ -57,6 +57,7 @@ source scripts/jenkins-common.sh
|
||||
|
||||
# Violations thresholds for failing the build
|
||||
PYLINT_THRESHOLD=7350
|
||||
JSHINT_THRESHOLD=3700
|
||||
|
||||
# If the environment variable 'SHARD' is not set, default to 'all'.
|
||||
# This could happen if you are trying to use this script from
|
||||
@@ -78,6 +79,10 @@ case "$TEST_SUITE" in
|
||||
paver run_quality -p 100
|
||||
|
||||
mkdir -p reports
|
||||
echo "Finding jshint violations and storing report..."
|
||||
PATH=$PATH:node_modules/.bin
|
||||
paver run_jshint -l $JSHINT_THRESHOLD > jshint.log || { cat jshint.log; EXIT=1; }
|
||||
echo "Running code complexity report (python)."
|
||||
paver run_complexity > reports/code_complexity.log || echo "Unable to calculate code complexity. Ignoring error."
|
||||
# Need to create an empty test result so the post-build
|
||||
# action doesn't fail the build.
|
||||
|
||||
Reference in New Issue
Block a user