Merge "gerrit: Add `approval-change` trigger"

doc/source/drivers/gerrit.rst

@@ -404,6 +404,14 @@ Trigger Configuration
       Example: ``Code-Review: 2`` matches a ``+2`` vote on the code
       review category.  Multiple approvals may be listed.
 
+   .. attr:: approval-change
+
+      This is only used for ``comment-added`` events. It works the same way as
+      ``approval``, with the additional requirement that the approval value
+      must have changed from its previous value. This means that it only
+      matches when a user modifies an approval score instead of any comment
+      where the score is present.
+
    .. attr:: email
 
       This is used for any event.  It takes a regex applied on the

releasenotes/notes/gerrit-approval-changes-9d0510e4ad71d990.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    The gerrit driver now supports a new synthetic trigger,
+    :attr:`pipeline.trigger.<gerrit source>.approval-change`. This is
+    similar to `approval`, but only triggers when the value of the
+    approval changes instead of simply being present.

tests/base.py

@@ -735,7 +735,7 @@ class FakeGerritChange(object):
         return event
 
     def addApproval(self, category, value, username='reviewer_john',
-                    granted_on=None, message='', tag=None):
+                    granted_on=None, message='', tag=None, old_value=None):
         if not granted_on:
             granted_on = time.time()
         approval = {
@@ -749,6 +749,8 @@ class FakeGerritChange(object):
             'grantedOn': int(granted_on),
             '__tag': tag,  # Not available in ssh api
         }
+        if old_value is not None:
+            approval['oldValue'] = str(old_value)
         for i, x in enumerate(self.patchsets[-1]['approvals'][:]):
             if x['by']['username'] == username and x['type'] == category:
                 del self.patchsets[-1]['approvals'][i]

tests/fixtures/layouts/gerrit-approval-change.yaml

@@ -0,0 +1,39 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval-change:
+            Code-Review: 2
+
+    require:
+      gerrit:
+        approval:
+          - Code-Review: 2
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+    nodeset:
+      nodes:
+        - label: ubuntu-xenial
+          name: controller
+
+- job:
+    name: check-job
+    run: playbooks/check.yaml
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - check-job
+

tests/unit/test_gerrit.py

@@ -1305,3 +1305,30 @@ class TestGerritDriver(ZuulTestCase):
         self.assertHistory([
             dict(name='check-job', result='SUCCESS', changes='1,1'),
         ])
+
+    @simple_layout('layouts/gerrit-approval-change.yaml')
+    def test_approval_change(self):
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.data['hashtags'] = ['check']
+        self.fake_gerrit.addEvent(A.addApproval('Code-Review', 2))
+        self.waitUntilSettled()
+
+        # Does not meet pipeline requirements, because old value is not present
+        self.assertHistory([])
+
+        # Still not sufficient because old value matches new value
+        self.fake_gerrit.addEvent(A.addApproval('Code-Review', 2, old_value=2))
+        self.waitUntilSettled()
+        self.assertHistory([])
+
+        # Old value present, but new value is insufficient
+        self.fake_gerrit.addEvent(A.addApproval('Code-Review', 0, old_value=2))
+        self.waitUntilSettled()
+        self.assertHistory([])
+
+        # This should work now
+        self.fake_gerrit.addEvent(A.addApproval('Code-Review', 2, old_value=0))
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='check-job', result='SUCCESS', changes='1,1'),
+        ])

zuul/driver/gerrit/gerritmodel.py

@@ -280,11 +280,11 @@ class GerritTriggerEvent(TriggerEvent):
 
 class GerritEventFilter(EventFilter):
     def __init__(self, connection_name, trigger, types=[], branches=[],
-                 refs=[], event_approvals={}, comments=[], emails=[],
-                 usernames=[], required_approvals=[], reject_approvals=[],
-                 added=[], removed=[],
-                 uuid=None, scheme=None, ignore_deletes=True,
-                 require=None, reject=None, parse_context=None):
+                 refs=[], event_approvals={}, event_approval_changes={},
+                 comments=[], emails=[], usernames=[], required_approvals=[],
+                 reject_approvals=[], added=[], removed=[], uuid=None,
+                 scheme=None, ignore_deletes=True, require=None, reject=None,
+                 parse_context=None):
 
         EventFilter.__init__(self, connection_name, trigger)
 
@@ -323,6 +323,7 @@ class GerritEventFilter(EventFilter):
         self.added = added
         self.removed = removed
         self.event_approvals = event_approvals
+        self.event_approval_changes = event_approval_changes
         self.uuid = uuid
         self.scheme = scheme
         self.ignore_deletes = ignore_deletes
@@ -346,6 +347,9 @@ class GerritEventFilter(EventFilter):
         if self.event_approvals:
             ret += ' event_approvals: %s' % ', '.join(
                 ['%s:%s' % a for a in self.event_approvals.items()])
+        if self.event_approval_changes:
+            ret += ' event_approval_changes: %s' % ', '.join(
+                ['%s:%s' % a for a in self.event_approval_changes.items()])
         if self._comments:
             ret += ' comments: %s' % ', '.join(self._comments)
         if self._emails:
@@ -457,6 +461,19 @@ class GerritEventFilter(EventFilter):
                 return FalseWithReason("Approvals %s do not match %s" % (
                     self.event_approvals, event.approvals))
 
+        for category, value in self.event_approval_changes.items():
+            matches_approval = False
+            for eapp in event.approvals:
+                if (eapp['description'] == category and
+                        int(eapp['value']) == int(value) and
+                        'oldValue' in eapp and
+                        int(eapp['value']) != int(eapp['oldValue'])):
+                    matches_approval = True
+            if not matches_approval:
+                return FalseWithReason(
+                    "Changed approvals %s do not match %s" % (
+                        self.event_approval_changes, event.approvals))
+
         # hashtags are ORed
         if self.added:
             matches_token = False

zuul/driver/gerrit/gerrittrigger.py

@@ -52,6 +52,10 @@ class GerritTrigger(BaseTrigger):
             for approval_dict in to_list(trigger.get('approval')):
                 for key, val in approval_dict.items():
                     approvals[key] = val
+            approval_changes = {}
+            for approval_dict in to_list(trigger.get('approval-change')):
+                for key, val in approval_dict.items():
+                    approval_changes[key] = val
             # Backwards compat for *_filter versions of these args
             attrname = 'comment' if 'comment' in trigger else 'comment_filter'
             with pcontext.confAttr(trigger, attrname) as attr:
@@ -96,6 +100,7 @@ class GerritTrigger(BaseTrigger):
                 branches=branches,
                 refs=refs,
                 event_approvals=approvals,
+                event_approval_changes=approval_changes,
                 comments=comments,
                 emails=emails,
                 usernames=usernames,
@@ -153,6 +158,7 @@ def getSchema():
         'ref': scalar_or_list(v.Any(ZUUL_REGEX, str)),
         'ignore-deletes': bool,
         'approval': scalar_or_list(variable_dict),
+        'approval-change': scalar_or_list(variable_dict),
         'require-approval': scalar_or_list(approval),
         'reject-approval': scalar_or_list(approval),
         'added': scalar_or_list(v.Any(ZUUL_REGEX, str)),