From 84f7a6fe43c2f7876185abb5157572dc93d9a867 Mon Sep 17 00:00:00 2001 From: Pushpit Kamboj Date: Sat, 9 May 2026 19:44:47 +0530 Subject: [PATCH 1/2] [fix] Change the imports for changelog to avoid circular import --- .../generate_changelog.py | 45 +++++++++++++++++-- .../test_generate_changelog.py | 19 ++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/.github/actions/bot-changelog-generator/generate_changelog.py b/.github/actions/bot-changelog-generator/generate_changelog.py index e59f2eee..a8c65bff 100644 --- a/.github/actions/bot-changelog-generator/generate_changelog.py +++ b/.github/actions/bot-changelog-generator/generate_changelog.py @@ -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 @@ -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]: diff --git a/.github/actions/bot-changelog-generator/test_generate_changelog.py b/.github/actions/bot-changelog-generator/test_generate_changelog.py index b804fbd9..5f97dd46 100644 --- a/.github/actions/bot-changelog-generator/test_generate_changelog.py +++ b/.github/actions/bot-changelog-generator/test_generate_changelog.py @@ -20,6 +20,7 @@ get_changelog_validation_errors, get_env_or_exit, get_linked_issues, + get_openwisp_commitizen, get_pr_commits, get_pr_details, get_pr_diff, @@ -527,6 +528,24 @@ 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", + ) + self.assertTrue(result.is_valid) + @patch("generate_changelog.get_openwisp_commitizen") def test_valid_feature_tag_rst(self, mock_get_commitizen): mock_plugin = MagicMock() From 7a233f0d8ef1f56777792f133b0e7fdd0f22e32c Mon Sep 17 00:00:00 2001 From: Pushpit Kamboj Date: Sat, 9 May 2026 22:53:31 +0530 Subject: [PATCH 2/2] [fix] Add more test cases --- .../test_generate_changelog.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/.github/actions/bot-changelog-generator/test_generate_changelog.py b/.github/actions/bot-changelog-generator/test_generate_changelog.py index 5f97dd46..37a7e049 100644 --- a/.github/actions/bot-changelog-generator/test_generate_changelog.py +++ b/.github/actions/bot-changelog-generator/test_generate_changelog.py @@ -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, @@ -546,6 +548,52 @@ def test_loads_commitizen_plugin_without_circular_import(self): ) 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()