Fix guarantee final pipeline

README.md

@@ -141,6 +141,9 @@ optional arguments:
                         Use merge commit when creating batches, so that the commits in the batch MR will be the same with in individual MRs. Requires sudo scope in the access token.
                            [env var: MARGE_USE_MERGE_COMMIT_BATCHES] (default: False)
   --skip-ci-batches     Skip CI when updating individual MRs when using batches   [env var: MARGE_SKIP_CI_BATCHES] (default: False)
+  --guarantee-final-pipeline
+                        Guaranteed final pipeline when assigned to marge-bot
+                        [env var: MARGE_GUARANTEE_FINAL_PIPELINE] (default: False)
 ```
 Here is a config file example
 ```yaml

marge/merge_request.py

@@ -235,6 +235,27 @@ def fetch_approvals(self):
     def fetch_commits(self):
         return self._api.call(GET('/projects/{0.project_id}/merge_requests/{0.iid}/commits'.format(self)))
 
+    def trigger_pipeline(self):
+        """
+        Triggers a pipeline for the merge request.
+
+        At first, try to trigger a merge request pipeline, which is different
+        from a normal Gitlab CI pipeline and should be configured[0].
+        If this fails due to unavailable merge request job definitions, trigger
+        a normal pipeline for the source branch.
+
+        [0]: https://docs.gitlab.com/ee/ci/pipelines/merge_request_pipelines.html
+        """
+        try:
+            return self._api.call(POST(
+                '/projects/{0.project_id}/merge_requests/{0.iid}/pipelines'.format(self)))
+        except gitlab.BadRequest as exc:
+            if 'No stages / jobs for this pipeline.' in exc.error_message.get('base', []):
+                log.info('The pipeline is not configured for MR jobs, triggering a usual pipeline.')
+                return self._api.call(POST(
+                    '/projects/{0.project_id}/pipeline?ref={0.source_branch}'.format(self)))
+            raise
+
 
 class MergeRequestRebaseFailed(Exception):
     pass

marge/single_merge_job.py

@@ -64,8 +64,9 @@ def update_merge_request_and_accept(self, approvals):
 
             if _updated_sha == actual_sha and self._options.guarantee_final_pipeline:
                 log.info('No commits on target branch to fuse, triggering pipeline...')
-                merge_request.comment("jenkins retry")
-                time.sleep(30)
+                pipeline_info = merge_request.trigger_pipeline()
+                log.info('Pipeline %s is triggered, waiting for it to finish...', pipeline_info.get('id'))
+                self.wait_for_ci_to_pass(merge_request, actual_sha)
 
             log.info(
                 'Commit id to merge %r into: %r (updated sha: %r)',

pylintrc

@@ -30,7 +30,7 @@ max-line-length=110
 [DESIGN]
 max-args=10
 max-attributes=15
-max-public-methods=35
+max-public-methods=36
 # Maximum number of locals for function / method body
 max-locals=25
 

tests/test_merge_request.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from marge.gitlab import Api, GET, POST, PUT, Version
+from marge.gitlab import Api, BadRequest, GET, POST, PUT, Version
 from marge.merge_request import MergeRequest, MergeRequestRebaseFailed
 import marge.user
 
@@ -228,6 +228,63 @@ def test_fetch_assigned_at(self):
         ))
         assert result == 1597733578.093
 
