Shard the quality tasks on jenkins

pavelib/paver_tests/test_paver_quality.py

@@ -300,28 +300,11 @@ def setUp(self):
         self.addCleanup(shutil.rmtree, self.report_dir)
         self.addCleanup(report_dir_patcher.stop)
 
-    @patch('__builtin__.open', mock_open())
-    def test_failure_on_diffquality_pep8(self):
-        """
-        If pep8 finds errors, pylint and eslint should still be run
-        """
-        # Mock _get_pep8_violations to return a violation
-        _mock_pep8_violations = MagicMock(
-            return_value=(1, ['lms/envs/common.py:32:2: E225 missing whitespace around operator'])
-        )
-        with patch('pavelib.quality._get_pep8_violations', _mock_pep8_violations):
-            with self.assertRaises(SystemExit):
-                pavelib.quality.run_quality("")
-
-        # Test that pep8, pylint and eslint were called by counting the calls to
-        # _get_pep8_violations (for pep8) and sh (5 for pylint & 1 for eslint)
-        self.assertEqual(_mock_pep8_violations.call_count, 1)
-        self.assertEqual(self._mock_paver_sh.call_count, 6)
-
     @patch('__builtin__.open', mock_open())
     def test_failure_on_diffquality_pylint(self):
         """
-        If diff-quality fails on pylint, the paver task should also fail
+        If diff-quality fails on pylint, the paver task should also fail, but
+        only after runnning diff-quality with eslint
         """
 
         # Underlying sh call must fail when it is running the pylint diff-quality task
@@ -331,11 +314,11 @@ def test_failure_on_diffquality_pylint(self):
                 with self.assertRaises(SystemExit):
                     pavelib.quality.run_quality("")
 
-        # Test that both pep8 and pylint were called by counting the calls
         # Assert that _get_pylint_violations (which calls "pylint") is called once
         self.assertEqual(_mock_pylint_violations.call_count, 1)
-        # And assert that sh was called 6 times (1 for pep8 & 1 for eslint).
-        # This means that even in the event of a diff-quality pylint failure, eslint and pep8 are still called.
+        # Assert that sh was called twice- once for diff quality with pylint
+        # and once for diff quality with eslint. This means that 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())
@@ -346,26 +329,23 @@ def test_failure_on_diffquality_eslint(self):
 
         # Underlying sh call must fail when it is running the eslint diff-quality task
         self._mock_paver_sh.side_effect = fail_on_eslint
-        _mock_pep8_violations = MagicMock(return_value=(0, []))
         _mock_pylint_violations = MagicMock(return_value=(0, []))
-        with patch('pavelib.quality._get_pep8_violations', _mock_pep8_violations):
-            with patch('pavelib.quality._get_pylint_violations', _mock_pylint_violations):
-                with self.assertRaises(SystemExit):
-                    pavelib.quality.run_quality("")
-                    self.assertRaises(BuildFailure)
+        with patch('pavelib.quality._get_pylint_violations', _mock_pylint_violations):
+            with self.assertRaises(SystemExit):
+                pavelib.quality.run_quality("")
+                self.assertRaises(BuildFailure)
         print self._mock_paver_sh.mock_calls
 
-        # Test that both pep8 and pylint were called by counting the calls
-        # Assert that _get_pep8_violations (which calls "pep8") is called once
-        _mock_pep8_violations.assert_called_once_with(clean=False)
+        # Test that pylint is called
         _mock_pylint_violations.assert_called_once_with(clean=False)
-        # And assert that sh was called once (the call to eslint)
-        self.assertEqual(self._mock_paver_sh.call_count, 1)
+        # Assert that sh was called twice- once for diff quality with pylint
+        # and once for diff quality with eslint
+        self.assertEqual(self._mock_paver_sh.call_count, 2)
 
     @patch('__builtin__.open', mock_open())
     def test_other_exception(self):
         """
-        If diff-quality fails for an unknown reason on the first run (pep8), then
+        If diff-quality fails for an unknown reason on the first run, then
         pylint should not be run
         """
         self._mock_paver_sh.side_effect = [Exception('unrecognized failure!'), 0]
@@ -379,5 +359,8 @@ def test_other_exception(self):
     def test_no_diff_quality_failures(self):
         # Assert nothing is raised
         pavelib.quality.run_quality("")
-        # And assert that sh was called 7 times (1 for pep8, 5 for pylint, and 1 for eslint)
+        # And assert that sh was called 7 times:
+        # 5 for pylint on each of the system directories
+        # 1 for diff_quality for pylint
+        # 1 for diff_quality for eslint
         self.assertEqual(self._mock_paver_sh.call_count, 7)

pavelib/paver_tests/utils.py

