From e5cab6189770d6c316e869ed714c9f8879d49c51 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Fri, 8 Sep 2017 18:31:02 -0400 Subject: [PATCH] Lint Sass files using the rules in stylelint-config-edx Fixes --- package.json | 4 +- pavelib/paver_tests/test_eslint.py | 8 ++-- pavelib/paver_tests/test_stylelint.py | 37 +++++++++++++++ pavelib/quality.py | 66 +++++++++++++++++++++++++-- scripts/all-tests.sh | 1 + scripts/circle-ci-tests.sh | 3 ++ scripts/generic-ci-tests.sh | 2 + stylelint.config.js | 3 ++ 8 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 pavelib/paver_tests/test_stylelint.py create mode 100644 stylelint.config.js diff --git a/package.json b/package.json index dfdc402be706..fa0be9b955f6 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,8 @@ "plato": "1.2.2", "selenium-webdriver": "3.4.0", "sinon": "2.3.5", - "squirejs": "^0.1.0" + "squirejs": "^0.1.0", + "@edx/stylelint-config-edx": "^1.1.0", + "stylelint-formatter-pretty": "^1.0.3" } } diff --git a/pavelib/paver_tests/test_eslint.py b/pavelib/paver_tests/test_eslint.py index 8a326cf0cdb7..a977fb756032 100644 --- a/pavelib/paver_tests/test_eslint.py +++ b/pavelib/paver_tests/test_eslint.py @@ -1,5 +1,5 @@ """ -Tests for paver quality tasks +Tests for Paver's Stylelint tasks. """ import unittest @@ -9,13 +9,13 @@ import pavelib.quality -class TestPaverESLint(unittest.TestCase): +class TestPaverStylelint(unittest.TestCase): """ - For testing run_eslint + For testing run_stylelint """ def setUp(self): - super(TestPaverESLint, self).setUp() + super(TestPaverStylelint, self).setUp() # Mock the paver @needs decorator self._mock_paver_needs = patch.object(pavelib.quality.run_eslint, 'needs').start() diff --git a/pavelib/paver_tests/test_stylelint.py b/pavelib/paver_tests/test_stylelint.py new file mode 100644 index 000000000000..b7d853b2461a --- /dev/null +++ b/pavelib/paver_tests/test_stylelint.py @@ -0,0 +1,37 @@ +""" +Tests for Paver's Stylelint tasks. +""" +import ddt +from mock import MagicMock, patch +from paver.easy import call_task + +from .utils import PaverTestCase + + +@ddt.ddt +class TestPaverStylelint(PaverTestCase): + """ + Tests for Paver's Stylelint tasks. + """ + + def setUp(self): + super(TestPaverStylelint, self).setUp() + pass + + @ddt.data( + [0, False], + [99, False], + [100, True], + ) + @ddt.unpack + def test_run_stylelint(self, violations_limit, should_pass): + """ + Verify that the quality task fails with Stylelint violations. + """ + _mock_stylelint_violations = MagicMock(return_value=100) + with patch('pavelib.quality._get_stylelint_violations', _mock_stylelint_violations): + if should_pass: + call_task('pavelib.quality.run_stylelint', options={"limit": violations_limit}) + else: + with self.assertRaises(SystemExit): + call_task('pavelib.quality.run_stylelint', options={"limit": violations_limit}) diff --git a/pavelib/quality.py b/pavelib/quality.py index 717db9dcb648..2b14ddd4f3bd 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -1,3 +1,5 @@ +# coding=utf-8 + """ Check code quality using pep8, pylint, and diff_quality. """ @@ -6,7 +8,7 @@ import re from string import join -from paver.easy import BuildFailure, cmdopts, needs, sh, task +from paver.easy import BuildFailure, call_task, cmdopts, needs, sh, task from openedx.core.djangolib.markup import HTML @@ -189,7 +191,7 @@ def _get_pep8_violations(): "{report_dir}/pep8.report".format(report_dir=report_dir) ) - return (count, violations_list) + return count, violations_list def _pep8_violations(report_file): @@ -315,6 +317,64 @@ def run_eslint(options): ) +def _get_stylelint_violations(): + """ + Returns the number of Stylelint violations. + """ + stylelint_report_dir = (Env.REPORT_DIR / "stylelint") + stylelint_report = stylelint_report_dir / "stylelint.report" + _prepare_report_dir(stylelint_report_dir) + formatter = 'node_modules/stylelint-formatter-pretty' + + sh( + "stylelint **/*.scss --custom-formatter={formatter} | tee {stylelint_report}".format( + formatter=formatter, + stylelint_report=stylelint_report, + ), + ignore_error=True + ) + + try: + return int(_get_count_from_last_line(stylelint_report, "eslint")) + except TypeError: + raise BuildFailure( + "Error. Number of eslint violations could not be found in {eslint_report}".format( + eslint_report=stylelint_report + ) + ) + + +@task +@needs('pavelib.prereqs.install_node_prereqs') +@cmdopts([ + ("limit=", "l", "limit for number of acceptable violations"), +]) +@timed +def run_stylelint(options): + """ + Runs stylelint on Sass files. + If limit option is passed, fails build if more violations than the limit are found. + """ + violations_limit = int(getattr(options, 'limit', -1)) + num_violations = _get_stylelint_violations() + + # Record the metric + _write_metric(num_violations, (Env.METRICS_DIR / "stylelint")) + + # Fail if number of violations is greater than the limit + if num_violations > violations_limit > -1: + raise BuildFailure( + "Stylelint failed with too many violations: ({count}).\nThe limit is {violations_limit}.".format( + count=num_violations, + violations_limit=violations_limit, + ) + ) + if num_violations > 0: + print("Stylelint succeeded with no more violations than {violations_limit}".format( + violations_limit=violations_limit, + )) + + @task @needs('pavelib.prereqs.install_python_prereqs') @cmdopts([ @@ -518,7 +578,7 @@ def _get_count_from_last_line(filename, file_type): This will return the number in the last line of a file. It is returning only the value (as a floating number). """ - last_line = _get_report_contents(filename, last_line_only=True) + last_line = _get_report_contents(filename, last_line_only=True).strip() if file_type is "python_complexity": # Example of the last line of a complexity report: "Average complexity: A (1.93953443446)" regex = r'\d+.\d+' diff --git a/scripts/all-tests.sh b/scripts/all-tests.sh index eb93c94c9a50..e705dfc64609 100755 --- a/scripts/all-tests.sh +++ b/scripts/all-tests.sh @@ -13,6 +13,7 @@ set -e # Violations thresholds for failing the build export PYLINT_THRESHOLD=3600 export ESLINT_THRESHOLD=9134 +export STYLELINT_THRESHOLD=18431 XSSLINT_THRESHOLDS=`cat scripts/xsslint_thresholds.json` export XSSLINT_THRESHOLDS=${XSSLINT_THRESHOLDS//[[:space:]]/} diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh index 10a6e2502a70..bcc201ff6389 100755 --- a/scripts/circle-ci-tests.sh +++ b/scripts/circle-ci-tests.sh @@ -65,6 +65,9 @@ else echo "Finding ESLint violations and storing report..." paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; } + echo "Finding Stylelint violations and storing report..." + paver run_stylelint -l $STYLELINT_THRESHOLD > stylelint.log || { cat stylelint.log; EXIT=1; } + # Run quality task. Pass in the 'fail-under' percentage to diff-quality paver run_quality -p 100 || EXIT=1 diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index cc94990569d2..65535c3f9c28 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -84,6 +84,8 @@ case "$TEST_SUITE" in echo "Finding ESLint violations and storing report..." paver run_eslint -l $ESLINT_THRESHOLD > eslint.log || { cat eslint.log; EXIT=1; } + echo "Finding Stylelint violations and storing report..." + paver run_stylelint -l $STYLELINT_THRESHOLD > stylelint.log || { cat stylelint.log; EXIT=1; } echo "Running code complexity report (python)." paver run_complexity || echo "Unable to calculate code complexity. Ignoring error." echo "Running xss linter report." diff --git a/stylelint.config.js b/stylelint.config.js new file mode 100644 index 000000000000..bd7769911708 --- /dev/null +++ b/stylelint.config.js @@ -0,0 +1,3 @@ +module.exports = { + extends: '@edx/stylelint-config-edx' +};