Merge "Case sensitive label matching"2.6.0

16 files changed, 105 insertions, 84 deletions

diff --git a/NEWS.rst b/NEWS.rst
index 5fef40af8..28a6ec264 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -12,6 +12,11 @@ Since 2.0.0:
   the Zuul server in smaller deployments.  Several configuration
   options have moved from the ``zuul`` section to ``merger``.
 
+* Gerrit label names must now be listed in your layout.yaml exactly as
+  they appear in Gerrit. This means case and special characters must
+  match. This change was made to accomodate Gerrit 2.13 which needs the
+  strings to match for changes to be successfully submitted.
+
 Since 1.3.0:
 
 * The Jenkins launcher is replaced with Gearman launcher.  An internal
diff --git a/tests/base.py b/tests/base.py
index 9dc412b5f..62f960b2e 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -103,9 +103,22 @@ class ChangeReference(git.Reference):
 
 
 class FakeChange(object):
-    categories = {'APRV': ('Approved', -1, 1),
-                  'CRVW': ('Code-Review', -2, 2),
-                  'VRFY': ('Verified', -2, 2)}
+    categories = {'Approved': ('Approved', -1, 1),
+                  'Code-Review': ('Code-Review', -2, 2),
+                  'Verified': ('Verified', -2, 2)}
+
+    # TODO(tobiash): This is used as a translation layer between the tests
+    #                which use lower case labels. This can be removed if all
+    #                tests are converted to use the correct casing.
+    categories_translation = {'approved': 'Approved',
+                              'code-review': 'Code-Review',
+                              'verified': 'Verified',
+                              'Approved': 'Approved',
+                              'Code-Review': 'Code-Review',
+                              'Verified': 'Verified',
+                              'CRVW': 'Code-Review',
+                              'APRV': 'Approved',
+                              'VRFY': 'Verified'}
 
     def __init__(self, gerrit, number, project, branch, subject,
                  status='NEW', upstream_root=None):
@@ -290,8 +303,8 @@ class FakeChange(object):
         if not granted_on:
             granted_on = time.time()
         approval = {
-            'description': self.categories[category][0],
-            'type': category,
+            'description': self.categories_translation[category],
+            'type': self.categories_translation[category],
             'value': str(value),
             'by': {
                 'username': username,
@@ -300,7 +313,8 @@ class FakeChange(object):
             'grantedOn': int(granted_on)
         }
         for i, x in enumerate(self.patchsets[-1]['approvals'][:]):
-            if x['by']['username'] == username and x['type'] == category:
+            if x['by']['username'] == username and \
+                    x['type'] == self.categories_translation[category]:
                 del self.patchsets[-1]['approvals'][i]
         self.patchsets[-1]['approvals'].append(approval)
         event = {'approvals': [approval],
diff --git a/tests/fixtures/layout-cloner.yaml b/tests/fixtures/layout-cloner.yaml
index e8b5dde08..0d5112928 100644
--- a/tests/fixtures/layout-cloner.yaml
+++ b/tests/fixtures/layout-cloner.yaml
@@ -6,10 +6,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: gate
     manager: DependentPipelineManager
@@ -18,17 +18,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
 
   - name: post
     manager: IndependentPipelineManager
diff --git a/tests/fixtures/layout-delayed-repo-init.yaml b/tests/fixtures/layout-delayed-repo-init.yaml
index 6caf6222a..04dc010ae 100644
--- a/tests/fixtures/layout-delayed-repo-init.yaml
+++ b/tests/fixtures/layout-delayed-repo-init.yaml
@@ -6,10 +6,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: post
     manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 projects:
diff --git a/tests/fixtures/layout-footer-message.yaml b/tests/fixtures/layout-footer-message.yaml
index 7977c19bc..fb7c95e78 100644
--- a/tests/fixtures/layout-footer-message.yaml
+++ b/tests/fixtures/layout-footer-message.yaml
@@ -10,21 +10,21 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
       smtp:
         to: success@example.org
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
       smtp:
         to: failure@example.org
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 projects:
diff --git a/tests/fixtures/layout-live-reconfiguration-functions.yaml b/tests/fixtures/layout-live-reconfiguration-functions.yaml
index e261a884b..695239f0b 100644
--- a/tests/fixtures/layout-live-reconfiguration-functions.yaml
+++ b/tests/fixtures/layout-live-reconfiguration-functions.yaml
@@ -9,17 +9,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 jobs:
diff --git a/tests/fixtures/layout-merge-failure.yaml b/tests/fixtures/layout-merge-failure.yaml
index 72bc9c9c0..9550466f5 100644
--- a/tests/fixtures/layout-merge-failure.yaml
+++ b/tests/fixtures/layout-merge-failure.yaml
@@ -6,10 +6,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: post
     manager: IndependentPipelineManager
@@ -26,22 +26,22 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     merge-failure:
       gerrit:
-        verified: -1
+        Verified: -1
       smtp:
         to: you@example.com
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 projects:
diff --git a/tests/fixtures/layout-rate-limit.yaml b/tests/fixtures/layout-rate-limit.yaml
index 9f6748c98..48d393209 100644
--- a/tests/fixtures/layout-rate-limit.yaml
+++ b/tests/fixtures/layout-rate-limit.yaml
@@ -6,13 +6,13 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
diff --git a/tests/fixtures/layout-repo-deleted.yaml b/tests/fixtures/layout-repo-deleted.yaml
index 967009a25..10168be5c 100644
--- a/tests/fixtures/layout-repo-deleted.yaml
+++ b/tests/fixtures/layout-repo-deleted.yaml
@@ -6,10 +6,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: post
     manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 projects:
diff --git a/tests/fixtures/layout-swift.yaml b/tests/fixtures/layout-swift.yaml
index acaaad8d4..2af5b244d 100644
--- a/tests/fixtures/layout-swift.yaml
+++ b/tests/fixtures/layout-swift.yaml
@@ -6,10 +6,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: post
     manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 jobs:
diff --git a/tests/fixtures/layout-zuultrigger-enqueued.yaml b/tests/fixtures/layout-zuultrigger-enqueued.yaml
index 8babd9e71..e052ec923 100644
--- a/tests/fixtures/layout-zuultrigger-enqueued.yaml
+++ b/tests/fixtures/layout-zuultrigger-enqueued.yaml
@@ -4,7 +4,7 @@ pipelines:
     source: gerrit
     require:
       approval:
-        - verified: -1
+        - Verified: -1
     trigger:
       gerrit:
         - event: patchset-created
@@ -13,10 +13,10 @@ pipelines:
           pipeline: gate
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: gate
     manager: DependentPipelineManager
@@ -24,25 +24,25 @@ pipelines:
     source: gerrit
     require:
       approval:
-        - verified: 1
+        - Verified: 1
     trigger:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
       zuul:
         - event: parent-change-enqueued
           pipeline: gate
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
 projects:
diff --git a/tests/fixtures/layout-zuultrigger-merged.yaml b/tests/fixtures/layout-zuultrigger-merged.yaml
index bb06ddef4..90fa579b3 100644
--- a/tests/fixtures/layout-zuultrigger-merged.yaml
+++ b/tests/fixtures/layout-zuultrigger-merged.yaml
@@ -7,10 +7,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: gate
     manager: DependentPipelineManager
@@ -20,17 +20,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
   - name: merge-check
@@ -42,7 +42,7 @@ pipelines:
         - event: project-change-merged
     merge-failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
 projects:
   - name: org/project
diff --git a/tests/fixtures/layout.yaml b/tests/fixtures/layout.yaml
index 2e48ff1d0..ba9d09ffc 100644
--- a/tests/fixtures/layout.yaml
+++ b/tests/fixtures/layout.yaml
@@ -9,10 +9,10 @@ pipelines:
         - event: patchset-created
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: post
     manager: IndependentPipelineManager
@@ -28,17 +28,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
     precedence: high
 
   - name: unused
@@ -48,7 +48,7 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
 
   - name: dup1
     manager: IndependentPipelineManager
@@ -57,10 +57,10 @@ pipelines:
         - event: change-restored
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: dup2
     manager: IndependentPipelineManager
@@ -69,10 +69,10 @@ pipelines:
         - event: change-restored
     success:
       gerrit:
-        verified: 1
+        Verified: 1
     failure:
       gerrit:
-        verified: -1
+        Verified: -1
 
   - name: conflict
     manager: DependentPipelineManager
@@ -81,17 +81,17 @@ pipelines:
       gerrit:
         - event: comment-added
           approval:
-            - approved: 1
+            - Approved: 1
     success:
       gerrit:
-        verified: 2
+        Verified: 2
         submit: true
     failure:
       gerrit:
-        verified: -2
+        Verified: -2
     start:
       gerrit:
-        verified: 0
+        Verified: 0
 
   - name: experimental
     manager: IndependentPipelineManager
diff --git a/tests/test_connection.py b/tests/test_connection.py
index f9f54f389..eb69b272a 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -60,7 +60,7 @@ class TestConnections(ZuulDBTestCase):
         self.waitUntilSettled()
 
         self.assertEqual(len(A.patchsets[-1]['approvals']), 1)
-        self.assertEqual(A.patchsets[-1]['approvals'][0]['type'], 'VRFY')
+        self.assertEqual(A.patchsets[-1]['approvals'][0]['type'], 'Verified')
         self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '1')
         self.assertEqual(A.patchsets[-1]['approvals'][0]['by']['username'],
                          'jenkins')
@@ -72,7 +72,7 @@ class TestConnections(ZuulDBTestCase):
         self.waitUntilSettled()
 
         self.assertEqual(len(B.patchsets[-1]['approvals']), 1)
-        self.assertEqual(B.patchsets[-1]['approvals'][0]['type'], 'VRFY')
+        self.assertEqual(B.patchsets[-1]['approvals'][0]['type'], 'Verified')
         self.assertEqual(B.patchsets[-1]['approvals'][0]['value'], '-1')
         self.assertEqual(B.patchsets[-1]['approvals'][0]['by']['username'],
                          'civoter')
diff --git a/zuul/model.py b/zuul/model.py
index b24a06bd2..405a348bb 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1123,7 +1123,8 @@ class BaseFilter(object):
             else:
                 if not isinstance(v, list):
                     v = [v]
-                if (normalizeCategory(approval['description']) != k or
+                if (normalizeCategory(approval['description']) !=
+                        normalizeCategory(k) or
                         int(approval['value']) not in v):
                     return False
         return True
@@ -1303,8 +1304,9 @@ class EventFilter(BaseFilter):
         for category, value in self.event_approvals.items():
             matches_approval = False
             for eapproval in event.approvals:
-                if (normalizeCategory(eapproval['description']) == category and
-                    int(eapproval['value']) == int(value)):
+                if (normalizeCategory(eapproval['description']) ==
+                        normalizeCategory(category) and
+                        int(eapproval['value']) == int(value)):
                     matches_approval = True
             if not matches_approval:
                 return False
diff --git a/zuul/source/gerrit.py b/zuul/source/gerrit.py
index fa495056c..b4eba5423 100644
--- a/zuul/source/gerrit.py
+++ b/zuul/source/gerrit.py
@@ -125,7 +125,7 @@ class GerritSource(BaseSource):
                             continue
                         elif label['status'] in ['NEED', 'REJECT']:
                             # It may be our own rejection, so we ignore
-                            if label['label'].lower() not in allow_needs:
+                            if label['label'] not in allow_needs:
                                 return False
                             continue
                         else: