Pass the correct value for merge_when_pipeline_succeeds

Passing this value as True makes gitlab assume that the MR must have
a pipeline, which means if it's doesn't, then the MR will be denied.
Instead we should use the value of project.only_merge_if_pipeline_succeeds,
As this will mean the gitlab logic will be correct:
   If the MR requires a pipeline to merge then we'll only merge if a pipeline
succeeds
   If the MR doesn't needs a pipeline to succeed (i.e. has this setting on the
project off) then we won't pass the parameter and hence the merge will be allowed

Fixes issue smarkets#240

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)
             ),
         ))

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

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)

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,32 @@ 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),
+        )
+        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