Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Install and configure eslint and eslint-config-edx (alongside JSHint for now)

Browse Source
This commit is contained in:
Brian Jacobel
2016-06-21 17:12:52 -04:00
parent 2c4ab11658
commit af76273393
11 changed files with 180 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
.eslintignore Normal file
View File

@@ -0,0 +1,57 @@
# Vendor files and generated test artifacts
**/vendor
test_root/staticfiles
# Vendor files living outside the /vendor/ dir
*.min.js
*-min.js
*.nocache.js
**/bootstrap*.js
**/jquery*.js
**/d3*.js
# Translations files
**/static/js/i18n
# Gitignored xmodule stuff
common/static/xmodule
# Coffeescript directories (don't lint autogenerated files)
cms/static/coffee
lms/static/coffee
common/static/coffee
common/lib/capa/capa/tests/test_files/js
# Symlinks into common/lib/xmodule/xmodule/js
cms/static/xmodule_js
lms/static/xmodule_js
# This directory is about half Coffee and half JS, things get messy here so just ignore all existing coffee paths
common/lib/xmodule/xmodule/js/spec/annotatable/display_spec.js
common/lib/xmodule/xmodule/js/spec/capa/display_spec.js
common/lib/xmodule/xmodule/js/spec/html/edit_spec.js
common/lib/xmodule/xmodule/js/spec/problem/edit_spec.js
common/lib/xmodule/xmodule/js/spec/problem/edit_spec_hint.js
common/lib/xmodule/xmodule/js/spec/tabs/edit.js
common/lib/xmodule/xmodule/js/src/annotatable/display.js
common/lib/xmodule/xmodule/js/src/capa/display.js
common/lib/xmodule/xmodule/js/src/conditional/display.js
common/lib/xmodule/xmodule/js/src/discussion/display.js
common/lib/xmodule/xmodule/js/src/html/display.js
common/lib/xmodule/xmodule/js/src/html/edit.js
common/lib/xmodule/xmodule/js/src/javascript_loader.js
common/lib/xmodule/xmodule/js/src/problem/edit.js
common/lib/xmodule/xmodule/js/src/raw/edit/json.js
common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.js
common/lib/xmodule/xmodule/js/src/raw/edit/xml.js
common/lib/xmodule/xmodule/js/src/sequence/display.js
common/lib/xmodule/xmodule/js/src/sequence/edit.js
common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.js
common/lib/xmodule/xmodule/js/src/vertical/edit.js

3
.eslintrc.json Normal file
View File

@@ -0,0 +1,3 @@
{
"extends": "eslint-config-edx"
}

8
package.json
View File

@@ -18,8 +18,8 @@
},
"devDependencies": {
"edx-custom-a11y-rules": "0.1.2",
"pa11y": "3.6.0",
"pa11y-reporter-1.0-json": "1.0.2",
"eslint": "^2.13.1",
"eslint-config-edx": "^1.2.0",
"jasmine-core": "^2.4.1",
"jasmine-jquery": "^2.1.1",
"jquery": "^2.1.4",
@@ -31,8 +31,10 @@
"karma-jasmine": "^0.3.8",
"karma-jasmine-html-reporter": "^0.2.0",
"karma-junit-reporter": "^0.4.1",
"karma-spec-reporter": "^0.0.20",
"karma-requirejs": "^0.2.6",
"karma-spec-reporter": "^0.0.20",
"pa11y": "3.6.0",
"pa11y-reporter-1.0-json": "1.0.2",
"plato": "1.2.2"
}
}

20
pavelib/paver_tests/test_jshint.py → pavelib/paver_tests/test_eslint.py
View File

@@ -8,16 +8,16 @@ import pavelib.quality
from paver.easy import BuildFailure
class TestPaverJsHint(unittest.TestCase):
class TestPaverESLint(unittest.TestCase):
"""
For testing run_jshint
For testing run_eslint
"""
def setUp(self):
super(TestPaverJsHint, self).setUp()
super(TestPaverESLint, self).setUp()
# Mock the paver @needs decorator
self._mock_paver_needs = patch.object(pavelib.quality.run_jshint, 'needs').start()
self._mock_paver_needs = patch.object(pavelib.quality.run_eslint, 'needs').start()
self._mock_paver_needs.return_value = 0
# Mock shell commands
@@ -31,20 +31,20 @@ class TestPaverJsHint(unittest.TestCase):
@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
def test_eslint_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
run_eslint encounters an error parsing the eslint output log
"""
mock_count.return_value = None
with self.assertRaises(BuildFailure):
pavelib.quality.run_jshint("")
pavelib.quality.run_eslint("")
@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
def test_eslint_vanilla(self, mock_count, mock_report_dir, mock_write_metric): # pylint: disable=unused-argument
"""
jshint finds violations, but a limit was not set
eslint finds violations, but a limit was not set
"""
mock_count.return_value = 1
pavelib.quality.run_jshint("")
pavelib.quality.run_eslint("")

34
pavelib/paver_tests/test_paver_quality.py
View File

@@ -61,7 +61,7 @@ class TestPaverQualityViolations(unittest.TestCase):
class TestPaverReportViolationsCounts(unittest.TestCase):
"""
For testing utility functions for getting counts from reports for
run_jshint, run_complexity, run_safelint, and run_safecommit_report.
run_eslint, run_complexity, run_safelint, and run_safecommit_report.
"""
def setUp(self):
@@ -79,16 +79,16 @@ class TestPaverReportViolationsCounts(unittest.TestCase):
self.addCleanup(self._mock_paver_needs.stop)
self.addCleanup(os.remove, self.f.name)
def test_get_jshint_violations_count(self):
def test_get_eslint_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, "jshint") # pylint: disable=protected-access
actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # 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, "jshint") # pylint: disable=protected-access
actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # pylint: disable=protected-access
self.assertEqual(actual_count, None)
def test_get_violations_count_truncated_report(self):
@@ -97,7 +97,7 @@ class TestPaverReportViolationsCounts(unittest.TestCase):
"""
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, "jshint") # pylint: disable=protected-access
actual_count = pavelib.quality._get_count_from_last_line(self.f.name, "eslint") # pylint: disable=protected-access
self.assertEqual(actual_count, None)
def test_complexity_value(self):
@@ -274,7 +274,7 @@ class TestPaverRunQuality(unittest.TestCase):
@patch('__builtin__.open', mock_open())
def test_failure_on_diffquality_pep8(self):
"""
If pep8 finds errors, pylint and jshint should still be run
If pep8 finds errors, pylint and eslint should still be run
"""
# Mock _get_pep8_violations to return a violation
_mock_pep8_violations = MagicMock(
@@ -284,8 +284,8 @@ class TestPaverRunQuality(unittest.TestCase):
with self.assertRaises(SystemExit):
pavelib.quality.run_quality("")
# Test that pep8, pylint, and jshint were called by counting the calls to
# _get_pep8_violations (for pep8) and sh (for diff-quality pylint & jshint)
# Test that pep8, pylint, and eslint were called by counting the calls to
# _get_pep8_violations (for pep8) and sh (for diff-quality pylint & eslint)
self.assertEqual(_mock_pep8_violations.call_count, 1)
self.assertEqual(self._mock_paver_sh.call_count, 2)
@@ -305,18 +305,18 @@ class TestPaverRunQuality(unittest.TestCase):
# Test that both pep8 and pylint were called by counting the calls
# Assert that _get_pep8_violations (which calls "pep8") is called once
self.assertEqual(_mock_pep8_violations.call_count, 1)
# And assert that sh was called twice (for the calls to pylint & jshint). This means that even in
# the event of a diff-quality pylint failure, jshint is still called.
# And assert that sh was called twice (for the calls to pylint & eslint). This means that even in
# the event of a diff-quality pylint failure, eslint is still called.
self.assertEqual(self._mock_paver_sh.call_count, 2)
@patch('__builtin__.open', mock_open())
def test_failure_on_diffquality_jshint(self):
def test_failure_on_diffquality_eslint(self):
"""
If diff-quality fails on jshint, the paver task should also fail
If diff-quality fails on eslint, the paver task should also fail
"""
# Underlying sh call must fail when it is running the jshint diff-quality task
self._mock_paver_sh.side_effect = CustomShMock().fail_on_jshint
# Underlying sh call must fail when it is running the eslint diff-quality task
self._mock_paver_sh.side_effect = CustomShMock().fail_on_eslint
_mock_pep8_violations = MagicMock(return_value=(0, []))
with patch('pavelib.quality._get_pep8_violations', _mock_pep8_violations):
with self.assertRaises(SystemExit):
@@ -349,7 +349,7 @@ class TestPaverRunQuality(unittest.TestCase):
pavelib.quality.run_quality("")
# Assert that _get_pep8_violations (which calls "pep8") is called once
self.assertEqual(_mock_pep8_violations.call_count, 1)
# And assert that sh was called twice (for the call to "pylint" & "jshint")
# And assert that sh was called twice (for the call to "pylint" & "eslint")
self.assertEqual(self._mock_paver_sh.call_count, 2)
@@ -370,12 +370,12 @@ class CustomShMock(object):
else:
return
def fail_on_jshint(self, arg):
def fail_on_eslint(self, arg):
"""
For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what
is going to fail when we pass in a percentage ("p") requirement.
"""
if "jshint" in arg:
if "eslint" in arg:
# Essentially mock diff-quality exiting with 1
paver.easy.sh("exit 1")
else:

76
pavelib/quality.py
View File

@@ -257,6 +257,7 @@ def run_complexity():
print "ERROR: Unable to calculate python-only code-complexity."
# @TODO: Remove, deprecated in favor of ESLint
@task
@needs('pavelib.prereqs.install_node_prereqs')
@cmdopts([
@@ -301,6 +302,50 @@ def run_jshint(options):
)
@task
@needs('pavelib.prereqs.install_node_prereqs')
@cmdopts([
("limit=", "l", "limit for number of acceptable violations"),
])
def run_eslint(options):
"""
Runs eslint on static asset directories.
If limit option is passed, fails build if more violations than the limit are found.
"""
eslint_report_dir = (Env.REPORT_DIR / "eslint")
eslint_report = eslint_report_dir / "eslint.report"
_prepare_report_dir(eslint_report_dir)
violations_limit = int(getattr(options, 'limit', -1))
sh(
"eslint --format=compact . | tee {eslint_report}".format(
eslint_report=eslint_report
),
ignore_error=True
)
try:
num_violations = int(_get_count_from_last_line(eslint_report, "eslint"))
except TypeError:
raise BuildFailure(
"Error. Number of eslint violations could not be found in {eslint_report}".format(
eslint_report=eslint_report
)
)
# Record the metric
_write_metric(num_violations, (Env.METRICS_DIR / "eslint"))
# Fail if number of violations is greater than the limit
if num_violations > violations_limit > -1:
raise BuildFailure(
"ESLint Failed. Too many violations ({count}).\nThe limit is {violations_limit}.".format(
count=num_violations, violations_limit=violations_limit
)
)
@task
@needs('pavelib.prereqs.install_python_prereqs')
@cmdopts([
@@ -451,9 +496,11 @@ def run_safecommit_report():
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
Used for things like reports/metrics/eslint, which will simply tell you the number of
eslint violations found
"""
Env.METRICS_DIR.makedirs_p()
with open(filename, "w") as metric_file:
metric_file.write(str(metric))
@@ -485,7 +532,9 @@ def _get_report_contents(filename, last_line_only=False):
with open(filename, 'r') as report_file:
if last_line_only:
lines = report_file.readlines()
return lines[len(lines) - 1]
for line in reversed(lines):
if line != '\n':
return line
else:
return report_file.read()
else:
@@ -503,7 +552,7 @@ def _get_count_from_last_line(filename, file_type):
# Example of the last line of a complexity report: "Average complexity: A (1.93953443446)"
regex = r'\d+.\d+'
else:
# Example of the last line of a jshint report (for example): "3482 errors"
# Example of the last line of a compact-formatted eslint report (for example): "62829 problems"
regex = r'^\d+'
try:
@@ -663,9 +712,14 @@ def run_quality(options):
pylint_files = get_violations_reports("pylint")
pylint_reports = u' '.join(pylint_files)
# @TODO: Remove, deprecated in favor of ESLint
jshint_files = get_violations_reports("jshint")
jshint_reports = u' '.join(jshint_files)
eslint_files = get_violations_reports("eslint")
eslint_reports = u' '.join(eslint_files)
pythonpath_prefix = (
"PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:cms:cms/djangoapps:"
"common:common/djangoapps:common/lib"
@@ -682,6 +736,7 @@ def run_quality(options):
):
diff_quality_percentage_pass = False
# @TODO: Remove, deprecated in favor of ESLint
# run diff-quality for jshint.
if not run_diff_quality(
violations_type="jshint",
@@ -693,6 +748,17 @@ def run_quality(options):
):
diff_quality_percentage_pass = False
# run diff-quality for eslint.
if not run_diff_quality(
violations_type="eslint",
prefix=pythonpath_prefix,
reports=eslint_reports,
percentage_string=percentage_string,
branch_string=compare_branch_string,
dquality_dir=dquality_dir
):
diff_quality_percentage_pass = False
# If one of the quality runs fails, then paver exits with an error when it is finished
if not diff_quality_percentage_pass:
raise BuildFailure("Diff-quality failure(s).")
@@ -702,7 +768,7 @@ def run_diff_quality(
violations_type=None, prefix=None, reports=None, percentage_string=None, branch_string=None, dquality_dir=None
):
"""
This executes the diff-quality commandline tool for the given violation type (e.g., pylint, jshint).
This executes the diff-quality commandline tool for the given violation type (e.g., pylint, eslint).
If diff-quality fails due to quality issues, this method returns False.
"""

2
requirements/edx/base.txt
View File

@@ -146,7 +146,7 @@ bok-choy==0.5.3
chrono==1.0.2
coverage==4.0.2
ddt==0.8.0
diff-cover==0.9.0
diff-cover==0.9.8
django-crum==0.5
django_nose==1.4.1
factory_boy==2.5.1

3
scripts/all-tests.sh
View File

@@ -12,7 +12,8 @@ set -e
# Violations thresholds for failing the build
export PYLINT_THRESHOLD=4175
export JSHINT_THRESHOLD=7550
export JSHINT_THRESHOLD=7550 # @TODO Remove, deprecated in favor of ESLint
export ESLINT_THRESHOLD=49019
SAFELINT_THRESHOLDS=`cat scripts/safelint_thresholds.json`
export SAFELINT_THRESHOLDS=${SAFELINT_THRESHOLDS//[[:space:]]/}

7
scripts/circle-ci-tests.sh
View File

@@ -60,10 +60,15 @@ else
paver run_pylint -l $PYLINT_THRESHOLD | tee pylint.log || EXIT=1
mkdir -p reports
echo "Finding jshint violations and storing report..."
PATH=$PATH:node_modules/.bin
# @TODO: Remove, deprecated in favor of ESLint
echo "Finding JSHint violations and storing report..."
paver run_jshint -l $JSHINT_THRESHOLD > jshint.log || { cat jshint.log; EXIT=1; }
echo "Finding ESLint violations and storing report..."
paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; }
# Run quality task. Pass in the 'fail-under' percentage to diff-quality
paver run_quality -p 100 || EXIT=1

8
scripts/generic-ci-tests.sh
View File

@@ -80,8 +80,14 @@ case "$TEST_SUITE" in
paver run_pylint -l $PYLINT_THRESHOLD > pylint.log || { cat pylint.log; EXIT=1; }
mkdir -p reports
echo "Finding jshint violations and storing report..."
# @TODO: Remove, deprecated in favor of ESLint
echo "Finding JSHint violations and storing report..."
paver run_jshint -l $JSHINT_THRESHOLD > jshint.log || { cat jshint.log; EXIT=1; }
echo "Finding ESLint violations and storing report..."
paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; }
echo "Running code complexity report (python)."
paver run_complexity > reports/code_complexity.log || echo "Unable to calculate code complexity. Ignoring error."
echo "Running safe template linter report."

2
scripts/safe_template_linter.py
View File

@@ -543,7 +543,7 @@ class SummaryResults(object):
print("{}: {}{} violations".format(rule_id, padding, self.totals_by_rule[rule_id]), file=out)
print("", file=out)
# matches output of jshint for simplicity
# matches output of eslint for simplicity
print("", file=out)
print("{} violations total".format(self.total_violations), file=out)