SERVER-59511: Improve error messages in validate_commit_message

3 files changed, 70 insertions, 45 deletions

diff --git a/buildscripts/tests/test_validate_commit_message.py b/buildscripts/tests/test_validate_commit_message.py
index 575a840f6f3..acd6008da44 100644
--- a/buildscripts/tests/test_validate_commit_message.py
+++ b/buildscripts/tests/test_validate_commit_message.py
@@ -2,11 +2,11 @@
 import itertools
 import unittest
 from typing import List
-from mock import MagicMock, patch
+from unittest.mock import MagicMock
 
-import evergreen
+from evergreen import EvergreenApi
 
-from buildscripts.validate_commit_message import main, STATUS_OK, STATUS_ERROR
+import buildscripts.validate_commit_message as under_test
 
 # pylint: disable=missing-docstring,no-self-use
 
@@ -18,13 +18,6 @@ INVALID_MESSAGES = [
     "Fix Lint",  # Fix lint is strict in terms of caps
 ]
 
-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 NS + "." + relative_name
-
 
 def create_mock_evg_client(code_change_messages: List[str]) -> MagicMock:
     mock_code_change = MagicMock()
@@ -33,7 +26,7 @@ def create_mock_evg_client(code_change_messages: List[str]) -> MagicMock:
     mock_patch = MagicMock()
     mock_patch.module_code_changes = [mock_code_change]
 
-    mock_evg_client = MagicMock()
+    mock_evg_client = MagicMock(spec_set=EvergreenApi)
     mock_evg_client.patch_by_id.return_value = mock_patch
     return mock_evg_client
 
@@ -48,8 +41,7 @@ def interleave_new_format(older):
 
 
 class ValidateCommitMessageTest(unittest.TestCase):
-    @patch.object(evergreen.RetryingEvergreenApi, "get_api")
-    def test_valid_commits(self, get_api_mock):
+    def test_valid_commits(self):
         messages = [
             "Fix lint",
             "EVG-1",  # Test valid projects with various number lengths
@@ -65,28 +57,29 @@ class ValidateCommitMessageTest(unittest.TestCase):
             "Import tools: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from branch mongodb-4.4",
             'Revert "Import wiredtiger: 58115abb6fbb3c1cc7bfd087d41a47347bce9a69 from branch mongodb-4.4"',
         ]
-        api_mock = create_mock_evg_client(interleave_new_format(messages))
+        mock_evg_api = create_mock_evg_client(interleave_new_format(messages))
 
-        get_api_mock.return_value = api_mock
-        self.assertTrue(main(["fake_version"]) == STATUS_OK)
+        is_valid = under_test.validate_commit_messages("version_id", mock_evg_api)
 
-    @patch.object(evergreen.RetryingEvergreenApi, "get_api")
-    def test_private(self, get_api_mock):
+        self.assertEqual(is_valid, under_test.STATUS_OK)
+
+    def test_private(self):
         messages = ["XYZ-1"]
-        api_mock = create_mock_evg_client(interleave_new_format(messages))
+        mock_evg_api = create_mock_evg_client(interleave_new_format(messages))
+
+        is_valid = under_test.validate_commit_messages("version_id", mock_evg_api)
 
-        get_api_mock.return_value = api_mock
-        self.assertTrue(main(["fake_version"]) == STATUS_ERROR)
+        self.assertEqual(is_valid, under_test.STATUS_ERROR)
 
-    @patch.object(evergreen.RetryingEvergreenApi, "get_api")
-    def test_private_with_public(self, get_api_mock):
+    def test_private_with_public(self):
         messages = [
             "Fix lint",
             "EVG-1",  # Test valid projects with various number lengths
             "SERVER-20",
             "XYZ-1"
         ]
-        api_mock = create_mock_evg_client(interleave_new_format(messages))
+        mock_evg_api = create_mock_evg_client(interleave_new_format(messages))
+
+        is_valid = under_test.validate_commit_messages("version_id", mock_evg_api)
 
-        get_api_mock.return_value = api_mock
-        self.assertTrue(main(["fake_version"]) == STATUS_ERROR)
+        self.assertEqual(is_valid, under_test.STATUS_ERROR)
diff --git a/buildscripts/validate_commit_message.py b/buildscripts/validate_commit_message.py
index e655b67b2e5..61fcce06f85 100755
--- a/buildscripts/validate_commit_message.py
+++ b/buildscripts/validate_commit_message.py
@@ -28,14 +28,28 @@
 #
 """Validate that the commit message is ok."""
 import argparse
