Further fix getting topic changes by git needs

The test helper method that handles fake gerrit queries had a bug
which would cause the "topic:" queries to return all open changes.

When we correct that, we can see, by virtue of a newly raised
execption that there was some unexercised code in getChangesByTopic
which is now exercised.  This change also corrects the exception
that is raised when mutating a set while iterating over it.

Change-Id: I1874482b2c28fd1082fcd56036afb20333232409

3 files changed, 30 insertions, 13 deletions

diff --git a/tests/base.py b/tests/base.py
index 1cfcecde5..c031497a2 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -1504,8 +1504,9 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
                 msg = msg[1:-1]
             l = [queryMethod(change) for change in self.changes.values()
                  if msg in change.data['commitMessage']]
-        elif query.startswith("status:"):
+        else:
             cut_off_time = 0
+            l = list(self.changes.values())
             parts = query.split(" ")
             for part in parts:
                 if part.startswith("-age"):
@@ -1513,17 +1514,18 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
                     cut_off_time = (
                         datetime.datetime.now().timestamp() - float(age[:-1])
                     )
-            l = [
-                queryMethod(change) for change in self.changes.values()
-                if change.data["lastUpdated"] >= cut_off_time
-            ]
-        elif query.startswith('topic:'):
-            topic = query[len('topic:'):].strip()
-            l = [queryMethod(change) for change in self.changes.values()
-                 if topic in change.data.get('topic', '')]
-        else:
-            # Query all open changes
-            l = [queryMethod(change) for change in self.changes.values()]
+                    l = [
+                        change for change in l
+                        if change.data["lastUpdated"] >= cut_off_time
+                    ]
+                if part.startswith('topic:'):
+                    topic = part[len('topic:'):].strip()
+                    l = [
+                        change for change in l
+                        if 'topic' in change.data
+                        and topic in change.data['topic']
+                    ]
+            l = [queryMethod(change) for change in l]
         return l
 
     def simpleQuerySSH(self, query, event=None):
diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py
index ac3dccf3b..4085e8b1b 100644
--- a/tests/unit/test_gerrit.py
+++ b/tests/unit/test_gerrit.py
@@ -827,6 +827,14 @@ class TestGerritFake(ZuulTestCase):
     config_file = "zuul-gerrit-github.conf"
     tenant_config_file = "config/circular-dependencies/main.yaml"
 
+    def _make_tuple(self, data):
+        ret = []
+        for c in data:
+            dep_change = c['number']
+            dep_ps = c['currentPatchSet']['number']
+            ret.append((int(dep_change), int(dep_ps)))
+        return sorted(ret)
+
     def _get_tuple(self, change_number):
         ret = []
         data = self.fake_gerrit.get(
@@ -903,6 +911,11 @@ class TestGerritFake(ZuulTestCase):
         ret = self.fake_gerrit._getSubmittedTogether(C1, None)
         self.assertEqual(ret, [(4, 1)])
 
+        # Test also the query used by the GerritConnection:
+        ret = self.fake_gerrit._simpleQuery('status:open topic:test-topic')
+        ret = self._make_tuple(ret)
+        self.assertEqual(ret, [(3, 1), (4, 1)])
+
 
 class TestGerritConnection(ZuulTestCase):
     config_file = 'zuul-gerrit-web.conf'
diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py
index 3bf2b3a14..b0bd3c448 100644
--- a/zuul/driver/gerrit/gerritsource.py
+++ b/zuul/driver/gerrit/gerritsource.py
@@ -164,7 +164,9 @@ class GerritSource(BaseSource):
             change = self.connection._getChange(change_key)
             changes[change_key] = change
 
-        for change in changes.values():
+        # Convert to list here because the recursive call can mutate
+        # the set.
+        for change in list(changes.values()):
             for git_change_ref in change.git_needs_changes:
                 change_key = ChangeKey.fromReference(git_change_ref)
                 if change_key in changes: