zuul
/
zuul
Code Issues Proposed changes

Report correctly when dequeuing dependent changes 

The change in commit b7179777f4
(change I98eb8b8817bbda57efc2ef5bfcc2a5076fe8f4fd) erroneously
altered the logic in formatReport.  The "dequeued_needing_change"
flag was only checked if the merger encountered a merge conflict.
So, a change whose parent was dequeued would only be reported as
having been dequeued if it had a merge conflict.  In the (frequent)
case that a change is dequeued because its parent failed tests or
otherwise failed to merge, but has no merge conflicts itself, the
flag was not checked and the usual path of displaying job results
was taken.  In our configuration, it is not possible to format
those job results (they are all SKIPPED), so Zuul would log an
exception and not report.

This change restores the original hierarchy of report types:

 1. Dequeued because it depends on a change that failed to merge.
 2. Merge conflict.
 3. Job report.

It also moves the bulk of the job report formatting to its own
method so that the logic around the report types is more clear,
and reduces redundancy introduced by the report footer option.
Finally, it updates a test to verify that the correct report types
are issued.

Story: #147
Change-Id: If1fe3af8b4d0896a9e43d8163bc2690663df0637
Co-Authored-By: James E. Blair <jeblair@hp.com>
This commit is contained in:
Jeremy Stanley 2014-08-02 16:10:56 +00:00
parent c6d4bc8683
commit 1083713e5a
2 changed files with 22 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
tests/test_scheduler.py
View File

@ -1028,14 +1028,19 @@ class TestScheduler(ZuulTestCase):
self.assertEqual(A.data['status'], 'MERGED')
self.assertEqual(A.reported, 2)
self.assertIn('Build succeeded', A.messages[1])
self.assertEqual(B.data['status'], 'NEW')
self.assertEqual(B.reported, 2)
self.assertIn('Build failed', B.messages[1])
self.assertEqual(C.data['status'], 'NEW')
self.assertEqual(C.reported, 2)
self.assertIn('depends on a change', C.messages[1])
self.assertEqual(D.data['status'], 'NEW')
self.assertEqual(D.reported, 2)
self.assertIn('depends on a change', D.messages[1])
self.assertEqual(E.data['status'], 'MERGED')
self.assertEqual(E.reported, 2)
self.assertIn('Build succeeded', E.messages[1])
self.assertEqual(len(self.history), 18)
def test_head_is_dequeued_once(self):

30
zuul/scheduler.py
View File

@ -1460,19 +1460,25 @@ class BasePipelineManager(object):
def formatReport(self, item):
ret = ''
if not self.pipeline.didMergerSucceed(item):
if item.dequeued_needing_change:
ret += 'This change depends on a change that failed to merge.\n'
elif 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 self.pipeline.didAllJobsSucceed(item):
ret += self.pipeline.success_message + '\n\n'
else:
ret += self.pipeline.failure_message + '\n\n'
ret += self._formatReportJobs(item)
if self.pipeline.footer_message:
ret += '\n' + self.pipeline.footer_message
return ret
def _formatReportJobs(self, item):
# Return the list of jobs portion of the report
ret = ''
if self.sched.config.has_option('zuul', 'url_pattern'):
url_pattern = self.sched.config.get('zuul', 'url_pattern')
@ -1523,8 +1529,6 @@ class BasePipelineManager(object):
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):