diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index 2d63dbee3f..16a5c24514 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -381,6 +381,44 @@ class TestGerritCircularDependencies(ZuulTestCase): self.assertEqual(B.data["status"], "MERGED") self.assertEqual(C.data["status"], "MERGED") + def test_dependency_on_merged_cycle(self): + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange("org/project", "master", "A") + B = self.fake_gerrit.addFakeChange("org/project1", "master", "B") + C = self.fake_gerrit.addFakeChange("org/project2", "master", "C") + + # A -> B -> C -> B (via commit-depends) + A.data["commitMessage"] = "{}\n\nDepends-On: {}\n".format( + A.subject, B.data["url"] + ) + B.data["commitMessage"] = "{}\n\nDepends-On: {}\n".format( + B.subject, C.data["url"] + ) + C.data["commitMessage"] = "{}\n\nDepends-On: {}\n".format( + C.subject, B.data["url"] + ) + + # Start jobs for A while B + C are still open so they get + # enqueued as a non-live item ahead of A. + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Fake merge of change B + C while those changes are + # still part of a non-live item as dependency for A. + B.setMerged() + C.setMerged() + self.fake_gerrit.addEvent(B.getChangeMergedEvent()) + self.fake_gerrit.addEvent(C.getChangeMergedEvent()) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertHistory([ + dict(name="project-job", result="SUCCESS", changes="3,1 2,1 1,1"), + ], ordered=False) + def test_dependent_change_on_cycle(self): self.executor_server.hold_jobs_in_build = True diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index b06e437dc4..bf7c14866f 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -1633,11 +1633,18 @@ class PipelineManager(metaclass=ABCMeta): # Verify that the cycle dependency graph is correct cycle = self.cycleForChange( item.changes[0], dependency_graph, item.event, debug=False) - cycle = cycle or [item.changes[0]] + cycle = set(cycle or [item.changes[0]]) item_cycle = set(item.changes) - if set(cycle) != item_cycle: + # We don't consider merged dependencies when building the + # dependency graph, so we need to ignore differences resulting + # from changes that have been merged in the meantime. Put any + # missing merged changes back in the cycle for comparison + # purposes. + merged_changes = set(c for c in (item_cycle - cycle) if c.is_merged) + cycle |= merged_changes + if cycle != item_cycle: log.info("Item cycle has changed: %s, now: %s, was: %s", item, - set(cycle), item_cycle) + cycle, item_cycle) self.removeItem(item) if item.live: self.reEnqueueChanges(item, item.changes)