From 2ef00dcdd51abbcf4f9928d3a53ec5186b37d661 Mon Sep 17 00:00:00 2001 From: marwin1991 Date: Mon, 5 Oct 2026 12:32:38 +0200 Subject: [PATCH 1/2] add wait logic in enable_auto_merge to handle GitLab's MR async updates, update corresponding tests --- test/ci_provider/gitlab/merge_request_test.py | 18 +++++++++++++----- valhalla/ci_provider/gitlab/merge_request.py | 13 ++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/test/ci_provider/gitlab/merge_request_test.py b/test/ci_provider/gitlab/merge_request_test.py index fb7d92b..91aab9d 100644 --- a/test/ci_provider/gitlab/merge_request_test.py +++ b/test/ci_provider/gitlab/merge_request_test.py @@ -182,7 +182,9 @@ def test_add_comment_exception(self, mock_warn, mock_info, mock_resolve, mock_ge @patch("valhalla.ci_provider.gitlab.merge_request.resolve") @patch("valhalla.ci_provider.gitlab.merge_request.info") @patch("valhalla.ci_provider.gitlab.merge_request.warn") - def test_enable_auto_merge(self, mock_warn, mock_info, mock_resolve, mock_get_project_id, + @patch("valhalla.ci_provider.gitlab.merge_request.Repo") + @patch("valhalla.ci_provider.gitlab.merge_request.time.sleep") + def test_enable_auto_merge(self, mock_sleep, mock_repo, mock_warn, mock_info, mock_resolve, mock_get_project_id, mock_get_gitlab_client): with patch.dict('os.environ', {'CI_COMMIT_BRANCH': 'feature-branch'}): # given: @@ -196,8 +198,10 @@ def test_enable_auto_merge(self, mock_warn, mock_info, mock_resolve, mock_get_pr mock_gitlab_client.projects.get.return_value = mock_project mock_get_gitlab_client.return_value = mock_gitlab_client - mock_mr_obj = MagicMock() - mock_project.mergerequests.get.return_value = mock_mr_obj + mock_repo.return_value.head.commit.hexsha = "new-sha" + stale_mr_obj = MagicMock(sha="old-sha") + mock_mr_obj = MagicMock(sha="new-sha") + mock_project.mergerequests.get.side_effect = [stale_mr_obj, mock_mr_obj] merge_request = GitLabValhallaMergeRequest() config = MergeRequestConfig(enabled=True, target_branch="main", title="T", description="D", reviewers=[], @@ -208,7 +212,9 @@ def test_enable_auto_merge(self, mock_warn, mock_info, mock_resolve, mock_get_pr hook.enable_auto_merge() # then: - mock_mr_obj.merge.assert_called_once_with(merge_when_pipeline_succeeds=True) + mock_sleep.assert_called_once() + stale_mr_obj.merge.assert_not_called() + mock_mr_obj.merge.assert_called_once_with(sha="new-sha", merge_when_pipeline_succeeds=True) mock_info.assert_any_call("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed") @patch("valhalla.ci_provider.gitlab.merge_request.get_gitlab_client") @@ -216,7 +222,9 @@ def test_enable_auto_merge(self, mock_warn, mock_info, mock_resolve, mock_get_pr @patch("valhalla.ci_provider.gitlab.merge_request.resolve") @patch("valhalla.ci_provider.gitlab.merge_request.info") @patch("valhalla.ci_provider.gitlab.merge_request.warn") - def test_enable_auto_merge_exception(self, mock_warn, mock_info, mock_resolve, mock_get_project_id, + @patch("valhalla.ci_provider.gitlab.merge_request.Repo") + @patch("valhalla.ci_provider.gitlab.merge_request.time.sleep") + def test_enable_auto_merge_exception(self, mock_sleep, mock_repo, mock_warn, mock_info, mock_resolve, mock_get_project_id, mock_get_gitlab_client): with patch.dict('os.environ', {'CI_COMMIT_BRANCH': 'feature-branch'}): # given: diff --git a/valhalla/ci_provider/gitlab/merge_request.py b/valhalla/ci_provider/gitlab/merge_request.py index 2ffa605..564bc12 100644 --- a/valhalla/ci_provider/gitlab/merge_request.py +++ b/valhalla/ci_provider/gitlab/merge_request.py @@ -1,6 +1,9 @@ import os +import time from typing import List +from git import Repo + from valhalla.ci_provider.git_host import MergeRequest from valhalla.ci_provider.gitlab.common import get_gitlab_client, get_project_id from valhalla.common.get_config import MergeRequestConfig @@ -53,8 +56,16 @@ def _add_comment(comment: str): def _enable_auto_merge(): try: + head_sha = Repo('.').head.commit.hexsha mr_obj = self.project.mergerequests.get(mr_iid, iid=True) - mr_obj.merge(merge_when_pipeline_succeeds=True) + # GitLab refreshes the MR asynchronously after push; enabling auto-merge before that + # (or without sha) makes GitLab abort it with "source branch was updated" + for _ in range(30): + if mr_obj.sha == head_sha: + break + time.sleep(2) + mr_obj = self.project.mergerequests.get(mr_iid, iid=True) + mr_obj.merge(sha=head_sha, merge_when_pipeline_succeeds=True) info("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed") except Exception as e: warn(f"Could not enable auto-merge for merge request because: {e}") From 30b2fa9f6ca2c1740ecb2b4121cfcdb31b852098 Mon Sep 17 00:00:00 2001 From: marwin1991 Date: Mon, 5 Oct 2026 13:02:29 +0200 Subject: [PATCH 2/2] improve enable_auto_merge to respect detailed_merge_status, handle 405 errors, and enhance tests --- CLAUDE.md | 10 ++++++++++ test/ci_provider/gitlab/merge_request_test.py | 15 ++++++++++----- valhalla/ci_provider/gitlab/merge_request.py | 9 ++++++--- 3 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..7da7f1d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,10 @@ +# Valhalla + +## Testy +System Python nie ma zależności ani `pytest`, a `python3 -m venv` nie działa (brak `ensurepip`). Używaj `uv` (za proxy z własnym CA potrzebne `--system-certs`): + +``` +uv venv /venv +VIRTUAL_ENV=/venv uv pip install --system-certs -r requirements.txt +/venv/bin/python -m pytest -q test +``` diff --git a/test/ci_provider/gitlab/merge_request_test.py b/test/ci_provider/gitlab/merge_request_test.py index 91aab9d..760ebf7 100644 --- a/test/ci_provider/gitlab/merge_request_test.py +++ b/test/ci_provider/gitlab/merge_request_test.py @@ -199,9 +199,10 @@ def test_enable_auto_merge(self, mock_sleep, mock_repo, mock_warn, mock_info, mo mock_get_gitlab_client.return_value = mock_gitlab_client mock_repo.return_value.head.commit.hexsha = "new-sha" - stale_mr_obj = MagicMock(sha="old-sha") - mock_mr_obj = MagicMock(sha="new-sha") - mock_project.mergerequests.get.side_effect = [stale_mr_obj, mock_mr_obj] + stale_mr_obj = MagicMock(sha="old-sha", detailed_merge_status="mergeable") + checking_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="checking") + mock_mr_obj = MagicMock(sha="new-sha", detailed_merge_status="mergeable") + mock_project.mergerequests.get.side_effect = [stale_mr_obj, checking_mr_obj, mock_mr_obj] merge_request = GitLabValhallaMergeRequest() config = MergeRequestConfig(enabled=True, target_branch="main", title="T", description="D", reviewers=[], @@ -212,8 +213,9 @@ def test_enable_auto_merge(self, mock_sleep, mock_repo, mock_warn, mock_info, mo hook.enable_auto_merge() # then: - mock_sleep.assert_called_once() + self.assertEqual(mock_sleep.call_count, 2) stale_mr_obj.merge.assert_not_called() + checking_mr_obj.merge.assert_not_called() mock_mr_obj.merge.assert_called_once_with(sha="new-sha", merge_when_pipeline_succeeds=True) mock_info.assert_any_call("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed") @@ -234,6 +236,8 @@ def test_enable_auto_merge_exception(self, mock_sleep, mock_repo, mock_warn, moc mock_project = MagicMock() mock_gitlab_client.projects.get.return_value = mock_project mock_get_gitlab_client.return_value = mock_gitlab_client + mock_repo.return_value.head.commit.hexsha = "new-sha" + mock_project.mergerequests.get.return_value = MagicMock(sha="new-sha", detailed_merge_status="need_rebase") mock_project.mergerequests.get.return_value.merge.side_effect = Exception("405 Method Not Allowed") merge_request = GitLabValhallaMergeRequest() @@ -245,7 +249,8 @@ def test_enable_auto_merge_exception(self, mock_sleep, mock_repo, mock_warn, moc hook.enable_auto_merge() # then: - mock_warn.assert_any_call("Could not enable auto-merge for merge request because: 405 Method Not Allowed") + mock_warn.assert_any_call("Could not enable auto-merge for merge request because: 405 Method Not Allowed " + "(detailed_merge_status: need_rebase)") @patch("valhalla.ci_provider.gitlab.merge_request.get_gitlab_client") @patch("valhalla.ci_provider.gitlab.merge_request.get_project_id") diff --git a/valhalla/ci_provider/gitlab/merge_request.py b/valhalla/ci_provider/gitlab/merge_request.py index 564bc12..6b9ad7f 100644 --- a/valhalla/ci_provider/gitlab/merge_request.py +++ b/valhalla/ci_provider/gitlab/merge_request.py @@ -55,20 +55,23 @@ def _add_comment(comment: str): warn(f"Could not add comment to merge request because: {e}") def _enable_auto_merge(): + mr_obj = None try: head_sha = Repo('.').head.commit.hexsha mr_obj = self.project.mergerequests.get(mr_iid, iid=True) # GitLab refreshes the MR asynchronously after push; enabling auto-merge before that - # (or without sha) makes GitLab abort it with "source branch was updated" + # (or without sha) makes GitLab abort it with "source branch was updated" or reject it with 405 for _ in range(30): - if mr_obj.sha == head_sha: + if (mr_obj.sha == head_sha and + getattr(mr_obj, 'detailed_merge_status', None) not in ('checking', 'preparing', 'unchecked')): break time.sleep(2) mr_obj = self.project.mergerequests.get(mr_iid, iid=True) mr_obj.merge(sha=head_sha, merge_when_pipeline_succeeds=True) info("Auto-merge enabled, merge request will be merged when approvals and pipeline succeed") except Exception as e: - warn(f"Could not enable auto-merge for merge request because: {e}") + status = getattr(mr_obj, 'detailed_merge_status', 'unknown') + warn(f"Could not enable auto-merge for merge request because: {e} (detailed_merge_status: {status})") return MergeRequestHook(mr_iid, _add_comment, _enable_auto_merge if merge_request_config.auto_merge else None)