NextERP/runbot
5
0
Fork 0
mirror of https://github.com/odoo/runbot.git synced 2025-03-15 23:45:44 +07:00
Code Issues Actions Packages Projects Releases Wiki Activity

[ADD] forwardport: support for branch filtering

Browse Source 
Add handling of branch filtering to the forwardport module:

* don't forward port (and trigger an error) when trying to port
  PRs to different next targets
* otherwise port normally

e.g. given a project with repos A and B and branches a, b and c, with
branch b being excluded from repo B:

* a PR merged into A.a will be forward-ported to A.b and A.c
* a PR merged into B.a will be forward-ported to B.c (skipping the
  excluded B.b)
* a PR set merged into (A.a, B.a) will *not* be forward-ported, and a
  message will be posted to each PR denoting the incompatibility
This commit is contained in:
Xavier Morel 2020-01-22 15:41:42 +01:00 committed by xmo-odoo
parent dd22f687bf
commit 60574d6fe2
2 changed files with 236 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
forwardport/models/project.py
View File

@ -11,6 +11,7 @@ means PR creation is trickier (as mergebot assumes opened event will always
lead to PR creation but fpbot wants to attach meaning to the PR when setting
it up), ...
"""
import ast
import base64
import contextlib
import datetime
@ -122,11 +123,11 @@ class Branch(models.Model):
b.fp_enabled = b.active and b.fp_target
# FIXME: this should be per-project...
def _forward_port_ordered(self):
def _forward_port_ordered(self, domain=[]):
""" Returns all branches in forward port order (from the lowest to
the highest — usually master)
"""
return self.search([], order=self._forward_port_ordering())
return self.search(domain, order=self._forward_port_ordering())
def _forward_port_ordering(self):
return ','.join(
@ -352,24 +353,6 @@ class PullRequests(models.Model):
})
return failed
def _forward_port_sequence(self):
# risk: we assume disabled branches are still at the correct location
# in the FP sequence (in case a PR was merged to a branch which is now
# disabled, could happen right around the end of the support window)
# (or maybe we can ignore this entirely and assume all relevant
# branches are active?)
fp_complete = self.env['runbot_merge.branch'].with_context(active_test=False)._forward_port_ordered()
candidates = iter(fp_complete)
for b in candidates:
if b == self.target:
break
# the candidates iterator is just past the current branch's target
for target in candidates:
if target.fp_enabled:
yield target
if target == self.limit_id:
break
def _find_next_target(self, reference):
""" Finds the branch between target and limit_id which follows
reference
@ -378,7 +361,9 @@ class PullRequests(models.Model):
return
# NOTE: assumes even disabled branches are properly sequenced, would
# probably be a good idea to have the FP view show all branches
branches = list(self.env['runbot_merge.branch'].with_context(active_test=False)._forward_port_ordered())
branches = list(self.env['runbot_merge.branch']
.with_context(active_test=False)
._forward_port_ordered(ast.literal_eval(self.repository.branch_filter)))
# get all branches between max(root.target, ref.target) (excluded) and limit (included)
from_ = max(branches.index(self.target), branches.index(reference.target))
@ -446,7 +431,8 @@ class PullRequests(models.Model):
ref = self[0]
base = ref.source_id or ref._get_root()
target = base._find_next_target(ref)
all_targets = [(p.source_id or p._get_root())._find_next_target(p) for p in self]
target = all_targets[0]
if target is None:
_logger.info(
"Will not forward-port %s: no next target",
@ -454,6 +440,32 @@ class PullRequests(models.Model):
)
return # QUESTION: do the prs need to be updated?
# check if all PRs in the batch have the same "next target" , bail if
# that's not the case as it doesn't make sense for forward one PR from
# a to b and a linked pr from a to c
different_target = next((t for t in all_targets if t != target), None)
if different_target:
different_pr = next(p for p, t in zip(self, all_targets) if t == different_target)
for pr, t in zip(self, all_targets):
linked, other = different_pr, different_target
if t != target:
linked, other = ref, target
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': pr.repository.id,
'pull_request': pr.number,
'token_field': 'fp_github_token',
'message': "This pull request can not be forward ported: "
"next branch is %r but linked pull request %s "
"has a next branch %r." % (
t.name, linked.display_name, other.name
)
})
_logger.warning(
"Cancelling forward-port of %s: found different next branches (%s)",
self, all_targets
)
return
proj = self.mapped('target.project_id')
if not proj.fp_github_token:
_logger.warning(

203
forwardport/tests/test_weird.py
View File

@ -20,7 +20,7 @@ def make_basic(env, config, make_repo, *, fp_token, fp_remote):
Projects = env['runbot_merge.project']
project = Projects.search([('name', '=', 'myproject')])
if not project:
project = env['runbot_merge.project'].create({
project = Projects.create({
'name': 'myproject',
'github_token': config['github']['token'],
'github_prefix': 'hansen',
@ -204,3 +204,204 @@ def test_failed_staging(env, config, make_repo):
assert pr3_head.to_check, "check that the commit was updated as to process"
env.run_crons()
assert not pr3_head.to_check, "check that the commit was processed"
class TestNotAllBranches:
""" Check that forward-ports don't behave completely insanely when not all
branches are supported on all repositories.
repo A branches a -> b -> c
a0 -> a1 -> a2 branch a
`-> a11 -> a22 branch b
`-> a111 branch c
repo B branches a -> c
b0 -> b1 -> b2 branch a
|
`-> b000 branch c
"""
@pytest.fixture
def repos(self, env, config, make_repo):
project = env['runbot_merge.project'].create({
'name': 'proj',
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'fp_github_token': config['github']['token'],
'branch_ids': [
(0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}),
(0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}),
(0, 0, {'name': 'c', 'fp_sequence': 0, 'fp_target': True}),
]
})
a = make_repo('A')
with a:
_, a_, _ = a.make_commits(
None,
Commit('a0', tree={'a': '0'}),
Commit('a1', tree={'a': '1'}),
Commit('a2', tree={'a': '2'}),
ref='heads/a'
)
b_, _ = a.make_commits(
a_,
Commit('a11', tree={'b': '11'}),
Commit('a22', tree={'b': '22'}),
ref='heads/b'
)
a.make_commits(b_, Commit('a111', tree={'c': '111'}), ref='heads/c')
a_dev = a.fork()
b = make_repo('B')
with b:
_, _a, _ = b.make_commits(
None,
Commit('b0', tree={'a': 'x'}),
Commit('b1', tree={'a': 'y'}),
Commit('b2', tree={'a': 'z'}),
ref='heads/a'
)
b.make_commits(_a, Commit('b000', tree={'c': 'x'}), ref='heads/c')
b_dev = b.fork()
project.write({
'repo_ids': [
(0, 0, {
'name': a.name,
'required_statuses': 'ci/runbot',
'fp_remote_target': a_dev.name,
}),
(0, 0, {
'name': b.name,
'required_statuses': 'ci/runbot',
'fp_remote_target': b_dev.name,
'branch_filter': '[("name", "in", ["a", "c"])]',
})
]
})
return project, a, a_dev, b, b_dev
def test_single_first(self, env, repos, config):
""" A merge in A.a should be forward-ported to A.b and A.c
"""
project, a, a_dev, b, _ = repos
with a, a_dev:
[c] = a_dev.make_commits('a', Commit('pr', tree={'pr': '1'}), ref='heads/change')
pr = a.make_pr(target='a', title="a pr", head=a_dev.owner + ':change')
a.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
p = env['runbot_merge.pull_requests'].search([('repository.name', '=', a.name), ('number', '=', pr.number)])
env.run_crons()
assert p.staging_id
with a, b:
for repo in a, b:
repo.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
a_head = a.commit('a')
assert a_head.message.startswith('pr\n\n')
assert a.read_tree(a_head) == {'a': '2', 'pr': '1'}
pr0, pr1 = env['runbot_merge.pull_requests'].search([], order='number')
with a:
a.post_status(pr1.head, 'success', 'ci/runbot')
env.run_crons()
pr0, pr1, pr2 = env['runbot_merge.pull_requests'].search([], order='number')
with a:
a.post_status(pr2.head, 'success', 'ci/runbot')
a.get_pr(pr2.number).post_comment(
'%s r+' % project.fp_github_name,
config['role_reviewer']['token'])
env.run_crons()
assert pr1.staging_id
assert pr2.staging_id
with a, b:
a.post_status('staging.b', 'success', 'ci/runbot')
a.post_status('staging.c', 'success', 'ci/runbot')
b.post_status('staging.c', 'success', 'ci/runbot')
env.run_crons()
assert pr0.state == 'merged'
assert pr1.state == 'merged'
assert pr2.state == 'merged'
assert a.read_tree(a.commit('b')) == {'a': '1', 'b': '22', 'pr': '1'}
assert a.read_tree(a.commit('c')) == {'a': '1', 'b': '11', 'c': '111', 'pr': '1'}
def test_single_second(self, env, repos, config):
""" A merge in B.a should "skip ahead" to B.c
"""
project, a, _, b, b_dev = repos
with b, b_dev:
[c] = b_dev.make_commits('a', Commit('pr', tree={'pr': '1'}), ref='heads/change')
pr = b.make_pr(target='a', title="a pr", head=b_dev.owner + ':change')
b.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with a, b:
a.post_status('staging.a', 'success', 'ci/runbot')
b.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
assert b.read_tree(b.commit('a')) == {'a': 'z', 'pr': '1'}
pr0, pr1 = env['runbot_merge.pull_requests'].search([], order='number')
with b:
b.post_status(pr1.head, 'success', 'ci/runbot')
b.get_pr(pr1.number).post_comment(
'%s r+' % project.fp_github_name,
config['role_reviewer']['token'])
env.run_crons()
with a, b:
a.post_status('staging.c', 'success', 'ci/runbot')
b.post_status('staging.c', 'success', 'ci/runbot')
env.run_crons()
assert pr0.state == 'merged'
assert pr1.state == 'merged'
assert b.read_tree(b.commit('c')) == {'a': 'y', 'c': 'x', 'pr': '1'}
def test_both_first(self, env, repos, config, users):
""" A merge in A.a, B.a should... not be forward-ported at all?
"""
project, a, a_dev, b, b_dev = repos
with a, a_dev:
[c_a] = a_dev.make_commits('a', Commit('pr a', tree={'pr': 'a'}), ref='heads/change')
pr_a = a.make_pr(target='a', title='a pr', head=a_dev.owner + ':change')
a.post_status(c_a, 'success', 'ci/runbot')
pr_a.post_comment('hansen r+', config['role_reviewer']['token'])
with b, b_dev:
[c_b] = b_dev.make_commits('a', Commit('pr b', tree={'pr': 'b'}), ref='heads/change')
pr_b = b.make_pr(target='a', title='b pr', head=b_dev.owner + ':change')
b.post_status(c_b, 'success', 'ci/runbot')
pr_b.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with a, b:
for repo in a, b:
repo.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
pr_a_id = env['runbot_merge.pull_requests'].search([
('repository.name', '=', a.name),
('number', '=', pr_a.number),
])
pr_b_id = env['runbot_merge.pull_requests'].search([
('repository.name', '=', b.name),
('number', '=', pr_b.number)
])
assert pr_a_id.state == pr_b_id.state == 'merged'
assert env['runbot_merge.pull_requests'].search([]) == pr_a_id | pr_b_id
# should have refused to create a forward port because the PRs have
# different next target
assert pr_a.comments == [
(users['reviewer'], 'hansen r+'),
(users['user'], "This pull request can not be forward ported: next "
"branch is 'b' but linked pull request %s#%d has a"
" next branch 'c'." % (b.name, pr_b.number)
)
]
assert pr_b.comments == [
(users['reviewer'], 'hansen r+'),
(users['user'], "This pull request can not be forward ported: next "
"branch is 'c' but linked pull request %s#%d has a"
" next branch 'b'." % (a.name, pr_a.number)
)
]