Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] migration_pr_check: Remind migration guidelines #267

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions environment.sample
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,6 @@ OCABOT_TWINE_REPOSITORIES="[('https://pypi.org/simple','https://upload.pypi.org/
# List of branches the bot will check to verify if user is the maintainer
# of module(s)
MAINTAINER_CHECK_ODOO_RELEASES=8.0,9.0,10.0,11.0,12.0,13.0,14.0,15.0

# Reminder of migration guidelines.
#MIGRATION_GUIDELINES_REMINDER=
17 changes: 17 additions & 0 deletions src/oca_github_bot/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,20 @@ def func_wrapper(*args, **kwargs):
# branches. For previous versions, the setuptools_odoo task is run and generates
# setup.py instead of pyproject.toml.
GEN_PYPROJECT_MIN_VERSION = os.environ.get("GEN_PYPROJECT_MIN_VERSION", "17.0")

MIGRATION_GUIDELINES_REMINDER = os.environ.get(
"MIGRATION_GUIDELINES_REMINDER",
"""Thanks for the contribution!
This appears to be a migration, \
so here you have a gentle reminder about the migration guidelines.

Please preserve commit history following technical method \
explained in https://github.com/OCA/maintainer-tools/wiki/#migration.
If the jump is between several versions, \
you have to modify the source branch in the main command \
to accommodate it to this circumstance.

You can also take a look on the project \
https://github.com/OCA/odoo-module-migrator/ to make easier migration.
""",
)
1 change: 1 addition & 0 deletions src/oca_github_bot/tasks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
main_branch_bot,
mention_maintainer,
migration_issue_bot,
migration_pr_check,
tag_approved,
tag_needs_review,
tag_ready_to_merge,
Expand Down
11 changes: 9 additions & 2 deletions src/oca_github_bot/tasks/migration_issue_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,18 @@
from ..utils import hide_secrets


def _create_or_find_branch_milestone(gh_repo, branch):
def _find_branch_milestone(gh_repo, branch):
for milestone in gh_repo.milestones():
if milestone.title == branch:
return milestone
return gh_repo.create_milestone(branch)
return None


def _create_or_find_branch_milestone(gh_repo, branch):
branch_milestone = _find_branch_milestone(gh_repo, branch)
if not branch_milestone:
branch_milestone = gh_repo.create_milestone(branch)

Check warning on line 25 in src/oca_github_bot/tasks/migration_issue_bot.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/tasks/migration_issue_bot.py#L25

Added line #L25 was not covered by tests
return branch_milestone


def _find_issue(gh_repo, milestone, target_branch):
Expand Down
78 changes: 78 additions & 0 deletions src/oca_github_bot/tasks/migration_pr_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Copyright 2022 Simone Rubino - TAKOBI
# Distributed under the MIT License (http://opensource.org/licenses/MIT).
import os.path
import re

from .. import config, github
from ..config import switchable
from ..manifest import addon_dirs_in
from ..process import check_call
from ..queue import getLogger, task
from .migration_issue_bot import _find_branch_milestone, _find_issue

_logger = getLogger(__name__)


def _get_added_modules(org, repo, gh_pr):
target_branch = gh_pr.base.ref
with github.temporary_clone(org, repo, target_branch) as clonedir:
# We need a list now because otherwise modules
# are yielded when the directory is already changed
existing_addons_paths = list(addon_dirs_in(clonedir, installable_only=True))

pr_branch = f"tmp-pr-{gh_pr.number}"
check_call(
["git", "fetch", "origin", f"refs/pull/{gh_pr.number}/head:{pr_branch}"],
cwd=clonedir,
)
check_call(["git", "checkout", pr_branch], cwd=clonedir)
pr_addons_paths = addon_dirs_in(clonedir, installable_only=True)

new_addons_paths = {
addon_path
for addon_path in pr_addons_paths
if addon_path not in existing_addons_paths
}
return new_addons_paths


