Merge "Fix gerrit errors from production"

tests/base.py

@@ -552,6 +552,8 @@ class FakeGerritChange(object):
         num = len(self.patchsets)
         files = {}
         for f in rev['files']:
+            if f['file'] == '/COMMIT_MSG':
+                continue
             files[f['file']] = {"status": f['type'][0]}  # ADDED -> A
         parent = '0000000000000000000000000000000000000000'
         if self.depends_on_change:
@@ -711,9 +713,12 @@ class GerritWebServer(object):
                 self.end_headers()
 
             def _get_change(self, change_id):
+                change_id = urllib.parse.unquote(change_id)
                 project, branch, change = change_id.split('~')
                 for c in fake_gerrit.changes.values():
-                    if c.data['id'] == change:
+                    if (c.data['id'] == change and
+                        c.data['branch'] == branch and
+                        c.data['project'] == project):
                         return c
 
             def review(self, change_id, revision, data):
@@ -3946,8 +3951,11 @@ class ZuulTestCase(BaseTestCase):
             self.sched.wake_event.wait(0.1)
 
     def waitForPoll(self, poller, timeout=30):
+        self.log.debug("Wait for poll on %s", poller)
         self.poller_events[poller].clear()
-        self.poller_events[poller].wait(30)
+        self.log.debug("Waiting for poll on %s", poller)
+        self.poller_events[poller].wait(timeout)
+        self.log.debug("Done waiting for poll on %s", poller)
 
     def logState(self):
         """ Log the current state of the system """

tests/fixtures/layouts/single-file-matcher.yaml

@@ -0,0 +1,28 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+
+- job:
+    name: irr-job
+    irrelevant-files:
+      - README
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - irr-job

tests/unit/test_gerrit.py

@@ -191,6 +191,26 @@ class TestGerritWeb(ZuulTestCase):
                       B.messages[0])
         self.assertEqual(B.comments, [])
 
+    @simple_layout('layouts/single-file-matcher.yaml')
+    def test_single_file(self):
+        # HTTP requests don't return a commit_msg entry in the files
+        # list, but the rest of zuul always expects one.  This test
+        # returns a single file to exercise the single-file code path
+        # in the files matcher.
+        files = {'README': 'please!\n'}
+
+        change = self.fake_gerrit.addFakeChange('org/project',
+                                                'master',
+                                                'test irrelevant-files',
+                                                files=files)
+        self.fake_gerrit.addEvent(change.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        tested_change_ids = [x.changes[0] for x in self.history
+                             if x.name == 'project-test-irrelevant-files']
+
+        self.assertEqual([], tested_change_ids)
+
 
 class TestFileComments(AnsibleZuulTestCase):
     config_file = 'zuul-gerrit-web.conf'

tests/unit/test_gerrit_crd.py

@@ -770,3 +770,7 @@ class TestGerritCRDAltBaseUrl(ZuulTestCase):
         self.assertEqual(self.history[0].changes, '2,1 1,1')
         tenant = self.sched.abide.tenants.get('tenant-one')
         self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0)
+
+
+class TestGerritCRDWeb(TestGerritCRD):
+    config_file = 'zuul-gerrit-web.conf'

tests/unit/test_gerrit_legacy_crd.py

@@ -626,3 +626,7 @@ class TestGerritLegacyCRD(ZuulTestCase):
         self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(2))
         self.waitUntilSettled()
         self.assertEqual(B.reported, 2)
+
+
+class TestGerritLegacyCRDWeb(TestGerritLegacyCRD):
+    config_file = 'zuul-gerrit-web.conf'

zuul/driver/gerrit/gerritconnection.py

@@ -1055,6 +1055,7 @@ class GerritConnection(BaseConnection):
             for x in range(1, 4):
                 try:
                     self.post('changes/%s/submit' % (changeid,), {})
+                    break
                 except Exception:
                     log.exception(
                         "Error submitting data to gerrit, attempt %s", x)
@@ -1153,6 +1154,7 @@ class GerritConnection(BaseConnection):
         sortkey = ''
         done = False
         offset = 0
+        query = urllib.parse.quote(query, safe='')
         while not done:
             # We don't actually want to limit to 500, but that's the
             # server-side default, and if we don't specify this, we
@@ -1326,7 +1328,8 @@ class GerritConnection(BaseConnection):
     def onLoad(self):
         self.log.debug("Starting Gerrit Connection/Watchers")
         try:
-            self._getRemoteVersion()
+            if self.session:
+                self._getRemoteVersion()
         except Exception:
             self.log.exception("Unable to determine remote Gerrit version")
 

zuul/driver/gerrit/gerritmodel.py

@@ -110,7 +110,7 @@ class GerritChange(Change):
         if str(current_revision['_number']) == self.patchset:
             self.ref = current_revision['ref']
             self.commit = data['current_revision']
-            files = current_revision.get('files', []).keys()
+            files = list(current_revision.get('files', []).keys())
             self.is_current_patchset = True
         else:
             self.is_current_patchset = False