From 017dbffd5effdfc6a39a4239a8dad66827ef4657 Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Tue, 2 Aug 2016 10:09:03 -0400 Subject: [PATCH] Remove ESLint from build scripts and script tests --- pavelib/paver_tests/test_jshint.py | 51 ---------------- pavelib/paver_tests/test_paver_quality.py | 72 +++-------------------- pavelib/quality.py | 61 ------------------- scripts/all-tests.sh | 1 - scripts/circle-ci-tests.sh | 4 -- scripts/generic-ci-tests.sh | 5 -- 6 files changed, 9 insertions(+), 185 deletions(-) delete mode 100644 pavelib/paver_tests/test_jshint.py diff --git a/pavelib/paver_tests/test_jshint.py b/pavelib/paver_tests/test_jshint.py deleted file mode 100644 index 36f78fa801..0000000000 --- a/pavelib/paver_tests/test_jshint.py +++ /dev/null @@ -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("") diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 8bddfd5e1c..7c2810c621 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -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 diff --git a/pavelib/quality.py b/pavelib/quality.py index 59eb5db9c7..4b98c88a6b 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -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", diff --git a/scripts/all-tests.sh b/scripts/all-tests.sh index 68cfe355dd..03284fbecc 100755 --- a/scripts/all-tests.sh +++ b/scripts/all-tests.sh @@ -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` diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh index e9738a4455..10a6e2502a 100755 --- a/scripts/circle-ci-tests.sh +++ b/scripts/circle-ci-tests.sh @@ -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; } diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index 24a289da37..1d5fc916ce 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -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."