@@ -65,8 +65,9 @@ def info(self, message, *args):
 
 def fail_on_eslint(*args):
     """
-    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.
+    For our tests, we need the call for diff-quality running eslint reports
+    to fail, since that is what is going to fail when we pass in a
+    percentage ("p") requirement.
     """
     if "eslint" in args[0]:
         # Essentially mock diff-quality exiting with 1
@@ -77,8 +78,9 @@ def fail_on_eslint(*args):
 
 def fail_on_pylint(*args):
     """
-    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.
+    For our tests, we need the call for diff-quality running pylint reports
+    to fail, since that is what is going to fail when we pass in a
+    percentage ("p") requirement.
     """
     if "pylint" in args[0]:
         # Essentially mock diff-quality exiting with 1

pavelib/quality.py

@@ -696,18 +696,18 @@ def run_quality(options):
     :param: p, diff-quality will fail if the quality percentage calculated is
         below this percentage. For example, if p is set to 80, and diff-quality finds
         quality of the branch vs the compare branch is less than 80%, then this task will fail.
-        This threshold would be applied to both pep8 and pylint.
     """
     # Directory to put the diff reports in.
     # This makes the folder if it doesn't already exist.
     dquality_dir = (Env.REPORT_DIR / "diff_quality").makedirs_p()
 
     # Save the pass variable. It will be set to false later if failures are detected.
-    diff_quality_percentage_pass = True
+    diff_quality_pass = True
+    failure_reasons = []
 
     def _lint_output(linter, count, violations_list, is_html=False, limit=0):
         """
