am 19598a2f: am 5a388898: Merge "Check the committer rather than the Gerrit owner."

* commit '19598a2f548bd545a41741f5b0964b74afb2f4ff':
  Check the committer rather than the Gerrit owner.

tools/bionicbb/gerrit.py

@@ -24,6 +24,11 @@ class GerritError(RuntimeError):
         super(GerritError, self).__init__('Error {}: {}'.format(code, url))
 
 
+def get_commit(change_id, revision):
+    return json.loads(
+        call('/changes/{}/revisions/{}/commit'.format(change_id, revision)))
+
+
 def call(endpoint, method='GET'):
     if method != 'GET':
         raise NotImplementedError('Currently only HTTP GET is supported.')

tools/bionicbb/gmail_listener.py

@@ -51,11 +51,14 @@ def get_headers(msg):
     return headers
 
 
-def should_skip_message(gerrit_info):
+def should_skip_message(info):
-    match = re.search(r'<(\S+)>$', gerrit_info['Owner'])
+    if info['MessageType'] in ('newchange', 'newpatchset', 'comment'):
-    if match:
+        commit = gerrit.get_commit(info['Change-Id'], info['PatchSet'])
-        return not match.group(1).endswith('@google.com')
+        committer = commit['committer']['email']
-    raise RuntimeError('Gerrit info missing Gerrit-Owner info.')
+        return not committer.endswith('@google.com')
+    else:
+        raise ValueError('should_skip_message() is only valid for new '
+                         'changes, patch sets, and commits.')
 
 
 def build_service():

tools/bionicbb/test_gmail_listener.py

@@ -1,20 +1,63 @@
 import gmail_listener
+import mock
 import unittest
 
 
-class TestGerritParsers(unittest.TestCase):
+class TestShouldSkipMessage(unittest.TestCase):
-    def test_should_skip_message(self):
+    def test_accepts_googlers(self):
-        info = gmail_listener.get_gerrit_info(
+        for message_type in ('newchange', 'newpatchset', 'comment'):
-            'Gerrit-Owner: Some Googler <somegoogler@google.com>\n')
+            with mock.patch('gerrit.get_commit') as mock_commit:
-        self.assertFalse(gmail_listener.should_skip_message(info))
+                mock_commit.return_value = {
+                    'committer': {'email': 'googler@google.com'}
+                }
 
-        info = gmail_listener.get_gerrit_info(
+                self.assertFalse(gmail_listener.should_skip_message({
-            'Gerrit-Owner: Fake Googler <fakegoogler@google.com.foo.com>\n')
+                    'MessageType': message_type,
-        self.assertTrue(gmail_listener.should_skip_message(info))
+                    'Change-Id': '',
+                    'PatchSet': '',
+                }))
 
-        info = gmail_listener.get_gerrit_info(
+    def test_rejects_non_googlers(self):
-            'Gerrit-Owner: John Doe <johndoe@example.com>\n')
+        for message_type in ('newchange', 'newpatchset', 'comment'):
-        self.assertTrue(gmail_listener.should_skip_message(info))
+            with mock.patch('gerrit.get_commit') as mock_commit:
+                mock_commit.return_value = {
+                    'committer': {'email': 'fakegoogler@google.com.fake.com'}
+                }
+
+                self.assertTrue(gmail_listener.should_skip_message({
+                    'MessageType': message_type,
+                    'Change-Id': '',
+                    'PatchSet': '',
+                }))
+
+            with mock.patch('gerrit.get_commit') as mock_commit:
+                mock_commit.return_value = {
+                    'committer': {'email': 'johndoe@example.com'}
+                }
+
+                self.assertTrue(gmail_listener.should_skip_message({
+                    'MessageType': message_type,
+                    'Change-Id': '',
+                    'PatchSet': '',
+                }))
+
+    def test_calls_gerrit_get_commit(self):  # pylint: disable=no-self-use
+        for message_type in ('newchange', 'newpatchset', 'comment'):
+            with mock.patch('gerrit.get_commit') as mock_commit:
+                gmail_listener.should_skip_message({
+                    'MessageType': message_type,
+                    'Change-Id': 'foo',
+                    'PatchSet': 'bar',
+                })
+            mock_commit.assert_called_once_with('foo', 'bar')
+
+            with mock.patch('gerrit.get_commit') as mock_commit:
+                gmail_listener.should_skip_message({
+                    'MessageType': message_type,
+                    'Change-Id': 'baz',
+                    'PatchSet': 'qux',
+                })
+            mock_commit.assert_called_once_with('baz', 'qux')
 
 
 if __name__ == '__main__':