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

Merge pull request #13146 from edx/bjacobel/eslint

Browse Source 
Remove JSHint dependency, config and paver/CI integration
This commit is contained in:
Brian Jacobel
2016-08-03 12:28:40 -04:00
committed by GitHub
parent 4029e6e0d8 9e67f15306
commit 0e81e0cebc
10 changed files with 12 additions and 352 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
.jshintignore
View File

@@ -1,9 +0,0 @@
**/vendor
cms/static/cms/js/build.js
cms/static/cms/js/spec/main.js
cms/static/js/i18n/**/*.js
lms/static/js/i18n/**/*.js
lms/static/lms/js/build.js
lms/static/lms/js/spec/main.js
node_modules
venv

154
.jshintrc
View File

@@ -1,154 +0,0 @@
// --------------------------------------------------------------------
// JSHint Configuration
// --------------------------------------------------------------------
//
// http://www.jshint.com/
// http://jshint.com/docs/options/
{
// == Enforcing Options ===============================================
"bitwise" : false, // Prohibits the use of bitwise operators such as ^ (XOR), | (OR) and others. Bitwise operators are very rare in JavaScript programs and quite often & is simply a mistyped &&.
"camelcase" : false, // Allows you to force all variable names to use either camelCase style or UPPER_CASE with underscores.
"curly" : true, // Requires you to always put curly braces around blocks in loops and conditionals. JavaScript allows you to omit curly braces when the block consists of only one statement
"eqeqeq" : true, // Prohibits the use of == and != in favor of === and !==.
"es3" : false, // Tells JSHint that your code needs to adhere to ECMAScript 3 specification. Use this option if you need your program to be executable in older browsers—such as Internet Explorer 6/7/8/9.
"forin" : true, // Requires all for in loops to filter object's items.
"freeze" : true, // Prohibits overwriting prototypes of native objects such as Array, Date and so on.
"immed" : true, // Prohibits the use of immediate function invocations without wrapping them in parentheses.
// "indent" : 4, // Enforces specific tab width for your code. Has no effect when "white" option is not used.
"latedef" : "nofunc", // Prohibits the use of a variable before it was defined. Setting this option to "nofunc" will allow function declarations to be ignored.
"newcap" : false, // Requires you to capitalize names of constructor functions.
"noarg" : true, // Prohibits the use of arguments.caller and arguments.callee.
"noempty" : true, // Warns when you have an empty block in your code.
"nonbsp" : true, // Warns about "non-breaking whitespace" characters.
"nonew" : true, // Prohibits the use of constructor functions for side-effects.
"plusplus" : false, // Prohibits the use of unary increment and decrement operators.
"quotmark" : false, // Enforces the consistency of quotation marks used throughout your code. It accepts three values: true, "single", and "double".
"undef" : true, // Prohibits the use of explicitly undeclared variables.
"unused" : true, // Warns when you define and never use your variables.
"strict" : true, // Requires all functions to run in ECMAScript 5's strict mode.
"trailing" : true, // Makes it an error to leave a trailing whitespace in your code.
"maxlen" : 120, // Lets you set the maximum length of a line.
//"maxparams" : 4, // Lets you set the max number of formal parameters allowed per function.
//"maxdepth" : 4, // Lets you control how nested do you want your blocks to be.
//"maxstatements" : 4, // Lets you set the max number of statements allowed per function.
//"maxcomplexity" : 4, // Lets you control cyclomatic complexity throughout your code.
// == Relaxing Options ================================================
"asi" : false, // Suppresses warnings about missing semicolons.
"boss" : false, // Suppresses warnings about the use of assignments in cases where comparisons are expected.
"debug" : false, // Suppresses warnings about the debugger statements in your code.
"eqnull" : false, // Suppresses warnings about == null comparisons.
"esnext" : false, // Tells JSHint that your code uses ECMAScript 6 specific syntax.
"evil" : false, // Suppresses warnings about the use of eval.
"expr" : false, // Suppresses warnings about the use of expressions where normally you would expect to see assignments or function calls.
"funcscope" : false, // Suppresses warnings about declaring variables inside of control structures while accessing them later from the outside.
"gcl" : false, // Makes JSHint compatible with Google Closure Compiler.
"globalstrict" : false, // Suppresses warnings about the use of global strict mode.
"iterator" : false, // Suppresses warnings about the __iterator__ property.
"lastsemic" : false, // Suppresses warnings about missing semicolons, but only when the semicolon is omitted for the last statement in a one-line block.
"laxbreak" : false, // Suppresses most of the warnings about possibly unsafe line breaks in your code.
"laxcomma" : false, // Suppresses warnings about comma-first coding style.
"loopfunc" : false, // Suppresses warnings about functions inside of loops.
"maxerr" : 100, // Set the maximum amount of warnings JSHint will produce before giving up.
"moz" : false, // Tells JSHint that your code uses Mozilla JavaScript extensions.
"notypeof" : false, // Suppresses warnings about invalid typeof operator values.
"proto" : false, // Suppresses warnings about the __proto__ property.
"scripturl" : false, // Suppresses warnings about the use of script-targeted URLs—such as javascript:...
"smarttabs" : false, // Suppresses warnings about mixed tabs and spaces when the latter are used for alignment only.
"shadow" : false, // Suppresses warnings about variable shadowing i.e. declaring a variable that had been already declared somewhere in the outer scope.
"sub" : false, // Suppresses warnings about using [] notation when it can be expressed in dot notation.
"supernew" : false, // Suppresses warnings about "weird" constructions like new function () { ... } and new Object;.
"validthis" : true, // Suppresses warnings about possible strict violations when the code is running in strict mode and you use this in a non-constructor function.
"noyield" : false, // Suppresses warnings about generator functions with no yield statement in them.
// == Environments ====================================================
//
// These options pre-define global variables that are exposed by
// popular JavaScript libraries and runtime environments—such as
// browser or node.js.
"browser" : true, // Defines globals exposed by modern browsers: all the way from good old document and navigator to the HTML5 FileReader and other new developments in the browser world.
"devel" : true, // Defines globals that are usually used for logging poor-man's debugging: console, alert, etc.
// The rest should remain `false`. Please see explanation for the "predef" parameter below.
"couch" : false, // Defines globals exposed by CouchDB.
"dojo" : false, // Defines globals exposed by the Dojo Toolkit
"jquery" : false, // Defines globals exposed by the jQuery JavaScript library.
"mootools" : false, // Defines globals exposed by the MooTools JavaScript framework.
"node" : false, // Defines globals available when your code is running inside of the Node runtime environment.
"nonstandard" : false, // Defines non-standard but widely adopted globals such as escape and unescape.
"phantom" : false, // Defines globals available when your core is running inside of the PhantomJS runtime environment.
"prototypejs" : false, // Defines globals exposed by the Prototype JavaScript framework.
"rhino" : false, // Defines globals available when your code is running inside of the Rhino runtime environment.
"worker" : false, // Defines globals available when your code is running inside of a Web Worker.
"wsh" : false, // Defines globals available when your code is running as a script for the Windows Script Host.
"yui" : false, // Defines globals exposed by the YUI JavaScript framework.
// == JSLint Legacy ===================================================
//
// These options are legacy from JSLint. Aside from bug fixes they will
// not be improved in any way and might be removed at any point.
// "nomen" : false, // Disallows the use of dangling _ in variables.
// "onevar" : false, // Allows only one var statement per function.
// "passfail" : false, // Makes JSHint stop on the first error or warning.
// "white" : false, // make JSHint check your source code against Douglas Crockford's JavaScript coding style.
// == Undocumented Options ============================================
//
// If you are using some global variable, for example `define`, or `$`, please
// make it available within your JS file by passing it to the wrapper anonymous
// function like so:
//
// (function (define, $) {
// 'use strict';
// // Your content goes here which uses `define`, and `$`.
// }).call(this, window.define, window.jQuery);
//
// The parameter "predef" should remain empty for this configuration file
// to remain as general as possible.
"predef": [
// JavaScript global libraries
"Backbone",
"jQuery",
"$",
"_",
// RequireJS globals
"define",
"require",
"RequireJS",
// Jasmine globals
"jasmine",
"describe", "xdescribe",
"it", "xit",
"spyOn",
"beforeEach",
"afterEach",
"expect",
"waitsFor",
"runs",
// jQuery-Jasmine globals
"loadFixtures",
"appendLoadFixtures",
"readFixtures",
"setFixtures",
"appendSetFixtures",
"spyOnEvent",
// Django i18n catalog globals
"interpolate",
"gettext",
"ngettext",
// Miscellaneous globals
"JSON",
// edX globals
"edx",
"XBlock"
]
}