-        Given a count & list of pep8 violations, pretty-print the pep8 output.
+        Given a count & list of pylint violations, pretty-print the output.
         If `is_html`, will print out with HTML markup.
         """
         if is_html:
@@ -740,36 +740,16 @@ def _lint_output(linter, count, violations_list, is_html=False, limit=0):
 
         return ''.join(lines)
 
-    # Run pep8 directly since we have 0 violations on master
-    (count, violations_list) = _get_pep8_violations(clean=False)
-
-    # Print number of violations to log
-    print _lint_output('pep8', count, violations_list)
-
-    # Also write the number of violations to a file
-    with open(dquality_dir / "diff_quality_pep8.html", "w") as f:
-        f.write(_lint_output('pep8', count, violations_list, is_html=True))
-
-    if count > 0:
-        diff_quality_percentage_pass = False
-
-    # Generate diff-quality html report for pylint, and print to console
     # If pylint reports exist, use those
     # Otherwise, `diff-quality` will call pylint itself
-
     (count, violations_list) = _get_pylint_violations(clean=False)
-
     _, upper_violations_limit, _, _ = _parse_pylint_options(options)
 
-    # Print number of violations to log
+    # Print total number of violations to log
     print _lint_output('pylint', count, violations_list, limit=upper_violations_limit)
-
-    # Also write the number of violations to a file
-    with open(dquality_dir / "diff_quality_pylint.html", "w") as f:
-        f.write(_lint_output('pylint', count, violations_list, is_html=True, limit=upper_violations_limit))
-
     if count > upper_violations_limit > -1:
-        diff_quality_percentage_pass = False
+        diff_quality_pass = False
+        failure_reasons.append('Too many total violations.')
 
     # ----- Set up for diff-quality pylint call -----
     # Set the string, if needed, to be used for the diff-quality --compare-branch switch.
@@ -784,22 +764,33 @@ def _lint_output(linter, count, violations_list, is_html=False, limit=0):
     if diff_threshold > -1:
         percentage_string = u'--fail-under={0}'.format(diff_threshold)
 
+    pylint_files = get_violations_reports("pylint")
+    pylint_reports = u' '.join(pylint_files)
+    if not run_diff_quality(
+        violations_type="pylint",
+        reports=pylint_reports,
+        percentage_string=percentage_string,
+        branch_string=compare_branch_string,
+        dquality_dir=dquality_dir
+    ):
+        diff_quality_pass = False
+        failure_reasons.append('Pylint violation(s) were found in the lines of code that were added or changed.')
+
     eslint_files = get_violations_reports("eslint")
     eslint_reports = u' '.join(eslint_files)
-
-    # run diff-quality for eslint.
     if not run_diff_quality(
             violations_type="eslint",
             reports=eslint_reports,
             percentage_string=percentage_string,
             branch_string=compare_branch_string,
             dquality_dir=dquality_dir
     ):
-        diff_quality_percentage_pass = False
+        diff_quality_pass = False
+        failure_reasons.append('Eslint violation(s) were found in the lines of code that were added or changed.')
 
     # If one of the quality runs fails, then paver exits with an error when it is finished
-    if not diff_quality_percentage_pass:
-        msg = "FAILURE: Diff-quality failure(s). This means that violations were found in the current changeset."
+    if not diff_quality_pass:
+        msg = "FAILURE: " + " ".join(failure_reasons)
         raise BuildFailure(msg)
 
 

scripts/all-tests.sh

@@ -11,10 +11,7 @@ set -e
 ###############################################################################
 
 # Violations thresholds for failing the build
-export LOWER_PYLINT_THRESHOLD=1000
-export UPPER_PYLINT_THRESHOLD=5900
-export ESLINT_THRESHOLD=5700
-export STYLELINT_THRESHOLD=973
+source scripts/thresholds.sh
 
 XSSLINT_THRESHOLDS=`cat scripts/xsslint_thresholds.json`
 export XSSLINT_THRESHOLDS=${XSSLINT_THRESHOLDS//[[:space:]]/}

scripts/generic-ci-tests.sh

@@ -100,28 +100,43 @@ function run_paver_quality {
 case "$TEST_SUITE" in
 
     "quality")
-        echo "Finding fixme's and storing report..."
-        run_paver_quality find_fixme || EXIT=1
-        echo "Finding pep8 violations and storing report..."
-        run_paver_quality run_pep8 || EXIT=1
-        echo "Finding pylint violations and storing in report..."
-        run_paver_quality run_pylint -l $LOWER_PYLINT_THRESHOLD:$UPPER_PYLINT_THRESHOLD || EXIT=1
 
         mkdir -p reports
 
-        echo "Finding ESLint violations and storing report..."
-        run_paver_quality run_eslint -l $ESLINT_THRESHOLD || EXIT=1
-        echo "Finding Stylelint violations and storing report..."
-        run_paver_quality run_stylelint -l $STYLELINT_THRESHOLD || EXIT=1
-        echo "Running code complexity report (python)."
-        run_paver_quality run_complexity || echo "Unable to calculate code complexity. Ignoring error."
-        echo "Running xss linter report."
-        run_paver_quality run_xsslint -t $XSSLINT_THRESHOLDS || EXIT=1
-        echo "Running safe commit linter report."
-        run_paver_quality run_xsscommitlint || EXIT=1
-        # Run quality task. Pass in the 'fail-under' percentage to diff-quality
-        echo "Running diff quality."
-        run_paver_quality run_quality -p 100 -l $LOWER_PYLINT_THRESHOLD:$UPPER_PYLINT_THRESHOLD || EXIT=1
+        case "$SHARD" in
+            1)
+                echo "Finding pylint violations and storing in report..."
+                run_paver_quality run_pylint --system=common  || { EXIT=1; }
+                ;;
+
+            2)
+                echo "Finding pylint violations and storing in report..."
+                run_paver_quality run_pylint --system=lms || { EXIT=1; }
+                ;;
+
+            3)
+                echo "Finding pylint violations and storing in report..."
+                run_paver_quality run_pylint --system="cms,openedx,pavelib" || { EXIT=1; }
+                ;;
+
+            4)
+                echo "Finding fixme's and storing report..."
+                run_paver_quality find_fixme || { EXIT=1; }
+                echo "Finding pep8 violations and storing report..."
+                run_paver_quality run_pep8 || { EXIT=1; }
+                echo "Finding ESLint violations and storing report..."
+                run_paver_quality run_eslint -l $ESLINT_THRESHOLD || { EXIT=1; }
+                echo "Finding Stylelint violations and storing report..."
+                run_paver_quality run_stylelint -l $STYLELINT_THRESHOLD || { EXIT=1; }
+                echo "Running code complexity report (python)."
+                run_paver_quality run_complexity || echo "Unable to calculate code complexity. Ignoring error."
+                echo "Running xss linter report."
+                run_paver_quality run_xsslint -t $XSSLINT_THRESHOLDS || { EXIT=1; }
+                echo "Running safe commit linter report."
+                run_paver_quality run_xsscommitlint || { EXIT=1; }
+                ;;
+
+        esac
 
         # Need to create an empty test result so the post-build
         # action doesn't fail the build.

scripts/jenkins-quality-diff.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+set -e
+
+# This script is used by the edx-platform-quality-check jenkins job.
+source scripts/jenkins-common.sh
+source scripts/thresholds.sh
+
+# Run quality task. Pass in the 'fail-under' percentage to diff-quality
+echo "Running diff quality."
+mkdir -p test_root/log/
+LOG_PREFIX=test_root/log/run_quality
+
+paver run_quality -p 100 -l $LOWER_PYLINT_THRESHOLD:$UPPER_PYLINT_THRESHOLD 2> $LOG_PREFIX.err.log > $LOG_PREFIX.out.log

scripts/thresholds.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+set -e
+
+export LOWER_PYLINT_THRESHOLD=1000
+export UPPER_PYLINT_THRESHOLD=5900
+export ESLINT_THRESHOLD=5700
+export STYLELINT_THRESHOLD=973