+    def test_trigger_pipeline_succeeds(self):
+        api = self.api
+        expected_result = object()
+
+        def side_effect(request):
+            if request.endpoint == '/projects/1234/merge_requests/54/pipelines':
+                return expected_result
+            return None
+
+        api.call = Mock(side_effect=side_effect)
+
+        result = self.merge_request.trigger_pipeline()
+
+        assert api.call.call_args_list == [
+            call(POST('/projects/1234/merge_requests/54/pipelines')),
+        ]
+
+        assert result == expected_result
+
+    def test_trigger_pipeline_fallback_succeeds(self):
+        api = self.api
+        expected_result = object()
+
+        def side_effect(request):
+            if request.endpoint == '/projects/1234/merge_requests/54/pipelines':
+                raise BadRequest(400, {'message': {'base': 'No stages / jobs for this pipeline.'}})
+            if request.endpoint == '/projects/1234/pipeline?ref=useless_new_feature':
+                return expected_result
+            return None
+
+        api.call = Mock(side_effect=side_effect)
+
+        result = self.merge_request.trigger_pipeline()
+
+        assert api.call.call_args_list == [
+            call(POST('/projects/1234/merge_requests/54/pipelines')),
+            call(POST('/projects/1234/pipeline?ref=useless_new_feature')),
+        ]
+
+        assert result == expected_result
+
+    def test_trigger_pipeline_fallback_fails(self):
+        api = self.api
+
+        def side_effect(request):
+            if request.endpoint == '/projects/1234/merge_requests/54/pipelines':
+                raise BadRequest(500, {'message': {'base': 'Another error.'}})
+
+        api.call = Mock(side_effect=side_effect)
+
+        with pytest.raises(BadRequest):
+            self.merge_request.trigger_pipeline()
+
+        assert api.call.call_args_list == [
+            call(POST('/projects/1234/merge_requests/54/pipelines')),
+        ]
+
     def _load(self, json):
         old_mock = self.api.call
         self.api.call = Mock(return_value=json)

tests/test_single_job.py

@@ -15,7 +15,7 @@
 import marge.project
 import marge.single_merge_job
 import marge.user
-from marge.gitlab import GET, PUT
+from marge.gitlab import GET, POST, PUT
 from marge.job import Fusion
 from marge.merge_request import MergeRequest
 from tests.git_repo_mock import RepoMock
@@ -64,6 +64,7 @@ def __init__(
         fork=False,
         expect_gitlab_rebase=False,
         merge_request_options=None,
+        guarantee_final_pipeline=False,
     ):
         super().__init__(
             initial_master_sha,
@@ -100,6 +101,24 @@ def __init__(
                to_state='pushed',
             )
 
+        if guarantee_final_pipeline:
+            # Corresponds to the `merge_request.trigger_pipeline()` call.
+            api.add_transition(
+                POST(
+                    '/projects/1234/merge_requests/{}/pipelines'.format(
+                        self.merge_request_info['iid'],
+                    ),
+                ),
+                Ok({}),
+                to_state='final_pipeline_triggered',
+            )
+            # Corresponds to `pipelines_by_branch()` by `wait_for_ci_to_pass`.
+            api.add_pipelines(
+                self.merge_request_info['source_project_id'],
+                _pipeline(sha1=rewritten_sha, status='success', ref=self.merge_request_info['source_branch']),
+                from_state=['final_pipeline_triggered'], to_state='pushed'
+            )
+
         api.add_pipelines(
             self.merge_request_info['source_project_id'],
             _pipeline(sha1=rewritten_sha, status='running', ref=self.merge_request_info['source_branch']),
@@ -208,14 +227,19 @@ def add_part_of(self, request):
     def add_reviewers(self, request):
         return request.param
 
+    @pytest.fixture(params=[True, False])
+    def guarantee_final_pipeline(self, request):
+        return request.param
+
     @pytest.fixture()
-    def options_factory(self, fusion, add_tested, add_reviewers, add_part_of):
+    def options_factory(self, fusion, add_tested, add_reviewers, add_part_of, guarantee_final_pipeline):
         def make_options(**kwargs):
             fixture_opts = {
                 'fusion': fusion,
                 'add_tested': add_tested,
                 'add_part_of': add_part_of,
                 'add_reviewers': add_reviewers,
+                'guarantee_final_pipeline': guarantee_final_pipeline,
             }
             assert not set(fixture_opts).intersection(kwargs)
             kwargs.update(fixture_opts)
@@ -255,9 +279,14 @@ def patch_sleep(self):
             yield
 
     @pytest.fixture()
-    def mocklab_factory(self, fork, fusion):
+    def mocklab_factory(self, fork, fusion, guarantee_final_pipeline):
         expect_rebase = fusion is Fusion.gitlab_rebase
-        return partial(SingleJobMockLab, fork=fork, expect_gitlab_rebase=expect_rebase)
+        return partial(
+            SingleJobMockLab,
+            fork=fork,
+            expect_gitlab_rebase=expect_rebase,
+            guarantee_final_pipeline=guarantee_final_pipeline,
+        )
 
     @pytest.fixture()
     def mocks_factory(self, mocklab_factory, options_factory, update_sha, rewrite_sha):
@@ -352,6 +381,16 @@ def test_succeeds_if_skipped(self, mocks):
             _pipeline(sha1=mocklab.rewritten_sha, status='skipped'),
             from_state=['skipped', 'merged'],
         )
+        # `pipelines_by_branch()` by `wait_for_ci_to_pass`.
+        api.add_pipelines(
+            mocklab.merge_request_info['source_project_id'],
+            _pipeline(
+                sha1=mocklab.rewritten_sha,
+                status='skipped',
+                ref=mocklab.merge_request_info['source_branch'],
+            ),
+            from_state=['final_pipeline_triggered'], to_state='passed'
+        )
         job.execute()
 
         assert api.state == 'merged'
@@ -390,6 +429,15 @@ def test_fails_if_ci_fails(self, mocks):
             _pipeline(sha1=mocklab.rewritten_sha, status='failed'),
             from_state=['failed'],
         )
