Return dependency cycle failure to user

This change changes the dependency cycle error handling in order
to report the error to the user via the standard report mechanics.

Change-Id: I7cdbd9b7b48fd612b2887007b70acedd7a8e3113
This commit is contained in:
Fabien Boucher 2019-07-24 12:12:51 +02:00
parent 5969c949b4
commit c2a33fcb9c
6 changed files with 30 additions and 24 deletions

View File

@ -183,7 +183,10 @@ class TestGerritToGithubCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertEqual(A.reported, 0)
self.assertEqual(A.reported, 1)
self.assertEqual(
A.messages[0],
"Build failed.\n\n\nWarning:\n Dependency cycle detected\n")
self.assertEqual(len(B.comments), 0)
self.assertEqual(A.data['status'], 'NEW')
self.assertFalse(B.is_merged)
@ -469,8 +472,8 @@ class TestGerritToGithubCRD(ZuulTestCase):
self.fake_github.emitEvent(A.getPullRequestEditedEvent())
self.waitUntilSettled()
# Dependency cycle injected so zuul should not have reported again on A
self.assertEqual(len(A.comments), 1)
# Dependency cycle injected so zuul should have reported again on A
self.assertEqual(len(A.comments), 2)
# Now if we update B to remove the depends-on, everything
# should be okay. B; A->B
@ -481,7 +484,7 @@ class TestGerritToGithubCRD(ZuulTestCase):
self.waitUntilSettled()
# Cycle was removed so now zuul should have reported again on A
self.assertEqual(len(A.comments), 2)
self.assertEqual(len(A.comments), 3)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(2))
self.waitUntilSettled()
@ -634,7 +637,7 @@ class TestGithubToGerritCRD(ZuulTestCase):
self.fake_github.emitEvent(A.addLabel('approved'))
self.waitUntilSettled()
self.assertEqual(len(A.comments), 0)
self.assertEqual(len(A.comments), 1)
self.assertEqual(B.reported, 0)
self.assertFalse(A.is_merged)
self.assertEqual(B.data['status'], 'NEW')
@ -930,8 +933,8 @@ class TestGithubToGerritCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
# Dependency cycle injected so zuul should not have reported again on A
self.assertEqual(A.reported, 1)
# Dependency cycle injected so zuul should have reported again on A
self.assertEqual(A.reported, 2)
# Now if we update B to remove the depends-on, everything
# should be okay. B; A->B
@ -942,7 +945,7 @@ class TestGithubToGerritCRD(ZuulTestCase):
self.waitUntilSettled()
# Cycle was removed so now zuul should have reported again on A
self.assertEqual(A.reported, 2)
self.assertEqual(A.reported, 3)
self.fake_github.emitEvent(B.getPullRequestEditedEvent())
self.waitUntilSettled()

View File

@ -335,7 +335,7 @@ class TestGerritCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertEqual(A.reported, 0)
self.assertEqual(A.reported, 1)
self.assertEqual(B.reported, 0)
self.assertEqual(A.data['status'], 'NEW')
self.assertEqual(B.data['status'], 'NEW')
@ -698,8 +698,8 @@ class TestGerritCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2))
self.waitUntilSettled()
# Dependency cycle injected so zuul should not have reported again on A
self.assertEqual(A.reported, 1)
# Dependency cycle injected so zuul should have reported again on A
self.assertEqual(A.reported, 2)
# Now if we update B to remove the depends-on, everything
# should be okay. B; A->B
@ -710,7 +710,7 @@ class TestGerritCRD(ZuulTestCase):
self.waitUntilSettled()
# Cycle was removed so now zuul should have reported again on A
self.assertEqual(A.reported, 2)
self.assertEqual(A.reported, 3)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(2))
self.waitUntilSettled()

View File

@ -270,7 +270,7 @@ class TestGerritLegacyCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertEqual(A.reported, 0)
self.assertEqual(A.reported, 1)
self.assertEqual(B.reported, 0)
self.assertEqual(A.data['status'], 'NEW')
self.assertEqual(B.data['status'], 'NEW')
@ -609,8 +609,8 @@ class TestGerritLegacyCRD(ZuulTestCase):
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2))
self.waitUntilSettled()
# Dependency cycle injected so zuul should not have reported again on A
self.assertEqual(A.reported, 1)
# Dependency cycle injected so zuul should have reported again on A
self.assertEqual(A.reported, 2)
# Now if we update B to remove the depends-on, everything
# should be okay. B; A->B
@ -621,7 +621,7 @@ class TestGerritLegacyCRD(ZuulTestCase):
self.waitUntilSettled()
# Cycle was removed so now zuul should have reported again on A
self.assertEqual(A.reported, 2)
self.assertEqual(A.reported, 3)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(2))
self.waitUntilSettled()

View File

@ -310,6 +310,17 @@ class PipelineManager(object):
(change, change.project))
return False
if history and change in history:
log.debug("Dependency cycle detected for "
"change %s in project %s" % (
change, change.project))
item = model.QueueItem(self, change, event)
item.warning("Dependency cycle detected")
actions = self.pipeline.failure_actions
item.setReportedResult('FAILURE')
self.sendReport(actions, item)
return False
if not self.enqueueChangesAhead(change, event, quiet,
ignore_requirements,
change_queue, history=history):

View File

@ -161,10 +161,6 @@ class DependentPipelineManager(PipelineManager):
change_queue, history=None):
log = get_annotated_logger(self.log, event)
if history and change in history:
# detected dependency cycle
log.warn("Dependency cycle detected")
return False
if hasattr(change, 'number'):
history = history or []
history = history + [change]

View File

@ -40,10 +40,6 @@ class IndependentPipelineManager(PipelineManager):
change_queue, history=None):
log = get_annotated_logger(self.log, event)
if history and change in history:
# detected dependency cycle
log.warn("Dependency cycle detected")
return False
if hasattr(change, 'number'):
history = history or []
history = history + [change]