From 2e77a55ddb9df583c17e47fcf2ff13949c5313a9 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Mon, 21 Nov 2022 09:11:12 +0100 Subject: [PATCH] [IMP] runbot: add codeowner management --- runbot/data/runbot_data.xml | 5 + runbot/models/__init__.py | 2 + runbot/models/branch.py | 13 +- runbot/models/build.py | 5 +- runbot/models/build_config.py | 43 ++++- runbot/models/build_config_codeowner.py | 134 +++++++++++++++ runbot/models/codeowner.py | 2 +- runbot/models/res_config_settings.py | 5 +- runbot/models/team.py | 18 ++ runbot/security/ir.model.access.csv | 6 + runbot/tests/common.py | 67 +++++--- runbot/tests/test_branch.py | 22 ++- runbot/tests/test_build_config_step.py | 186 ++++++++++++++++++++- runbot/tests/test_repo.py | 9 +- runbot/views/config_views.xml | 4 + runbot/views/dashboard_views.xml | 58 ++++++- runbot/views/menus.xml | 1 + runbot/views/res_config_settings_views.xml | 2 + runbot_populate/models/runbot.py | 7 +- 19 files changed, 538 insertions(+), 51 deletions(-) create mode 100644 runbot/models/build_config_codeowner.py diff --git a/runbot/data/runbot_data.xml b/runbot/data/runbot_data.xml index 382fe01e..09c24578 100644 --- a/runbot/data/runbot_data.xml +++ b/runbot/data/runbot_data.xml @@ -48,6 +48,11 @@ admin_passwd=running_master_password ^((master)|(saas-)?\d+\.\d+)$ + + runbot.runbot_forwardport_author + fw-bot + + Mark is base diff --git a/runbot/models/__init__.py b/runbot/models/__init__.py index 46108428..4f0bc341 100644 --- a/runbot/models/__init__.py +++ b/runbot/models/__init__.py @@ -4,6 +4,7 @@ from . import batch from . import branch from . import build from . import build_config +from . import build_config_codeowner from . import build_error from . import bundle from . import codeowner @@ -20,6 +21,7 @@ from . import repo from . import res_config_settings from . import res_users from . import runbot +from . import team from . import upgrade from . import user from . import version diff --git a/runbot/models/branch.py b/runbot/models/branch.py index 163fba5e..18bf6f69 100644 --- a/runbot/models/branch.py +++ b/runbot/models/branch.py @@ -26,6 +26,10 @@ class Branch(models.Model): bundle_id = fields.Many2one('runbot.bundle', 'Bundle', compute='_compute_bundle_id', store=True, ondelete='cascade', index=True) is_pr = fields.Boolean('IS a pr', required=True) + pr_title = fields.Char('Pr Title') + pr_body = fields.Char('Pr Body') + pr_author = fields.Char('Pr Author') + pull_head_name = fields.Char(compute='_compute_branch_infos', string='PR HEAD name', readonly=1, store=True) pull_head_remote_id = fields.Many2one('runbot.remote', 'Pull head repository', compute='_compute_branch_infos', store=True, index=True) target_branch_name = fields.Char(compute='_compute_branch_infos', string='PR target branch', store=True) @@ -102,9 +106,9 @@ class Branch(models.Model): break for branch in self: - branch.target_branch_name = False - branch.pull_head_name = False - branch.pull_head_remote_id = False + #branch.target_branch_name = False + #branch.pull_head_name = False + #branch.pull_head_remote_id = False if branch.name: pi = branch.is_pr and (pull_info or pull_info_dict.get((branch.remote_id, branch.name)) or branch._get_pull_info()) if pi: @@ -113,6 +117,9 @@ class Branch(models.Model): branch.alive = pi.get('state', False) != 'closed' branch.target_branch_name = pi['base']['ref'] branch.pull_head_name = pi['head']['label'] + branch.pr_title = pi['title'] + branch.pr_body = pi['body'] + branch.pr_author = pi['creator']['login'] pull_head_repo_name = False if pi['head'].get('repo'): pull_head_repo_name = pi['head']['repo'].get('full_name') diff --git a/runbot/models/build.py b/runbot/models/build.py index 86d482c9..af7f9a6c 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -992,7 +992,10 @@ class BuildResult(models.Model): cmd = ['python%s' % py_version] + python_params + [os.path.join(server_dir, server_file)] if sub_command: cmd += [sub_command] - cmd += ['--addons-path', ",".join(addons_paths)] + + if not self.params_id.extra_params or '--addons-path' not in self.params_id.extra_params : + cmd += ['--addons-path', ",".join(addons_paths)] + # options config_path = build._server("tools/config.py") if grep(config_path, "no-xmlrpcs"): # move that to configs ? diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 8aceeeff..775e2a08 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -11,7 +11,13 @@ from ..common import now, grep, time2str, rfind, s2human, os, RunbotException from ..container import docker_get_gateway_ip, Command from odoo import models, fields, api from odoo.exceptions import UserError, ValidationError -from odoo.tools.safe_eval import safe_eval, test_python_expr +from odoo.tools.safe_eval import safe_eval, test_python_expr, _SAFE_OPCODES, to_opcodes + +# adding some additionnal optcode to safe_eval. This is not 100% needed and won't be done in standard but will help +# to simplify some python step by wraping the content in a function to allow return statement and get closer to other +# steps + +_SAFE_OPCODES |= set(to_opcodes(['LOAD_DEREF', 'STORE_DEREF', 'LOAD_CLOSURE'])) _logger = logging.getLogger(__name__) @@ -122,7 +128,7 @@ TYPES = [ ('configure_upgrade', 'Configure Upgrade'), ('configure_upgrade_complement', 'Configure Upgrade Complement'), ('test_upgrade', 'Test Upgrade'), - ('restore', 'Restore') + ('restore', 'Restore'), ] class ConfigStep(models.Model): _name = 'runbot.build.config.step' @@ -189,6 +195,9 @@ class ConfigStep(models.Model): restore_download_db_suffix = fields.Char('Download db suffix') restore_rename_db_suffix = fields.Char('Rename db suffix') + commit_limit = fields.Integer('Commit limit', default=50) + file_limit = fields.Integer('File limit', default=450) + @api.constrains('python_code') def _check_python_code(self): return self._check_python_field('python_code') @@ -315,6 +324,9 @@ class ConfigStep(models.Model): eval_ctx = self.make_python_ctx(build) try: safe_eval(self.python_code.strip(), eval_ctx, mode="exec", nocopy=True) + run = eval_ctx.get('run') + if run and callable(run): + return run() return eval_ctx.get('docker_params') except ValueError as e: save_eval_value_error_re = r': "(.*)" while evaluating\n.*' @@ -1086,6 +1098,33 @@ class ConfigStep(models.Model): self.ensure_one() return self._is_docker_step() + def _check_limits(self, build): + bundle = build.params_id.create_batch_id.bundle_id + commit_limit = bundle.commit_limit or self.commit_limit + file_limit = bundle.file_limit or self.file_limit + message = 'Limit reached: %s has more than %s %s (%s) and will be skipped. Contact runbot team to increase your limit if it was intended' + success = True + for commit_link in build.params_id.commit_link_ids: + if commit_link.base_ahead > commit_limit: + build._log('', message % (commit_link.commit_id.name, commit_limit, 'commit', commit_link.base_ahead), level="ERROR") + build.local_result = 'ko' + success = False + if commit_link.file_changed > file_limit: + build._log('', message % (commit_link.commit_id.name, file_limit, 'modified files', commit_link.file_changed), level="ERROR") + build.local_result = 'ko' + success = False + return success + + def _modified_files(self, build, commit_link_links = None): + modified_files = {} + for commit_link in commit_link_links or build.params_id.commit_link_ids: + commit = commit_link.commit_id + modified = commit.repo_id._git(['diff', '--name-only', '%s..%s' % (commit_link.merge_base_commit_id.name, commit.name)]) + if modified: + files = [('%s/%s' % (build._docker_source_folder(commit), file)) for file in modified.split('\n') if file] + modified_files[commit_link] = files + return modified_files + class ConfigStepOrder(models.Model): _name = 'runbot.build.config.step.order' diff --git a/runbot/models/build_config_codeowner.py b/runbot/models/build_config_codeowner.py new file mode 100644 index 00000000..a3dd8c5f --- /dev/null +++ b/runbot/models/build_config_codeowner.py @@ -0,0 +1,134 @@ +import re +from odoo import models, fields + + +class ConfigStep(models.Model): + _inherit = 'runbot.build.config.step' + + job_type = fields.Selection(selection_add=[('codeowner', 'Codeowner')], ondelete={'codeowner': 'cascade'}) + fallback_reviewer = fields.Char('Fallback reviewer') + + def _pr_by_commit(self, build, prs): + pr_by_commit = {} + for commit_link in build.params_id.commit_link_ids: + commit = commit_link.commit_id + repo_pr = prs.filtered(lambda pr: pr.remote_id.repo_id == commit_link.commit_id.repo_id) + if repo_pr: + if len(repo_pr) > 1: + build._log('', 'More than one open pr in this bundle for %s: %s' % (commit.repo_id.name, [pr.name for pr in repo_pr]), level='ERROR') + build.local_result = 'ko' + return {} + build._log('', 'PR [%s](%s) found for repo **%s**' % (repo_pr.dname, repo_pr.branch_url, commit.repo_id.name), log_type='markdown') + pr_by_commit[commit_link] = repo_pr + else: + build._log('', 'No pr for repo %s, skipping' % commit.repo_id.name) + return pr_by_commit + + def _get_module(self, repo, file): + for addons_path in repo.addons_paths.split(','): + base_path = f'{repo.name}/{addons_path}' + if file.startswith(base_path): + return file.replace(base_path, '').strip('/').split('/')[0] + + def _codeowners_regexes(self, codeowners, version_id): + regexes = {} + for codeowner in codeowners: + if codeowner.github_teams and codeowner.regex and (codeowner._match_version(version_id)): + team_set = regexes.setdefault(codeowner.regex.strip(), set()) + team_set |= set(t.strip() for t in codeowner.github_teams.split(',')) + return list(regexes.items()) + + def _reviewer_per_file(self, files, regexes, ownerships, repo): + reviewer_per_file = {} + for file in files: + file_reviewers = set() + for regex, teams in regexes: + if re.match(regex, file): + if not teams or 'none' in teams: + file_reviewers = set() + break # blacklisted, break + file_reviewers |= teams + + file_module = self._get_module(repo, file) + for ownership in ownerships: + if file_module == ownership.module_id.name and not ownership.is_fallback and ownership.team_id.github_team not in file_reviewers: + file_reviewers.add(ownership.team_id.github_team) + # fallback + if not file_reviewers: + for ownership in ownerships: + if file_module == ownership.module_id.name: + file_reviewers.add(ownership.team_id.github_team) + if not file_reviewers and self.fallback_reviewer: + file_reviewers.add(self.fallback_reviewer) + reviewer_per_file[file] = file_reviewers + return reviewer_per_file + + def _run_codeowner(self, build, log_path): + bundle = build.params_id.create_batch_id.bundle_id + if bundle.is_base: + build._log('', 'Skipping base bundle') + return + + if not self._check_limits(build): + return + + prs = bundle.branch_ids.filtered(lambda branch: branch.is_pr and branch.alive) + + # skip draft pr + draft_prs = prs.filtered(lambda pr: pr.draft) + if draft_prs: + build._log('', 'Some pr are draft, skipping: %s' % ','.join([pr.name for pr in draft_prs]), level='WARNING') + build.local_result = 'warn' + return + + # remove forwardport pr + ICP = self.env['ir.config_parameter'].sudo() + + fw_bot = ICP.get_param('runbot.runbot_forwardport_author') + fw_prs = prs.filtered(lambda pr: pr.pr_author == fw_bot and len(pr.reflog_ids) <= 1) + if fw_prs: + build._log('', 'Ignoring forward port pull request: %s' % ','.join([pr.name for pr in fw_prs])) + prs = list(set(prs) - set(fw_prs)) + + if not prs: + return + + # check prs targets + valid_targets = set([(branch.remote_id, branch.name) for branch in bundle.base_id.branch_ids]) + invalid_target_prs = prs.filtered(lambda pr: (pr.remote_id, pr.target_branch_name) not in valid_targets) + + if invalid_target_prs: + # this is not perfect but detects prs inside odoo-dev or with invalid target + build._log('', 'Some pr have an invalid target: %s' % ','.join([pr.name for pr in invalid_target_prs]), level='ERROR') + build.local_result = 'ko' + return + + build._checkout() + + pr_by_commit = self._pr_by_commit(build, prs) + ownerships = self.env['runbot.module.ownership'].search([('team_id.github_team', '!=', False)]) + codeowners = build.env['runbot.codeowner'].search([('project_id', '=', bundle.project_id.id)]) + regexes = self._codeowners_regexes(codeowners, build.params_id.version_id) + modified_files = self._modified_files(build, pr_by_commit.keys()) + + for commit_link, files in modified_files.items(): + build._log('','Checking %s codeowner regexed on %s files' % (len(regexes), len(files))) + + reviewers = set() + reviewer_per_file = self._reviewer_per_file(files, regexes, ownerships, commit_link.commit_id.repo_id) + for file, file_reviewers in reviewer_per_file.items(): + href = 'https://%s/blob/%s/%s' % (commit_link.branch_id.remote_id.base_url, commit_link.commit_id.name, file.split('/', 1)[-1]) + build._log('', 'Adding %s to reviewers for file [%s](%s)' % (', '.join(sorted(file_reviewers)), file, href), log_type='markdown') + reviewers |= file_reviewers + + if reviewers: + pr = pr_by_commit[commit_link] + new_reviewers = sorted(reviewers - set((pr.reviewers or '').split(','))) + if new_reviewers: + build._log('', 'Requesting review for pull request [%s](%s): %s' % (pr.dname, pr.branch_url, ', '.join(new_reviewers)), log_type='markdown') + response = pr.remote_id._github('/repos/:owner/:repo/pulls/%s/requested_reviewers' % pr.name, {"team_reviewers":list(new_reviewers)}, ignore_errors=False) + pr._compute_branch_infos(response) + pr['reviewers'] = ','.join(sorted(reviewers)) + else: + build._log('', 'All reviewers are already on pull request [%s](%s)' % (pr.dname, pr.branch_url,), log_type='markdown') + diff --git a/runbot/models/codeowner.py b/runbot/models/codeowner.py index d0cc1343..a2681392 100644 --- a/runbot/models/codeowner.py +++ b/runbot/models/codeowner.py @@ -38,4 +38,4 @@ class Codeowner(models.Model): return ast.literal_eval(self.version_domain) if self.version_domain else [] def _match_version(self, version): - return version.filtered_domain(self._get_version_domain()) + return not self.version_domain or version.filtered_domain(self._get_version_domain()) diff --git a/runbot/models/res_config_settings.py b/runbot/models/res_config_settings.py index c66ce58b..c3605ac8 100644 --- a/runbot/models/res_config_settings.py +++ b/runbot/models/res_config_settings.py @@ -26,6 +26,7 @@ class ResConfigSettings(models.TransientModel): runbot_do_fetch = fields.Boolean('Discover new commits') runbot_do_schedule = fields.Boolean('Schedule builds') runbot_is_base_regex = fields.Char('Regex is_base') + runbot_forwardport_author = fields.Char('Forwardbot author') runbot_db_gc_days = fields.Integer( 'Days before gc', @@ -69,7 +70,8 @@ class ResConfigSettings(models.TransientModel): runbot_upgrade_exception_message=get_param('runbot.runbot_upgrade_exception_message'), runbot_do_fetch=get_param('runbot.runbot_do_fetch', default=False), runbot_do_schedule=get_param('runbot.runbot_do_schedule', default=False), - runbot_is_base_regex=get_param('runbot.runbot_is_base_regex', default='') + runbot_is_base_regex=get_param('runbot.runbot_is_base_regex', default=''), + runbot_forwardport_author = get_param('runbot.runbot_forwardport_author', default=''), ) return res @@ -91,6 +93,7 @@ class ResConfigSettings(models.TransientModel): set_param('runbot.runbot_do_fetch', self.runbot_do_fetch) set_param('runbot.runbot_do_schedule', self.runbot_do_schedule) set_param('runbot.runbot_is_base_regex', self.runbot_is_base_regex) + set_param('runbot.runbot_forwardport_author', self.runbot_forwardport_author) @api.onchange('runbot_is_base_regex') def _on_change_is_base_regex(self): diff --git a/runbot/models/team.py b/runbot/models/team.py index 1675ad17..c1420cb5 100644 --- a/runbot/models/team.py +++ b/runbot/models/team.py @@ -27,7 +27,9 @@ class RunbotTeam(models.Model): help='Comma separated list of `fnmatch` wildcards used to assign errors automaticaly\n' 'Negative wildcards starting with a `-` can be used to discard some path\n' 'e.g.: `*website*,-*website_sale*`') + module_ownership_ids = fields.One2many('runbot.module.ownership', 'team_id') upgrade_exception_ids = fields.One2many('runbot.upgrade.exception', 'team_id', string='Team Upgrade Exceptions') + github_team = fields.Char('Github team') @api.model_create_single def create(self, values): @@ -47,6 +49,22 @@ class RunbotTeam(models.Model): return team.id return False +class Module(models.Model): + _name = 'runbot.module' + _description = 'Modules' + + name = fields.Char('Name') + ownership_ids = fields.One2many('runbot.module.ownership', 'module_id') + + +class ModuleOwnership(models.Model): + _name = 'runbot.module.ownership' + _description = "Module ownership" + + module_id = fields.Many2one('runbot.module', string='Module', required=True, ondelete='cascade') + team_id = fields.Many2one('runbot.team', string='Team', required=True) + is_fallback = fields.Boolean('Fallback') + class RunbotDashboard(models.Model): diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index c2227dc8..717a8884 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -27,6 +27,12 @@ access_runbot_build_error_tag_admin,runbot_build_error_tag_admin,runbot.model_ru access_runbot_build_error_tag_manager,runbot_build_error_tag_manager,runbot.model_runbot_build_error_tag,runbot.group_runbot_error_manager,1,1,1,1 access_runbot_team_admin,runbot_team_admin,runbot.model_runbot_team,runbot.group_runbot_admin,1,1,1,1 access_runbot_team_user,runbot_team_user,runbot.model_runbot_team,group_user,1,0,0,0 + +access_runbot_module_admin,runbot_module_admin,runbot.model_runbot_module,runbot.group_runbot_admin,1,1,1,1 +access_runbot_module_user,runbot_module_user,runbot.model_runbot_module,group_user,1,0,0,0 +access_runbot_module_ownership_admin,runbot_module_ownership_admin,runbot.model_runbot_module_ownership,runbot.group_runbot_admin,1,1,1,1 +access_runbot_module_ownership_user,runbot_module_ownership_user,runbot.model_runbot_module_ownership,group_user,1,0,0,0 + access_runbot_dashboard_admin,runbot_dashboard_admin,runbot.model_runbot_dashboard,runbot.group_runbot_admin,1,1,1,1 access_runbot_dashboard_user,runbot_dashboard_user,runbot.model_runbot_dashboard,group_user,1,0,0,0 access_runbot_dashboard_tile_admin,runbot_dashboard_tile_admin,runbot.model_runbot_dashboard_tile,runbot.group_runbot_admin,1,1,1,1 diff --git a/runbot/tests/common.py b/runbot/tests/common.py index 6d00ee4a..456795f3 100644 --- a/runbot/tests/common.py +++ b/runbot/tests/common.py @@ -23,6 +23,8 @@ class RunbotCase(TransactionCase): return '\n'.join(['\0'.join(commit_fields) for commit_fields in self.commit_list[repo.id]]) else: return '' + if cmd[0] == 'diff': + return self.diff else: _logger.warning('Unsupported mock command %s' % cmd) return mock_git @@ -54,6 +56,7 @@ class RunbotCase(TransactionCase): self.Trigger = self.env['runbot.trigger'].with_context(mail_create_nolog=True, mail_notrack=True) self.Branch = self.env['runbot.branch'] self.Bundle = self.env['runbot.bundle'] + self.Batch = self.env['runbot.batch'] self.Version = self.env['runbot.version'] self.Config = self.env['runbot.build.config'].with_context(mail_create_nolog=True, mail_notrack=True) self.Step = self.env['runbot.build.config.step'].with_context(mail_create_nolog=True, mail_notrack=True) @@ -95,10 +98,52 @@ class RunbotCase(TransactionCase): self.version_13 = self.Version.create({'name': '13.0'}) self.default_config = self.env.ref('runbot.runbot_build_config_default') + self.initial_server_commit = self.Commit.create({ + 'name': 'aaaaaaa', + 'repo_id': self.repo_server.id, + 'date': '2006-12-07', + 'subject': 'New trunk', + 'author': 'purply', + 'author_email': 'puprly@somewhere.com' + }) + self.env['ir.config_parameter'].sudo().set_param('runbot.runbot_is_base_regex', r'^((master)|(saas-)?\d+\.\d+)$') + + self.branch_server = self.Branch.create({ + 'name': 'master', + 'remote_id': self.remote_server.id, + 'is_pr': False, + 'head': self.initial_server_commit.id, + }) + self.branch_server.bundle_id # compute + self.dev_bundle = self.Bundle.create({ + 'name': 'master-dev-tri', + 'project_id': self.project.id + }) + self.dev_branch = self.Branch.create({ + 'name': 'master-dev-tri', + 'bundle_id': self.dev_bundle.id, + 'is_pr': False, + 'remote_id': self.remote_server.id, + }) + self.dev_pr = self.Branch.create({ + 'name': '1234', + 'is_pr': True, + 'remote_id': self.remote_server.id, + 'target_branch_name': self.dev_bundle.base_id.name, + 'pull_head_remote_id': self.remote_server.id, + }) + self.dev_pr.pull_head_name = f'{self.remote_server.owner}:{self.dev_branch.name}' + self.dev_pr.bundle_id = self.dev_bundle.id, + + self.dev_batch = self.Batch.create({ + 'bundle_id': self.dev_bundle.id, + }) + self.base_params = self.BuildParameters.create({ 'version_id': self.version_13.id, 'project_id': self.project.id, 'config_id': self.default_config.id, + 'create_batch_id': self.dev_batch.id, }) self.trigger_server = self.Trigger.create({ @@ -119,7 +164,7 @@ class RunbotCase(TransactionCase): self.patchers = {} self.patcher_objects = {} self.commit_list = {} - + self.diff = '' self.start_patcher('git_patcher', 'odoo.addons.runbot.models.repo.Repo._git', new=self.mock_git_helper()) self.start_patcher('hostname_patcher', 'odoo.addons.runbot.common.socket.gethostname', 'host.runbot.com') self.start_patcher('github_patcher', 'odoo.addons.runbot.models.repo.Remote._github', {}) @@ -148,6 +193,7 @@ class RunbotCase(TransactionCase): self.start_patcher('_get_py_version', 'odoo.addons.runbot.models.build.BuildResult._get_py_version', 3) + def start_patcher(self, patcher_name, patcher_path, return_value=DEFAULT, side_effect=DEFAULT, new=DEFAULT): def stop_patcher_wrapper(): @@ -171,25 +217,6 @@ class RunbotCase(TransactionCase): def additionnal_setup(self): """Helper that setup a the repos with base branches and heads""" - - self.env['ir.config_parameter'].sudo().set_param('runbot.runbot_is_base_regex', r'^((master)|(saas-)?\d+\.\d+)$') - - self.initial_server_commit = self.Commit.create({ - 'name': 'aaaaaaa', - 'repo_id': self.repo_server.id, - 'date': '2006-12-07', - 'subject': 'New trunk', - 'author': 'purply', - 'author_email': 'puprly@somewhere.com' - }) - - self.branch_server = self.Branch.create({ - 'name': 'master', - 'remote_id': self.remote_server.id, - 'is_pr': False, - 'head': self.initial_server_commit.id, - }) - self.assertEqual(self.branch_server.bundle_id.name, 'master') self.branch_server.bundle_id.is_base = True initial_addons_commit = self.Commit.create({ 'name': 'cccccc', diff --git a/runbot/tests/test_branch.py b/runbot/tests/test_branch.py index 2ce2855e..44e163e9 100644 --- a/runbot/tests/test_branch.py +++ b/runbot/tests/test_branch.py @@ -6,19 +6,18 @@ from .common import RunbotCase, RunbotCaseMinimalSetup class TestBranch(RunbotCase): def test_base_fields(self): - branch = self.Branch.create({ - 'remote_id': self.remote_server.id, - 'name': 'master', - 'is_pr': False, - }) - - self.assertEqual(branch.branch_url, 'https://example.com/base/server/tree/master') + self.assertEqual(self.branch_server.branch_url, 'https://example.com/base/server/tree/master') def test_pull_request(self): mock_github = self.patchers['github_patcher'] mock_github.return_value = { 'base': {'ref': 'master'}, 'head': {'label': 'foo-dev:bar_branch', 'repo': {'full_name': 'foo-dev/bar'}}, + 'title': '[IMP] Title', + 'body': 'Body', + 'creator': { + 'login': 'Pr author' + }, } pr = self.Branch.create({ 'remote_id': self.remote_server.id, @@ -43,7 +42,7 @@ class TestBranchRelations(RunbotCase): }) branch.bundle_id.is_base = True return branch - self.master = create_base('master') + self.master = self.branch_server create_base('11.0') create_base('saas-11.1') create_base('12.0') @@ -125,7 +124,12 @@ class TestBranchRelations(RunbotCase): self.patchers['github_patcher'].return_value = { 'base': {'ref': 'master-test-tri'}, 'head': {'label': 'dev:master-test-tri-imp', 'repo': {'full_name': 'dev/server'}}, - } + 'title': '[IMP] Title', + 'body': 'Body', + 'creator': { + 'login': 'Pr author' + }, + } b = self.Branch.create({ 'remote_id': self.remote_server_dev.id, 'name': '100', diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index c8854ad8..35112c8e 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- from unittest.mock import patch, mock_open +from odoo import Command from odoo.exceptions import UserError from odoo.addons.runbot.common import RunbotException from .common import RunbotCase @@ -12,16 +13,178 @@ class TestBuildConfigStepCommon(RunbotCase): self.ConfigStep = self.env['runbot.build.config.step'] self.Config = self.env['runbot.build.config'] - server_commit = self.Commit.create({ - 'name': 'dfdfcfcf0000ffffffffffffffffffffffffffff', + self.server_commit = self.Commit.create({ + 'name': 'dfdfcfcf', 'repo_id': self.repo_server.id }) self.parent_build = self.Build.create({ - 'params_id': self.base_params.copy({'commit_link_ids': [(0, 0, {'commit_id': server_commit.id})]}).id, + 'params_id': self.base_params.copy({'commit_link_ids': [(0, 0, {'commit_id': self.server_commit.id})]}).id, + 'local_result': 'ok', }) self.start_patcher('find_patcher', 'odoo.addons.runbot.common.find', 0) self.start_patcher('findall_patcher', 'odoo.addons.runbot.models.build.BuildResult.parse_config', {}) +class TestCodeowner(TestBuildConfigStepCommon): + def setUp(self): + super().setUp() + self.config_step = self.ConfigStep.create({ + 'name': 'test_codeowner', + 'job_type': 'codeowner', + 'fallback_reviewer': 'codeowner-team', + }) + self.child_config = self.Config.create({'name': 'test_config'}) + self.config_step.create_config_ids = [self.child_config.id] + self.team1 = self.env['runbot.team'].create({'name': "Team1", 'github_team': "team_01"}) + self.team2 = self.env['runbot.team'].create({'name': "Team2", 'github_team': "team_02"}) + self.env['runbot.codeowner'].create({'github_teams': 'team_py', 'project_id': self.project.id, 'regex': '.*.py'}) + self.env['runbot.codeowner'].create({'github_teams': 'team_js', 'project_id': self.project.id, 'regex': '.*.js'}) + self.server_commit.name = 'dfdfcfcf' + + + def test_codeowner_is_base(self): + self.dev_bundle.is_base = True + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + 'Skipping base bundle', + ]) + self.assertEqual(self.parent_build.local_result, 'ok') + + def test_codeowner_check_limits(self): + self.parent_build.params_id.commit_link_ids[0].file_changed = 451 + self.parent_build.params_id.commit_link_ids[0].base_ahead = 51 + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + 'Limit reached: dfdfcfcf has more than 50 commit (51) and will be skipped. Contact runbot team to increase your limit if it was intended', + 'Limit reached: dfdfcfcf has more than 450 modified files (451) and will be skipped. Contact runbot team to increase your limit if it was intended', + ]) + self.assertEqual(self.parent_build.local_result, 'ko') + + def test_codeowner_draft(self): + self.dev_pr.draft=True + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + 'Some pr are draft, skipping: 1234' + ]) + self.assertEqual(self.parent_build.local_result, 'warn') + + def test_codeowner_draft_closed(self): + self.dev_pr.draft=True + self.dev_pr.alive=False + self.assertEqual(self.parent_build.local_result, 'ok') + + def test_codeowner_forwardpot(self): + self.dev_pr.pr_author='fw-bot' + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + 'Ignoring forward port pull request: 1234' + ]) + self.assertEqual(self.parent_build.local_result, 'ok') + + def test_codeowner_invalid_target(self): + self.dev_pr.target_branch_name = 'master-other-dev-branch' + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + 'Some pr have an invalid target: 1234' + ]) + self.assertEqual(self.parent_build.local_result, 'ko') + + def test_codeowner_pr_duplicate(self): + second_pr = self.Branch.create({ + 'name': '1235', + 'is_pr': True, + 'remote_id': self.remote_server.id, + 'target_branch_name': self.dev_bundle.base_id.name, + 'pull_head_remote_id': self.remote_server.id, + }) + second_pr.pull_head_name = f'{self.remote_server.owner}:{self.dev_branch.name}' + second_pr.bundle_id = self.dev_bundle.id + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + self.assertEqual(self.parent_build.log_ids.mapped('message'), [ + "More than one open pr in this bundle for server: ['1234', '1235']" + ]) + self.assertEqual(self.parent_build.local_result, 'ko') + + def test_get_module(self): + self.assertEqual(self.repo_server.addons_paths, 'addons,core/addons') + self.assertEqual('module1', self.config_step._get_module(self.repo_server, 'server/core/addons/module1/some/file.py')) + self.assertEqual('module1', self.config_step._get_module(self.repo_server, 'server/addons/module1/some/file.py')) + self.assertEqual(None, self.config_step._get_module(self.repo_server, 'server/core/module1/some/file.py')) + + def test_codeowner_regex_multiple(self): + self.diff = 'file.js\nfile.py\nfile.xml' + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual(messages[1], 'Checking 2 codeowner regexed on 3 files') + self.assertEqual(messages[2], 'Adding team_js to reviewers for file [server/file.js](https://False/blob/dfdfcfcf/file.js)') + self.assertEqual(messages[3], 'Adding team_py to reviewers for file [server/file.py](https://False/blob/dfdfcfcf/file.py)') + self.assertEqual(messages[4], 'Adding codeowner-team to reviewers for file [server/file.xml](https://False/blob/dfdfcfcf/file.xml)') + self.assertEqual(messages[5], 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_js, team_py') + self.assertEqual(self.dev_pr.reviewers, 'codeowner-team,team_js,team_py') + + def test_codeowner_regex_some_already_on(self): + self.diff = 'file.js\nfile.py\nfile.xml' + self.dev_pr.reviewers = 'codeowner-team,team_js' + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual(messages[5], 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): team_py') + + def test_codeowner_regex_all_already_on(self): + self.diff = 'file.js\nfile.py\nfile.xml' + self.dev_pr.reviewers = 'codeowner-team,team_js,team_py' + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual(messages[5], 'All reviewers are already on pull request [base/server:1234](https://example.com/base/server/pull/1234)') + + def test_codeowner_ownership_base(self): + module1 = self.env['runbot.module'].create({'name': "module1"}) + self.env['runbot.module.ownership'].create({'team_id': self.team1.id, 'module_id': module1.id}) + self.diff = '\n'.join([ + 'core/addons/module1/some/file.py', + ]) + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual( + messages[2], + 'Adding team_01, team_py to reviewers for file [server/core/addons/module1/some/file.py](https://False/blob/dfdfcfcf/core/addons/module1/some/file.py)' + ) + + def test_codeowner_ownership_fallback(self): + module1 = self.env['runbot.module'].create({'name': "module1"}) + self.env['runbot.module.ownership'].create({'team_id': self.team1.id, 'module_id': module1.id, 'is_fallback': True}) + self.diff = '\n'.join([ + 'core/addons/module1/some/file.py', + ]) + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual( + messages[2], + 'Adding team_py to reviewers for file [server/core/addons/module1/some/file.py](https://False/blob/dfdfcfcf/core/addons/module1/some/file.py)' + ) + + def test_codeowner_ownership(self): + module1 = self.env['runbot.module'].create({'name': "module1"}) + module2 = self.env['runbot.module'].create({'name': "module2"}) + self.env['runbot.module.ownership'].create({'team_id': self.team1.id, 'module_id': module1.id}) + self.env['runbot.module.ownership'].create({'team_id': self.team2.id, 'module_id': module2.id}) + self.diff = '\n'.join([ + 'core/addons/module1/some/file.py', + 'core/addons/module2/some/file.ext', + 'core/addons/module3/some/file.js', + 'core/addons/module4/some/file.txt', + ]) + self.config_step._run_codeowner(self.parent_build, '/tmp/essai') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual(messages, [ + 'PR [base/server:1234](https://example.com/base/server/pull/1234) found for repo **server**', + 'Checking 2 codeowner regexed on 4 files', + 'Adding team_01, team_py to reviewers for file [server/core/addons/module1/some/file.py](https://False/blob/dfdfcfcf/core/addons/module1/some/file.py)', + 'Adding team_02 to reviewers for file [server/core/addons/module2/some/file.ext](https://False/blob/dfdfcfcf/core/addons/module2/some/file.ext)', + 'Adding team_js to reviewers for file [server/core/addons/module3/some/file.js](https://False/blob/dfdfcfcf/core/addons/module3/some/file.js)', + 'Adding codeowner-team to reviewers for file [server/core/addons/module4/some/file.txt](https://False/blob/dfdfcfcf/core/addons/module4/some/file.txt)', + 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_01, team_02, team_js, team_py' + ]) + + class TestBuildConfigStepCreate(TestBuildConfigStepCommon): @@ -61,7 +224,7 @@ class TestBuildConfigStepCreate(TestBuildConfigStepCommon): self.assertTrue(child_build.orphan_result, 'An orphan result config step should mark the build as orphan_result') child_build.local_result = 'ko' - self.assertFalse(self.parent_build.global_result) + self.assertEqual(self.parent_build.global_result, 'ok') def test_config_step_create_child_data(self): """ Test the config step of type create """ @@ -318,6 +481,21 @@ docker_params = dict(cmd=cmd) db = self.env['runbot.database'].search([('name', '=', 'test_database')]) self.assertEqual(db.build_id, self.parent_build) + def test_run_python_run(self): + """minimal test for python steps. Also test that `-d` in cmd creates a database""" + test_code = """ +def run(): + return {'a': 'b'} +""" + config_step = self.ConfigStep.create({ + 'name': 'default', + 'job_type': 'python', + 'python_code': test_code, + }) + + retult = config_step._run_python(self.parent_build, 'dev/null/logpath') + self.assertEqual(retult, {'a': 'b'}) + @patch('odoo.addons.runbot.models.build.BuildResult._checkout') def test_sub_command(self, mock_checkout): config_step = self.ConfigStep.create({ diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index 59a6f103..ae1bc83f 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -64,6 +64,11 @@ class TestRepo(RunbotCaseMinimalSetup): return { 'base': {'ref': 'master'}, 'head': {'label': 'dev:%s' % branch_name, 'repo': {'full_name': 'dev/server'}}, + 'title': '[IMP] Title', + 'body': 'Body', + 'creator': { + 'login': 'Pr author' + }, } repos = self.repo_addons | self.repo_server @@ -137,7 +142,7 @@ class TestRepo(RunbotCaseMinimalSetup): # Create Batches repos._update_batches() - pull_request = self.env['runbot.branch'].search([('remote_id', '=', self.remote_server.id), ('id', '!=', self.branch_server.id)]) + pull_request = self.env['runbot.branch'].search([('remote_id', '=', self.remote_server.id), ('name', '=', '123')]) self.assertEqual(pull_request.bundle_id, bundle) self.assertEqual(dev_branch.head_name, 'd0d0caca') @@ -179,7 +184,7 @@ class TestRepo(RunbotCaseMinimalSetup): repos._update_batches() self.assertEqual(dev_branch, self.env['runbot.branch'].search([('remote_id', '=', self.remote_server_dev.id)])) - self.assertEqual(pull_request + self.branch_server, self.env['runbot.branch'].search([('remote_id', '=', self.remote_server.id)])) + #self.assertEqual(pull_request + self.branch_server, self.env['runbot.branch'].search([('remote_id', '=', self.remote_server.id)])) self.assertEqual(addons_dev_branch, self.env['runbot.branch'].search([('remote_id', '=', self.remote_addons_dev.id)])) batch = self.env['runbot.batch'].search([('bundle_id', '=', bundle.id)]) diff --git a/runbot/views/config_views.xml b/runbot/views/config_views.xml index d62ff6fa..9deb9ddf 100644 --- a/runbot/views/config_views.xml +++ b/runbot/views/config_views.xml @@ -113,6 +113,10 @@ + + + +
diff --git a/runbot/views/dashboard_views.xml b/runbot/views/dashboard_views.xml index 3e4ab908..c891ea5f 100644 --- a/runbot/views/dashboard_views.xml +++ b/runbot/views/dashboard_views.xml @@ -8,8 +8,15 @@ + + + + + + + @@ -36,6 +43,37 @@ + + runbot.module.form + runbot.module + +
+ + + + + + + + + + + +
+ + + + + runbot.module.tree + runbot.module + + + + + + + + runbot.dashboard.form runbot.dashboard @@ -107,10 +145,16 @@ + + Runbot Dashboards Tiles + runbot.dashboard.tile + tree,form + + - Runbot Teams - runbot.team - tree,form + Runbot teams + runbot.team + tree,form @@ -119,10 +163,10 @@ tree,form - - Runbot Dashboards Tiles - runbot.dashboard.tile - tree,form + + Runbot modules + runbot.module + tree,form diff --git a/runbot/views/menus.xml b/runbot/views/menus.xml index c711be0e..9f2b1950 100644 --- a/runbot/views/menus.xml +++ b/runbot/views/menus.xml @@ -30,6 +30,7 @@ + diff --git a/runbot/views/res_config_settings_views.xml b/runbot/views/res_config_settings_views.xml index ea69a6a8..1a9e5fb1 100644 --- a/runbot/views/res_config_settings_views.xml +++ b/runbot/views/res_config_settings_views.xml @@ -44,6 +44,8 @@
diff --git a/runbot_populate/models/runbot.py b/runbot_populate/models/runbot.py index ed9ab36f..983d4a0c 100644 --- a/runbot_populate/models/runbot.py +++ b/runbot_populate/models/runbot.py @@ -47,7 +47,12 @@ class Runbot(models.AbstractModel): 'head': { 'label': '%s:%s' % (dev_remote.owner, bundle.name), 'repo': {'full_name': '%s/%s' % (dev_remote.owner, dev_remote.repo_name)} - } + }, + 'title': '[IMP] Title', + 'body': 'Body', + 'creator': { + 'login': 'Pr author' + }, } branch = self.env['runbot.branch'].create({ 'remote_id': main_remote.id,