diff --git a/marge/branch.py b/marge/branch.py new file mode 100644 index 00000000..f0ef33ea --- /dev/null +++ b/marge/branch.py @@ -0,0 +1,25 @@ +from . import gitlab + + +GET = gitlab.GET + + +class Branch(gitlab.Resource): + + @classmethod + def fetch_by_name(cls, project_id, branch, api): + info = api.call(GET( + '/projects/{project_id}/repository/branches/{branch}'.format( + project_id=project_id, + branch=branch, + ), + )) + return cls(api, info) + + @property + def name(self): + return self.info['name'] + + @property + def protected(self): + return self.info['protected'] diff --git a/marge/job.py b/marge/job.py index 3b3e46dc..937151be 100644 --- a/marge/job.py +++ b/marge/job.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta from . import git +from .branch import Branch from .interval import IntervalUnion from .project import Project from .user import User @@ -145,6 +146,10 @@ def wait_for_ci_to_pass(self, merge_request, commit_sha=None): log.info('CI for MR !%s passed', merge_request.iid) return + if ci_status == 'skipped': + log.info('CI for MR !%s skipped', merge_request.iid) + return + if ci_status == 'failed': raise CannotMerge('CI failed!') @@ -196,7 +201,7 @@ def fetch_source_project(self, merge_request): remote = 'source' remote_url = source_project.ssh_url_to_repo self._repo.fetch( - remote=remote, + remote_name=remote, remote_url=remote_url, ) return source_project, remote_url, remote @@ -254,7 +259,7 @@ def update_from_target_branch_and_push( if updated_sha == target_sha: raise CannotMerge('these changes already exist in branch `{}`'.format(target_branch)) rewritten_sha = self.add_trailers(merge_request) or updated_sha - branch_rewritten = True + branch_rewritten = rewritten_sha != updated_sha repo.push(source_branch, source_repo_url=source_repo_url, force=True) changes_pushed = True except git.GitError: @@ -263,6 +268,12 @@ def update_from_target_branch_and_push( if not branch_rewritten: raise CannotMerge('failed on filter-branch; check my logs!') if not changes_pushed: + if ( + branch_rewritten and Branch.fetch_by_name( + merge_request.source_project_id, merge_request.source_branch, self._api, + ).protected + ): + raise CannotMerge('Sorry, I can\'t push rewritten changes to protected branches!') if self.opts.use_merge_strategy: raise CannotMerge('failed to push merged changes, check my logs!') else: @@ -278,8 +289,6 @@ def update_from_target_branch_and_push( if source_branch != 'master': repo.checkout_branch('master') repo.remove_branch(source_branch) - else: - assert source_repo_url is not None def _get_reviewer_names_and_emails(approvals, api): diff --git a/tests/gitlab_api_mock.py b/tests/gitlab_api_mock.py index 88ab6fb8..4d410645 100644 --- a/tests/gitlab_api_mock.py +++ b/tests/gitlab_api_mock.py @@ -24,7 +24,7 @@ def commit(commit_id, status): class MockLab(object): # pylint: disable=too-few-public-methods - def __init__(self, gitlab_url=None): + def __init__(self, gitlab_url=None, fork=False, merge_request_options=None): self.gitlab_url = gitlab_url = gitlab_url or 'http://git.example.com' self.api = api = Api(gitlab_url=gitlab_url, auth_token='no-token', initial_state='initial') @@ -35,7 +35,9 @@ def __init__(self, gitlab_url=None): api.add_user(self.user_info, is_current=True) self.project_info = dict(test_project.INFO) + self.forked_project_info = {**self.project_info, **{'id': 4321}} api.add_project(self.project_info) + api.add_project(self.forked_project_info) self.commit_info = dict(test_commit.INFO) api.add_commit(self.project_info['id'], self.commit_info) @@ -58,6 +60,10 @@ def __init__(self, gitlab_url=None): 'work_in_progress': False, 'web_url': 'http://git.example.com/group/project/merge_request/666', } + if merge_request_options is not None: + self.merge_request_info.update(merge_request_options) + if fork: + self.merge_request_info.update({'iid': 55, 'source_project_id': '4321'}) api.add_merge_request(self.merge_request_info) self.initial_master_sha = '505e' @@ -70,7 +76,11 @@ def __init__(self, gitlab_url=None): ) api.add_approvals(self.approvals_info) api.add_transition( - GET('/projects/1234/repository/branches/master'), + GET( + '/projects/1234/repository/branches/{target}'.format( + target=self.merge_request_info['target_branch'], + ), + ), Ok({'commit': {'id': self.initial_master_sha}}), ) diff --git a/tests/test_batch_job.py b/tests/test_batch_job.py index 21270bf2..1971e8a4 100644 --- a/tests/test_batch_job.py +++ b/tests/test_batch_job.py @@ -1,5 +1,5 @@ # pylint: disable=protected-access -from unittest.mock import ANY, Mock, patch +from unittest.mock import ANY, patch, create_autospec import pytest @@ -13,15 +13,23 @@ from tests.gitlab_api_mock import MockLab, Ok, commit -# pylint: disable=attribute-defined-outside-init class TestBatchJob(object): - def setup_method(self, _method): - self.mocklab = MockLab() - self.api = self.mocklab.api + @pytest.fixture(params=[True, False]) + def fork(self, request): + return request.param - def get_batch_merge_job(self, **batch_merge_kwargs): - api, mocklab = self.api, self.mocklab + @pytest.fixture() + def mocklab(self, fork): + return MockLab(fork=fork) + @pytest.fixture() + def api(self, mocklab): + return mocklab.api + + def _mock_merge_request(self, **options): + return create_autospec(marge.merge_request.MergeRequest, spec_set=True, **options) + + def get_batch_merge_job(self, api, mocklab, **batch_merge_kwargs): project_id = mocklab.project_info['id'] merge_request_iid = mocklab.merge_request_info['iid'] @@ -29,29 +37,29 @@ def get_batch_merge_job(self, **batch_merge_kwargs): params = { 'api': api, - 'user': marge.user.User.myself(self.api), + 'user': marge.user.User.myself(api), 'project': marge.project.Project.fetch_by_id(project_id, api), - 'repo': Mock(marge.git.Repo), + 'repo': create_autospec(marge.git.Repo, spec_set=True), 'options': MergeJobOptions.default(), 'merge_requests': [merge_request] } params.update(batch_merge_kwargs) return BatchMergeJob(**params) - def test_remove_batch_branch(self): - repo = Mock() - batch_merge_job = self.get_batch_merge_job(repo=repo) + def test_remove_batch_branch(self, api, mocklab): + repo = create_autospec(marge.git.Repo, spec_set=True) + batch_merge_job = self.get_batch_merge_job(api, mocklab, repo=repo) batch_merge_job.remove_batch_branch() repo.remove_branch.assert_called_once_with( BatchMergeJob.BATCH_BRANCH_NAME, ) - def test_close_batch_mr(self): + def test_close_batch_mr(self, api, mocklab): with patch('marge.batch_job.MergeRequest') as mr_class: - batch_mr = Mock() + batch_mr = self._mock_merge_request() mr_class.search.return_value = [batch_mr] - batch_merge_job = self.get_batch_merge_job() + batch_merge_job = self.get_batch_merge_job(api, mocklab) batch_merge_job.close_batch_mr() params = { @@ -68,12 +76,12 @@ def test_close_batch_mr(self): ) batch_mr.close.assert_called_once() - def test_create_batch_mr(self): + def test_create_batch_mr(self, api, mocklab): with patch('marge.batch_job.MergeRequest') as mr_class: - batch_mr = Mock() + batch_mr = self._mock_merge_request() mr_class.create.return_value = batch_mr - batch_merge_job = self.get_batch_merge_job() + batch_merge_job = self.get_batch_merge_job(api, mocklab) target_branch = 'master' r_batch_mr = batch_merge_job.create_batch_mr(target_branch) @@ -90,26 +98,27 @@ def test_create_batch_mr(self): ) assert r_batch_mr is batch_mr - def test_get_mrs_with_common_target_branch(self): + def test_get_mrs_with_common_target_branch(self, api, mocklab): master_mrs = [ - Mock(target_branch='master'), - Mock(target_branch='master'), + self._mock_merge_request(target_branch='master'), + self._mock_merge_request(target_branch='master'), ] non_master_mrs = [ - Mock(target_branch='non_master'), - Mock(target_branch='non_master'), + self._mock_merge_request(target_branch='non_master'), + self._mock_merge_request(target_branch='non_master'), ] batch_merge_job = self.get_batch_merge_job( + api, mocklab, merge_requests=non_master_mrs + master_mrs, ) r_maser_mrs = batch_merge_job.get_mrs_with_common_target_branch('master') assert r_maser_mrs == master_mrs @patch.object(BatchMergeJob, 'get_mr_ci_status') - def test_ensure_mergeable_mr_ci_not_ok(self, bmj_get_mr_ci_status): - batch_merge_job = self.get_batch_merge_job() + def test_ensure_mergeable_mr_ci_not_ok(self, bmj_get_mr_ci_status, api, mocklab): + batch_merge_job = self.get_batch_merge_job(api, mocklab) bmj_get_mr_ci_status.return_value = 'failed' - merge_request = Mock( + merge_request = self._mock_merge_request( assignee_id=batch_merge_job._user.id, state='opened', work_in_progress=False, @@ -121,19 +130,19 @@ def test_ensure_mergeable_mr_ci_not_ok(self, bmj_get_mr_ci_status): assert str(exc_info.value) == 'This MR has not passed CI.' - def test_push_batch(self): - batch_merge_job = self.get_batch_merge_job() + def test_push_batch(self, api, mocklab): + batch_merge_job = self.get_batch_merge_job(api, mocklab) batch_merge_job.push_batch() batch_merge_job._repo.push.assert_called_once_with( BatchMergeJob.BATCH_BRANCH_NAME, force=True, ) - def test_ensure_mr_not_changed(self): + def test_ensure_mr_not_changed(self, api, mocklab): with patch('marge.batch_job.MergeRequest') as mr_class: - batch_merge_job = self.get_batch_merge_job() - merge_request = Mock() - changed_merge_request = Mock() + batch_merge_job = self.get_batch_merge_job(api, mocklab) + merge_request = self._mock_merge_request() + changed_merge_request = self._mock_merge_request() mr_class.fetch_by_iid.return_value = changed_merge_request with pytest.raises(CannotMerge): @@ -145,24 +154,27 @@ def test_ensure_mr_not_changed(self): batch_merge_job._api, ) - def test_fuse_mr_when_target_branch_was_moved(self): - batch_merge_job = self.get_batch_merge_job() - merge_request = Mock(target_branch='master') + def test_fuse_mr_when_target_branch_was_moved(self, api, mocklab): + batch_merge_job = self.get_batch_merge_job(api, mocklab) + merge_request = self._mock_merge_request(target_branch='master') with pytest.raises(CannotBatch) as exc_info: batch_merge_job.accept_mr(merge_request, 'abc') assert str(exc_info.value) == 'Someone was naughty and by-passed marge' - def test_fuse_mr_when_source_branch_was_moved(self): - api, mocklab = self.api, self.mocklab - batch_merge_job = self.get_batch_merge_job() - merge_request = Mock( - source_project_id=batch_merge_job._project.id, + def test_fuse_mr_when_source_branch_was_moved(self, api, mocklab): + batch_merge_job = self.get_batch_merge_job(api, mocklab) + merge_request = self._mock_merge_request( + source_project_id=mocklab.merge_request_info['source_project_id'], target_branch='master', - source_branch=self.mocklab.merge_request_info['source_branch'], + source_branch=mocklab.merge_request_info['source_branch'], ) api.add_transition( - GET('/projects/1234/repository/branches/useless_new_feature'), + GET( + '/projects/{project_iid}/repository/branches/useless_new_feature'.format( + project_iid=mocklab.merge_request_info['source_project_id'], + ), + ), Ok({'commit': commit(commit_id='abc', status='running')}), ) diff --git a/tests/test_job.py b/tests/test_job.py index d12e89ca..737f7ef6 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -1,20 +1,28 @@ # pylint: disable=protected-access from datetime import timedelta -from unittest.mock import ANY, Mock, patch +from unittest.mock import ANY, Mock, patch, create_autospec import pytest -import marge.interval from marge.job import CannotMerge, MergeJob, MergeJobOptions, SkipMerge +import marge.interval +import marge.git +import marge.gitlab +import marge.merge_request +import marge.project +import marge.user class TestJob(object): + def _mock_merge_request(self, **options): + return create_autospec(marge.merge_request.MergeRequest, spec_set=True, **options) + def get_merge_job(self, **merge_kwargs): params = { - 'api': Mock(), - 'user': Mock(), - 'project': Mock(), - 'repo': Mock(), + 'api': create_autospec(marge.gitlab.Api, spec_set=True), + 'user': create_autospec(marge.user.User, spec_set=True), + 'project': create_autospec(marge.project.Project, spec_set=True), + 'repo': create_autospec(marge.git.Repo, spec_set=True), 'options': MergeJobOptions.default(), } params.update(merge_kwargs) @@ -22,7 +30,7 @@ def get_merge_job(self, **merge_kwargs): def test_get_source_project_when_is_target_project(self): merge_job = self.get_merge_job() - merge_request = Mock() + merge_request = self._mock_merge_request() merge_request.source_project_id = merge_job._project.id r_source_project = merge_job.get_source_project(merge_request) assert r_source_project is merge_job._project @@ -30,7 +38,7 @@ def test_get_source_project_when_is_target_project(self): def test_get_source_project_when_is_fork(self): with patch('marge.job.Project') as project_class: merge_job = self.get_merge_job() - merge_request = Mock() + merge_request = self._mock_merge_request() r_source_project = merge_job.get_source_project(merge_request) project_class.fetch_by_id.assert_called_once_with( @@ -41,10 +49,12 @@ def test_get_source_project_when_is_fork(self): assert r_source_project is project_class.fetch_by_id.return_value def test_get_mr_ci_status(self): - with patch('marge.job.Pipeline') as pipeline_class: - pipeline_class.pipelines_by_branch.return_value = [Mock(sha='abc', status='success')] + with patch('marge.job.Pipeline', autospec=True) as pipeline_class: + pipeline_class.pipelines_by_branch.return_value = [ + Mock(spec=pipeline_class, sha='abc', status='success'), + ] merge_job = self.get_merge_job() - merge_request = Mock(sha='abc') + merge_request = self._mock_merge_request(sha='abc') r_ci_status = merge_job.get_mr_ci_status(merge_request) @@ -57,7 +67,7 @@ def test_get_mr_ci_status(self): def test_ensure_mergeable_mr_not_assigned(self): merge_job = self.get_merge_job() - merge_request = Mock( + merge_request = self._mock_merge_request( state='opened', work_in_progress=False, squash=False, @@ -68,7 +78,7 @@ def test_ensure_mergeable_mr_not_assigned(self): def test_ensure_mergeable_mr_state_not_ok(self): merge_job = self.get_merge_job() - merge_request = Mock( + merge_request = self._mock_merge_request( assignee_id=merge_job._user.id, state='merged', work_in_progress=False, @@ -80,7 +90,7 @@ def test_ensure_mergeable_mr_state_not_ok(self): def test_ensure_mergeable_mr_not_approved(self): merge_job = self.get_merge_job() - merge_request = Mock( + merge_request = self._mock_merge_request( assignee_id=merge_job._user.id, state='opened', work_in_progress=False, @@ -95,7 +105,7 @@ def test_ensure_mergeable_mr_not_approved(self): def test_ensure_mergeable_mr_wip(self): merge_job = self.get_merge_job() - merge_request = Mock( + merge_request = self._mock_merge_request( assignee_id=merge_job._user.id, state='opened', work_in_progress=True, @@ -108,7 +118,7 @@ def test_ensure_mergeable_mr_wip(self): def test_ensure_mergeable_mr_squash_and_trailers(self): merge_job = self.get_merge_job(options=MergeJobOptions.default(add_reviewers=True)) - merge_request = Mock( + merge_request = self._mock_merge_request( assignee_id=merge_job._user.id, state='opened', work_in_progress=False, @@ -125,7 +135,7 @@ def test_ensure_mergeable_mr_squash_and_trailers(self): def test_unassign_from_mr(self): merge_job = self.get_merge_job() - merge_request = Mock() + merge_request = self._mock_merge_request() # when we are not the author merge_job.unassign_from_mr(merge_request) diff --git a/tests/test_single_job.py b/tests/test_single_job.py index a00c66f0..8caad0cf 100644 --- a/tests/test_single_job.py +++ b/tests/test_single_job.py @@ -1,6 +1,10 @@ import contextlib +from collections import namedtuple from datetime import timedelta -from unittest.mock import ANY, Mock, patch +from functools import partial +from unittest.mock import ANY, patch, create_autospec + +import pytest import marge.commit import marge.interval @@ -26,51 +30,67 @@ def _commit(commit_id, status): } -def _pipeline(sha1, status): +def _branch(name, protected=False): + return { + 'name': name, + 'protected': protected, + } + + +def _pipeline(sha1, status, ref='useless_new_feature'): return { 'id': 47, 'status': status, - 'ref': 'useless_new_feature', + 'ref': ref, 'sha': sha1, } class SingleJobMockLab(MockLab): - def __init__(self, gitlab_url=None): - super().__init__(gitlab_url) + def __init__(self, gitlab_url=None, fork=False, merge_request_options=None): + super().__init__(gitlab_url, fork=fork, merge_request_options=merge_request_options) api = self.api self.rewritten_sha = rewritten_sha = 'af7a' api.add_pipelines( - self.project_info['id'], - _pipeline(sha1=rewritten_sha, status='running'), + self.merge_request_info['source_project_id'], + _pipeline(sha1=rewritten_sha, status='running', ref=self.merge_request_info['source_branch']), from_state='pushed', to_state='passed', ) api.add_pipelines( - self.project_info['id'], - _pipeline(sha1=rewritten_sha, status='success'), + self.merge_request_info['source_project_id'], + _pipeline(sha1=rewritten_sha, status='success', ref=self.merge_request_info['source_branch']), from_state=['passed', 'merged'], ) + source_project_id = self.merge_request_info['source_project_id'] api.add_transition( - GET('/projects/1234/repository/branches/useless_new_feature'), + GET( + '/projects/{}/repository/branches/{}'.format( + source_project_id, self.merge_request_info['source_branch'], + ), + ), Ok({'commit': _commit(commit_id=rewritten_sha, status='running')}), from_state='pushed', ) api.add_transition( - GET('/projects/1234/repository/branches/useless_new_feature'), + GET( + '/projects/{}/repository/branches/{}'.format( + source_project_id, self.merge_request_info['source_branch'], + ), + ), Ok({'commit': _commit(commit_id=rewritten_sha, status='success')}), from_state='passed' ) api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=self.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Ok({}), - from_state='passed', to_state='merged', + from_state=['passed', 'skipped'], to_state='merged', ) api.add_merge_request(dict(self.merge_request_info, state='merged'), from_state='merged') api.add_transition( - GET('/projects/1234/repository/branches/master'), + GET('/projects/1234/repository/branches/{}'.format(self.merge_request_info['target_branch'])), Ok({'commit': {'id': self.rewritten_sha}}), from_state='merged' ) @@ -98,7 +118,10 @@ def assign_to_author(): author_assigned = True self.api.add_transition( - PUT('/projects/1234/merge_requests/54', args={'assignee_id': self.author_id}), + PUT( + '/projects/1234/merge_requests/{iid}'.format(iid=self.merge_request_info['iid']), + args={'assignee_id': self.author_id}, + ), assign_to_author, ) error_note = "I couldn't merge this branch: %s" % message @@ -117,56 +140,213 @@ def branch_update(self, side_effect=None): marge.single_merge_job.SingleMergeJob, 'update_from_target_branch_and_push', side_effect=side_effect, + autospec=True, ): yield -# pylint: disable=attribute-defined-outside-init -@patch('time.sleep') class TestUpdateAndAccept(object): + TestParams = namedtuple('TestParams', ['fork', 'source_project_id']) - def setup_method(self, _method): - self.mocklab = SingleJobMockLab() - self.api = self.mocklab.api + @pytest.fixture( + params=[ + TestParams(fork=True, source_project_id=4321), + TestParams(fork=False, source_project_id=1234), + ] + ) + def test_params(self, request): + return request.param + + @pytest.fixture(autouse=True) + def patch_sleep(self): + with patch('time.sleep'): + yield - def make_job(self, options=None): - api, mocklab = self.api, self.mocklab + @pytest.fixture() + def mocklab(self, test_params): + return SingleJobMockLab(fork=test_params.fork) + @pytest.fixture() + def mocklab_factory(self, test_params): + return partial(SingleJobMockLab, fork=test_params.fork) + + @pytest.fixture() + def api(self, mocklab): + return mocklab.api + + def make_job(self, api, mocklab, options=None): project_id = mocklab.project_info['id'] merge_request_iid = mocklab.merge_request_info['iid'] project = marge.project.Project.fetch_by_id(project_id, api) merge_request = MergeRequest.fetch_by_iid(project_id, merge_request_iid, api) - repo = Mock(marge.git.Repo) + repo = create_autospec(marge.git.Repo, spec_set=True) options = options or marge.job.MergeJobOptions.default() - user = marge.user.User.myself(self.api) + user = marge.user.User.myself(api) return marge.single_merge_job.SingleMergeJob( api=api, user=user, project=project, merge_request=merge_request, repo=repo, options=options, ) - def test_succeeds_first_time(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_succeeds_first_time(self, api, mocklab): with mocklab.branch_update(): - job = self.make_job(marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False)) + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) job.execute() assert api.state == 'merged' assert api.notes == [] - def test_fails_on_not_acceptable_if_master_did_not_move(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_succeeds_with_updated_branch(self, api, mocklab): + api.add_transition( + GET( + '/projects/1234/repository/branches/{source}'.format( + source=mocklab.merge_request_info['source_branch'], + ), + ), + Ok({'commit': {'id': mocklab.rewritten_sha}}), + from_state='initial', to_state='pushed', + ) + with patch.object( + marge.single_merge_job.SingleMergeJob, + 'add_trailers', + side_effect=lambda *_: mocklab.push_updated()[2], + autospec=True, + ): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) + job.execute() + + assert api.state == 'merged' + assert api.notes == [] + + def test_succeeds_if_skipped(self, api, mocklab): + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='running'), + from_state='pushed', to_state='skipped', + ) + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='skipped'), + from_state=['skipped', 'merged'], + ) + + with mocklab.branch_update(): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) + job.execute() + + assert api.state == 'merged' + assert api.notes == [] + + def test_succeeds_if_source_is_master(self, mocklab_factory): + mocklab = mocklab_factory( + merge_request_options={'source_branch': 'master', 'target_branch': 'production'}, + ) + api = mocklab.api + api.add_transition( + GET( + '/projects/1234/repository/branches/{source}'.format( + source=mocklab.merge_request_info['source_branch'], + ), + ), + Ok({'commit': {'id': mocklab.rewritten_sha}}), + from_state='initial', to_state='pushed', + ) + with patch.object( + marge.single_merge_job.SingleMergeJob, + 'add_trailers', + side_effect=lambda *_: mocklab.push_updated()[2], + autospec=True, + ): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) + job.execute() + + assert api.state == 'merged' + assert api.notes == [] + + def test_fails_if_ci_fails(self, api, mocklab): + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='running'), + from_state='pushed', to_state='failed', + ) + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='failed'), + from_state=['failed'], + ) + + with mocklab.branch_update(): + with mocklab.expected_failure("CI failed!"): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(), + ) + job.execute() + + assert api.state == 'failed' + + def test_fails_if_ci_canceled(self, api, mocklab): + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='running'), + from_state='pushed', to_state='canceled', + ) + api.add_pipelines( + mocklab.merge_request_info['source_project_id'], + _pipeline(sha1=mocklab.rewritten_sha, status='canceled'), + from_state=['canceled'], + ) + + with mocklab.branch_update(): + with mocklab.expected_failure("Someone canceled the CI."): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(), + ) + job.execute() + + assert api.state == 'canceled' + + def test_fails_on_not_acceptable_if_master_did_not_move( + self, api, mocklab, test_params + ): new_branch_head_sha = '99ba110035' api.add_transition( - GET('/projects/1234/repository/branches/useless_new_feature'), + GET( + '/projects/{source_project_id}/repository/branches/useless_new_feature'.format( + source_project_id=test_params.source_project_id, + ), + ), Ok({'commit': _commit(commit_id=new_branch_head_sha, status='success')}), from_state='pushed', to_state='pushed_but_head_changed' ) with mocklab.branch_update(): with mocklab.expected_failure("Someone pushed to branch while we were trying to merge"): - job = self.make_job(marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False)) + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) job.execute() assert api.state == 'pushed_but_head_changed' @@ -174,18 +354,40 @@ def test_fails_on_not_acceptable_if_master_did_not_move(self, unused_time_sleep) "I couldn't merge this branch: Someone pushed to branch while we were trying to merge", ] - def test_succeeds_second_time_if_master_moved(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_fails_if_branch_is_protected( + self, api, mocklab, test_params + ): + api.add_transition( + GET( + '/projects/{source_project_id}/repository/branches/useless_new_feature'.format( + source_project_id=test_params.source_project_id, + ), + ), + Ok(_branch('useless_new_feature', protected=True)), + from_state='initial', to_state='protected' + ) + with mocklab.expected_failure("Sorry, I can't push rewritten changes to protected branches!"): + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) + job.repo.push.side_effect = marge.git.GitError() + job.execute() + + assert api.state == 'protected' + + def test_succeeds_second_time_if_master_moved(self, api, mocklab, test_params): moved_master_sha = 'fafafa' first_rewritten_sha = '1o1' api.add_pipelines( - mocklab.project_info['id'], + mocklab.merge_request_info['source_project_id'], _pipeline(sha1=first_rewritten_sha, status='success'), from_state=['pushed_but_master_moved', 'merged_rejected'], ) api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict( sha=first_rewritten_sha, should_remove_source_branch=True, @@ -196,7 +398,11 @@ def test_succeeds_second_time_if_master_moved(self, unused_time_sleep): from_state='pushed_but_master_moved', to_state='merge_rejected', ) api.add_transition( - GET('/projects/1234/repository/branches/useless_new_feature'), + GET( + '/projects/{source_project_id}/repository/branches/useless_new_feature'.format( + source_project_id=test_params.source_project_id, + ), + ), Ok({'commit': _commit(commit_id=first_rewritten_sha, status='success')}), from_state='pushed_but_master_moved' ) @@ -216,7 +422,11 @@ def push_effects(): yield moved_master_sha, 'deadbeef', mocklab.rewritten_sha with mocklab.branch_update(side_effect=push_effects()): - job = self.make_job(marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False)) + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(add_tested=True, add_reviewers=False), + ) job.execute() assert api.state == 'merged' @@ -224,12 +434,11 @@ def push_effects(): "My job would be easier if people didn't jump the queue and push directly... *sigh*", ] - def test_handles_races_for_merging(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_handles_races_for_merging(self, api, mocklab): rewritten_sha = mocklab.rewritten_sha api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Error(marge.gitlab.NotFound(404, {'message': '404 Branch Not Found'})), @@ -240,17 +449,16 @@ def test_handles_races_for_merging(self, unused_time_sleep): from_state='someone_else_merged', ) with mocklab.branch_update(): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'someone_else_merged' assert api.notes == [] - def test_handles_request_becoming_wip_after_push(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_handles_request_becoming_wip_after_push(self, api, mocklab): rewritten_sha = mocklab.rewritten_sha api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Error(marge.gitlab.MethodNotAllowed(405, {'message': '405 Method Not Allowed'})), @@ -262,17 +470,16 @@ def test_handles_request_becoming_wip_after_push(self, unused_time_sleep): ) message = 'The request was marked as WIP as I was processing it (maybe a WIP commit?)' with mocklab.branch_update(), mocklab.expected_failure(message): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'now_is_wip' assert api.notes == ["I couldn't merge this branch: %s" % message] - def test_guesses_git_hook_error_on_merge_refusal(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_guesses_git_hook_error_on_merge_refusal(self, api, mocklab): rewritten_sha = mocklab.rewritten_sha api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Error(marge.gitlab.MethodNotAllowed(405, {'message': '405 Method Not Allowed'})), @@ -287,17 +494,16 @@ def test_guesses_git_hook_error_on_merge_refusal(self, unused_time_sleep): 'is rejecting my commits; maybe my email needs to be white-listed?' ) with mocklab.branch_update(), mocklab.expected_failure(message): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'rejected_by_git_hook' assert api.notes == ["I couldn't merge this branch: %s" % message] - def test_discovers_if_someone_closed_the_merge_request(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_discovers_if_someone_closed_the_merge_request(self, api, mocklab): rewritten_sha = mocklab.rewritten_sha api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Error(marge.gitlab.MethodNotAllowed(405, {'message': '405 Method Not Allowed'})), @@ -309,17 +515,16 @@ def test_discovers_if_someone_closed_the_merge_request(self, unused_time_sleep): ) message = 'Someone closed the merge request while I was attempting to merge it.' with mocklab.branch_update(), mocklab.expected_failure(message): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'oops_someone_closed_it' assert api.notes == ["I couldn't merge this branch: %s" % message] - def test_tells_explicitly_that_gitlab_refused_to_merge(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_tells_explicitly_that_gitlab_refused_to_merge(self, api, mocklab): rewritten_sha = mocklab.rewritten_sha api.add_transition( PUT( - '/projects/1234/merge_requests/54/merge', + '/projects/1234/merge_requests/{iid}/merge'.format(iid=mocklab.merge_request_info['iid']), dict(sha=rewritten_sha, should_remove_source_branch=True, merge_when_pipeline_succeeds=True), ), Error(marge.gitlab.MethodNotAllowed(405, {'message': '405 Method Not Allowed'})), @@ -327,18 +532,17 @@ def test_tells_explicitly_that_gitlab_refused_to_merge(self, unused_time_sleep): ) message = "Gitlab refused to merge this request and I don't know why!" with mocklab.branch_update(), mocklab.expected_failure(message): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'rejected_for_mysterious_reasons' assert api.notes == ["I couldn't merge this branch: %s" % message] - def test_wont_merge_wip_stuff(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_wont_merge_wip_stuff(self, api, mocklab): wip_merge_request = dict(mocklab.merge_request_info, work_in_progress=True) api.add_merge_request(wip_merge_request, from_state='initial') with mocklab.expected_failure("Sorry, I can't merge requests marked as Work-In-Progress!"): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'initial' @@ -346,8 +550,7 @@ def test_wont_merge_wip_stuff(self, unused_time_sleep): "I couldn't merge this branch: Sorry, I can't merge requests marked as Work-In-Progress!", ] - def test_wont_merge_branches_with_autosquash_if_rewriting(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_wont_merge_branches_with_autosquash_if_rewriting(self, api, mocklab): autosquash_merge_request = dict(mocklab.merge_request_info, squash=True) api.add_merge_request(autosquash_merge_request, from_state='initial') admin_user = dict(mocklab.user_info, is_admin=True) @@ -357,35 +560,42 @@ def test_wont_merge_branches_with_autosquash_if_rewriting(self, unused_time_slee for rewriting_opt in ('add_tested', 'add_reviewers'): with mocklab.expected_failure(message): - job = self.make_job(marge.job.MergeJobOptions.default(**{rewriting_opt: True})) + job = self.make_job( + api, + mocklab, + options=marge.job.MergeJobOptions.default(**{rewriting_opt: True}), + ) job.execute() assert api.state == 'initial' with mocklab.branch_update(): - job = self.make_job() + job = self.make_job(api, mocklab) job.execute() assert api.state == 'merged' - @patch('marge.job.log') - def test_waits_for_approvals(self, mock_log, unused_time_sleep): - api, mocklab = self.api, self.mocklab + @patch('marge.job.log', autospec=True) + def test_waits_for_approvals(self, mock_log, api, mocklab): with mocklab.branch_update(): job = self.make_job( - marge.job.MergeJobOptions.default(approval_timeout=timedelta(seconds=5), reapprove=True)) + api, + mocklab, + options=marge.job.MergeJobOptions.default( + approval_timeout=timedelta(seconds=5), reapprove=True, + ), + ) job.execute() mock_log.info.assert_any_call('Checking if approvals have reset') mock_log.debug.assert_any_call('Approvals haven\'t reset yet, sleeping for %s secs', ANY) assert api.state == 'merged' - def test_fails_if_changes_already_exist(self, unused_time_sleep): - api, mocklab = self.api, self.mocklab + def test_fails_if_changes_already_exist(self, api, mocklab): expected_message = 'these changes already exist in branch `{}`'.format( mocklab.merge_request_info['target_branch'], ) with mocklab.expected_failure(expected_message): - job = self.make_job() + job = self.make_job(api, mocklab) job.repo.rebase.return_value = mocklab.initial_master_sha job.repo.get_commit_hash.return_value = mocklab.initial_master_sha job.execute()