6
docs/en_us/internal/testing.rst
View File

@@ -802,13 +802,13 @@ To view JavaScript code style quality run this command.
::
paver run_jshint
paver run_eslint
- This command also comes with a ``--limit`` switch, this is an example of that switch.
::
paver run_jshint --limit=700
paver run_eslint --limit=50000
@@ -829,7 +829,7 @@ Two tools are available for evaluating complexity of edx-platform code:
::
plato -q -x common/static/js/vendor/ -t common -l .jshintrc -r -d jscomplexity common/static/js/
plato -q -x common/static/js/vendor/ -t common -e .eslintrc.json -r -d jscomplexity common/static/js/

1
package.json
View File

@@ -23,7 +23,6 @@
"jasmine-core": "^2.4.1",
"jasmine-jquery": "^2.1.1",
"jquery": "^2.1.4",
"jshint": "^2.7.0",
"karma": "^0.13.22",
"karma-chrome-launcher": "^0.2.3",
"karma-coverage": "^0.5.5",

51
pavelib/paver_tests/test_jshint.py
View File

@@ -1,51 +0,0 @@
"""
Tests for paver quality tasks
"""
import unittest
from mock import patch
import pavelib.quality
from paver.easy import BuildFailure
# @TODO Deprecated, remove in favor of ESLint
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("")

