diff --git a/gitlab/v4/objects/merge_requests.py b/gitlab/v4/objects/merge_requests.py index 9ff72f95a..3a878e257 100644 --- a/gitlab/v4/objects/merge_requests.py +++ b/gitlab/v4/objects/merge_requests.py @@ -342,7 +342,7 @@ def merge( if merge_when_pipeline_succeeds: data["merge_when_pipeline_succeeds"] = True - server_data = self.manager.gitlab.http_put(path, query_data=data, **kwargs) + server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs) self._update_attrs(server_data) diff --git a/tools/functional/api/test_merge_requests.py b/tools/functional/api/test_merge_requests.py index c5de5ebc5..e7682345e 100644 --- a/tools/functional/api/test_merge_requests.py +++ b/tools/functional/api/test_merge_requests.py @@ -1,6 +1,9 @@ +import time + import pytest import gitlab +import gitlab.v4.objects def test_merge_requests(project): @@ -95,3 +98,68 @@ def test_merge_request_merge(project): with pytest.raises(gitlab.GitlabMRClosedError): # Two merge attempts should raise GitlabMRClosedError mr.merge() + + +def test_merge_request_should_remove_source_branch( + project, merge_request, wait_for_sidekiq +) -> None: + """Test to ensure + https://github.com/python-gitlab/python-gitlab/issues/1120 is fixed. + Bug reported that they could not use 'should_remove_source_branch' in + mr.merge() call""" + + source_branch = "remove_source_branch" + mr = merge_request(source_branch=source_branch) + + mr.merge(should_remove_source_branch=True) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + # Wait until it is merged + mr_iid = mr.iid + for _ in range(60): + mr = project.mergerequests.get(mr_iid) + if mr.merged_at is not None: + break + time.sleep(0.5) + assert mr.merged_at is not None + time.sleep(0.5) + + # Ensure we can NOT get the MR branch + with pytest.raises(gitlab.exceptions.GitlabGetError): + project.branches.get(source_branch) + + +def test_merge_request_large_commit_message( + project, merge_request, wait_for_sidekiq +) -> None: + """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1452 + is fixed. + Bug reported that very long 'merge_commit_message' in mr.merge() would + cause an error: 414 Request too large + """ + + source_branch = "large_commit_message" + mr = merge_request(source_branch=source_branch) + + merge_commit_message = "large_message\r\n" * 1_000 + assert len(merge_commit_message) > 10_000 + + mr.merge(merge_commit_message=merge_commit_message) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + # Wait until it is merged + mr_iid = mr.iid + for _ in range(60): + mr = project.mergerequests.get(mr_iid) + if mr.merged_at is not None: + break + time.sleep(0.5) + assert mr.merged_at is not None + time.sleep(0.5) + + # Ensure we can get the MR branch + project.branches.get(source_branch) diff --git a/tools/functional/conftest.py b/tools/functional/conftest.py index 89b3dda12..634713d81 100644 --- a/tools/functional/conftest.py +++ b/tools/functional/conftest.py @@ -201,6 +201,77 @@ def project(gl): print(f"Project already deleted: {e}") +@pytest.fixture(scope="function") +def merge_request(project, wait_for_sidekiq): + """Fixture used to create a merge_request. + + It will create a branch, add a commit to the branch, and then create a + merge request against project.default_branch. The MR will be returned. + + When finished any created merge requests and branches will be deleted. + + NOTE: No attempt is made to restore project.default_branch to its previous + state. So if the merge request is merged then its content will be in the + project.default_branch branch. + """ + + to_delete = [] + + def _merge_request(*, source_branch: str): + # Wait for processes to be done before we start... + # NOTE(jlvillal): Sometimes the CI would give a "500 Internal Server + # Error". Hoping that waiting until all other processes are done will + # help with that. + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + project.refresh() # Gets us the current default branch + project.branches.create( + {"branch": source_branch, "ref": project.default_branch} + ) + # NOTE(jlvillal): Must create a commit in the new branch before we can + # create an MR that will work. + project.files.create( + { + "file_path": f"README.{source_branch}", + "branch": source_branch, + "content": "Initial content", + "commit_message": "New commit in new branch", + } + ) + mr = project.mergerequests.create( + { + "source_branch": source_branch, + "target_branch": project.default_branch, + "title": "Should remove source branch", + "remove_source_branch": True, + } + ) + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + mr_iid = mr.iid + for _ in range(60): + mr = project.mergerequests.get(mr_iid) + if mr.merge_status != "checking": + break + time.sleep(0.5) + assert mr.merge_status != "checking" + + to_delete.append((mr.iid, source_branch)) + return mr + + yield _merge_request + + for mr_iid, source_branch in to_delete: + project.mergerequests.delete(mr_iid) + try: + project.branches.delete(source_branch) + except gitlab.exceptions.GitlabDeleteError: + # Ignore if branch was already deleted + pass + + @pytest.fixture(scope="module") def project_file(project): """File fixture for tests requiring a project with files and branches."""