Merge pull request #3 from MiraGeoscience/DEVOPS-404

DEVOPS-404: Fix pre-commit hook preparing commit message

Author: Swendart

mirageoscience/hooks/__init__.py

@@ -6,4 +6,4 @@
 #  All rights reserved.                                                        '
 # ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
-__version__ = "1.0.0"
+__version__ = "1.0.1"

mirageoscience/hooks/git_message_hook.py

@@ -31,7 +31,7 @@ class JiraPattern:
         making sure it gets compiled only once."""
 
         __pattern = re.compile(
-            r"(?:GEOPY|GI|GA|GMS|VPem1D|VPem3D|VPmg|UBCGIF|LICMGR)-\d+"
+            r"(?:\w*!)?\s*\S?\b((?:GEOPY|GI|GA|GMS|VPem1D|VPem3D|VPmg|UBCGIF|LICMGR|DEVOPS|QA)-\d+)"
         )
 
         @staticmethod
@@ -41,8 +41,29 @@ def get():
 
     # use re.match() rather than re.search() to enforce the JIRA reference to be at the beginning
     match = re.match(JiraPattern.get(), text.strip())
-    return match.group(0) if match else ""
+    return match.group(1) if match else ""
 
+def get_message_prefix_bang(line: str) -> str:
+    """Capture the standard commit message prefix, if any, such as 'fixup!', 'amend!',
+        etc.
+
+        :return: the standard commit message prefix if found, else empty string.
+        """
+    class BangPattern:
+        """Internal class that encapsulates the regular expression for the Bnag pattern,
+        making sure it gets compiled only once."""
+
+        __pattern = re.compile(
+            r"(\w*!\s)"
+        )
+
+        @staticmethod
+        def get():
+            """:return: the compiled regular expression for the JIRA pattern"""
+            return BangPattern.__pattern
+    # use re.match() rather than re.search() to enforce pattern at the beginning
+    match = re.match(BangPattern.get(), line.strip())
+    return match.group(1) if match else ""
 
 def get_branch_name() -> str | None:
     """:return: the name of the current branch"""
@@ -51,6 +72,7 @@ def get_branch_name() -> str | None:
         shlex.split("git branch --list"),
         stdout=subprocess.PIPE,
         text=True,
+        check=False
     )
 
     if git_proc.returncode != 0:
@@ -100,18 +122,20 @@ def check_commit_message(filepath: str) -> tuple[bool, str]:
 
     message_jira_id = ""
     first_line = None
-    with open(filepath) as message_file:
+    with open(filepath, encoding="utf-8") as message_file:
         for line in message_file:
             if not line.startswith("#") and len(line.strip()) > 0:
                 # test only the first non-comment line that is not empty
                 # (should we reject messages with empty first line?)
-                first_line = line
+                first_line = line.strip()
+                prefix_bang = get_message_prefix_bang(first_line)
+                first_line = first_line[len(prefix_bang) :].strip()
                 message_jira_id = get_jira_id(first_line)
                 break
     assert first_line is not None
 
     if not branch_jira_id and not (
-        message_jira_id or first_line.strip().lower().startswith("merge")
+        message_jira_id or (not prefix_bang and first_line.lower().startswith("merge"))
     ):
         return (
             False,
@@ -121,7 +145,8 @@ def check_commit_message(filepath: str) -> tuple[bool, str]:
     if branch_jira_id and message_jira_id and branch_jira_id != message_jira_id:
         return (
             False,
-            f"Different JIRA ID in commit message {message_jira_id} and in branch name {branch_jira_id}.",
+            f"Different JIRA ID in commit message {message_jira_id} "
+            f"and in branch name {branch_jira_id}.",
         )
 
     stripped_message_line = ""
@@ -176,24 +201,31 @@ def prepare_commit_msg(filepath: str, source: str | None = None) -> None:
     if source not in [None, "message", "template"]:
         return
 
+    prefix_bang = ""
     with open(
         filepath,
         "r+",
+        encoding="utf-8"
     ) as message_file:
         message_has_jira_id = False
         message_lines = message_file.readlines()
         for line_index, line_content in enumerate(message_lines):
             if not line_content.startswith("#"):
                 # test only the first non-comment line
+                line_content = line_content.strip()
+                prefix_bang = get_message_prefix_bang(line_content)
+                line_content = line_content[len(prefix_bang):].strip()
                 message_jira_id = get_jira_id(line_content)
                 if not message_jira_id:
-                    message_lines[line_index] = branch_jira_id + ": " + line_content
+                    message_lines[line_index] = (
+                        f"{prefix_bang}[{branch_jira_id}] {line_content}\n"
+                    )
                 message_has_jira_id = True
                 break
 
         if not message_has_jira_id:
             # message is empty or all lines are comments: insert JIRA ID at the very beginning
-            message_lines.insert(0, branch_jira_id + ": ")
+            message_lines.insert(0, f"{prefix_bang}[{branch_jira_id}]\n")
 
         message_file.seek(0, 0)
         message_file.write("".join(message_lines))

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "mirageoscience.pre-commit-hooks"
 
-version = "1.0.0"
+version = "1.0.1"
 
 license = "MIT"
 description = ""
@@ -25,6 +25,7 @@ python = "^3.10"
 Pygments = "*"
 pylint = "*"
 pytest = "*"
+pytest-mock = "*"
 pytest-cov = "*"
 tomli = "*"
 

tests/__init__.py

@@ -0,0 +1,7 @@
+# ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+#  Copyright (c) 2024 Mira Geoscience Ltd.                                     '
+#                                                                              '
+#  This file is part of mirageoscience.pre-commit-hooks package.               '
+#                                                                              '
+#  All rights reserved.                                                        '
+# ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

tests/git_message_hook.py

@@ -0,0 +1,98 @@
+# ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+#  Copyright (c) 2024 Mira Geoscience Ltd.                                     '
+#                                                                              '
+#  This file is part of mirageoscience.pre-commit-hooks package.               '
+#                                                                              '
+#  All rights reserved.                                                        '
+# ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+from __future__ import annotations
+import pytest
+
+from mirageoscience.hooks.git_message_hook import *
+
+
+@pytest.fixture
+def mock_get_branch_name(mocker):
+    def _mock_get_branch_name(branch_name):
+        mocker.patch('mirageoscience.hooks.git_message_hook.get_branch_name',
+                     return_value=branch_name)
+        return get_jira_id(branch_name)
+    return _mock_get_branch_name
+
+
+def test_get_jira_id():
+    text = "[GEOPY-1233] Git commit message"
+    assert get_jira_id(text) == "GEOPY-1233"
+
+
+def test_get_message_prefix_bang_with_bang():
+    """Tests if get_message_prefix_bang can extract the prefix bang from a line."""
+    line = "fixup! This is a fix"
+    expected_prefix_bang = "fixup! "
+    actual_prefix_bang = get_message_prefix_bang(line)
+    assert actual_prefix_bang == expected_prefix_bang
+
+
+def test_get_message_prefix_bang_no_bang():
+    """Tests if get_message_prefix_bang returns empty string for a line without bang."""
+    line = "This is a commit message"
+    expected_prefix_bang = ""
+    actual_prefix_bang = get_message_prefix_bang(line)
+    assert actual_prefix_bang == expected_prefix_bang
+
+
+def test_check_commit_message_valid_with_message_jira(mock_get_branch_name):
+    """Test avec identifiant JIRA dans le message de commit"""
+    branch_name = "feature_branch"
+    mock_get_branch_name(branch_name)
+    message_content = "GEOPY-123 Fix a bug xx"
+    filepath = "test_commit_message.txt"
+    with open(filepath, "w") as f:
+        f.write(message_content)
+
+    is_valid, error_message = check_commit_message(filepath)
+    assert is_valid
+    assert error_message == ""
+
+
+def test_check_commit_message_invalid_no_jira(mock_get_branch_name):
+    """Test without JIRA id in the branch name or message content"""
+    branch_name = "feature_branch"
+    mock_get_branch_name(branch_name)
+    message_content = "Fix a bug"
+    filepath = "test_commit_message.txt"
+    with open(filepath, "w") as f:
+        f.write(message_content)
+
+    is_valid, error_message = check_commit_message(filepath)
+    assert not is_valid
+    assert error_message == "Either the branch name or the commit message must start with a JIRA ID."
+
+
+def test_check_commit_message_invalid_different_jira(mock_get_branch_name):
+    """Test with different JIRA id in the branch name and in the message content"""
+    branch_name = "GEOPY-123_fix_bug"
+    mock_get_branch_name(branch_name)
+    message_content = "GI-456 Fix a bug"
+    filepath = "test_commit_message.txt"
+    with open(filepath, "w") as f:
+        f.write(message_content)
+
+    is_valid, error_message = check_commit_message(filepath)
+    assert not is_valid
+    assert error_message.startswith("Different JIRA ID in commit message")
+
+
+def test_check_commit_message_invalid_short_message(mock_get_branch_name):
+    """Test with a too short message content"""
+    branch_name = "GEOPY-123_fix_bug"
+    mock_get_branch_name(branch_name)
+    message_content = "Fix"
+    filepath = "test_commit_message.txt"
+    with open(filepath, "w") as f:
+        f.write(message_content)
+
+    is_valid, error_message = check_commit_message(filepath)
+    assert not is_valid
+    assert error_message.startswith("First line of commit message must be at least")

tests/test_commit_message.txt

@@ -0,0 +1 @@
+Fix