Skip to content

Commit

Permalink
fix: use POST method and return dict in `cancel_merge_when_pipeline_s…
Browse files Browse the repository at this point in the history
…ucceeds()` (#2350)

* Call was incorrectly using a `PUT` method when should have used a
    `POST` method.
  * Changed return type to a `dict` as GitLab only returns
    {'status': 'success'} on success. Since the function didn't work
    previously, this should not impact anyone.
  * Updated the test fixture `merge_request` to add ability to create
    a pipeline.
  * Added functional test for `mr.cancel_merge_when_pipeline_succeeds()`

Fixes: #2349
  • Loading branch information
JohnVillalovos committed Oct 31, 2022
1 parent 2974966 commit bd82d74
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 9 deletions.
8 changes: 7 additions & 1 deletion docs/gl_objects/merge_requests.rst
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,14 @@ Accept a MR::

mr.merge()

Cancel a MR when the build succeeds::
Schedule a MR to merge after the pipeline(s) succeed::

mr.merge(merge_when_pipeline_succeeds=True)

Cancel a MR from merging when the pipeline succeeds::

# Cancel a MR from being merged that had been previously set to
# 'merge_when_pipeline_succeeds=True'
mr.cancel_merge_when_pipeline_succeeds()

List commits of a MR::
Expand Down
15 changes: 8 additions & 7 deletions gitlab/v4/objects/merge_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ class ProjectMergeRequest(

@cli.register_custom_action("ProjectMergeRequest")
@exc.on_http_error(exc.GitlabMROnBuildSuccessError)
def cancel_merge_when_pipeline_succeeds(
self, **kwargs: Any
) -> "ProjectMergeRequest":
def cancel_merge_when_pipeline_succeeds(self, **kwargs: Any) -> Dict[str, str]:
"""Cancel merge when the pipeline succeeds.
Args:
Expand All @@ -179,17 +177,20 @@ def cancel_merge_when_pipeline_succeeds(
request
Returns:
ProjectMergeRequest
dict of the parsed json returned by the server
"""

path = (
f"{self.manager.path}/{self.encoded_id}/cancel_merge_when_pipeline_succeeds"
)
server_data = self.manager.gitlab.http_put(path, **kwargs)
server_data = self.manager.gitlab.http_post(path, **kwargs)
# 2022-10-30: The docs at
# https://docs.gitlab.com/ee/api/merge_requests.html#cancel-merge-when-pipeline-succeeds
# are incorrect in that the return value is actually just:
# {'status': 'success'} for a successful cancel.
if TYPE_CHECKING:
assert isinstance(server_data, dict)
self._update_attrs(server_data)
return ProjectMergeRequest(self.manager, server_data)
return server_data

@cli.register_custom_action("ProjectMergeRequest")
@exc.on_http_error(exc.GitlabListError)
Expand Down
16 changes: 16 additions & 0 deletions tests/functional/api/test_merge_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,27 @@ def test_merge_request_reset_approvals(gitlab_url, project, wait_for_sidekiq):
assert mr.reset_approvals()


def test_cancel_merge_when_pipeline_succeeds(project, merge_request, wait_for_sidekiq):
mr = merge_request(source_branch="test_merge_request_merge", create_pipeline=True)
# Set to merge when the pipeline succeeds, which should never happen
mr.merge(merge_when_pipeline_succeeds=True)
wait_for_sidekiq(timeout=60)

mr = project.mergerequests.get(mr.iid)
assert mr.merged_at is None
assert mr.merge_when_pipeline_succeeds is True
cancel = mr.cancel_merge_when_pipeline_succeeds()
assert cancel == {"status": "success"}


def test_merge_request_merge(project, merge_request, wait_for_sidekiq):
mr = merge_request(source_branch="test_merge_request_merge")
mr.merge()
wait_for_sidekiq(timeout=60)

mr = project.mergerequests.get(mr.iid)
assert mr.merged_at is not None
assert mr.merge_when_pipeline_succeeds is False
with pytest.raises(gitlab.GitlabMRClosedError):
# Two merge attempts should raise GitlabMRClosedError
mr.merge()
Expand Down
17 changes: 16 additions & 1 deletion tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ def merge_request(project, wait_for_sidekiq):

to_delete = []

def _merge_request(*, source_branch: str):
def _merge_request(*, source_branch: str, create_pipeline: bool = False):
# 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
Expand All @@ -401,6 +401,21 @@ def _merge_request(*, source_branch: str):
"commit_message": "New commit in new branch",
}
)
if create_pipeline:
project.files.create(
{
"file_path": ".gitlab-ci.yml",
"branch": source_branch,
"content": """
test:
rules:
- if: '$CI_PIPELINE_SOURCE == "merge_request_event"'
script:
- sleep 24h # We don't expect this to finish
""",
"commit_message": "Add a simple pipeline",
}
)
mr = project.mergerequests.create(
{
"source_branch": source_branch,
Expand Down

0 comments on commit bd82d74

Please sign in to comment.