Reset jobs behind non-mergeable cycle

In the case of a dependency cycle, we check the mergeability of
each change in the item before we try to merge any of them, and
dequeue the item if it looks like one of them won't be able to
merge.  However, that bypasses the normal behavior where we reset
changes behind failing items, which could lead to merging changes
that were tested with changes ahead that did not merge.

To correct this, update the cycle-can-not-be-merged dequeue stanza
with a reset, to mirror the stanza below which handles the failure
of any individual change to merge.

Change-Id: I52a9fc2da4dd89131722d69d2b5dea886eb3d51c
This commit is contained in:
James E. Blair 2024-03-13 08:59:57 -07:00
parent da07d5ff5e
commit c2103f7058
2 changed files with 30 additions and 0 deletions

View File

@ -4031,8 +4031,10 @@ class TestGithubCircularDependencies(ZuulTestCase):
self.executor_server.hold_jobs_in_build = True
A = self.fake_github.openFakePullRequest("gh/project", "master", "A")
B = self.fake_github.openFakePullRequest("gh/project1", "master", "B")
C = self.fake_github.openFakePullRequest("gh/project1", "master", "B")
A.addReview('derp', 'APPROVED')
B.addReview('derp', 'APPROVED')
C.addReview('derp', 'APPROVED')
B.addLabel("approved")
# A <-> B
@ -4045,6 +4047,10 @@ class TestGithubCircularDependencies(ZuulTestCase):
self.fake_github.emitEvent(A.addLabel("approved"))
self.waitUntilSettled()
# Put an indepedente change behind this pair to test that it
# gets reset.
self.fake_github.emitEvent(C.addLabel("approved"))
self.waitUntilSettled()
# Change draft status of A so it can no longer merge. Note that we
# don't send an event to test the "github doesn't send an event"
@ -4060,6 +4066,7 @@ class TestGithubCircularDependencies(ZuulTestCase):
self.assertEqual(len(B.comments), 2)
self.assertFalse(A.is_merged)
self.assertFalse(B.is_merged)
self.assertTrue(C.is_merged)
self.assertIn("failed to merge",
A.comments[-1])
@ -4078,6 +4085,24 @@ class TestGithubCircularDependencies(ZuulTestCase):
B.comments[-1]))
self.assertFalse(re.search('Change .*? is needed',
B.comments[-1]))
self.assertHistory([
dict(name="project1-job", result="SUCCESS",
changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
dict(name="project-vars-job", result="SUCCESS",
changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
dict(name="project-job", result="SUCCESS",
changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"),
dict(name="project1-job", result="SUCCESS",
changes=(f"{B.number},{B.head_sha} {A.number},{A.head_sha} "
f"{C.number},{C.head_sha}")),
dict(name="project-vars-job", result="SUCCESS",
changes=(f"{B.number},{B.head_sha} {A.number},{A.head_sha} "
f"{C.number},{C.head_sha}")),
dict(name="project1-job", result="SUCCESS",
changes=f"{C.number},{C.head_sha}"),
dict(name="project-vars-job", result="SUCCESS",
changes=f"{C.number},{C.head_sha}"),
], ordered=False)
def test_dependency_refresh(self):
# Test that when two changes are put into a cycle, the

View File

@ -1733,6 +1733,11 @@ class PipelineManager(metaclass=ABCMeta):
self.reportItem(item)
except exceptions.MergeFailure:
pass
for item_behind in item.items_behind:
log.info("Resetting builds for %s because the "
"item ahead, %s, can not be merged" %
(item_behind, item))
self.cancelJobs(item_behind)
self.dequeueItem(item)
return (True, nnfi)