Merge "Allow merge failures to have unique reporters."

doc/source/zuul.rst

@@ -238,6 +238,12 @@ explanation of each of the parameters::
   reported back to Gerrit when at least one voting build fails.
   Defaults to "Build failed."
 
+**merge-failure-message**
+  An optional field that supplies the introductory text in message
+  reported back to Gerrit when a change fails to merge with the
+  current state of the repository.
+  Defaults to "Merge failed."
+
 **footer-message**
   An optional field to supply additional information after test results.
   Useful for adding information about the CI system such as debugging
@@ -413,6 +419,12 @@ explanation of each of the parameters::
   Uses the same syntax as **success**, but describes what Zuul should
   do if at least one job fails.
 
+**merge-failure**
+  Uses the same syntax as **success**, but describes what Zuul should
+  do if it is unable to merge in the patchset. If no merge-failure
+  reporters are listed then the ``failure`` reporters will be used to
+  notify of unsuccessful merges.
+
 **start**
   Uses the same syntax as **success**, but describes what Zuul should
   do when a change is added to the pipeline manager.  This can be used,

tests/fixtures/layout-merge-failure.yaml

@@ -0,0 +1,56 @@
+pipelines:
+  - name: check
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+  - name: post
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: ref-updated
+          ref: ^(?!refs/).*$
+
+  - name: gate
+    manager: DependentPipelineManager
+    failure-message: Build failed.  For information on how to proceed, see http://wiki.example.org/Test_Failures
+    merge-failure-message: "The merge failed! For more information..."
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - approved: 1
+    success:
+      gerrit:
+        verified: 2
+        submit: true
+    failure:
+      gerrit:
+        verified: -2
+    merge-failure:
+      gerrit:
+        verified: -1
+      smtp:
+        to: you@example.com
+    start:
+      gerrit:
+        verified: 0
+    precedence: high
+
+projects:
+  - name: org/project
+    check:
+      - project-merge:
+        - project-test1
+        - project-test2
+    gate:
+      - project-merge:
+        - project-test1
+        - project-test2

tests/fixtures/layouts/bad_merge_failure.yaml

@@ -0,0 +1,39 @@
+pipelines:
+  - name: check
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+    merge-failure-message:
+
+  - name: gate
+    manager: DependentPipelineManager
+    failure-message: Build failed.  For information on how to proceed, see http://wiki.example.org/Test_Failures
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - approved: 1
+    success:
+      gerrit:
+        verified: 2
+        submit: true
+    failure:
+      gerrit:
+        verified: -2
+    merge-failure:
+    start:
+      gerrit:
+        verified: 0
+    precedence: high
+
+projects:
+  - name: org/project
+    check:
+      - project-check

tests/fixtures/layouts/good_merge_failure.yaml

@@ -0,0 +1,53 @@
+pipelines:
+  - name: check
+    manager: IndependentPipelineManager
+    merge-failure-message: "Could not merge the change. Please rebase..."
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+  - name: post
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: ref-updated
+          ref: ^(?!refs/).*$
+    merge-failure:
+      gerrit:
+        verified: -1
+
+  - name: gate
+    manager: DependentPipelineManager
+    failure-message: Build failed.  For information on how to proceed, see http://wiki.example.org/Test_Failures
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - approved: 1
+    success:
+      gerrit:
+        verified: 2
+        submit: true
+    failure:
+      gerrit:
+        verified: -2
+    merge-failure:
+      gerrit:
+        verified: -1
+      smtp:
+        to: you@example.com
+    start:
+      gerrit:
+        verified: 0
+    precedence: high
+
+projects:
+  - name: org/project
+    check:
+      - project-check

tests/test_scheduler.py

