Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions .github/actions/bot-changelog-generator/generate_changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
GEMINI_MODEL: Model to use (default: 'gemini-2.5-flash-lite')
"""

import importlib.util
import os
import re
import runpy
import secrets
import subprocess
import sys
from html import escape
from types import ModuleType

from google import genai
from google.genai import types
Expand Down Expand Up @@ -399,10 +402,46 @@ def build_prompt(


def get_openwisp_commitizen():
"""Load the local OpenWISP Commitizen plugin lazily."""
from openwisp_utils.releaser.commitizen import OpenWispCommitizen
"""Load the plugin without triggering Commitizen plugin auto-discovery."""
spec = importlib.util.find_spec("openwisp_utils.releaser.commitizen")
if spec is None or not spec.origin:
raise ImportError("Could not locate openwisp_utils.releaser.commitizen")

class _BaseCommitizen:
def __init__(self, config):
self.config = config

class _ValidationResult:
def __init__(self, is_valid: bool, errors: list[str] | None = None):
self.is_valid = is_valid
self.errors = errors or []

fake_commitizen = ModuleType("commitizen")
fake_commitizen_cz = ModuleType("commitizen.cz")
fake_commitizen_base = ModuleType("commitizen.cz.base")
fake_commitizen_base.BaseCommitizen = _BaseCommitizen
fake_commitizen_base.ValidationResult = _ValidationResult
fake_commitizen.cz = fake_commitizen_cz
fake_commitizen_cz.base = fake_commitizen_base

previous_modules = {
name: sys.modules.get(name)
for name in ("commitizen", "commitizen.cz", "commitizen.cz.base")
}

return OpenWispCommitizen(_CommitizenConfig())
try:
sys.modules["commitizen"] = fake_commitizen
sys.modules["commitizen.cz"] = fake_commitizen_cz
sys.modules["commitizen.cz.base"] = fake_commitizen_base
plugin_class = runpy.run_path(spec.origin)["OpenWispCommitizen"]
finally:
for name, previous in previous_modules.items():
if previous is None:
sys.modules.pop(name, None)
else:
sys.modules[name] = previous

return plugin_class(_CommitizenConfig())


def get_commit_message_validation_errors(text: str) -> list[str]:
Expand Down
67 changes: 67 additions & 0 deletions .github/actions/bot-changelog-generator/test_generate_changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

import os
import sys
from types import SimpleNamespace

# Add the directory to path for importing (must be before local imports)
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
import unittest # noqa: E402
from unittest.mock import MagicMock, patch # noqa: E402

import generate_changelog # noqa: E402
from generate_changelog import ( # noqa: E402
CHANGELOG_BOT_MARKER,
CHANGELOG_COMMENT_INTRO,
Expand All @@ -20,6 +22,7 @@
get_changelog_validation_errors,
get_env_or_exit,
get_linked_issues,
get_openwisp_commitizen,
get_pr_commits,
get_pr_details,
get_pr_diff,
Expand Down Expand Up @@ -527,6 +530,70 @@ def test_raises_on_request_error(self, mock_request):
class TestValidateChangelogOutput(unittest.TestCase):
"""Tests for validate_changelog_output function."""

def test_loads_commitizen_plugin_without_circular_import(self):
plugin = get_openwisp_commitizen()
self.assertEqual(plugin.__class__.__name__, "OpenWispCommitizen")

result = plugin.validate_commit_message(
commit_msg=(
"[feature] Added new functionality #123\n\n"
"Adds the new behavior with a user-focused summary.\n\n"
"Closes #123"
),
pattern=MagicMock(),
allow_abort=False,
allowed_prefixes=[],
max_msg_length=COMMIT_SUBJECT_LIMIT,
commit_hash="TEST",
)
Comment on lines +537 to +548

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use the real schema pattern in this regression test.

Line 541 passes MagicMock() as pattern, which can make regex checks trivially succeed and leaves the production path untested. Compile plugin.schema_pattern() here so this actually verifies the validation flow the bot uses.

Suggested change
+import re
...
         result = plugin.validate_commit_message(
             commit_msg=(
                 "[feature] Added new functionality `#123`\n\n"
                 "Adds the new behavior with a user-focused summary.\n\n"
                 "Closes `#123`"
             ),
-            pattern=MagicMock(),
+            pattern=re.compile(plugin.schema_pattern()),
             allow_abort=False,
             allowed_prefixes=[],
             max_msg_length=COMMIT_SUBJECT_LIMIT,
             commit_hash="TEST",
         )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
result = plugin.validate_commit_message(
commit_msg=(
"[feature] Added new functionality #123\n\n"
"Adds the new behavior with a user-focused summary.\n\n"
"Closes #123"
),
pattern=MagicMock(),
allow_abort=False,
allowed_prefixes=[],
max_msg_length=COMMIT_SUBJECT_LIMIT,
commit_hash="TEST",
)
result = plugin.validate_commit_message(
commit_msg=(
"[feature] Added new functionality `#123`\n\n"
"Adds the new behavior with a user-focused summary.\n\n"
"Closes `#123`"
),
pattern=re.compile(plugin.schema_pattern()),
allow_abort=False,
allowed_prefixes=[],
max_msg_length=COMMIT_SUBJECT_LIMIT,
commit_hash="TEST",
)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/actions/bot-changelog-generator/test_generate_changelog.py around
lines 535 - 546, The test uses MagicMock() for the pattern which bypasses real
regex validation; replace the MagicMock with the actual compiled schema by
calling plugin.schema_pattern() (or plugin.schema_pattern().compile() if
schema_pattern returns a string) and pass that to plugin.validate_commit_message
in the test so the real validation path is exercised; update the test assertion
if needed to reflect true success/failure using the produced pattern from
plugin.schema_pattern().

self.assertTrue(result.is_valid)

@patch("generate_changelog.importlib.util.find_spec", return_value=None)
def test_raises_if_commitizen_plugin_module_cannot_be_located(self, mock_find_spec):
with self.assertRaises(ImportError) as context:
get_openwisp_commitizen()

self.assertIn(
"Could not locate openwisp_utils.releaser.commitizen",
str(context.exception),
)
mock_find_spec.assert_called_once_with("openwisp_utils.releaser.commitizen")

@patch(
"generate_changelog.importlib.util.find_spec",
return_value=SimpleNamespace(origin="/tmp/fake_commitizen.py"),
)
@patch("generate_changelog.runpy.run_path", side_effect=RuntimeError("boom"))
def test_restores_sys_modules_if_plugin_loading_fails(
self, mock_run_path, mock_find_spec
):
existing_commitizen = object()
existing_commitizen_cz = object()

with patch.dict(
generate_changelog.sys.modules,
{
"commitizen": existing_commitizen,
"commitizen.cz": existing_commitizen_cz,
},
clear=False,
):
with self.assertRaises(RuntimeError) as context:
get_openwisp_commitizen()

self.assertEqual(str(context.exception), "boom")
self.assertIs(
generate_changelog.sys.modules["commitizen"], existing_commitizen
)
self.assertIs(
generate_changelog.sys.modules["commitizen.cz"],
existing_commitizen_cz,
)
self.assertNotIn("commitizen.cz.base", generate_changelog.sys.modules)

mock_find_spec.assert_called_once_with("openwisp_utils.releaser.commitizen")
mock_run_path.assert_called_once_with("/tmp/fake_commitizen.py")

@patch("generate_changelog.get_openwisp_commitizen")
def test_valid_feature_tag_rst(self, mock_get_commitizen):
mock_plugin = MagicMock()
Expand Down
Loading