diff --git a/conftest.py b/conftest.py index 388ae239..498605b3 100644 --- a/conftest.py +++ b/conftest.py @@ -1087,7 +1087,6 @@ class Model: return self._env(self._model, name, self._ids, *args, **kwargs) def __setattr__(self, fieldname, value): - assert self._fields[fieldname]['type'] not in ('many2one', 'one2many', 'many2many') self._env(self._model, 'write', self._ids, {fieldname: value}) def __iter__(self): diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index fd03822b..8d38f047 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -1,8 +1,9 @@ +# coding: utf-8 + import ast import base64 import collections import datetime -import functools import io import itertools import json @@ -10,7 +11,6 @@ import logging import os import pprint import re -import sys import time from itertools import takewhile @@ -65,9 +65,6 @@ class Project(models.Model): "will lead to webhook rejection. Should only use ASCII." ) - def _create_stagings(self, commit=False): - pass - def _check_stagings(self, commit=False): for staging in self.search([]).mapped('branch_ids.active_staging_id'): staging.check_status() @@ -217,6 +214,20 @@ class Project(models.Model): """, (self.id, name)) return bool(self.env.cr.rowcount) +class StatusConfiguration(models.Model): + _name = 'runbot_merge.repository.status' + _description = "required statuses on repositories" + _rec_name = 'context' + _log_access = False + + context = fields.Char(required=True) + repo_id = fields.Many2one('runbot_merge.repository', required=True, ondelete='cascade') + branch_ids = fields.Many2many('runbot_merge.branch', 'runbot_merge_repository_status_branch', 'status_id', 'branch_id') + + def _for_branch(self, branch): + assert branch._name == 'runbot_merge.branch', \ + f'Expected branch, got {branch}' + return self.filtered(lambda st: not st.branch_ids or branch in st.branch_ids) class Repository(models.Model): _name = _description = 'runbot_merge.repository' _order = 'sequence, id' @@ -224,11 +235,8 @@ class Repository(models.Model): sequence = fields.Integer(default=50) 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' - ) + status_ids = fields.One2many('runbot_merge.repository.status', 'repo_id', string="Required Statuses") + branch_filter = fields.Char(default='[(1, "=", 1)]', help="Filter branches valid for this repository") substitutions = fields.Text( "label substitutions", @@ -237,6 +245,22 @@ class Repository(models.Model): All substitutions are tentatively applied sequentially to the input. """) + @api.model + def create(self, vals): + if 'status_ids' in vals: + return super().create(vals) + + st = vals.pop('required_statuses', 'legal/cla,ci/runbot') + if st: + vals['status_ids'] = [(0, 0, {'context': c}) for c in st.split(',')] + return super().create(vals) + + def write(self, vals): + st = vals.pop('required_statuses', None) + if st: + vals['status_ids'] = [(5, 0, {})] + [(0, 0, {'context': c}) for c in st.split(',')] + return super().write(vals) + def github(self, token_field='github_token'): return github.GH(self.project_id[token_field], self.name) @@ -662,27 +686,27 @@ class PullRequests(models.Model): pr.blocked = 'linked pr %s is not ready' % unready.display_name continue - @api.depends('head', 'repository.required_statuses') + @api.depends('head', 'repository.status_ids') def _compute_statuses(self): Commits = self.env['runbot_merge.commit'] - for s in self: - c = Commits.search([('sha', '=', s.head)]) + for pr in self: + c = Commits.search([('sha', '=', pr.head)]) if not (c and c.statuses): - s.status = s.statuses = False + pr.status = pr.statuses = False continue statuses = json.loads(c.statuses) - s.statuses = pprint.pformat(statuses) + pr.statuses = pprint.pformat(statuses) st = 'success' - for ci in filter(None, (s.repository.required_statuses or '').split(',')): - v = state_(statuses, ci) or 'pending' + for ci in pr.repository.status_ids._for_branch(pr.target): + v = state_(statuses, ci.context) or 'pending' if v in ('error', 'failure'): st = 'failure' break if v == 'pending': st = 'pending' - s.status = st + pr.status = st @api.depends('batch_ids.active') def _compute_active_batch(self): @@ -944,7 +968,7 @@ class PullRequests(models.Model): # targets failed = self.browse(()) for pr in self: - required = filter(None, (pr.repository.required_statuses or '').split(',')) + required = pr.repository.status_ids._for_branch(pr.target).mapped('context') success = True for ci in required: @@ -1438,7 +1462,7 @@ class Commit(models.Model): def _auto_init(self): res = super(Commit, self)._auto_init() self._cr.execute(""" - CREATE INDEX IF NOT EXISTS runbot_merge_unique_statuses + CREATE INDEX IF NOT EXISTS runbot_merge_unique_statuses ON runbot_merge_commit USING hash (sha) """) @@ -1523,10 +1547,9 @@ class Stagings(models.Model): } # maps commits to the statuses they need required_statuses = [ - (head, repos[repo].required_statuses.split(',')) + (head, repos[repo].status_ids._for_branch(s.target).mapped('context')) for repo, head in json.loads(s.heads).items() if not repo.endswith('^') - if repos[repo].required_statuses ] # maps commits to their statuses cmap = { diff --git a/runbot_merge/security/ir.model.access.csv b/runbot_merge/security/ir.model.access.csv index 87191796..cc5a4668 100644 --- a/runbot_merge/security/ir.model.access.csv +++ b/runbot_merge/security/ir.model.access.csv @@ -1,6 +1,7 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_runbot_merge_project_admin,Admin access to project,model_runbot_merge_project,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_repository_admin,Admin access to repo,model_runbot_merge_repository,runbot_merge.group_admin,1,1,1,1 +access_runbot_merge_repository_status_admin,Admin access to repo statuses,model_runbot_merge_repository_status,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_branch_admin,Admin access to branches,model_runbot_merge_branch,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_pull_requests_admin,Admin access to PR,model_runbot_merge_pull_requests,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_pull_requests_tagging_admin,Admin access to tagging,model_runbot_merge_pull_requests_tagging,runbot_merge.group_admin,1,1,1,1 diff --git a/runbot_merge/tests/remote.py b/runbot_merge/tests/remote.py deleted file mode 100644 index e69de29b..00000000 diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 18112be6..331a2687 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -982,7 +982,7 @@ class TestNoRequiredStatus: def test_basic(self, env, repo, config): """ check that mergebot can work on a repo with no CI at all """ - env['runbot_merge.repository'].search([('name', '=', repo.name)]).required_statuses = False + env['runbot_merge.repository'].search([('name', '=', repo.name)]).status_ids = False with repo: m = repo.make_commit(None, 'initial', None, tree={'0': '0'}) repo.make_ref('heads/master', m) @@ -1004,7 +1004,7 @@ class TestNoRequiredStatus: assert pr.state == 'merged' def test_updated(self, env, repo, config): - env['runbot_merge.repository'].search([('name', '=', repo.name)]).required_statuses = False + env['runbot_merge.repository'].search([('name', '=', repo.name)]).status_ids = False 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_by_branch.py b/runbot_merge/tests/test_by_branch.py new file mode 100644 index 00000000..f51098aa --- /dev/null +++ b/runbot_merge/tests/test_by_branch.py @@ -0,0 +1,96 @@ +import pytest + +from utils import Commit + + +@pytest.fixture +def repo(env, project, make_repo, users, setreviewers): + r = make_repo('repo') + project.write({ + 'repo_ids': [(0, 0, { + 'name': r.name, + 'status_ids': [ + (0, 0, {'context': 'ci'}), + # require the lint status on master + (0, 0, { + 'context': 'lint', + 'branch_ids': [(4, project.branch_ids.id, False)] + }), + ] + })], + }) + setreviewers(*project.repo_ids) + return r + +def test_status_applies(env, repo, config): + """ If branches are associated with a repo status, only those branch should + require the status on their PRs & stagings + """ + with repo: + m = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master') + + [c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change') + pr = repo.make_pr(target='master', title="super change", head='change') + pr_id = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', repo.name), + ('number', '=', pr.number) + ]) + assert pr_id.state == 'opened' + + with repo: + repo.post_status(c, 'success', 'ci') + env.run_crons('runbot_merge.process_updated_commits') + assert pr_id.state == 'opened' + + with repo: + repo.post_status(c, 'success', 'lint') + env.run_crons('runbot_merge.process_updated_commits') + assert pr_id.state == 'validated' + + with repo: + pr.post_comment('hansen r+', config['role_reviewer']['token']) + env.run_crons() + + st = env['runbot_merge.stagings'].search([]) + assert st.state == 'pending' + with repo: + repo.post_status('staging.master', 'success', 'ci') + env.run_crons('runbot_merge.process_updated_commits') + assert st.state == 'pending' + with repo: + repo.post_status('staging.master', 'success', 'lint') + env.run_crons('runbot_merge.process_updated_commits') + assert st.state == 'success' + +def test_status_skipped(env, project, repo, config): + """ Branches not associated with a repo status should not require the status + on their PRs or stagings + """ + # add a second branch for which the lint status doesn't apply + project.write({'branch_ids': [(0, 0, {'name': 'maintenance'})]}) + with repo: + m = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/maintenance') + + [c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change') + pr = repo.make_pr(target='maintenance', title="super change", head='change') + pr_id = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', repo.name), + ('number', '=', pr.number) + ]) + assert pr_id.state == 'opened' + + with repo: + repo.post_status(c, 'success', 'ci') + env.run_crons('runbot_merge.process_updated_commits') + assert pr_id.state == 'validated' + + with repo: + pr.post_comment('hansen r+', config['role_reviewer']['token']) + env.run_crons() + + st = env['runbot_merge.stagings'].search([]) + assert st.state == 'pending' + with repo: + repo.post_status('staging.maintenance', 'success', 'ci') + env.run_crons('runbot_merge.process_updated_commits') + assert st.state == 'success' diff --git a/runbot_merge/views/mergebot.xml b/runbot_merge/views/mergebot.xml index 1d348c4d..566d5dbf 100644 --- a/runbot_merge/views/mergebot.xml +++ b/runbot_merge/views/mergebot.xml @@ -5,13 +5,6 @@
-

@@ -33,11 +26,11 @@ - + - + @@ -53,6 +46,32 @@ + + Repository form + runbot_merge.repository + + + +
+

+
+ + + + + + + + + + + + +
+ +
+
+ Projects runbot_merge.project