From 8ca6d29a919ea86076c1c60f79219a9035eba56f Mon Sep 17 00:00:00 2001 From: Simone Rubino Date: Mon, 6 Nov 2023 16:43:14 +0100 Subject: [PATCH 1/2] [REF] Maintainers identification --- src/oca_github_bot/manifest.py | 124 +++++++++++------- .../tasks/mention_maintainer.py | 19 +-- tests/test_manifest.py | 47 ++++++- 3 files changed, 119 insertions(+), 71 deletions(-) diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py index 6589b713..bbc02c4c 100644 --- a/src/oca_github_bot/manifest.py +++ b/src/oca_github_bot/manifest.py @@ -7,8 +7,6 @@ import re from typing import Tuple -import requests - from . import config from .github import git_get_current_branch, github_user_can_push from .process import check_call, check_output @@ -70,9 +68,11 @@ def get_manifest_file_name(addon_dir): return None -def get_manifest_path(addon_dir): +def get_manifest_path(addon_dir, cwd=None): + if cwd is None: + cwd = os.getcwd() for manifest_name in MANIFEST_NAMES: - manifest_path = os.path.join(addon_dir, manifest_name) + manifest_path = os.path.join(cwd, addon_dir, manifest_name) if os.path.exists(manifest_path): return manifest_path return None @@ -82,8 +82,8 @@ def parse_manifest(manifest: bytes) -> dict: return ast.literal_eval(manifest.decode("utf-8")) -def get_manifest(addon_dir): - manifest_path = get_manifest_path(addon_dir) +def get_manifest(addon_dir, cwd=None): + manifest_path = get_manifest_path(addon_dir, cwd=cwd) if not manifest_path: raise NoManifestFound(f"no manifest found in {addon_dir}") with open(manifest_path, "rb") as f: @@ -99,13 +99,11 @@ def set_manifest_version(addon_dir, version): f.write(manifest) -def is_maintainer(username, addon_dirs): - for addon_dir in addon_dirs: - try: - manifest = get_manifest(addon_dir) - except NoManifestFound: - return False - maintainers = manifest.get("maintainers", []) +def is_maintainer(username, addon_dirs, main_branches=None, cwd=None): + addon_to_maintainers = get_maintainers( + addon_dirs, main_branches=main_branches, cwd=cwd + ) + for _addon, maintainers in addon_to_maintainers.items(): if username not in maintainers: return False return True @@ -247,46 +245,72 @@ def user_can_push(gh, org, repo, username, addons_dir, target_branch): if other_changes or not modified_addon_dirs: return False # if we are modifying addons only, then the user must be maintainer of - # all of them on the target branch - current_branch = git_get_current_branch(cwd=addons_dir) - try: - check_call(["git", "checkout", target_branch], cwd=addons_dir) - result = is_maintainer(username, modified_addon_dirs) - finally: - check_call(["git", "checkout", current_branch], cwd=addons_dir) + # all of them + main_branches = [target_branch] + config.MAINTAINER_CHECK_ODOO_RELEASES + return is_maintainer( + username, modified_addon_dirs, main_branches=main_branches, cwd=addons_dir + ) - if result: - return True - other_branches = config.MAINTAINER_CHECK_ODOO_RELEASES - if target_branch in other_branches: - other_branches.remove(target_branch) +def get_maintainers_current_branch(addon_dirs, cwd=None): + """Get maintainer for each addon in `addon_dirs`. - return is_maintainer_other_branches( - org, repo, username, modified_addons, other_branches - ) + :return: Dictionary {'addon_dir': } + """ + addon_to_maintainers = dict() + for addon_dir in addon_dirs: + try: + manifest = get_manifest(addon_dir, cwd=cwd) + except NoManifestFound: + maintainers = set() + else: + maintainers = manifest.get("maintainers", set()) + addon_to_maintainers[addon_dir] = maintainers + return addon_to_maintainers -def is_maintainer_other_branches(org, repo, username, modified_addons, other_branches): - for addon in modified_addons: - is_maintainer = False - for branch in other_branches: - manifest_file = ( - "__openerp__.py" if float(branch) < 10.0 else "__manifest__.py" - ) - url = ( - f"https://github.com/{org}/{repo}/raw/{branch}/{addon}/{manifest_file}" - ) - _logger.debug("Looking for maintainers in %s" % url) - r = requests.get( - url, allow_redirects=True, headers={"Cache-Control": "no-cache"} - ) - if r.ok: - manifest = parse_manifest(r.content) - if username in manifest.get("maintainers", []): - is_maintainer = True - break +def get_maintainers(addon_dirs, main_branches=None, cwd=None): + if cwd is None: + cwd = os.getcwd() + current_branch = git_get_current_branch(cwd=cwd) + if main_branches is None: + main_branches = [current_branch] - if not is_maintainer: - return False - return True + branch_to_addon_to_maintainers = {} + for branch in main_branches: + try: + check_call(["git", "checkout", branch], cwd=cwd) + branch_to_addon_to_maintainers[branch] = get_maintainers_current_branch( + addon_dirs, + cwd=cwd, + ) + finally: + check_call(["git", "checkout", current_branch], cwd=cwd) + + # Transform + # { + # "16.0": { + # "date_range": { + # "user_1", + # }, + # }, + # "14.0": { + # "date_range": { + # "user_2", + # }, + # } + # } + # to + # { + # "date_range": { + # "user_1", + # "user_2", + # }, + # } + + result = {} + for _branch, addon_to_maintainers in branch_to_addon_to_maintainers.items(): + for addon, maintainers in addon_to_maintainers.items(): + addon_maintainers = result.setdefault(addon, set()) + addon_maintainers.update(maintainers) + return result diff --git a/src/oca_github_bot/tasks/mention_maintainer.py b/src/oca_github_bot/tasks/mention_maintainer.py index 4b4649e1..04a4d610 100644 --- a/src/oca_github_bot/tasks/mention_maintainer.py +++ b/src/oca_github_bot/tasks/mention_maintainer.py @@ -5,7 +5,7 @@ from ..config import switchable from ..manifest import ( addon_dirs_in, - get_manifest, + get_maintainers, git_modified_addon_dirs, is_addon_dir, ) @@ -24,7 +24,10 @@ def mention_maintainer(org, repo, pr, dry_run=False): with github.temporary_clone(org, repo, target_branch) as clonedir: # Get maintainers existing before the PR changes addon_dirs = addon_dirs_in(clonedir, installable_only=True) - maintainers_dict = get_maintainers(addon_dirs) + maintainers_dict = get_maintainers( + addon_dirs, + cwd=clonedir, + ) # Get list of addons modified in the PR. pr_branch = f"tmp-pr-{pr}" @@ -81,15 +84,3 @@ def get_adopt_mention(pr_opener): if config.ADOPT_AN_ADDON_MENTION: return config.ADOPT_AN_ADDON_MENTION.format(pr_opener=pr_opener) return None - - -def get_maintainers(addon_dirs): - """Get maintainer for each addon in `addon_dirs`. - - :return: Dictionary {'addon_dir': } - """ - addon_maintainers_dict = dict() - for addon_dir in addon_dirs: - maintainers = get_manifest(addon_dir).get("maintainers", []) - addon_maintainers_dict.setdefault(addon_dir, maintainers) - return addon_maintainers_dict diff --git a/tests/test_manifest.py b/tests/test_manifest.py index b160dcf9..a953bec3 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -1,6 +1,6 @@ # Copyright (c) ACSONE SA/NV 2018 # Distributed under the MIT License (http://opensource.org/licenses/MIT). - +import json import subprocess import pytest @@ -20,7 +20,6 @@ is_addon_dir, is_addons_dir, is_maintainer, - is_maintainer_other_branches, set_manifest_version, ) @@ -242,10 +241,44 @@ def test_is_maintainer(tmp_path): assert not is_maintainer("u1", [tmp_path / "not_an_addon"]) -def test_is_maintainer_other_branches(): - assert is_maintainer_other_branches( - "OCA", "mis-builder", "sbidoul", {"mis_builder"}, ["12.0"] +def test_is_maintainer_other_branches(git_clone): + addon_name = "addon_maintainer_other_branches" + + # create `addon_name` on `other_branch` with maintainer `maintainer` + maintainer = "maintainer" + other_branch = "other_branch_maintainer" + subprocess.check_call(["git", "checkout", "-b", other_branch], cwd=git_clone) + addon1_dir = git_clone / addon_name + addon1_dir.mkdir(exist_ok=True) + manifest = { + "name": addon_name, + "maintainers": [ + maintainer, + ], + } + (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest)) + subprocess.check_call(["git", "add", addon_name], cwd=git_clone) + subprocess.check_call( + ["git", "commit", "-m", "[BOT] Add with maintainer"], cwd=git_clone + ) + + # create `addon_name` on current branch with no maintainers + branch = "current_branch_no_maintainer" + subprocess.check_call(["git", "checkout", "-b", branch], cwd=git_clone) + addon1_dir = git_clone / addon_name + addon1_dir.mkdir(exist_ok=True) + manifest = { + "name": addon_name, + } + (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest)) + subprocess.check_call(["git", "add", addon_name], cwd=git_clone) + subprocess.check_call( + ["git", "commit", "-m", "[BOT] Add without maintainer"], cwd=git_clone + ) + + assert is_maintainer( + maintainer, {addon_name}, main_branches=[other_branch], cwd=git_clone ) - assert not is_maintainer_other_branches( - "OCA", "mis-builder", "fpdoo", {"mis_builder"}, ["12.0"] + assert not is_maintainer( + "fpdoo", {addon_name}, main_branches=[other_branch], cwd=git_clone ) From f26a5681f796fecbd7571153c9c68bc5302b661c Mon Sep 17 00:00:00 2001 From: Simone Rubino Date: Tue, 7 Nov 2023 09:58:14 +0100 Subject: [PATCH 2/2] [IMP] Mention maintainers of other branches --- src/oca_github_bot/manifest.py | 6 +- .../tasks/mention_maintainer.py | 26 ++++++- tests/test_mention_maintainer.py | 68 +++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py index bbc02c4c..efe155bb 100644 --- a/src/oca_github_bot/manifest.py +++ b/src/oca_github_bot/manifest.py @@ -36,13 +36,13 @@ def is_addons_dir(addons_dir, installable_only=False): return any(addon_dirs_in(addons_dir, installable_only)) -def is_addon_dir(addon_dir, installable_only=False): +def is_addon_dir(addon_dir, installable_only=False, cwd=None): """Test if a directory contains an Odoo addon.""" if not installable_only: - return bool(get_manifest_path(addon_dir)) + return bool(get_manifest_path(addon_dir, cwd=cwd)) else: try: - return get_manifest(addon_dir).get("installable", True) + return get_manifest(addon_dir, cwd=cwd).get("installable", True) except NoManifestFound: return False diff --git a/src/oca_github_bot/tasks/mention_maintainer.py b/src/oca_github_bot/tasks/mention_maintainer.py index 04a4d610..2cdd2217 100644 --- a/src/oca_github_bot/tasks/mention_maintainer.py +++ b/src/oca_github_bot/tasks/mention_maintainer.py @@ -24,8 +24,11 @@ def mention_maintainer(org, repo, pr, dry_run=False): with github.temporary_clone(org, repo, target_branch) as clonedir: # Get maintainers existing before the PR changes addon_dirs = addon_dirs_in(clonedir, installable_only=True) + other_branches = config.MAINTAINER_CHECK_ODOO_RELEASES + main_branches = [target_branch] + other_branches maintainers_dict = get_maintainers( addon_dirs, + main_branches=main_branches, cwd=clonedir, ) @@ -41,9 +44,30 @@ def mention_maintainer(org, repo, pr, dry_run=False): # Remove not installable addons # (case where an addon becomes no more installable). modified_addon_dirs = [ - d for d in modified_addon_dirs if is_addon_dir(d, installable_only=True) + d + for d in modified_addon_dirs + if is_addon_dir(d, installable_only=True, cwd=clonedir) ] + # Get maintainer of new addons in other branches + if other_branches: + new_addons = { + addon for addon in modified_addon_dirs if addon not in addon_dirs + } + if new_addons: + new_maintainers_dict = get_maintainers( + new_addons, + main_branches=other_branches, + cwd=clonedir, + ) + maintainers_dict.update( + { + new_addon: new_maintainers_dict[new_addon] + for new_addon in new_addons + } + ) + modified_addon_dirs.extend(new_addons) + modified_addons_maintainers = set() for modified_addon in modified_addon_dirs: addon_maintainers = maintainers_dict.get(modified_addon, list()) diff --git a/tests/test_mention_maintainer.py b/tests/test_mention_maintainer.py index ddf93661..f79d12d0 100644 --- a/tests/test_mention_maintainer.py +++ b/tests/test_mention_maintainer.py @@ -1,9 +1,14 @@ # Copyright 2019 Simone Rubino - Agile Business Group # Distributed under the MIT License (http://opensource.org/licenses/MIT). + +import json import shutil +import subprocess import pytest +from oca_github_bot import config +from oca_github_bot.github import git_get_current_branch from oca_github_bot.tasks.mention_maintainer import mention_maintainer from .common import make_addon, set_config @@ -13,6 +18,8 @@ def test_maintainer_mentioned(git_clone, mocker): github_mock = mocker.patch("oca_github_bot.tasks.mention_maintainer.github") github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) + pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) addon_name = "addon1" addon_dir = make_addon(git_clone, addon_name, maintainers=["themaintainer"]) @@ -33,6 +40,8 @@ def test_added_maintainer_not_mentioned(git_clone, mocker): """Only maintainers existing before the PR will be mentioned.""" github_mock = mocker.patch("oca_github_bot.tasks.mention_maintainer.github") github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) + pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) addon_name = "addon1" pre_pr_addon = make_addon(git_clone, addon_name, maintainers=["themaintainer"]) @@ -68,6 +77,8 @@ def pr_edited_addon(_args, **_kwargs): def test_multi_maintainer_one_mention(git_clone, mocker): github_mock = mocker.patch("oca_github_bot.tasks.mention_maintainer.github") github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) + pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) addon_dirs = list() addon_names = ["addon1", "addon2"] @@ -95,6 +106,7 @@ def test_pr_by_maintainer_no_mention(git_clone, mocker): github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request pr_mock.return_value.user.login = themaintainer + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) addon_dirs = list() addon_names = ["addon1", "addon2"] @@ -116,6 +128,8 @@ def test_pr_by_maintainer_no_mention(git_clone, mocker): def test_no_maintainer_adopt_module(git_clone, mocker): github_mock = mocker.patch("oca_github_bot.tasks.mention_maintainer.github") github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) + pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) addon_name = "addon1" addon_dir = make_addon(git_clone, addon_name) @@ -130,3 +144,57 @@ def test_no_maintainer_adopt_module(git_clone, mocker): mention_maintainer("org", "repo", "pr") github_mock.gh_call.assert_called_once() + + +@pytest.mark.vcr() +def test_mention_maintainer_other_branches(git_clone, mocker): + addon_name = "addon_maintainer_other_branches" + + # create `addon_name` on `other_branch` with maintainer `maintainer` + maintainer = "maintainer" + maintainer_branch = "other_branch_maintainer" + subprocess.check_call(["git", "checkout", "-b", maintainer_branch], cwd=git_clone) + addon1_dir = git_clone / addon_name + addon1_dir.mkdir(exist_ok=True) + manifest = { + "name": addon_name, + "maintainers": [ + maintainer, + ], + } + (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest)) + subprocess.check_call(["git", "add", addon_name], cwd=git_clone) + subprocess.check_call( + ["git", "commit", "-m", "[BOT] Add with maintainer"], cwd=git_clone + ) + + # create `addon_name` on current branch with no maintainers + branch = "current_branch_no_maintainer" + subprocess.check_call(["git", "checkout", "-b", branch], cwd=git_clone) + addon1_dir = git_clone / addon_name + addon1_dir.mkdir(exist_ok=True) + manifest = { + "name": addon_name, + } + (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest)) + subprocess.check_call(["git", "add", addon_name], cwd=git_clone) + subprocess.check_call( + ["git", "commit", "-m", "[BOT] Add without maintainer"], cwd=git_clone + ) + + github_mock = mocker.patch("oca_github_bot.tasks.mention_maintainer.github") + github_mock.temporary_clone.return_value.__enter__.return_value = str(git_clone) + pr_mock = github_mock.login.return_value.__enter__.return_value.pull_request + pr_mock.return_value.base.ref = git_get_current_branch(git_clone) + + modified_addons_mock = mocker.patch( + "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs" + ) + modified_addons_mock.return_value = [git_clone / addon_name], False, {addon_name} + mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call") + + config.MAINTAINER_CHECK_ODOO_RELEASES = [maintainer_branch] + mention_maintainer("org", "repo", "pr") + config.MAINTAINER_CHECK_ODOO_RELEASES = [] + + assert "@" + maintainer in github_mock.gh_call.mock_calls[0][1][1]