+import os
 import re
 import sys
 import logging
-from evergreen import RetryingEvergreenApi
+from evergreen import RetryingEvergreenApi, EvergreenApi
 
-EVG_CONFIG_FILE = "./.evergreen.yml"
+EVG_CONFIG_FILE = "~/.evergreen.yml"
 LOGGER = logging.getLogger(__name__)
 
+ERROR_MSG = """
+################################################################################
+Encountered an invalid commit message. Please correct to the commit message to
+continue. 
+
+Commit message should start with a Public Jira ticket, an "Import" for wiredtiger
+or tools, or a "Revert" message.
+
+{error_msg} on '{branch}':
+'{commit_message}'
+################################################################################
+"""
+
 COMMON_PUBLIC_PATTERN = r'''
     ((?P<revert>Revert)\s+[\"\']?)?                         # Revert (optional)
     ((?P<ticket>(?:EVG|SERVER|WT)-[0-9]+)[\"\']?\s*)               # ticket identifier
@@ -122,6 +136,34 @@ PRIVATE_PATTERNS = [
 """private patterns."""
 
 
+def validate_commit_messages(version_id: str, evg_api: EvergreenApi) -> int:
+    """
+    Validate the commit messages for the given build.
+
+    :param version_id: ID of version to validate.
+    :param evg_api: Evergreen API client.
+    :return: True if all commit messages were valid.
+    """
+    found_error = False
+    code_changes = evg_api.patch_by_id(version_id).module_code_changes
+    for change in code_changes:
+        for message in change.commit_messages:
+            if any(valid_pattern.match(message) for valid_pattern in VALID_PATTERNS):
+                continue
+            elif any(private_pattern.match(message) for private_pattern in PRIVATE_PATTERNS):
+                print(
+                    ERROR_MSG.format(error_msg="Reference to a private project",
+                                     branch=change.branch_name, commit_message=message))
+                found_error = True
+            else:
+                print(
+                    ERROR_MSG.format(error_msg="Commit without a ticket", branch=change.branch_name,
+                                     commit_message=message))
+                found_error = True
+
+    return STATUS_ERROR if found_error else STATUS_OK
+
+
 def main(argv=None):
     """Execute Main function to validate commit messages."""
     parser = argparse.ArgumentParser(
@@ -132,24 +174,12 @@ def main(argv=None):
         metavar="version id",
         help="The id of the version to validate",
     )
+    parser.add_argument("--evg-config-file", default=EVG_CONFIG_FILE,
+                        help="Path to evergreen configuration file containing auth information.")
     args = parser.parse_args(argv)
-    evg_api = RetryingEvergreenApi.get_api(config_file=EVG_CONFIG_FILE)
-
-    code_changes = evg_api.patch_by_id(args.version_id).module_code_changes
-
-    for change in code_changes:
-        for message in change.commit_messages:
-            if any(valid_pattern.match(message) for valid_pattern in VALID_PATTERNS):
-                continue
-            elif any(private_pattern.match(message) for private_pattern in PRIVATE_PATTERNS):
-                error_type = "Found a reference to a private project"
-            else:
-                error_type = "Found a commit without a ticket"
-            if error_type:
-                LOGGER.error(f"{error_type}\n{message}")  # pylint: disable=logging-fstring-interpolation
-                return STATUS_ERROR
+    evg_api = RetryingEvergreenApi.get_api(config_file=os.path.expanduser(args.evg_config_file))
 
-    return STATUS_OK
+    return validate_commit_messages(args.version_id, evg_api)
 
 
 if __name__ == "__main__":
diff --git a/evergreen/commit_message_validate.sh b/evergreen/commit_message_validate.sh
index cf2edcd2cf0..3d7825ae6e1 100644
--- a/evergreen/commit_message_validate.sh
+++ b/evergreen/commit_message_validate.sh
@@ -7,5 +7,7 @@ set -o verbose
 set -o errexit
 if [ "${is_commit_queue}" = "true" ]; then
   activate_venv
-  $python buildscripts/validate_commit_message.py ${version_id}
+  $python buildscripts/validate_commit_message.py \
+    --evg-config-file ./.evergreen.yml \
+    ${version_id}
 fi