summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Bradford <david.bradford@mongodb.com>2020-01-24 12:43:39 -0500
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2020-01-24 19:17:25 +0000
commit544cd7662f2d8ba2d4067d5bc0f960c07ff40ce4 (patch)
tree9c2890b131516c48983f49ad860c6b77ce5c345b
parentaed4edcbf74f914fbcdd831937fa581aa436c5c6 (diff)
downloadmongo-544cd7662f2d8ba2d4067d5bc0f960c07ff40ce4.tar.gz
Revert "SERVER-45074 validate commit message against JIRA"
This reverts commit 1da2398abc11189a6d7706cfba12fa71a693e129.
-rw-r--r--buildscripts/tests/test_validate_commit_message.py480
-rwxr-xr-xbuildscripts/validate_commit_message.py421
-rw-r--r--etc/evergreen.yml2
3 files changed, 61 insertions, 842 deletions
diff --git a/buildscripts/tests/test_validate_commit_message.py b/buildscripts/tests/test_validate_commit_message.py
index 4682fb02014..1cfd37a1d0c 100644
--- a/buildscripts/tests/test_validate_commit_message.py
+++ b/buildscripts/tests/test_validate_commit_message.py
@@ -1,20 +1,11 @@
"""Unit tests for the evergreen_task_timeout script."""
-import logging
+
import unittest
-from http import HTTPStatus
-from http.client import HTTPConnection
+from mock import MagicMock, patch
-import requests.exceptions
-from jira import JIRAError
-from mock import MagicMock, patch, PropertyMock
+from buildscripts.validate_commit_message import main, STATUS_OK, STATUS_ERROR, GIT_SHOW_COMMAND
-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
@@ -28,450 +19,41 @@ NS = "buildscripts.validate_commit_message"
def ns(relative_name): # pylint: disable=invalid-name
- """Return a full name from a name relative to the tested module's name space."""
+ """Return a full name from a name relative to the test module"s name space."""
return NS + "." + relative_name
-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):
+class ValidateCommitMessageTest(unittest.TestCase):
+ def test_valid(self):
messages = [
- _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))
-
-
-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 _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
-
-
-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')
+ ["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"]
]
- 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)
+ self.assertTrue(all(main(message) == STATUS_OK for message in messages))
+ def test_private(self):
+ self.assertEqual(main(["XYZ-1"]), STATUS_ERROR)
-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)
+ def test_catch_all(self):
+ self.assertTrue(all(main(message) == STATUS_ERROR for message in INVALID_MESSAGES))
- if formatter is None:
- expected = '%(levelname)s: %(message)s'
- else:
- expected = formatter
+ def test_ignore_warnings(self):
+ self.assertTrue(all(main(["-i"] + message) == STATUS_OK for message in INVALID_MESSAGES))
- 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)
+ 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)
diff --git a/buildscripts/validate_commit_message.py b/buildscripts/validate_commit_message.py
index 37973faa584..03283b9d940 100755
--- a/buildscripts/validate_commit_message.py
+++ b/buildscripts/validate_commit_message.py
@@ -28,398 +28,36 @@
#
"""Validate that the commit message is ok."""
import argparse
-import collections
-import logging
+import os
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 = [
- # 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
+ 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
]
-"""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
-
+PRIVATE_PATTERNS = [re.compile(r"^[A-Z]+-[0-9]+")]
-def validate_author(issue: Type[Issue], ticket: Dict, author: str) -> Violation:
- """
- Validate that the issue author is correct.
+STATUS_OK = 0
+STATUS_ERROR = 1
- :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
+GIT_SHOW_COMMAND = ["git", "show", "-1", "-s", "--format=%s"]
-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.
- """
+def main(argv=None):
+ """Execute Main function to validate commit messages."""
parser = argparse.ArgumentParser(
usage="Validate the commit message. "
"It validates the latest message when no arguments are provided.")
parser.add_argument(
- "-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",
+ "-i",
action="store_true",
- dest="warning_as_errors",
- help="treat warnings as errors.",
+ dest="ignore_warnings",
+ help="Ignore all warnings.",
default=False,
)
- parser.add_argument("-v", "--verbosity", action="count", default=0,
- help="increase output verbosity")
parser.add_argument(
"message",
metavar="commit message",
@@ -427,25 +65,24 @@ def parse_args(argv: List[str]) -> Type[argparse.ArgumentParser]:
help="The commit message to validate",
)
args = parser.parse_args(argv)
- return args
-
-
-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)
+ 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)
- return handle_violations(ticket, message, violations, args.warning_as_errors)
+ 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
if __name__ == "__main__":
diff --git a/etc/evergreen.yml b/etc/evergreen.yml
index c2ef9a260ec..f6c081dcf8b 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" -a "${author}"
+ $python buildscripts/validate_commit_message.py "$commit_message_content"
fi
- <<: *task_template