diff --git a/pavelib/paver_tests/test_paver_quality.py b/pavelib/paver_tests/test_paver_quality.py index 8a26d747fb..ba9bb1b762 100644 --- a/pavelib/paver_tests/test_paver_quality.py +++ b/pavelib/paver_tests/test_paver_quality.py @@ -323,9 +323,10 @@ class TestPaverRunQuality(PaverTestCase): # Test that pylint is called _mock_pylint_violations.assert_called_once_with(clean=False) - # Assert that sh was called twice- once for diff quality with pylint + # Assert that sh was called four times - once to get the comparison commit hash, + # once to get the current commit hash, once for diff quality with pylint, # and once for diff quality with eslint - self.assertEqual(self._mock_paver_sh.call_count, 2) + self.assertEqual(self._mock_paver_sh.call_count, 4) @patch('__builtin__.open', mock_open()) def test_other_exception(self): diff --git a/pavelib/paver_tests/utils.py b/pavelib/paver_tests/utils.py index 931ca2f5d9..fd67b4c0d3 100644 --- a/pavelib/paver_tests/utils.py +++ b/pavelib/paver_tests/utils.py @@ -2,6 +2,7 @@ import os from unittest import TestCase +from uuid import uuid4 from paver import tasks from paver.easy import BuildFailure @@ -62,7 +63,7 @@ class MockEnvironment(tasks.Environment): self.messages.append(unicode(output)) -def fail_on_eslint(*args): +def fail_on_eslint(*args, **kwargs): """ 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 @@ -71,7 +72,10 @@ def fail_on_eslint(*args): if "eslint" in args[0]: raise BuildFailure('Subprocess return code: 1') else: - return + if kwargs.get('capture', False): + return uuid4().hex + else: + return def fail_on_npm_install(*args, **kwargs): # pylint: disable=unused-argument diff --git a/pavelib/quality.py b/pavelib/quality.py index 7ebbc402c5..f4f39a0d43 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -826,41 +826,41 @@ def run_quality(options): 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. - compare_branch = getattr(options, 'compare_branch', None) - compare_branch_string = u'' - if compare_branch: - compare_branch_string = u'--compare-branch={0}'.format(compare_branch) + # Set the string to be used for the diff-quality --compare-branch switch. + compare_branch = getattr(options, 'compare_branch', u'origin/master') + compare_commit = sh('git merge-base HEAD {}'.format(compare_branch), capture=True).strip() + if sh('git rev-parse HEAD', capture=True).strip() != compare_commit: + compare_branch_string = u'--compare-branch={0}'.format(compare_commit) - # Set the string, if needed, to be used for the diff-quality --fail-under switch. - diff_threshold = int(getattr(options, 'percentage', -1)) - percentage_string = u'' - if diff_threshold > -1: - percentage_string = u'--fail-under={0}'.format(diff_threshold) + # Set the string, if needed, to be used for the diff-quality --fail-under switch. + diff_threshold = int(getattr(options, 'percentage', -1)) + percentage_string = u'' + 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) - if not run_diff_quality( - violations_type="eslint", - reports=eslint_reports, + 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('Eslint violation(s) were found in the lines of code that were added or changed.') + ): + 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) + 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_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_pass: