diff --git a/NEWS.rst b/NEWS.rst index 5fef40af82..28a6ec2646 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 9dc412b5f1..62f960b2e4 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 e8b5dde08f..0d5112928e 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 6caf6222a2..04dc010ae9 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 7977c19bc8..fb7c95e780 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 e261a884b4..695239f0bc 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 72bc9c9c09..9550466f59 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 9f6748c98d..48d3932091 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 967009a25e..10168be5cf 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 acaaad8d42..2af5b244d0 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 8babd9e71d..e052ec923b 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 bb06ddef45..90fa579b35 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 2e48ff1d0c..ba9d09ffcf 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 f9f54f3894..eb69b272a7 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 b24a06bd25..405a348bb7 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 fa495056cb..b4eba54234 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: