From 7dfa973b57d66b0eed963b051f540e740f453e23 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 21 Jan 2020 14:00:11 +0100 Subject: [PATCH] [IMP] runbot_merge, forwardport: move required statuses to repository Allows more flexibility in project composition as different repositories can each have their own CI passes. --- forwardport/tests/test_weird.py | 2 +- forwardport/tests/utils.py | 2 +- runbot_merge/__manifest__.py | 1 + .../migrations/11.0.1.1/pre-migration.py | 17 ++++++ runbot_merge/models/pull_requests.py | 33 ++++++----- runbot_merge/tests/conftest.py | 1 - runbot_merge/tests/test_basic.py | 7 ++- runbot_merge/tests/test_multirepo.py | 57 ++++++++++++++++++- runbot_merge/views/mergebot.xml | 4 +- 9 files changed, 99 insertions(+), 25 deletions(-) create mode 100644 runbot_merge/migrations/11.0.1.1/pre-migration.py diff --git a/forwardport/tests/test_weird.py b/forwardport/tests/test_weird.py index 9feff8bc..f872bea1 100644 --- a/forwardport/tests/test_weird.py +++ b/forwardport/tests/test_weird.py @@ -25,7 +25,6 @@ def make_basic(env, config, make_repo, *, fp_token, fp_remote): 'github_token': config['github']['token'], 'github_prefix': 'hansen', 'fp_github_token': fp_token and config['github']['token'], - 'required_statuses': 'legal/cla,ci/runbot', 'branch_ids': [ (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}), (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}), @@ -59,6 +58,7 @@ def make_basic(env, config, make_repo, *, fp_token, fp_remote): project.write({ 'repo_ids': [(0, 0, { 'name': prod.name, + 'required_statuses': 'legal/cla,ci/runbot', 'fp_remote_target': fp_remote and other.name, })], }) diff --git a/forwardport/tests/utils.py b/forwardport/tests/utils.py index 5da2c2d3..f126f053 100644 --- a/forwardport/tests/utils.py +++ b/forwardport/tests/utils.py @@ -55,7 +55,6 @@ def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproje 'github_token': config['github']['token'], 'github_prefix': 'hansen', 'fp_github_token': config['github']['token'], - 'required_statuses': 'legal/cla,ci/runbot', 'branch_ids': [ (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}), (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}), @@ -89,6 +88,7 @@ def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproje project.write({ 'repo_ids': [(0, 0, { 'name': prod.name, + 'required_statuses': 'legal/cla,ci/runbot', 'fp_remote_target': other.name, })], }) diff --git a/runbot_merge/__manifest__.py b/runbot_merge/__manifest__.py index 14d9e961..5d38d16f 100644 --- a/runbot_merge/__manifest__.py +++ b/runbot_merge/__manifest__.py @@ -1,5 +1,6 @@ { 'name': 'merge bot', + 'version': '1.1', 'depends': ['contacts', 'website'], 'data': [ 'security/security.xml', diff --git a/runbot_merge/migrations/11.0.1.1/pre-migration.py b/runbot_merge/migrations/11.0.1.1/pre-migration.py new file mode 100644 index 00000000..edecc416 --- /dev/null +++ b/runbot_merge/migrations/11.0.1.1/pre-migration.py @@ -0,0 +1,17 @@ +def migrate(cr, version): + """ Moved the required_statuses field from the project to the repository so + different repos can have different CI requirements within a project + """ + # create column on repo + cr.execute("ALTER TABLE runbot_merge_repository ADD COLUMN required_statuses varchar") + # copy data from project + cr.execute(""" + UPDATE runbot_merge_repository r + SET required_statuses = ( + SELECT required_statuses + FROM runbot_merge_project + WHERE id = r.project_id + ) + """) + # drop old column on project + cr.execute("ALTER TABLE runbot_merge_project DROP COLUMN required_statuses") diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 24adcd65..fae62130 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -41,11 +41,6 @@ class Project(models.Model): "target branches of PR this project handles." ) - required_statuses = fields.Char( - help="Comma-separated list of status contexts which must be "\ - "`success` for a PR or staging to be valid", - default='legal/cla,ci/runbot' - ) ci_timeout = fields.Integer( default=60, required=True, help="Delay (in minutes) before a staging is considered timed out and failed" @@ -217,6 +212,11 @@ class Repository(models.Model): name = fields.Char(required=True) project_id = fields.Many2one('runbot_merge.project', required=True) + required_statuses = fields.Char( + help="Comma-separated list of status contexts which must be "\ + "`success` for a PR or staging to be valid", + default='legal/cla,ci/runbot' + ) def github(self, token_field='github_token'): return github.GH(self.project_id[token_field], self.name) @@ -586,7 +586,7 @@ class PullRequests(models.Model): for pr in self: pr.blocked = pr.id not in stageable - @api.depends('head', 'repository.project_id.required_statuses') + @api.depends('head', 'repository.required_statuses') def _compute_statuses(self): Commits = self.env['runbot_merge.commit'] for s in self: @@ -599,7 +599,7 @@ class PullRequests(models.Model): s.statuses = pprint.pformat(statuses) st = 'success' - for ci in s.repository.project_id.required_statuses.split(','): + for ci in s.repository.required_statuses.split(','): v = state_(statuses, ci) or 'pending' if v in ('error', 'failure'): st = 'failure' @@ -851,7 +851,7 @@ class PullRequests(models.Model): # targets failed = self.browse(()) for pr in self: - required = filter(None, pr.repository.project_id.required_statuses.split(',')) + required = filter(None, pr.repository.required_statuses.split(',')) success = True for ci in required: @@ -1416,18 +1416,23 @@ class Stagings(models.Model): if s.state != 'pending': continue - heads = [ - head for repo, head in json.loads(s.heads).items() + repos = { + repo.name: repo + for repo in self.env['runbot_merge.repository'].search([]) + } + repomap = { + head: repos[repo] + for repo, head in json.loads(s.heads).items() if not repo.endswith('^') - ] + } commits = Commits.search([ - ('sha', 'in', heads) + ('sha', 'in', list(repomap)) ]) update_timeout_limit = False - reqs = [r.strip() for r in s.target.project_id.required_statuses.split(',')] st = 'success' for c in commits: + reqs = [r.strip() for r in repomap[c.sha].required_statuses.split(',')] statuses = json.loads(c.statuses) for v in map(lambda n: state_(statuses, n), reqs): if st == 'failure' or v in ('error', 'failure'): @@ -1441,7 +1446,7 @@ class Stagings(models.Model): assert v == 'success' # mark failure as soon as we find a failed status, but wait until # all commits are known & not pending to mark a success - if st == 'success' and len(commits) < len(heads): + if st == 'success' and len(commits) < len(repomap): st = 'pending' vals = {'state': st} diff --git a/runbot_merge/tests/conftest.py b/runbot_merge/tests/conftest.py index c1bab557..b33698dd 100644 --- a/runbot_merge/tests/conftest.py +++ b/runbot_merge/tests/conftest.py @@ -36,5 +36,4 @@ def project(env, config): 'github_token': config['github']['token'], 'github_prefix': 'hansen', 'branch_ids': [(0, 0, {'name': 'master'})], - 'required_statuses': 'legal/cla,ci/runbot', }) diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 48bef128..50b66a68 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -15,7 +15,10 @@ from test_utils import re_matches, get_partner, _simple_init @pytest.fixture def repo(project, make_repo): r = make_repo('repo') - project.write({'repo_ids': [(0, 0, {'name': r.name})]}) + project.write({'repo_ids': [(0, 0, { + 'name': r.name, + 'required_statuses': 'legal/cla,ci/runbot' + })]}) return r def test_trivial_flow(env, repo, page, users, config): @@ -937,7 +940,7 @@ def test_reopen_state(env, repo): def test_no_required_statuses(env, repo, config): """ check that mergebot can work on a repo with no CI at all """ - env['runbot_merge.project'].search([]).required_statuses = '' + env['runbot_merge.repository'].search([('name', '=', repo.name)]).required_statuses = '' with repo: m = repo.make_commit(None, 'initial', None, tree={'0': '0'}) repo.make_ref('heads/master', m) diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 4c9ebe81..36714005 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -14,19 +14,28 @@ from test_utils import re_matches, get_partner @pytest.fixture def repo_a(project, make_repo): repo = make_repo('a') - project.write({'repo_ids': [(0, 0, {'name': repo.name})]}) + project.write({'repo_ids': [(0, 0, { + 'name': repo.name, + 'required_statuses': 'legal/cla,ci/runbot' + })]}) return repo @pytest.fixture def repo_b(project, make_repo): repo = make_repo('b') - project.write({'repo_ids': [(0, 0, {'name': repo.name})]}) + project.write({'repo_ids': [(0, 0, { + 'name': repo.name, + 'required_statuses': 'legal/cla,ci/runbot' + })]}) return repo @pytest.fixture def repo_c(project, make_repo): repo = make_repo('c') - project.write({'repo_ids': [(0, 0, {'name': repo.name})]}) + project.write({'repo_ids': [(0, 0, { + 'name': repo.name, + 'required_statuses': 'legal/cla,ci/runbot' + })]}) return repo def make_pr(repo, prefix, trees, *, target='master', user, @@ -137,6 +146,48 @@ def test_stage_match(env, project, repo_a, repo_b, config): assert 'Related: {}#{}'.format(repo_b.name, pr_b.number) in repo_a.commit('master').message assert 'Related: {}#{}'.format(repo_a.name, pr_a.number) in repo_b.commit('master').message +def test_stage_different_statuses(env, project, repo_a, repo_b, config): + project.batch_limit = 1 + + env['runbot_merge.repository'].search([ + ('name', '=', repo_b.name) + ]).write({ + 'required_statuses': 'foo/bar', + }) + + with repo_a: + make_branch(repo_a, 'master', 'initial', {'a': 'a_0'}) + pr_a = make_pr( + repo_a, 'do-a-thing', [{'a': 'a_1'}], + user=config['role_user']['token'], + reviewer=config['role_reviewer']['token'], + ) + repo_a.post_status(pr_a.head, 'success', 'foo/bar') + with repo_b: + make_branch(repo_b, 'master', 'initial', {'a': 'b_0'}) + pr_b = make_pr(repo_b, 'do-a-thing', [{'a': 'b_1'}], + user=config['role_user']['token'], + reviewer=config['role_reviewer']['token'], + ) + env.run_crons() + # since the labels are the same but the statuses on pr_b are not the + # expected ones, pr_a should be blocked on pr_b, which should be approved + # but not validated / ready + pr_a_id = to_pr(env, pr_a) + pr_b_id = to_pr(env, pr_b) + assert pr_a_id.state == 'ready' + assert not pr_a_id.staging_id + assert pr_a_id.blocked + assert pr_b_id.state == 'approved' + assert not pr_b_id.staging_id + + with repo_b: + repo_b.post_status(pr_b.head, 'success', 'foo/bar') + env.run_crons() + + assert pr_a_id.state == pr_b_id.state == 'ready' + assert pr_a_id.staging_id == pr_b_id.staging_id + def test_unmatch_patch(env, project, repo_a, repo_b, config): """ When editing files via the UI for a project you don't have write access to, a branch called patch-XXX is automatically created in your diff --git a/runbot_merge/views/mergebot.xml b/runbot_merge/views/mergebot.xml index 10ac7e87..36bbbfee 100644 --- a/runbot_merge/views/mergebot.xml +++ b/runbot_merge/views/mergebot.xml @@ -19,9 +19,6 @@ - - - @@ -38,6 +35,7 @@ +