@@ -3731,3 +3731,82 @@ For CI problems and help debugging, contact ci@example.org"""
 
         self.assertEqual(failure_body, self.smtp_messages[0]['body'])
         self.assertEqual(success_body, self.smtp_messages[1]['body'])
+
+    def test_merge_failure_reporters(self):
+        """Check that the config is set up correctly"""
+
+        self.config.set('zuul', 'layout_config',
+                        'tests/fixtures/layout-merge-failure.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        self.assertEqual(
+            "Merge Failed.\n\nThis change was unable to be automatically "
+            "merged with the current state of the repository. Please rebase "
+            "your change and upload a new patchset.",
+            self.sched.layout.pipelines['check'].merge_failure_message)
+        self.assertEqual(
+            "The merge failed! For more information...",
+            self.sched.layout.pipelines['gate'].merge_failure_message)
+
+        self.assertEqual(
+            len(self.sched.layout.pipelines['check'].merge_failure_actions), 1)
+        self.assertEqual(
+            len(self.sched.layout.pipelines['gate'].merge_failure_actions), 2)
+
+        self.assertTrue(isinstance(
+            self.sched.layout.pipelines['check'].merge_failure_actions[0].
+            reporter, zuul.reporter.gerrit.Reporter))
+
+        self.assertTrue(
+            (
+                isinstance(self.sched.layout.pipelines['gate'].
+                           merge_failure_actions[0].reporter,
+                           zuul.reporter.smtp.Reporter) and
+                isinstance(self.sched.layout.pipelines['gate'].
+                           merge_failure_actions[1].reporter,
+                           zuul.reporter.gerrit.Reporter)
+            ) or (
+                isinstance(self.sched.layout.pipelines['gate'].
+                           merge_failure_actions[0].reporter,
+                           zuul.reporter.gerrit.Reporter) and
+                isinstance(self.sched.layout.pipelines['gate'].
+                           merge_failure_actions[1].reporter,
+                           zuul.reporter.smtp.Reporter)
+            )
+        )
+
+    def test_merge_failure_reports(self):
+        """Check that when a change fails to merge the correct message is sent
+        to the correct reporter"""
+        self.config.set('zuul', 'layout_config',
+                        'tests/fixtures/layout-merge-failure.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        # Check a test failure isn't reported to SMTP
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.addApproval('CRVW', 2)
+        self.worker.addFailTest('project-test1', A)
+        self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
+        self.waitUntilSettled()
+
+        self.assertEqual(3, len(self.history))  # 3 jobs
+        self.assertEqual(0, len(self.smtp_messages))
+
+        # Check a merge failure is reported to SMTP
+        # B should be merged, but C will conflict with B
+        B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
+        B.addPatchset(['conflict'])
+        C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
+        C.addPatchset(['conflict'])
+        B.addApproval('CRVW', 2)
+        C.addApproval('CRVW', 2)
+        self.fake_gerrit.addEvent(B.addApproval('APRV', 1))
+        self.fake_gerrit.addEvent(C.addApproval('APRV', 1))
+        self.waitUntilSettled()
+
+        self.assertEqual(6, len(self.history))  # A and B jobs
+        self.assertEqual(1, len(self.smtp_messages))
+        self.assertEqual('The merge failed! For more information...',
+                         self.smtp_messages[0]['body'])

zuul/layoutvalidator.py

@@ -79,11 +79,13 @@ class LayoutSchema(object):
                 'description': str,
                 'success-message': str,
                 'failure-message': str,
+                'merge-failure-message': str,
                 'footer-message': str,
                 'dequeue-on-new-patchset': bool,
                 'trigger': trigger,
                 'success': report_actions,
                 'failure': report_actions,
+                'merge-failure': report_actions,
                 'start': report_actions,
                 'window': window,
                 'window-floor': window_floor,

zuul/model.py

@@ -63,6 +63,7 @@ class Pipeline(object):
         self.name = name
         self.description = None
         self.failure_message = None
+        self.merge_failure_message = None
         self.success_message = None
         self.footer_message = None
         self.dequeue_on_new_patchset = True
@@ -171,6 +172,11 @@ class Pipeline(object):
                 return False
         return True
 
+    def didMergerSucceed(self, item):
+        if item.current_build_set.unable_to_merge:
+            return False
+        return True
+
     def didAnyJobFail(self, item):
         for job in self.getJobs(item.change):
             if not job.voting:
@@ -206,9 +212,8 @@ class Pipeline(object):
                 fakebuild.result = 'SKIPPED'
                 item.addBuild(fakebuild)
 
-    def setUnableToMerge(self, item, msg):
+    def setUnableToMerge(self, item):
         item.current_build_set.unable_to_merge = True
-        item.current_build_set.unable_to_merge_message = msg
         root = self.getJobTree(item.change.project)
         for job in root.getJobs():
             fakebuild = Build(job, None)
@@ -677,7 +682,6 @@ class BuildSet(object):
         self.commit = None
         self.zuul_url = None
         self.unable_to_merge = False
-        self.unable_to_merge_message = None
         self.failing_reasons = []
         self.merge_state = self.NEW
 

zuul/scheduler.py

@@ -226,6 +226,11 @@ class Scheduler(threading.Thread):
             pipeline.precedence = precedence
             pipeline.failure_message = conf_pipeline.get('failure-message',
                                                          "Build failed.")
+            pipeline.merge_failure_message = conf_pipeline.get(
+                'merge-failure-message', "Merge Failed.\n\nThis change was "
+                "unable to be automatically merged with the current state of "
+                "the repository. Please rebase your change and upload a new "
+                "patchset.")
             pipeline.success_message = conf_pipeline.get('success-message',
                                                          "Build succeeded.")
             pipeline.footer_message = conf_pipeline.get('footer-message', "")
@@ -233,7 +238,7 @@ class Scheduler(threading.Thread):
                 'dequeue-on-new-patchset', True)
 
             action_reporters = {}
-            for action in ['start', 'success', 'failure']:
+            for action in ['start', 'success', 'failure', 'merge-failure']:
                 action_reporters[action] = []
                 if conf_pipeline.get(action):
                     for reporter_name, params \
@@ -247,6 +252,11 @@ class Scheduler(threading.Thread):
             pipeline.start_actions = action_reporters['start']
             pipeline.success_actions = action_reporters['success']
             pipeline.failure_actions = action_reporters['failure']
+            if len(action_reporters['merge-failure']) > 0:
+                pipeline.merge_failure_actions = \
+                    action_reporters['merge-failure']
+            else:
+                pipeline.merge_failure_actions = action_reporters['failure']
 
             pipeline.window = conf_pipeline.get('window', 20)
             pipeline.window_floor = conf_pipeline.get('window-floor', 3)
@@ -936,6 +946,8 @@ class BasePipelineManager(object):
         self.log.info("    %s" % self.pipeline.success_actions)
         self.log.info("  On failure:")
         self.log.info("    %s" % self.pipeline.failure_actions)
+        self.log.info("  On merge-failure:")
+        self.log.info("    %s" % self.pipeline.merge_failure_actions)
 
     def getSubmitAllowNeeds(self):
         # Get a list of code review labels that are allowed to be
@@ -1334,10 +1346,7 @@ class BasePipelineManager(object):
             build_set.commit = item.change.newrev
         if not build_set.commit:
             self.log.info("Unable to merge change %s" % item.change)
-            msg = ("This change was unable to be automatically merged "
-                   "with the current state of the repository. Please "
-                   "rebase your change and upload a new patchset.")
-            self.pipeline.setUnableToMerge(item, msg)
+            self.pipeline.setUnableToMerge(item)
 
     def reportItem(self, item):
         if item.reported:
@@ -1370,10 +1379,12 @@ class BasePipelineManager(object):
         self.log.debug("Reporting change %s" % item.change)
         ret = True  # Means error as returned by trigger.report
         if self.pipeline.didAllJobsSucceed(item):
-            self.log.debug("success %s %s" % (self.pipeline.success_actions,
-                                              self.pipeline.failure_actions))
+            self.log.debug("success %s" % (self.pipeline.success_actions))
             actions = self.pipeline.success_actions
             item.setReportedResult('SUCCESS')
+        elif not self.pipeline.didMergerSucceed(item):
+            actions = self.pipeline.merge_failure_actions
+            item.setReportedResult('MERGER_FAILURE')
         else:
             actions = self.pipeline.failure_actions
             item.setReportedResult('FAILURE')
@@ -1395,66 +1406,72 @@ class BasePipelineManager(object):
 
     def formatReport(self, item):
         ret = ''
+
+        if not self.pipeline.didMergerSucceed(item):
+            ret += self.pipeline.merge_failure_message
+            if item.dequeued_needing_change:
+                ret += ('\n\nThis change depends on a change that failed to '
+                        'merge.')
+            if self.pipeline.footer_message:
+                ret += '\n\n' + self.pipeline.footer_message
+            return ret
+
         if self.pipeline.didAllJobsSucceed(item):
             ret += self.pipeline.success_message + '\n\n'
         else:
             ret += self.pipeline.failure_message + '\n\n'
 
-        if item.dequeued_needing_change:
-            ret += "This change depends on a change that failed to merge."
-        elif item.current_build_set.unable_to_merge_message:
-            ret += item.current_build_set.unable_to_merge_message
+        if self.sched.config.has_option('zuul', 'url_pattern'):
+            url_pattern = self.sched.config.get('zuul', 'url_pattern')
         else:
-            if self.sched.config.has_option('zuul', 'url_pattern'):
-                url_pattern = self.sched.config.get('zuul', 'url_pattern')
+            url_pattern = None
+
+        for job in self.pipeline.getJobs(item.change):
+            build = item.current_build_set.getBuild(job.name)
+            result = build.result
+            pattern = url_pattern
+            if result == 'SUCCESS':
+                if job.success_message:
+                    result = job.success_message
+                if job.success_pattern:
+                    pattern = job.success_pattern
+            elif result == 'FAILURE':
+                if job.failure_message:
+                    result = job.failure_message
+                if job.failure_pattern:
+                    pattern = job.failure_pattern
+            if pattern:
+                url = pattern.format(change=item.change,
+                                     pipeline=self.pipeline,
+                                     job=job,
+                                     build=build)
             else:
-                url_pattern = None
-            for job in self.pipeline.getJobs(item.change):
-                build = item.current_build_set.getBuild(job.name)
-                result = build.result
-                pattern = url_pattern
-                if result == 'SUCCESS':
-                    if job.success_message:
-                        result = job.success_message
-                    if job.success_pattern:
-                        pattern = job.success_pattern
-                elif result == 'FAILURE':
-                    if job.failure_message:
-                        result = job.failure_message
-                    if job.failure_pattern:
-                        pattern = job.failure_pattern
-                if pattern:
-                    url = pattern.format(change=item.change,
-                                         pipeline=self.pipeline,
-                                         job=job,
-                                         build=build)
-                else:
-                    url = build.url or job.name
-                if not job.voting:
-                    voting = ' (non-voting)'
-                else:
-                    voting = ''
-                if self.report_times and build.end_time and build.start_time:
-                    dt = int(build.end_time - build.start_time)
-                    m, s = divmod(dt, 60)
-                    h, m = divmod(m, 60)
-                    if h:
-                        elapsed = ' in %dh %02dm %02ds' % (h, m, s)
-                    elif m:
-                        elapsed = ' in %dm %02ds' % (m, s)
-                    else:
-                        elapsed = ' in %ds' % (s)
+                url = build.url or job.name
+            if not job.voting:
+                voting = ' (non-voting)'
+            else:
+                voting = ''
+            if self.report_times and build.end_time and build.start_time:
+                dt = int(build.end_time - build.start_time)
+                m, s = divmod(dt, 60)
+                h, m = divmod(m, 60)
+                if h:
+                    elapsed = ' in %dh %02dm %02ds' % (h, m, s)
+                elif m:
+                    elapsed = ' in %dm %02ds' % (m, s)
                 else:
-                    elapsed = ''
-                name = ''
-                if self.sched.config.has_option('zuul', 'job_name_in_report'):
-                    if self.sched.config.getboolean('zuul',
-                                                    'job_name_in_report'):
-                        name = job.name + ' '
-                ret += '- %s%s : %s%s%s\n' % (name, url, result, elapsed,
-                                              voting)
-            ret += '\n'
-        ret += self.pipeline.footer_message
+                    elapsed = ' in %ds' % (s)
+            else:
+                elapsed = ''
+            name = ''
+            if self.sched.config.has_option('zuul', 'job_name_in_report'):
+                if self.sched.config.getboolean('zuul',
+                                                'job_name_in_report'):
+                    name = job.name + ' '
+            ret += '- %s%s : %s%s%s\n' % (name, url, result, elapsed,
+                                          voting)
+        if self.pipeline.footer_message:
+            ret += '\n' + self.pipeline.footer_message
         return ret
 
     def formatDescription(self, build):