str_to_bool issues, and merge_request missing issues (#21)

* fix: str_to_bool not working as expectedd

* test: test if other valid yaml boolean types work

---------

Co-authored-by: nm1 <[email protected]>

test/common/get_config_test.py

@@ -1,49 +1,125 @@
-# import unittest
-# from unittest.mock import mock_open, patch
-#
-# from valhalla.common.get_config import get_config
-#
-#
-# class GetConfigTest(unittest.TestCase):
-#
-#     def setUp(self):
-#         self.config_path = 'test_config.yml'
-#         self.mock_yml_content = """
-#         git_host: gitlab
-#         commit_before_release:
-#             enabled: True
-#             before:
-#               - echo "test"
-#               - echo "test2"
-#         """
-#
-#     @patch('builtins.open', new_callable=mock_open, read_data="""
-#         git_host: gitlab
-#         commit_before_release:
-#             enabled: True
-#             before:
-#               - echo "test"
-#               - echo "test2"
-#         """)
-#     def test_get_config(self, mock_open_file):
-#         config = get_config(self.config_path)
-#
-#         mock_open_file.assert_called_once_with(self.config_path)
-#
-#         self.assertEqual(config.git_host, 'gitlab')
-#         self.assertEqual(config.commit_before_release.enabled, True)
-#         self.assertEqual(config.commit_before_release.before_commands, ['echo "test"', 'echo "test2"'])
-#
-#         mock_open_file.assert_called_once_with(self.config_path)
-#
-#     @patch('builtins.open', side_effect=FileNotFoundError)
-#     def test_get_config_file_not_found(self, mock_open_file):
-#         with self.assertRaises(SystemExit) as context:
-#             get_config(self.config_path)
-#
-#         mock_open_file.assert_called_once_with(self.config_path)
-#         self.assertEqual(context.exception.code, -1)
-#
-#
-# if __name__ == '__main__':
-#     unittest.main()
+import unittest
+from unittest.mock import mock_open, patch
+
+from valhalla.common.get_config import get_config
+
+
+class GetConfigTest(unittest.TestCase):
+
+    def setUp(self):
+        self.config_path = "test_config.yml"
+        self.mock_yml_content = """
+        git_host: gitlab
+        commit_before_release:
+            enabled: True
+            before:
+              - echo "test"
+              - echo "test2"
+        """
+
+    @patch(
+        "builtins.open",
+        new_callable=mock_open,
+        read_data="""
+        git_host: gitlab
+        commit_before_release:
+            enabled: True
+            msg: test commit message
+            before:
+              - echo "test"
+              - echo "test2"
+        release:
+            description:
+                from_command: "cat changelog/v{VERSION}/version_summary.md"
+        merge_request:
+            enabled: On
+            title: test mr title
+        """,
+    )
+    def test_get_config(self, mock_open_file):
+        config = get_config(self.config_path)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+        self.assertEqual(config.git_host, "gitlab")
+        self.assertEqual(config.commit_before_release.enabled, True)
+        self.assertEqual(
+            config.commit_before_release.before_commands,
+            ['echo "test"', 'echo "test2"'],
+        )
+        self.assertEqual(config.merge_request.enabled, True)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+    @patch(
+        "builtins.open",
+        new_callable=mock_open,
+        read_data="""
+        git_host: gitlab
+        commit_before_release:
+            enabled: True
+            msg: test commit message
+            before:
+              - echo "test"
+              - echo "test2"
+        release:
+            description:
+                from_command: "cat changelog/v{VERSION}/version_summary.md"
+        """,
+    )
+    def test_get_config_no_mr_section(self, mock_open_file):
+        config = get_config(self.config_path)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+        self.assertEqual(config.merge_request.enabled, False)
+        self.assertEqual(config.merge_request.target_branch, None)
+        self.assertEqual(config.merge_request.title, None)
+        self.assertEqual(config.merge_request.description, None)
+        self.assertEqual(config.merge_request.reviewers, None)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+    @patch(
+        "builtins.open",
+        new_callable=mock_open,
+        read_data="""
+        git_host: gitlab
+        commit_before_release:
+            enabled: True
+            msg: test commit message
+            before:
+              - echo "test"
+              - echo "test2"
+        release:
+            description:
+                from_command: "cat changelog/v{VERSION}/version_summary.md"
+        merge_request:
+            enabled: False
+            title: test mr title
+        """,
+    )
+    def test_get_config_mr_disabled(self, mock_open_file):
+        config = get_config(self.config_path)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+        self.assertEqual(config.merge_request.enabled, False)
+        self.assertEqual(config.merge_request.target_branch, None)
+        self.assertEqual(config.merge_request.title, "test mr title")
+        self.assertEqual(config.merge_request.description, None)
+        self.assertEqual(config.merge_request.reviewers, None)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+
+    @patch("builtins.open", side_effect=FileNotFoundError)
+    def test_get_config_file_not_found(self, mock_open_file):
+        with self.assertRaises(SystemExit) as context:
+            get_config(self.config_path)
+
+        mock_open_file.assert_called_once_with(self.config_path)
+        self.assertEqual(context.exception.code, -1)
+
+
+if __name__ == "__main__":
+    unittest.main()

valhalla/common/get_config.py

@@ -168,7 +168,7 @@ def get_commit_part(commit_config_dict: dict) -> CommitConfig | None:
     if commit_config_dict is None:
         return None
 
-    enabled = str_to_bool(get_from_dict(commit_config_dict, 'enabled', True))
+    enabled = get_from_dict(commit_config_dict, 'enabled', True)
     commit_other_options_required = enabled
 
     git_username = get_from_dict(commit_config_dict, 'username', False)
@@ -223,7 +223,10 @@ def get_release_assets_links_config_part(links_list_of_dicts: List[dict]) -> Lis
 
 
 def get_merge_request_part(merge_request_dict: dict) -> MergeRequestConfig:
-    enabled = str_to_bool(get_from_dict(merge_request_dict, 'enabled', True))
+    if merge_request_dict is None:
+        return MergeRequestConfig(False, None, None, None, None)
+
+    enabled = get_from_dict(merge_request_dict, 'enabled', True)
     merge_request_other_options_required = enabled
 
     target_branch = get_from_dict(merge_request_dict, 'target_branch', False)
@@ -234,16 +237,6 @@ def get_merge_request_part(merge_request_dict: dict) -> MergeRequestConfig:
     reviewers = get_from_dict(merge_request_dict, 'reviewers', False)
     return MergeRequestConfig(enabled, target_branch, title, description, reviewers)
 
-
-def str_to_bool(value: str) -> bool:
-    if "True" or "true":
-        return True
-    if "False" or "false":
-        return False
-    warn("Could not parse boolean value for input: " + value + " using False instead")
-    return False
-
-
 def get_from_dict(d: dict, key: str, required: bool):
     try:
         return d[key]