+        api.add_pipelines(
+            mocklab.merge_request_info['source_project_id'],
+            _pipeline(
+                sha1=mocklab.rewritten_sha,
+                status='failed',
+                ref=mocklab.merge_request_info['source_branch'],
+            ),
+            from_state=['final_pipeline_triggered'], to_state='failed',
+        )
 
         with mocklab.expected_failure("CI failed!"):
             job.execute()
@@ -408,6 +456,15 @@ def test_fails_if_ci_canceled(self, mocks):
             _pipeline(sha1=mocklab.rewritten_sha, status='canceled'),
             from_state=['canceled'],
         )
+        api.add_pipelines(
+            mocklab.merge_request_info['source_project_id'],
+            _pipeline(
+                sha1=mocklab.rewritten_sha,
+                status='canceled',
+                ref=mocklab.merge_request_info['source_branch'],
+            ),
+            from_state=['final_pipeline_triggered'], to_state='canceled',
+        )
 
         with mocklab.expected_failure("Someone canceled the CI."):
             job.execute()
@@ -426,6 +483,16 @@ def test_fails_on_not_acceptable_if_master_did_not_move(self, mocks):
             Ok({'commit': _commit(commit_id=new_branch_head_sha, status='success')}),
             from_state='pushed', to_state='pushed_but_head_changed'
         )
+        api.add_pipelines(
+            mocklab.merge_request_info['source_project_id'],
+            _pipeline(
+                sha1=new_branch_head_sha,
+                status='success',
+                ref=mocklab.merge_request_info['source_branch'],
+            ),
+            from_state=['pushed_but_head_changed']
+        )
+
         with mocklab.expected_failure("Someone pushed to branch while we were trying to merge"):
             job.execute()
 
@@ -541,6 +608,27 @@ def push_effects(remote_url, remote_branch, old_sha, new_sha):
             Ok({'commit': _commit(commit_id=moved_master_sha, status='success')}),
             from_state='merge_rejected'
         )
+        # Overwrite original `guarantee_final_pipeline` transition: no need in
+        # the state changing here.
+        api.add_transition(
+            POST(
+                '/projects/1234/merge_requests/{}/pipelines'.format(
+                    mocklab.merge_request_info['iid'],
+                ),
+            ),
+            Ok({}),
+        )
+        # Register additional pipeline check introduced by
+        # `guarantee_final_pipeline`.
+        api.add_pipelines(
+            mocklab.merge_request_info['source_project_id'],
+            _pipeline(
+                sha1=first_rewritten_sha,
+                status='success',
+                ref=mocklab.merge_request_info['source_branch']
+            ),
+            from_state=['pushed_but_master_moved', 'merge_rejected'],
+        )
         if fusion is Fusion.gitlab_rebase:
             rebase_url = '/projects/{project_id}/merge_requests/{iid}/rebase'.format(
                 project_id=mocklab.merge_request_info['project_id'],