diff options
author | Jim O'Leary <jim.oleary@mongodb.com> | 2020-01-23 17:52:51 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2020-01-23 17:52:51 +0000 |
commit | 1da2398abc11189a6d7706cfba12fa71a693e129 (patch) | |
tree | f345490dc3f61f8d306a5385dee829f0f229a0c8 | |
parent | 50444adc39207f2f4b3bd261e6fe399fb3cb3a6d (diff) | |
download | mongo-1da2398abc11189a6d7706cfba12fa71a693e129.tar.gz |
SERVER-45074 validate commit message against JIRA
-rw-r--r-- | buildscripts/tests/test_validate_commit_message.py | 480 | ||||
-rwxr-xr-x | buildscripts/validate_commit_message.py | 421 | ||||
-rw-r--r-- | etc/evergreen.yml | 2 |
3 files changed, 842 insertions, 61 deletions
diff --git a/buildscripts/tests/test_validate_commit_message.py b/buildscripts/tests/test_validate_commit_message.py index 1cfd37a1d0c..4682fb02014 100644 --- a/buildscripts/tests/test_validate_commit_message.py +++ b/buildscripts/tests/test_validate_commit_message.py @@ -1,11 +1,20 @@ """Unit tests for the evergreen_task_timeout script.""" - +import logging import unittest -from mock import MagicMock, patch +from http import HTTPStatus +from http.client import HTTPConnection -from buildscripts.validate_commit_message import main, STATUS_OK, STATUS_ERROR, GIT_SHOW_COMMAND +import requests.exceptions +from jira import JIRAError +from mock import MagicMock, patch, PropertyMock +from buildscripts.validate_commit_message import Status, jira_client, \ + configure_logging, validate_public_ticket, get_full_message, GIT_SHOW_COMMAND, \ + validate_message, handle_violations, Violation, validate_ticket, find_ticket, main # pylint: disable=missing-docstring,no-self-use +from buildscripts.validate_commit_message import find_matching_pattern, VALID_PATTERNS + +AUTHOR = 'jim' INVALID_MESSAGES = [ [""], # You must provide a message @@ -19,41 +28,450 @@ NS = "buildscripts.validate_commit_message" def ns(relative_name): # pylint: disable=invalid-name - """Return a full name from a name relative to the test module"s name space.""" + """Return a full name from a name relative to the tested module's name space.""" return NS + "." + relative_name -class ValidateCommitMessageTest(unittest.TestCase): - def test_valid(self): +class GetFullMessageTest(unittest.TestCase): + def test_no_message(self): + with patch(ns('subprocess')) as mock_subprocess: + expected = "output" + mock_subprocess.check_output.return_value = bytearray(expected, "utf-8") + self.assertEqual(expected, get_full_message([])) + mock_subprocess.check_output.assert_called_once_with(GIT_SHOW_COMMAND) + + def test_empty_message(self): + with patch(ns('subprocess')) as mock_subprocess: + # blank / empty string is false + self.assertFalse(get_full_message([""])) + mock_subprocess.check_output.assert_not_called() + + def test_message(self): + with patch(ns('subprocess')) as mock_subprocess: + expected = "this is a test" + self.assertEqual(expected, get_full_message(expected.split())) + mock_subprocess.check_output.assert_not_called() + + +class FindTicketTest(unittest.TestCase): + def test_last_git_commit_success(self): messages = [ - ["Fix lint"], - ["EVG-1"], # Test valid projects with various number lengths - ["SERVER-20"], - ["WT-300"], - ["SERVER-44338"], - ["Revert EVG-5"], - ["Revert SERVER-60"], - ["Revert WT-700"], - ["Revert 'SERVER-8000"], - ['Revert "SERVER-90000'], - ["Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from branch mongodb-4.4"], - ["Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from branch mongodb-4.4"] + _make_expected("Fix lint", lint='Fix lint', public=True), + _make_expected( + 'SERVER-44612 ' + 'this is the message\n\n' + '(cherry picked from commit beef)', revert=None, ticket='SERVER-44612', + backport='(cherry picked from commit beef)', public=True), + _make_expected('SERVER-45074 this is a test (cherry picked from', revert=None, + ticket='SERVER-45074', backport='(cherry picked from', public=True), + _make_expected('SERVER-45074 this is a test (cherry picked from commit beef)', + revert=None, ticket='SERVER-45074', + backport='(cherry picked from commit beef)', public=True), + _make_expected('SERVER-20', revert=None, ticket='SERVER-20', backport=None, + public=True), + _make_expected('WT-300', revert=None, ticket='WT-300', backport=None, public=True), + _make_expected('SERVER-44338', revert=None, ticket='SERVER-44338', backport=None, + public=True), + _make_expected('Revert EVG-5', revert='Revert', ticket='EVG-5', backport=None, + public=True), + _make_expected('Revert SERVER-60', revert='Revert', ticket='SERVER-60', backport=None, + public=True), + _make_expected('Revert WT-700', revert='Revert', ticket='WT-700', backport=None, + public=True), + _make_expected('Revert \'SERVER-8000', revert='Revert', ticket='SERVER-8000', + backport=None, public=True), + _make_expected('Revert \'SERVER-8000\'', revert='Revert', ticket='SERVER-8000', + backport=None, public=True), + _make_expected('Revert "SERVER-90000"', revert='Revert', ticket='SERVER-90000', + backport=None, public=True), + _make_expected('Revert "SERVER-90000"', revert='Revert', ticket='SERVER-90000', + backport=None, public=True), + _make_expected( + 'Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from ' + 'branch mongodb-4.4', + imported='Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69' + ' from branch mongodb-4.4', public=True), + _make_expected( + 'Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from ' + 'branch mongodb-4.4', + imported='Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69' + ' from branch mongodb-4.4', public=True), + _make_expected('PRIVATE-1 this is a test', ticket='PRIVATE-1', public=False), + _make_expected('this is a test'), ] + self.assertTrue(all(find_ticket(message) == expected for message, expected in messages)) - self.assertTrue(all(main(message) == STATUS_OK for message in messages)) - def test_private(self): - self.assertEqual(main(["XYZ-1"]), STATUS_ERROR) +class FindMatchingPatternTest(unittest.TestCase): + def test_last_git_commit_success(self): + messages = [ + _make_expected("Fix lint", lint='Fix lint'), + _make_expected( + 'SERVER-44612 ' + 'this is the message\n\n' + '(cherry picked from commit beef)', revert=None, ticket='SERVER-44612', + backport='(cherry picked from commit beef)'), + _make_expected('SERVER-45074 this is a test (cherry picked from', revert=None, + ticket='SERVER-45074', backport='(cherry picked from'), + _make_expected('SERVER-45074 this is a test (cherry picked from commit beef)', + revert=None, ticket='SERVER-45074', + backport='(cherry picked from commit beef)'), + _make_expected('SERVER-20', revert=None, ticket='SERVER-20', backport=None), + _make_expected('WT-300', revert=None, ticket='WT-300', backport=None), + _make_expected('SERVER-44338', revert=None, ticket='SERVER-44338', backport=None), + _make_expected('Revert EVG-5', revert='Revert', ticket='EVG-5', backport=None), + _make_expected('Revert SERVER-60', revert='Revert', ticket='SERVER-60', backport=None), + _make_expected('Revert WT-700', revert='Revert', ticket='WT-700', backport=None), + _make_expected('Revert \'SERVER-8000', revert='Revert', ticket='SERVER-8000', + backport=None), + _make_expected('Revert \'SERVER-8000\'', revert='Revert', ticket='SERVER-8000', + backport=None), + _make_expected('Revert "SERVER-90000"', revert='Revert', ticket='SERVER-90000', + backport=None), + _make_expected('Revert "SERVER-90000"', revert='Revert', ticket='SERVER-90000', + backport=None), + _make_expected( + 'Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from ' + 'branch mongodb-4.4', + imported='Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69' + ' from branch mongodb-4.4'), + _make_expected( + 'Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from ' + 'branch mongodb-4.4', + imported='Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69' + ' from branch mongodb-4.4'), + ] + self.assertTrue( + all( + find_matching_pattern(message, VALID_PATTERNS) == expected + for message, expected in messages)) - def test_catch_all(self): - self.assertTrue(all(main(message) == STATUS_ERROR for message in INVALID_MESSAGES)) - def test_ignore_warnings(self): - self.assertTrue(all(main(["-i"] + message) == STATUS_OK for message in INVALID_MESSAGES)) +def _mock_issue(author, status='open', display="Jim O'Leary"): + mock_assignee = MagicMock(name='assignee', displayName=display) + type(mock_assignee).name = PropertyMock(return_value=author) + mock_fields = MagicMock(name='fields', status=status, assignee=mock_assignee) + mock_issue = MagicMock(name='issue', fields=mock_fields) + return mock_issue - def test_last_git_commit_success(self): - with patch( - ns("subprocess.check_output"), - return_value=bytearray('SERVER-1111 this is a test', 'utf-8')) as check_output_mock: - self.assertEqual(main([]), 0) - check_output_mock.assert_called_with(GIT_SHOW_COMMAND) + +class ValidateMessageTest(unittest.TestCase): + def _test_empty_message(self, message=''): + mock_jira = MagicMock(name='jira') + ticket, validations = validate_message(message, AUTHOR, mock_jira) + self.assertFalse(ticket) + self.assertEqual(1, len(validations)) + self.assertEqual(Status.ERROR, validations[0].status) + self.assertIn('empty commit', validations[0].message) + + def test_empty_message(self): + self._test_empty_message('') + + def test_blank_message(self): + self._test_empty_message(' ') + + def test_validate(self): + mock_jira = MagicMock(name='jira') + message = 'message' + ticket = {} + validations = [] + with patch(ns('find_ticket'), return_value=ticket) as mock_find_ticket, \ + patch(ns('validate_ticket'), return_value=validations) as mock_validate_ticket: + actual_ticket, actual_validations = validate_message(message, AUTHOR, mock_jira) + self.assertEqual(actual_ticket, ticket) + self.assertEqual(actual_validations, validations) + mock_find_ticket.assert_called_once_with(message) + mock_validate_ticket.assert_called_once_with(ticket, AUTHOR, mock_jira) + + +class ValidateTicketTest(unittest.TestCase): + def test_empty_ticket(self): + mock_jira = MagicMock(name='jira') + ticket = {} + validations = validate_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(validations)) + self.assertEqual(Status.WARNING, validations[0].status) + self.assertIn('commit without a ticket', validations[0].message) + + def test_public_ticket(self): + mock_jira = MagicMock(name='jira') + ticket = dict(public=True) + expected = [] + with patch(ns('validate_public_ticket'), + return_value=expected) as mock_validate_public_ticket: + self.assertEqual(expected, validate_ticket(ticket, AUTHOR, mock_jira)) + mock_validate_public_ticket.assert_called_once_with(ticket, AUTHOR, mock_jira) + + def test_private_ticket(self): + mock_jira = MagicMock(name='jira') + ticket = dict(public=False, ticket='PRIVATE-1') + expected = [] + with patch(ns('validate_public_ticket'), return_value=expected): + validations = validate_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(validations)) + self.assertEqual(Status.ERROR, validations[0].status) + self.assertIn('private project', validations[0].message) + + +class ValidatePublicTicketTest(unittest.TestCase): + def test_no_issue(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = None + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(len(faults), 1) + self.assertEqual(faults[0].status, Status.WARNING) + + def test_closed(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = _mock_issue(AUTHOR, status='closed') + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(len(faults), 1) + self.assertEqual(faults[0].status, Status.ERROR) + self.assertIn('status cannot be', faults[0].message) + + def test_different_author(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = _mock_issue('not ' + AUTHOR) + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(len(faults), 1) + self.assertEqual(faults[0].status, Status.WARNING) + self.assertIn('assignee is not author', faults[0].message) + + def test_multiple(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = _mock_issue('not ' + AUTHOR, status='closed') + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(len(faults), 2) + self.assertEqual(len([fault for fault in faults if fault.status == Status.WARNING]), 1) + self.assertEqual(len([fault for fault in faults if fault.status == Status.ERROR]), 1) + self.assertTrue(any(['assignee is not author' in fault.message for fault in faults])) + self.assertTrue(any(['status cannot be' in fault.message for fault in faults])) + + def test_backport_not_author(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = _mock_issue('not ' + AUTHOR) + ticket = dict(ticket='SERVER-45074', backport='(cherry picled from X)') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertFalse(faults) + + def test_backport_author(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.return_value = _mock_issue(AUTHOR) + ticket = dict(ticket='SERVER-45074', backport='(cherry picled from X)') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertFalse(faults) + + def test_connection_exception(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.side_effect = requests.exceptions.ConnectionError('boom') + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(faults)) + self.assertTrue(faults[0].status == Status.WARNING) + self.assertIn('unexpected connection exception', faults[0].message) + + def test_not_found(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.side_effect = JIRAError(status_code=HTTPStatus.NOT_FOUND) + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(faults)) + self.assertTrue(faults[0].status == Status.ERROR) + self.assertIn('not found', faults[0].message) + + def test_permission(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.side_effect = JIRAError(status_code=HTTPStatus.UNAUTHORIZED) + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(faults)) + self.assertTrue(faults[0].status == Status.ERROR) + self.assertIn('private', faults[0].message) + + def test_other_jira_exception(self): + mock_jira = MagicMock(name='jira') + mock_jira.issue.side_effect = JIRAError() + ticket = dict(ticket='SERVER-45074') + faults = validate_public_ticket(ticket, AUTHOR, mock_jira) + self.assertEqual(1, len(faults)) + self.assertTrue(faults[0].status == Status.WARNING) + self.assertIn('unexpected jira error', faults[0].message) + + +class HandleValidationTest(unittest.TestCase): + def test_empty(self): + ticket = {} + message = 'this is a test' + validations = [] + warnings_as_errors = False + self.assertEqual(Status.OK, + handle_violations(ticket, message, validations, warnings_as_errors)) + + def test_error(self): + ticket = {} + message = 'this is a test' + validations = [Violation(Status.ERROR, 'private project: PRIVATE-1')] + warnings_as_errors = False + self.assertEqual(Status.ERROR, + handle_violations(ticket, message, validations, warnings_as_errors)) + + def _test_warnings(self, validations=None, warnings_as_errors=False): + ticket = {} + message = 'this is a test' + if validations is None: + validations = [ + Violation(Status.WARNING, 'found a commit without a ticket'), + Violation(Status.WARNING, 'another warning') + ] + self.assertEqual(Status.OK if not warnings_as_errors else Status.ERROR, + handle_violations(ticket, message, validations, warnings_as_errors)) + + def test_single_warning(self): + self._test_warnings([Violation(Status.WARNING, 'found a commit without a ticket')]) + + def test_multiple_warnings(self): + self._test_warnings() + + def test_warning_as_error(self): + self._test_warnings(warnings_as_errors=True) + + def test_warnings_and_errors(self): + ticket = {} + message = 'this is a test' + validations = [ + Violation(Status.ERROR, 'private project: PRIVATE-1'), + Violation(Status.WARNING, 'found a commit without a ticket') + ] + warnings_as_errors = False + self.assertEqual(Status.ERROR, + handle_violations(ticket, message, validations, warnings_as_errors)) + + +def _make_expected(message, **kwargs): + return message, dict(**kwargs) + + +class JiraClientTest(unittest.TestCase): + def _test_exception(self, exception): + with patch(ns("JIRA"), side_effect=exception("boom")): + self.assertIsNone(jira_client('server', verbose=False)) + + def test_connection_exceptions(self): + """test connection exceptions.""" + self._test_exception(requests.exceptions.ConnectionError) + + def test_jira_exception(self): + """test jira exceptions.""" + self._test_exception(JIRAError) + + def test_value_exception(self): + """test ValueError exceptions.""" + self._test_exception(ValueError) + + def test_exception(self): + """test Exception exceptions.""" + self._test_exception(Exception) + + def test_issue(self): + mock_jira = MagicMock() + # mock_jira.issue.return_value = 'issue' + with patch(ns("JIRA"), return_value=mock_jira) as mock_ctor: + jira = jira_client('server') + mock_ctor.assert_called_once_with('server', logging=False) + mock_jira.issue.assert_called_once_with('SERVER-1') + self.assertEqual(mock_jira, jira) + + +class ConfigureLoggingTest(unittest.TestCase): + def _test_configure_formatter(self, formatter=None): + mock_root_logger = MagicMock(name="root_logger") + with patch.object(logging, 'basicConfig') as mock_basic_config, \ + patch.object(logging, 'getLogger') as mock_get_logger: + mock_get_logger.return_value = mock_root_logger + if formatter is None: + configure_logging(0) + else: + configure_logging(0, formatter=formatter) + + if formatter is None: + expected = '%(levelname)s: %(message)s' + else: + expected = formatter + + mock_basic_config.assert_called_once_with(format=expected) + mock_root_logger.setLevel.assert_called_once_with(logging.WARNING) + + def test_configure_default_formatter(self): + self._test_configure_formatter() + + def test_configure_formatter(self): + self._test_configure_formatter(formatter='format') + + def _test_configure_level(self, level, expected, debuglevel=0): + old_debuglevel = HTTPConnection.debuglevel + try: + mock_root_logger = MagicMock(name="root_logger") + with patch.object(logging, 'basicConfig'), \ + patch.object(logging, 'getLogger') as mock_get_logger: + mock_get_logger.return_value = mock_root_logger + configure_logging(level) + + mock_root_logger.setLevel.assert_called_once_with(expected) + self.assertEqual(HTTPConnection.debuglevel, debuglevel) + finally: + HTTPConnection.debuglevel = old_debuglevel + + def test_configure_level_0(self): + self._test_configure_level(0, logging.WARNING) + + def test_configure_level_1(self): + self._test_configure_level(1, logging.INFO) + + def test_configure_level_2(self): + self._test_configure_level(2, logging.DEBUG, debuglevel=1) + + def test_configure_level_3(self): + self._test_configure_level(3, logging.DEBUG, debuglevel=1) + + +class MainTest(unittest.TestCase): + def _test_helper(self, args=None, violations=None, expected=Status.OK): + if violations is None: + violations = [] + if args is None: + args = [] + args = ['SERVER-45074 this is a test', '-a', AUTHOR] + args + with patch(ns('jira_client')), patch(ns('validate_public_ticket'), return_value=violations): + self.assertEqual(expected, main(args)) + + def test_no_violations(self): + self._test_helper() + + def test_warning(self): + violations = [Violation(Status.WARNING, 'just a warning')] + self._test_helper(violations=violations) + + def test_warning_as_error(self): + violations = [Violation(Status.WARNING, 'just a warning')] + args = ['-W'] + self._test_helper(args=args, violations=violations, expected=Status.ERROR) + + def test_error(self): + violations = [Violation(Status.ERROR, 'just an error')] + self._test_helper(violations=violations, expected=Status.ERROR) + + def test_no_author(self): + args = ['SERVER-45074 this is a test'] + with self.assertRaises(SystemExit) as cm: + main(args) + exception = cm.exception + self.assertEqual(exception.code, 2) + + def test_author(self): + args = ['SERVER-45074 this is a test', '-a', AUTHOR] + with patch(ns('JIRA'), side_effect=requests.exceptions.ConnectionError): + self.assertEqual(main(args), 0) diff --git a/buildscripts/validate_commit_message.py b/buildscripts/validate_commit_message.py index 03283b9d940..37973faa584 100755 --- a/buildscripts/validate_commit_message.py +++ b/buildscripts/validate_commit_message.py @@ -28,36 +28,398 @@ # """Validate that the commit message is ok.""" import argparse -import os +import collections +import logging import re import subprocess import sys +from contextlib import contextmanager +from enum import IntEnum +from http import HTTPStatus +from http.client import HTTPConnection # py3 +from typing import Dict, List, Type, Tuple, Optional, Match, Any + +import requests.exceptions +from jira import JIRA, Issue +from jira.exceptions import JIRAError VALID_PATTERNS = [ - re.compile(r"^Fix lint$"), # Allow "Fix lint" as the sole commit summary - re.compile(r'^(Revert ["\']?)?(EVG|SERVER|WT)-[0-9]+'), # These are public tickets - re.compile(r'^Import (wiredtiger|tools):'), # These are public tickets + # NOTE: re.VERBOSE is for visibility / debugging. As such significant white space must be + # escaped (e.g ' ' to \s). + re.compile( + r''' + ^ + ((?P<revert>Revert)\s*[\"\']?)? # Revert (optional) + ((?P<ticket>(?:EVG|SERVER|WT)-[0-9]+)\s*) # ticket identifier + #(?P<body>(?:(?!\(cherry\spicked\sfrom).)*)? # To also capture the body + ((?:(?!\(cherry\spicked\sfrom).)*)? # negative lookahead backport + (?P<backport>\(cherry\spicked\sfrom.*)? # back port (optional) + ''', re.MULTILINE | re.DOTALL | re.VERBOSE), + re.compile(r'(?P<lint>^Fix lint$)'), # Allow "Fix lint" as the sole commit summary + re.compile(r'(?P<imported>^Import (wiredtiger|tools): .*)'), # These are public tickets ] -PRIVATE_PATTERNS = [re.compile(r"^[A-Z]+-[0-9]+")] +"""valid public patterns.""" + +PRIVATE_PATTERNS = [re.compile(r'^(?P<ticket>[A-Z]+-[0-9]+)')] +"""private patterns.""" + +INVALID_JIRA_STATUS = ('closed', ) +"""List of lower cased invalid jira status strings.""" + +GIT_SHOW_COMMAND = ['git', 'show', '-1', '-s', '--format=%s'] +"""git command line to get the last commit message.""" + +DEFAULT_JIRA = 'https://jira.mongodb.org' +LOGGER = logging.getLogger(__name__) + + +class Status(IntEnum): + """Status enumeration values.""" + + OK = 0 + ERROR = 1 + WARNING = 2 + + +class Violation(collections.namedtuple('Fault', ['status', 'message'])): + """validation issue holder.""" + + def __str__(self): + return str(self.message) + + +def get_full_message(message: List[str]) -> str: + """ + Convert the message to a single string or get the last git commit message. + + If the input list is empty then the last git commit message is used. + + :param message: A list of the message components. + :return: The message. + """ + LOGGER.info('get commit message') + if not message: + LOGGER.info('Validating last git commit message') + result = subprocess.check_output(GIT_SHOW_COMMAND) + message = result.decode('utf-8') + else: + message = " ".join(message) + LOGGER.info('Validating commit message \'%s\'', message) + return message + + +def find_ticket(message: str) -> Dict: + """ + Find ticket data in message. + + :param message: The commit message. + :return: A dict of the commit message components (may be empty). + """ + ticket = find_matching_pattern(message, VALID_PATTERNS) + if ticket: + ticket['public'] = True + else: + ticket = find_matching_pattern(message, PRIVATE_PATTERNS) + if ticket: + ticket['public'] = False + return ticket + + +def find_matching_pattern(message: str, patterns: List[Match]) -> Dict: + """ + Find the first matching pattern. + + :param message: The commit message. + :param patterns: A list of regular expressions. + :return: A dict of the commit message components (may be empty). + """ + for valid_pattern in patterns: + matching_pattern = valid_pattern.match(message) + # pattern matches and there is a ticket + if matching_pattern: + return matching_pattern.groupdict() + return {} + + +def validate_message(message: str, author: str, + jira: Optional[Type[JIRA]]) -> Tuple[Dict, List[Violation]]: + """ + Validate the commit message. + + :param message: The commit message. + :param author: The author. + :param jira: The jira connection. + :return: The ticket dict and violations. + """ + LOGGER.info('validating message') + if not message.strip(): + ticket = {} + violations = [Violation(Status.ERROR, 'found empty commit message')] + else: + ticket = find_ticket(message) + violations = validate_ticket(ticket, author, jira) + return ticket, violations + + +def validate_ticket(ticket: Dict, author: str, jira: Optional[Type[JIRA]]) -> List[Violation]: + """ + Validate the ticket and commit message. + + :param ticket: The extract ticket information. + :param author: The author. + :param jira: The jira connection. + :return: The violations. + """ + violations = [] + + if not ticket: + violations.append(Violation(Status.WARNING, 'found a commit without a ticket')) + elif ticket['public']: + violations = validate_public_ticket(ticket, author, jira) + else: + tid = ticket['ticket'] + violations.append(Violation(Status.ERROR, f'private project: {tid}')) + + return violations + + +def validate_status(issue: Type[Issue]) -> Optional[Violation]: + """ + Validate that the issue status is allowed. + + :param issue: The jira issue. + :return: A violation (if applicable). + """ + status = str(issue.fields.status).lower() + if status in INVALID_JIRA_STATUS: + return Violation(Status.ERROR, f'status cannot be {status}') + return None + -STATUS_OK = 0 -STATUS_ERROR = 1 +def validate_author(issue: Type[Issue], ticket: Dict, author: str) -> Violation: + """ + Validate that the issue author is correct. -GIT_SHOW_COMMAND = ["git", "show", "-1", "-s", "--format=%s"] + :param issue: The jira issue. + :param ticket: The ticket data. + :param author: The expected author. + :return: A violation (if applicable). + """ + assignee = issue.fields.assignee + if assignee.name != author: + details = (f'assignee is not author \'{assignee.name}\'(' + f' \'{assignee.displayName}\') != \'{author}\'') + if not ticket.get('backport', False): + return Violation(Status.WARNING, details) + else: + LOGGER.debug('%s but this is a backport', details) + return None -def main(argv=None): - """Execute Main function to validate commit messages.""" +def validate_public_ticket(ticket: Dict, author: str, jira: Type[JIRA], + verbose: bool = False) -> List[Violation]: + """ + Validate the status of a public ticket. + + :param ticket: The extract ticket information. + :param author: The author. + :param jira: The jira connection. + :param verbose: A flag to enable / disable verbose output. + :return: The violations. + """ + violations = [] + ticket_id = ticket['ticket'] + try: + if jira is not None: + with silence(verbose): + issue = jira.issue(ticket_id) + if issue: + violation = validate_status(issue) + if violation: + violations.append(violation) + + violation = validate_author(issue, ticket, author) + if violation: + violations.append(violation) + + else: + LOGGER.debug('unable to fully validate issue \'%s\'', ticket_id) + violations.append( + Violation(Status.WARNING, f'{ticket_id}: unable to validate with jira')) + except requests.exceptions.ConnectionError: + LOGGER.debug('%s: unexpected connection exception', exc_info=True) + violations.append( + Violation(Status.WARNING, f'{ticket_id}: unexpected connection exception')) + except JIRAError as ex: + LOGGER.debug('unexpected jira exception', exc_info=True) + if ex.status_code == HTTPStatus.NOT_FOUND: + violation = Violation(Status.ERROR, f'{ticket_id}: not found') + elif ex.status_code == HTTPStatus.UNAUTHORIZED: + violation = Violation(Status.ERROR, f'{ticket_id}: private (unauthorized)') + else: + violation = Violation(Status.WARNING, + f'{ticket_id}: unexpected jira error {ex.status_code}') + violations.append(violation) + except ValueError: + LOGGER.debug('unexpected exception', exc_info=True) + violations.append(Violation(Status.WARNING, f'{ticket_id}: unexpected exception')) + + return violations + + +def handle_violations(ticket: Dict, message: str, violations: List[Violation], + warning_as_errors: bool) -> Type[Status]: + """ + Handle any validation issues found. + + :param ticket: The extract ticket information. + :param message: The commit message. + :param violations: The validation violations. + :param warning_as_errors: If True then treat all violations as errors. + :return: The Status.ERROR if no errors or warning_as_errors. + """ + LOGGER.info('handle validation issues') + if warning_as_errors: + errors = violations + warnings = [] + else: + errors = [validation for validation in violations if validation.status == Status.ERROR] + warnings = [validation for validation in violations if validation.status == Status.WARNING] + + if errors: + LOGGER.error("%s\n\t%s", ticket['ticket'] if ticket and 'ticket' in ticket else message, + "\n\t".join(error.message for error in errors)) + + if warnings: + LOGGER.warning("%s\n\t%s", ticket['ticket'] if ticket and 'ticket' in ticket else message, + "\n\t".join(warning.message for warning in warnings)) + + return Status.ERROR if errors else Status.OK + + +def jira_client(jira_server: str, verbose: bool = False) -> Optional[Type[JIRA]]: + """ + Connect to jira. + + Create a connection to jira_server and validate that SERVER-1 is accessible. + :param jira_server: The jira server endpoint to connect and validate. + :param verbose: A flag controlling th verbosity of the checks. The requests and jira package + are very verbose by default. + :return: The jira client instance if all went well. + """ + try: + # requests and JIRA can be very verbose. + LOGGER.info('connecting to %s', jira_server) + with silence(verbose): + jira = JIRA(jira_server, logging=verbose) + # check the status of a known / existing ticket. A JIRAError with a status code of + # 404 Not Found maybe returned or a 401 unauthorized. + jira.issue("SERVER-1") + return jira + except (requests.exceptions.ConnectionError, JIRAError, ValueError) as ex: + # These are recoverable / ignorable exceptions. We print exception the full stack trace + # when debugging / verbose output is requested. + # ConnectionErrors relate to networking. + # JIRAError, ValueError refer to invalid / unexpected responses. + class_name = _get_class_name(ex) + if isinstance(ex, requests.exceptions.ConnectionError): + details = f'{class_name}: unable to connect to {jira_server}' + elif isinstance(ex, JIRAError): + details = f'{class_name}: unable to access {jira_server}, status: {ex.status_code}' + elif isinstance(ex, ValueError): + details = f'{class_name}: communication error with {jira_server}' + LOGGER.debug(details, exc_info=True) + except Exception as ex: # pylint: disable=broad-except + # recoverable / ignorable exceptions but unknown so print trace. + LOGGER.debug('%s unknown error: %s', _get_class_name(ex), jira_server, exc_info=True) + return None + + +def configure_logging(level: int, formatter: str = '%(levelname)s: %(message)s'): + """ + Configure logging. + + :param level: The log level verbosity. 0 logs warnings and above. 1 enabled info and above, all + other values are DEBUG and above. + :param formatter: The log formatter. + """ + # level 0 is the default level (warning or greater). + logging.basicConfig(format=formatter) + root_logger = logging.getLogger() + debuglevel = 0 + if level == 0: + level = logging.WARNING + elif level == 1: + level = logging.INFO + elif level >= 2: + debuglevel = 1 + level = logging.DEBUG + + root_logger.setLevel(level) + HTTPConnection.debuglevel = debuglevel + + +def _get_class_name(obj: Any) -> str: + """Get the class name without package.""" + return type(obj).__name__ + + +@contextmanager +def silence(disable: bool = False): + """ + Silence logging within this scope. + + :param disable: A flag to programmatically enable / disable the silence functionality. Useful + for debugging. + """ + logger = logging.getLogger() + old = logger.disabled + try: + if not disable: + logger.disabled = True + yield + finally: + logger.disabled = old + + +def parse_args(argv: List[str]) -> Type[argparse.ArgumentParser]: + """ + Parse the command line args. + + :param argv: The command line arguments. + :return: The parsed arguments. + """ parser = argparse.ArgumentParser( usage="Validate the commit message. " "It validates the latest message when no arguments are provided.") parser.add_argument( - "-i", + "-a", + '--author', + dest="author", + nargs='?', + const=1, + type=str, + help="Your jira username of the author. This value must match the JIRA assignee.", + required=True, + ) + parser.add_argument( + "-j", + dest="jira_server", + nargs='?', + const=1, + type=str, + help="The jira server location. Defaults to '" + DEFAULT_JIRA + "'", + default=DEFAULT_JIRA, + ) + parser.add_argument( + "-W", action="store_true", - dest="ignore_warnings", - help="Ignore all warnings.", + dest="warning_as_errors", + help="treat warnings as errors.", default=False, ) + parser.add_argument("-v", "--verbosity", action="count", default=0, + help="increase output verbosity") parser.add_argument( "message", metavar="commit message", @@ -65,24 +427,25 @@ def main(argv=None): help="The commit message to validate", ) args = parser.parse_args(argv) + return args - if not args.message: - print('Validating last git commit message') - result = subprocess.check_output(GIT_SHOW_COMMAND) - message = result.decode('utf-8') - else: - message = " ".join(args.message) - if any(valid_pattern.match(message) for valid_pattern in VALID_PATTERNS): - status = STATUS_OK - elif any(private_pattern.match(message) for private_pattern in PRIVATE_PATTERNS): - print("ERROR: found a reference to a private project\n{message}".format(message=message)) - status = STATUS_ERROR - else: - print("{message_type}: found a commit without a ticket\n{message}".format( - message_type="WARNING" if args.ignore_warnings else "ERROR", message=message)) - status = STATUS_OK if args.ignore_warnings else STATUS_ERROR - return status +def main(argv: List = None) -> Type[Status]: + """ + Execute main function to validate commit messages. + + :param argv: The command line arguments. + :return: Status.OK if the commit message validation passed other wise Status.ERROR. + """ + + args = parse_args(argv) + configure_logging(level=args.verbosity) + jira = jira_client(args.jira_server) + + message = get_full_message(args.message) + ticket, violations = validate_message(message, args.author, jira) + + return handle_violations(ticket, message, violations, args.warning_as_errors) if __name__ == "__main__": diff --git a/etc/evergreen.yml b/etc/evergreen.yml index 7f401b6fd3b..2f87c6b85b9 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -8243,7 +8243,7 @@ tasks: commit_message_content=$(cat commit_message.txt) ${activate_virtualenv} - $python buildscripts/validate_commit_message.py "$commit_message_content" + $python buildscripts/validate_commit_message.py "$commit_message_content" -a "${author}" fi - <<: *task_template |