TE-2808 Fix diff-quality comparison target

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):

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

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: