Merge "Reject changes with cleanspecs."
This commit is contained in:
commit
5cf46f81ea
@ -29,6 +29,12 @@ def get_commit(change_id, revision):
|
||||
call('/changes/{}/revisions/{}/commit'.format(change_id, revision)))
|
||||
|
||||
|
||||
def get_files_for_revision(change_id, revision):
|
||||
return json.loads(
|
||||
call('/changes/{}/revisions/{}/files'.format(
|
||||
change_id, revision))).keys()
|
||||
|
||||
|
||||
def call(endpoint, method='GET'):
|
||||
if method != 'GET':
|
||||
raise NotImplementedError('Currently only HTTP GET is supported.')
|
||||
|
@ -19,6 +19,7 @@ import httplib
|
||||
import httplib2
|
||||
import jenkinsapi
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import requests
|
||||
import termcolor
|
||||
@ -51,15 +52,35 @@ def get_headers(msg):
|
||||
return headers
|
||||
|
||||
|
||||
def should_skip_message(info):
|
||||
if info['MessageType'] in ('newchange', 'newpatchset', 'comment'):
|
||||
commit = gerrit.get_commit(info['Change-Id'], info['PatchSet'])
|
||||
committer = commit['committer']['email']
|
||||
return not committer.endswith('@google.com')
|
||||
else:
|
||||
raise ValueError('should_skip_message() is only valid for new '
|
||||
def is_untrusted_committer(change_id, patch_set):
|
||||
# TODO(danalbert): Needs to be based on the account that made the comment.
|
||||
commit = gerrit.get_commit(change_id, patch_set)
|
||||
committer = commit['committer']['email']
|
||||
return not committer.endswith('@google.com')
|
||||
|
||||
|
||||
def contains_cleanspec(change_id, patch_set):
|
||||
files = gerrit.get_files_for_revision(change_id, patch_set)
|
||||
return 'CleanSpec.mk' in [os.path.basename(f) for f in files]
|
||||
|
||||
|
||||
def should_skip_build(info):
|
||||
if info['MessageType'] not in ('newchange', 'newpatchset', 'comment'):
|
||||
raise ValueError('should_skip_build() is only valid for new '
|
||||
'changes, patch sets, and commits.')
|
||||
|
||||
change_id = info['Change-Id']
|
||||
patch_set = info['PatchSet']
|
||||
|
||||
checks = [
|
||||
is_untrusted_committer,
|
||||
contains_cleanspec,
|
||||
]
|
||||
for check in checks:
|
||||
if check(change_id, patch_set):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def build_service():
|
||||
from apiclient.discovery import build
|
||||
@ -214,7 +235,7 @@ def build_project(gerrit_info, dry_run, lunch_target=None):
|
||||
|
||||
|
||||
def handle_change(gerrit_info, _, dry_run):
|
||||
if should_skip_message(gerrit_info):
|
||||
if should_skip_build(gerrit_info):
|
||||
return True
|
||||
return build_project(gerrit_info, dry_run)
|
||||
handle_newchange = handle_change
|
||||
@ -246,8 +267,7 @@ def handle_comment(gerrit_info, body, dry_run):
|
||||
if 'Verified+1' in body:
|
||||
drop_rejection(gerrit_info, dry_run)
|
||||
|
||||
# TODO(danalbert): Needs to be based on the account that made the comment.
|
||||
if should_skip_message(gerrit_info):
|
||||
if should_skip_build(gerrit_info):
|
||||
return True
|
||||
|
||||
command_map = {
|
||||
|
@ -3,61 +3,71 @@ import mock
|
||||
import unittest
|
||||
|
||||
|
||||
class TestShouldSkipMessage(unittest.TestCase):
|
||||
def test_accepts_googlers(self):
|
||||
class TestShouldSkipBuild(unittest.TestCase):
|
||||
@mock.patch('gmail_listener.contains_cleanspec')
|
||||
@mock.patch('gerrit.get_commit')
|
||||
def test_accepts_googlers(self, mock_commit, *other_checks):
|
||||
mock_commit.return_value = {
|
||||
'committer': {'email': 'googler@google.com'}
|
||||
}
|
||||
|
||||
for other_check in other_checks:
|
||||
other_check.return_value = False
|
||||
|
||||
for message_type in ('newchange', 'newpatchset', 'comment'):
|
||||
with mock.patch('gerrit.get_commit') as mock_commit:
|
||||
mock_commit.return_value = {
|
||||
'committer': {'email': 'googler@google.com'}
|
||||
}
|
||||
self.assertFalse(gmail_listener.should_skip_build({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
|
||||
self.assertFalse(gmail_listener.should_skip_message({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
@mock.patch('gmail_listener.contains_cleanspec')
|
||||
@mock.patch('gerrit.get_commit')
|
||||
def test_rejects_googlish_domains(self, mock_commit, *other_checks):
|
||||
mock_commit.return_value = {
|
||||
'committer': {'email': 'fakegoogler@google.com.fake.com'}
|
||||
}
|
||||
|
||||
for other_check in other_checks:
|
||||
other_check.return_value = False
|
||||
|
||||
def test_rejects_non_googlers(self):
|
||||
for message_type in ('newchange', 'newpatchset', 'comment'):
|
||||
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_build({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
|
||||
self.assertTrue(gmail_listener.should_skip_message({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
@mock.patch('gmail_listener.contains_cleanspec')
|
||||
@mock.patch('gerrit.get_commit')
|
||||
def test_rejects_non_googlers(self, mock_commit, *other_checks):
|
||||
mock_commit.return_value = {
|
||||
'committer': {'email': 'johndoe@example.com'}
|
||||
}
|
||||
|
||||
with mock.patch('gerrit.get_commit') as mock_commit:
|
||||
mock_commit.return_value = {
|
||||
'committer': {'email': 'johndoe@example.com'}
|
||||
}
|
||||
for other_check in other_checks:
|
||||
other_check.return_value = False
|
||||
|
||||
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')
|
||||
self.assertTrue(gmail_listener.should_skip_build({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
|
||||
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')
|
||||
@mock.patch('gmail_listener.is_untrusted_committer')
|
||||
@mock.patch('gerrit.get_files_for_revision')
|
||||
def test_skips_cleanspecs(self, mock_files, *other_checks):
|
||||
mock_files.return_value = ['foo/CleanSpec.mk']
|
||||
for other_check in other_checks:
|
||||
other_check.return_value = False
|
||||
|
||||
for message_type in ('newchange', 'newpatchset', 'comment'):
|
||||
self.assertTrue(gmail_listener.should_skip_build({
|
||||
'MessageType': message_type,
|
||||
'Change-Id': '',
|
||||
'PatchSet': '',
|
||||
}))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
|
Loading…
Reference in New Issue
Block a user