def is_migration_pr(org, repo, pr):
"""
Determine if the PR is a migration.
"""
with github.login() as gh:
gh_repo = gh.repository(org, repo)
gh_pr = gh_repo.pull_request(pr)
target_branch = gh_pr.base.ref
milestone = _find_branch_milestone(gh_repo, target_branch)
gh_migration_issue = _find_issue(gh_repo, milestone, target_branch)

# The PR is mentioned in the migration issue
pr_regex = re.compile(rf"#({gh_pr.number})")
found_pr = pr_regex.findall(gh_migration_issue.body)
if found_pr:
return True

Check warning on line 54 in src/oca_github_bot/tasks/migration_pr_check.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/tasks/migration_pr_check.py#L54

Added line #L54 was not covered by tests

# The added module is mentioned in the migration issue
new_addons_paths = _get_added_modules(org, repo, gh_pr)
new_addons = map(os.path.basename, new_addons_paths)
for addon in new_addons:
module_regex = re.compile(rf"- \[[ x]] {addon}")
found_module = module_regex.search(gh_migration_issue.body)
if found_module:
return True

# The Title of the PR contains [MIG]
pr_title = gh_pr.title
if "[MIG]" in pr_title:
return True
return False


@task()
@switchable("comment_migration_guidelines")
def comment_migration_guidelines(org, repo, pr, dry_run=False):
migration_reminder = config.MIGRATION_GUIDELINES_REMINDER

Check warning on line 75 in src/oca_github_bot/tasks/migration_pr_check.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/tasks/migration_pr_check.py#L75

Added line #L75 was not covered by tests
with github.login() as gh:
gh_pr = gh.pull_request(org, repo, pr)
return github.gh_call(gh_pr.create_comment, migration_reminder)

Check warning on line 78 in src/oca_github_bot/tasks/migration_pr_check.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/tasks/migration_pr_check.py#L77-L78

Added lines #L77 - L78 were not covered by tests
1 change: 1 addition & 0 deletions src/oca_github_bot/webhooks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
on_pr_green_label_needs_review,
on_pr_open_label_new_contributor,
on_pr_open_mention_maintainer,
on_pr_open_migration_check,
on_pr_review,
on_push_to_main_branch,
on_status_merge_bot,
Expand Down
21 changes: 21 additions & 0 deletions src/oca_github_bot/webhooks/on_pr_open_migration_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Copyright 2022 Simone Rubino - TAKOBI
# Distributed under the MIT License (http://opensource.org/licenses/MIT).

import logging

from ..router import router
from ..tasks.migration_pr_check import comment_migration_guidelines, is_migration_pr

_logger = logging.getLogger(__name__)


@router.register("pull_request", action="opened")
async def on_pr_open_migration_check(event, *args, **kwargs):
"""
Whenever a PR is opened, if it is a migration PR,
remind the migration guidelines.
"""
org, repo = event.data["repository"]["full_name"].split("/")
pr = event.data["pull_request"]["number"]

Check warning on line 19 in src/oca_github_bot/webhooks/on_pr_open_migration_check.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/webhooks/on_pr_open_migration_check.py#L18-L19

Added lines #L18 - L19 were not covered by tests
if is_migration_pr(org, repo, pr):
comment_migration_guidelines.delay(org, repo, pr)

Check warning on line 21 in src/oca_github_bot/webhooks/on_pr_open_migration_check.py

View check run for this annotation

Codecov / codecov/patch

src/oca_github_bot/webhooks/on_pr_open_migration_check.py#L21

Added line #L21 was not covered by tests
158 changes: 158 additions & 0 deletions tests/test_migration_pr_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# Copyright 2022 Simone Rubino - TAKOBI
# Distributed under the MIT License (http://opensource.org/licenses/MIT).

import pytest

from oca_github_bot.tasks.migration_pr_check import is_migration_pr

MIGRATION_PR_PATH = "oca_github_bot.tasks.migration_pr_check"


def _get_addons_gen_mock(pr_new_modules=None):
"""
Return a callable that returns a list of modules.
The list contains `pr_new_modules` only after first call.
"""
if pr_new_modules is None:
pr_new_modules = list()

