diff --git a/marge/merge_request.py b/marge/merge_request.py index 6d87553a..bce1bbf8 100644 --- a/marge/merge_request.py +++ b/marge/merge_request.py @@ -158,12 +158,12 @@ def rebase(self): raise TimeoutError('Waiting for merge request to be rebased by GitLab') - def accept(self, remove_branch=False, sha=None): + def accept(self, remove_branch=False, sha=None, merge_when_pipeline_succeeds=True): return self._api.call(PUT( '/projects/{0.project_id}/merge_requests/{0.iid}/merge'.format(self), dict( should_remove_source_branch=remove_branch, - merge_when_pipeline_succeeds=True, + merge_when_pipeline_succeeds=merge_when_pipeline_succeeds, sha=sha or self.sha, # if provided, ensures what is merged is what we want (or fails) ), )) diff --git a/marge/single_merge_job.py b/marge/single_merge_job.py index c42d0a14..f4ed83ef 100644 --- a/marge/single_merge_job.py +++ b/marge/single_merge_job.py @@ -80,7 +80,11 @@ def update_merge_request_and_accept(self, approvals): self.ensure_mergeable_mr(merge_request) try: - merge_request.accept(remove_branch=merge_request.force_remove_source_branch, sha=actual_sha) + merge_request.accept( + remove_branch=merge_request.force_remove_source_branch, + sha=actual_sha, + merge_when_pipeline_succeeds=bool(target_project.only_allow_merge_if_pipeline_succeeds), + ) except gitlab.NotAcceptable as err: new_target_sha = Commit.last_on_branch(self._project.id, merge_request.target_branch, api).id # target_branch has moved under us since we updated, just try again diff --git a/tests/test_merge_request.py b/tests/test_merge_request.py index 19189c0d..8d36808f 100644 --- a/tests/test_merge_request.py +++ b/tests/test_merge_request.py @@ -150,7 +150,7 @@ def test_rebase_was_in_progress_no_error(self): self.merge_request.rebase() self.api.call.assert_has_calls([call(req) for (req, resp) in expected]) - def test_accept(self): + def test_accept_remove_branch(self): self._load(dict(INFO, sha='badc0de')) for boolean in (True, False): @@ -165,6 +165,8 @@ def test_accept(self): )) self.api.call.reset_mock() + def test_accept_sha(self): + self._load(dict(INFO, sha='badc0de')) self.merge_request.accept(sha='g00dc0de') self.api.call.assert_called_once_with(PUT( '/projects/1234/merge_requests/54/merge', @@ -175,6 +177,18 @@ def test_accept(self): ) )) + def test_accept_merge_when_pipeline_succeeds(self): + self._load(dict(INFO, sha='badc0de')) + self.merge_request.accept(merge_when_pipeline_succeeds=False) + self.api.call.assert_called_once_with(PUT( + '/projects/1234/merge_requests/54/merge', + dict( + merge_when_pipeline_succeeds=False, + should_remove_source_branch=False, + sha='badc0de', + ) + )) + def test_fetch_all_opened_for_me(self): api = self.api mr1, mr_not_me, mr2 = INFO, dict(INFO, assignees=[{'id': _MARGE_ID+1}], id=679), dict(INFO, id=678) diff --git a/tests/test_single_job.py b/tests/test_single_job.py index d632c904..0a40d5c6 100644 --- a/tests/test_single_job.py +++ b/tests/test_single_job.py @@ -20,6 +20,7 @@ from marge.merge_request import MergeRequest from tests.git_repo_mock import RepoMock from tests.gitlab_api_mock import Error, Ok, MockLab +from tests.test_project import INFO as TEST_PROJECT_INFO import tests.test_commit as test_commit @@ -585,6 +586,33 @@ def test_handles_races_for_merging(self, mocks): assert api.state == 'someone_else_merged' assert api.notes == [] + @pytest.mark.parametrize("only_allow_merge_if_pipeline_succeeds", [True, False]) + def test_calculates_merge_when_pipeline_succeeds_correctly( + self, mocks, only_allow_merge_if_pipeline_succeeds + ): + mocklab, api, job = mocks + rewritten_sha = mocklab.rewritten_sha + project_info = dict(TEST_PROJECT_INFO) + project_info["only_allow_merge_if_pipeline_succeeds"] = only_allow_merge_if_pipeline_succeeds + api.add_project(project_info) + api.add_transition( + PUT( + '/projects/{pid}/merge_requests/{iid}/merge'.format( + iid=mocklab.merge_request_info['iid'], + pid=project_info["id"] + ), + dict( + sha=rewritten_sha, + should_remove_source_branch=True, + merge_when_pipeline_succeeds=only_allow_merge_if_pipeline_succeeds + ), + ), + Ok(True), + to_state='merged', + ) + job.execute() + assert api.state == 'merged' + def test_handles_request_becoming_wip_after_push(self, mocks): mocklab, api, job = mocks rewritten_sha = mocklab.rewritten_sha