Merge "Fix getting Gerrit topic changes with git needs"

2 files changed, 68 insertions, 3 deletions

diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py
index a3f9dda33..28ca528b5 100644
--- a/tests/unit/test_circular_dependencies.py
+++ b/tests/unit/test_circular_dependencies.py
@@ -2246,6 +2246,70 @@ class TestGerritCircularDependencies(ZuulTestCase):
         self.assertEqual(B.data["status"], "MERGED")
 
     @simple_layout('layouts/deps-by-topic.yaml')
+    def test_deps_by_topic_git_needs(self):
+        A = self.fake_gerrit.addFakeChange('org/project1', "master", "A",
+                                           topic='test-topic')
+        B = self.fake_gerrit.addFakeChange('org/project2', "master", "B",
+                                           topic='test-topic')
+        C = self.fake_gerrit.addFakeChange('org/project2', "master", "C",
+                                           topic='other-topic')
+        D = self.fake_gerrit.addFakeChange('org/project1', "master", "D",
+                                           topic='other-topic')
+
+        # Git level dependency between B and C
+        B.setDependsOn(C, 1)
+
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+        self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
+        self.fake_gerrit.addEvent(D.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertEqual(len(A.patchsets[-1]["approvals"]), 1)
+        self.assertEqual(A.patchsets[-1]["approvals"][0]["type"], "Verified")
+        self.assertEqual(A.patchsets[-1]["approvals"][0]["value"], "1")
+
+        self.assertEqual(len(B.patchsets[-1]["approvals"]), 1)
+        self.assertEqual(B.patchsets[-1]["approvals"][0]["type"], "Verified")
+        self.assertEqual(B.patchsets[-1]["approvals"][0]["value"], "1")
+
+        self.assertEqual(len(C.patchsets[-1]["approvals"]), 1)
+        self.assertEqual(C.patchsets[-1]["approvals"][0]["type"], "Verified")
+        self.assertEqual(C.patchsets[-1]["approvals"][0]["value"], "1")
+
+        self.assertEqual(len(D.patchsets[-1]["approvals"]), 1)
+        self.assertEqual(D.patchsets[-1]["approvals"][0]["type"], "Verified")
+        self.assertEqual(D.patchsets[-1]["approvals"][0]["value"], "1")
+
+        # We're about to add approvals to changes without adding the
+        # triggering events to Zuul, so that we can be sure that it is
+        # enqueing the changes based on dependencies, not because of
+        # triggering events.  Since it will have the changes cached
+        # already (without approvals), we need to clear the cache
+        # first.
+        for connection in self.scheds.first.connections.connections.values():
+            connection.maintainCache([], max_age=0)
+
+        A.addApproval("Code-Review", 2)
+        B.addApproval("Code-Review", 2)
+        C.addApproval("Code-Review", 2)
+        D.addApproval("Code-Review", 2)
+        A.addApproval("Approved", 1)
+        C.addApproval("Approved", 1)
+        D.addApproval("Approved", 1)
+        self.fake_gerrit.addEvent(B.addApproval("Approved", 1))
+        self.waitUntilSettled()
+
+        self.assertEqual(A.reported, 3)
+        self.assertEqual(B.reported, 3)
+        self.assertEqual(C.reported, 3)
+        self.assertEqual(D.reported, 3)
+        self.assertEqual(A.data["status"], "MERGED")
+        self.assertEqual(B.data["status"], "MERGED")
+        self.assertEqual(C.data["status"], "MERGED")
+        self.assertEqual(D.data["status"], "MERGED")
+
+    @simple_layout('layouts/deps-by-topic.yaml')
     def test_deps_by_topic_new_patchset(self):
         # Make sure that we correctly update the change cache on new
         # patchsets.
diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py
index f42e93254..3bf2b3a14 100644
--- a/zuul/driver/gerrit/gerritsource.py
+++ b/zuul/driver/gerrit/gerritsource.py
@@ -165,10 +165,11 @@ class GerritSource(BaseSource):
             changes[change_key] = change
 
         for change in changes.values():
-            for git_key in change.git_needs_changes:
-                if git_key in changes:
+            for git_change_ref in change.git_needs_changes:
+                change_key = ChangeKey.fromReference(git_change_ref)
+                if change_key in changes:
                     continue
-                git_change = self.getChange(git_key)
+                git_change = self.getChange(change_key)
                 if not git_change.topic or git_change.topic == topic:
                     continue
                 self.getChangesByTopic(git_change.topic, changes)