diff --git a/test/common/logger_test.py b/test/common/logger_test.py index 41ef5f2..7762565 100644 --- a/test/common/logger_test.py +++ b/test/common/logger_test.py @@ -10,6 +10,7 @@ def tearDown(self): from valhalla.common import logger logger.MR_HOOK = None logger.MR_HOOK_COMMENTS_COUNT = 0 + logger.PENDING_MR_COMMENTS = [] @patch('builtins.print') def test_info(self, mock_print): @@ -153,6 +154,48 @@ def test_exit_after_50_comments(self, mock_exit, mock_print): # cleanup logger.MR_HOOK_COMMENTS_COUNT = 0 + @patch('builtins.print') + def test_pending_warn_and_error_flushed_when_hook_set_later(self, mock_print): + # given: errors logged before MR hook is initialized + warn("early warn") + error("early error") + + # when: MR hook is initialized later + mr_hook = MagicMock() + init_logger_mr_hook(mr_hook) + + # then: pending comments are flushed in order + mr_hook.add_comment.assert_any_call("[WARN] early warn") + mr_hook.add_comment.assert_any_call("[ERROR] early error") + self.assertEqual(mr_hook.add_comment.call_count, 2) + + @patch('builtins.print') + def test_pending_buffer_cleared_after_flush(self, mock_print): + # given: an early error and a hook set up + error("early error") + mr_hook = MagicMock() + init_logger_mr_hook(mr_hook) + + # when: a new hook is set up afterwards + mr_hook.reset_mock() + another_hook = MagicMock() + init_logger_mr_hook(another_hook) + + # then: previously flushed comments are not re-flushed + another_hook.add_comment.assert_not_called() + + @patch('builtins.print') + def test_info_not_buffered_before_hook(self, mock_print): + # given: an info log before MR hook is initialized + info("informational") + + # when: MR hook is initialized later + mr_hook = MagicMock() + init_logger_mr_hook(mr_hook) + + # then: info messages are never sent to MR + mr_hook.add_comment.assert_not_called() + @patch('builtins.print') def test_resolve_author_in_log(self, mock_print): # given: diff --git a/test/main_start_test.py b/test/main_start_test.py index dc24260..1df1b0f 100644 --- a/test/main_start_test.py +++ b/test/main_start_test.py @@ -53,3 +53,41 @@ def __init__(self, text): self.assertEqual(args[2], "1.2.3") # merge request config is default and disabled in our test YAML self.assertEqual(mock_create_mr.call_count, 1) + + def test_start_creates_mr_and_posts_error_when_version_empty(self): + # given: version is empty and from_config does not resolve it (e.g. command failed) + mock_vtr = MagicMock() + mock_vtr.is_version_empty.return_value = True + mock_vtr.get_config_file_path.return_value = "test/resources/valhalla.yml" + + provider_hook = MagicMock() + + def _mock_requests_get(url): + class R: + status_code = 200 + + def __init__(self, text): + self.text = text + + with open("test/resources/valhalla-extended.yml", "r") as f: + return R(f.read()) + + with patch('valhalla.main.__version_to_release', return_value=mock_vtr), \ + patch('valhalla.extends.valhalla_extends.requests.get', side_effect=_mock_requests_get), \ + patch('valhalla.main.get_valhalla_token', return_value='TOKEN'), \ + patch('os.environ.get', side_effect=fake_environ_get), \ + patch('valhalla.ci_provider.git_host.GitHost.get_author', return_value='AUTHOR'), \ + patch('valhalla.main.create_merge_request', return_value=provider_hook) as mock_create_mr: + from valhalla.main import start + + # when: running start should exit with error + with self.assertRaises(SystemExit): + start() + + # then: MR is created so the error is posted as a comment + self.assertEqual(mock_create_mr.call_count, 1) + # The "Version to release is empty" error is posted to the MR + posted = "\n".join( + call_args[0][0] for call_args in provider_hook.add_comment.call_args_list + ) + self.assertIn("Version to release is empty", posted) diff --git a/valhalla/common/logger.py b/valhalla/common/logger.py index b981877..e23845e 100644 --- a/valhalla/common/logger.py +++ b/valhalla/common/logger.py @@ -3,6 +3,7 @@ TOKEN: str = "not_set" MR_HOOK: MergeRequestHook | None = None MR_HOOK_COMMENTS_COUNT: int = 0 +PENDING_MR_COMMENTS: list = [] # Allows to hide sensitive data @@ -12,14 +13,26 @@ def init_logger(token: str): def init_logger_mr_hook(mr_hook: MergeRequestHook): - global MR_HOOK + global MR_HOOK, MR_HOOK_COMMENTS_COUNT, PENDING_MR_COMMENTS MR_HOOK = mr_hook + if MR_HOOK is None or not PENDING_MR_COMMENTS: + PENDING_MR_COMMENTS = [] + return + + pending = PENDING_MR_COMMENTS + PENDING_MR_COMMENTS = [] + for comment in pending: + if MR_HOOK_COMMENTS_COUNT >= 50: + break + MR_HOOK.add_comment(comment) + MR_HOOK_COMMENTS_COUNT += 1 + def log_message(level, msg): from valhalla.common import resolver import sys - global TOKEN, MR_HOOK, MR_HOOK_COMMENTS_COUNT + global TOKEN, MR_HOOK, MR_HOOK_COMMENTS_COUNT, PENDING_MR_COMMENTS msg = str(msg) msg = resolver.resolve(msg, suppress_log=True) msg = msg.replace(TOKEN, "*" * len(TOKEN)) @@ -34,14 +47,20 @@ def log_message(level, msg): else: formatted_msg += " \n" + line_to_print - if MR_HOOK is not None and (level == "WARN" or level == "ERROR"): - if MR_HOOK_COMMENTS_COUNT >= 50: - error_msg = f"[ERROR] Too many comments added to Merge Request (limit: 50). Please fix previous warnings." - MR_HOOK.add_comment(error_msg) - print(error_msg) - sys.exit(1) - MR_HOOK.add_comment(formatted_msg) - MR_HOOK_COMMENTS_COUNT += 1 + if level != "WARN" and level != "ERROR": + return + + if MR_HOOK is None: + PENDING_MR_COMMENTS.append(formatted_msg) + return + + if MR_HOOK_COMMENTS_COUNT >= 50: + error_msg = f"[ERROR] Too many comments added to Merge Request (limit: 50). Please fix previous warnings." + MR_HOOK.add_comment(error_msg) + print(error_msg) + sys.exit(1) + MR_HOOK.add_comment(formatted_msg) + MR_HOOK_COMMENTS_COUNT += 1 def info(msg): diff --git a/valhalla/main.py b/valhalla/main.py index 4b34046..f80d195 100644 --- a/valhalla/main.py +++ b/valhalla/main.py @@ -78,6 +78,12 @@ def start(): version_to_release.from_config(config) if version_to_release.is_version_empty(): + try: + mr_hook = create_merge_request(git_host, config.merge_request) + except Exception as e: + info(f"Could not create merge request for error reporting: {e}") + mr_hook = MergeRequestHook.Skip() + init_logger_mr_hook(mr_hook) error( f"Version to release is empty, exiting! Create branch with name {BASE_PREFIX}X.X.X or just {BASE_PREFIX}\n" f"and define in valhalla.yml version to release. You can also you VALHALLA_RELEASE_CMD to define it.\n"