72
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_eslint, run_complexity, run_safelint, and run_safecommit_report.
run_eslint, run_complexity, run_safelint, and run_safecommit_report.
"""
def setUp(self):
@@ -79,28 +79,6 @@ class TestPaverReportViolationsCounts(unittest.TestCase):
self.addCleanup(self._mock_paver_needs.stop)
self.addCleanup(os.remove, self.f.name)
# @TODO: Deprecated, remove in favor of ESLint
def test_get_jshint_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
self.assertEqual(actual_count, 3000)
def test_get_jshint_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
self.assertEqual(actual_count, None)
def test_get_jshint_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, "jshint") # pylint: disable=protected-access
self.assertEqual(actual_count, None)
def test_get_eslint_violations_count(self):
with open(self.f.name, 'w') as f:
f.write("3000 violations found")
@@ -306,10 +284,10 @@ class TestPaverRunQuality(unittest.TestCase):
with self.assertRaises(SystemExit):
pavelib.quality.run_quality("")
# Test that pep8, pylint, jshint and eslint were called (@TODO remove JSHint) by counting the calls to
# 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, 3)
self.assertEqual(self._mock_paver_sh.call_count, 2)
@patch('__builtin__.open', mock_open())
def test_failure_on_diffquality_pylint(self):
@@ -327,29 +305,9 @@ 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 3x (for the calls to pylint & jshint & eslint). (@TODO remove JSHint)
# 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, 3)
# @TODO: Deprecated, remove in favor of ESLint
@patch('__builtin__.open', mock_open())
def test_failure_on_diffquality_jshint(self):
"""
If diff-quality fails on jshint, 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
_mock_pep8_violations = MagicMock(return_value=(0, []))
with patch('pavelib.quality._get_pep8_violations', _mock_pep8_violations):
with self.assertRaises(SystemExit):
pavelib.quality.run_quality("")
self.assertRaises(BuildFailure)
# 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 3x (for the calls to pep8 , jshint and pylint) @TODO remove this test
self.assertEqual(self._mock_paver_sh.call_count, 3)
self.assertEqual(self._mock_paver_sh.call_count, 2)
@patch('__builtin__.open', mock_open())
def test_failure_on_diffquality_eslint(self):
@@ -367,8 +325,8 @@ 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 3x (for the calls to pep8, jshint and pylint) @TODO, remove jshint and decrement
self.assertEqual(self._mock_paver_sh.call_count, 3)
# And assert that sh was called twice (for the calls to pep8 and pylint)
self.assertEqual(self._mock_paver_sh.call_count, 2)
@patch('__builtin__.open', mock_open())
def test_other_exception(self):
@@ -391,8 +349,8 @@ 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 3x (for the call to "pylint" & "jshint" & "eslint") (@TODO, remove jshint)
self.assertEqual(self._mock_paver_sh.call_count, 3)
# And assert that sh was called twice (for the call to "pylint" & "eslint")
self.assertEqual(self._mock_paver_sh.call_count, 2)
class CustomShMock(object):
@@ -412,18 +370,6 @@ class CustomShMock(object):
else:
return
# @TODO: Deprecated, remove in favor of ESLint
def fail_on_jshint(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:
# Essentially mock diff-quality exiting with 1
paver.easy.sh("exit 1")
else:
return
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

61
pavelib/quality.py
View File

@@ -257,51 +257,6 @@ 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([
("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)
sh(
"jshint . --config .jshintrc >> {jshint_report}".format(
jshint_report=jshint_report
),
ignore_error=True
)
try:
num_violations = int(_get_count_from_last_line(jshint_report, "jshint"))
except TypeError:
raise BuildFailure(
"Error. Number of jshint violations could not be found in {jshint_report}".format(
jshint_report=jshint_report
)
)
# Record the metric
_write_metric(num_violations, (Env.METRICS_DIR / "jshint"))
# Fail if number of violations is greater than the limit
if num_violations > violations_limit > -1:
raise BuildFailure(
"JSHint Failed. Too many violations ({count}).\nThe limit is {violations_limit}.".format(
count=num_violations, violations_limit=violations_limit
)
)
@task
@needs('pavelib.prereqs.install_node_prereqs')
@cmdopts([
@@ -713,10 +668,6 @@ 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)
@@ -736,18 +687,6 @@ 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",
prefix=pythonpath_prefix,
reports=jshint_reports,
percentage_string=percentage_string,
branch_string=compare_branch_string,
dquality_dir=dquality_dir
):
diff_quality_percentage_pass = False
# run diff-quality for eslint.
if not run_diff_quality(
violations_type="eslint",

1
scripts/all-tests.sh
View File

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

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

@@ -62,10 +62,6 @@ else
mkdir -p reports
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; }

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

@@ -81,13 +81,8 @@ case "$TEST_SUITE" in
mkdir -p reports
# @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."