class AddonsGenMock:
def __init__(self):
self.addons_gen_calls = 0

def __call__(self, *args, **kwargs):
# First time, only the existing addons are returned
existing_addons = ["existing_addon"]
if self.addons_gen_calls > 0:
# After that, return also `pr_new_modules`
if pr_new_modules:
existing_addons.extend(pr_new_modules)
self.addons_gen_calls += 1
return existing_addons

return AddonsGenMock()


@pytest.mark.vcr()
def test_no_new_module(mocker):
"""
If a PR does not add a new module, then it is not a migration.
"""
mocker.patch("%s.github" % MIGRATION_PR_PATH)
mocker.patch("%s.check_call" % MIGRATION_PR_PATH)

migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH)
migration_issue.return_value.body = "Migration Issue Body"

addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH)
addons_gen.side_effect = _get_addons_gen_mock()

is_migration = is_migration_pr("org", "repo", "pr")
assert not is_migration


@pytest.mark.vcr()
def test_new_module_no_migration(mocker):
"""
If a PR adds a new module but the module is not in the migration issue,
then it is not a migration.
"""
mocker.patch("%s.github" % MIGRATION_PR_PATH)
mocker.patch("%s.check_call" % MIGRATION_PR_PATH)

migration_issue_body = """
Modules to migrate:
- [ ] a_module
"""
migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH)
migration_issue.return_value.body = migration_issue_body

addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH)
addons_gen.side_effect = _get_addons_gen_mock()

is_migration = is_migration_pr("org", "repo", "pr")
assert not is_migration


@pytest.mark.vcr()
def test_new_module_migration(mocker):
"""
If a PR adds a new module and the module is in the migration issue,
then it is a migration.
"""
mocker.patch("%s.github" % MIGRATION_PR_PATH)
mocker.patch("%s.check_call" % MIGRATION_PR_PATH)

addon_name = "migrated_module"
migration_issue_body = f"""
Modules to migrate:
- [ ] {addon_name}
"""
migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH)
migration_issue.return_value.body = migration_issue_body

addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH)
addons_gen.side_effect = _get_addons_gen_mock([addon_name])

is_migration = is_migration_pr("org", "repo", "pr")
assert is_migration


@pytest.mark.vcr()
def test_migration_comment(mocker):
"""
If a PR adds a new module and it is in the migration issue,
then it is a migration.
"""
github_mock = mocker.patch("%s.github" % MIGRATION_PR_PATH)
mocker.patch("%s.check_call" % MIGRATION_PR_PATH)

addon_name = "migrated_module"
migration_issue_body = f"""
Modules to migrate:
- [ ] {addon_name}
"""
migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH)
migration_issue.return_value.body = migration_issue_body

addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH)
addons_gen.side_effect = _get_addons_gen_mock([addon_name])

gh_context = mocker.MagicMock()
github_mock_login_cm = github_mock.login.return_value
github_mock_login_cm.__enter__.return_value = gh_context

is_migration = is_migration_pr("org", "repo", "pr")
assert is_migration


@pytest.mark.vcr()
def test_pr_title(mocker):
"""
If a PR has [MIG] in its Title,
then it is a migration.
"""
github_mock = mocker.patch("%s.github" % MIGRATION_PR_PATH)
mocker.patch("%s.check_call" % MIGRATION_PR_PATH)

migration_issue_body = """
Modules to migrate
"""
migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH)
migration_issue.return_value.body = migration_issue_body

addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH)
addons_gen.side_effect = _get_addons_gen_mock(["another_module"])

pr_title = "[MIG] migrated_module"
gh_pr = mocker.MagicMock()
gh_pr.title = pr_title
gh_repo = mocker.MagicMock()
gh_repo.pull_request.return_value = gh_pr
gh_context = mocker.MagicMock()
gh_context.repository.return_value = gh_repo
github_mock_login_cm = github_mock.login.return_value
github_mock_login_cm.__enter__.return_value = gh_context

is_migration = is_migration_pr("org", "repo", "pr")
assert is_migration
Loading