[IMP] forwardport: move limits tests into their own file

test_simple is starting to get pretty big and we'll need some more
tests for #213
This commit is contained in:
Xavier Morel 2019-09-30 15:18:44 +02:00 committed by xmo-odoo
parent e49b112447
commit 01ff6d13db
3 changed files with 270 additions and 258 deletions

View File

@ -0,0 +1,210 @@
# -*- coding: utf-8 -*-
import collections
import pytest
from utils import *
Description = collections.namedtuple('Restriction', 'source limit')
def test_configure(env, config, make_repo):
""" Checks that configuring an FP limit on a PR is respected
* limits to not the latest
* limits to the current target (= no FP)
* limits to an earlier branch (???)
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
descriptions = [
Description(source='a', limit='b'),
Description(source='b', limit='b'),
Description(source='b', limit='a'),
]
originals = []
with prod:
for i, descr in enumerate(descriptions):
[c] = prod.make_commits(
descr.source, Commit('c %d' % i, tree={str(i): str(i)}),
ref='heads/branch%d' % i,
)
pr = prod.make_pr(target=descr.source, head='branch%d'%i)
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to %s' % (bot_name, descr.limit), config['role_reviewer']['token'])
originals.append(pr.number)
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
env.run_crons()
# should have created a single FP PR for 0, none for 1 and none for 2
prs = env['runbot_merge.pull_requests'].search([], order='number')
assert len(prs) == 4
assert prs[-1].parent_id == prs[0]
assert prs[0].number == originals[0]
assert prs[1].number == originals[1]
assert prs[2].number == originals[2]
def test_self_disabled(env, config, make_repo):
""" Allow setting target as limit even if it's disabled
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
branch_a.fp_target = False
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head='mybranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to a' % bot_name, config['role_reviewer']['token'])
env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)])
assert pr_id.limit_id == branch_a
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
def test_ignore(env, config, make_repo):
""" Provide an "ignore" command which is equivalent to setting the limit
to target
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head='mybranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s ignore' % bot_name, config['role_reviewer']['token'])
env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)])
assert pr_id.limit_id == branch_a
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
@pytest.mark.parametrize('enabled', ['active', 'fp_target'])
def test_disable(env, config, make_repo, users, enabled):
""" Checks behaviour if the limit target is disabled:
* disable target while FP is ongoing -> skip over (and stop there so no FP)
* forward-port over a disabled branch
* request a disabled target as limit
Disabling (with respect to forward ports) can be performed by marking the
branch as !active (which also affects mergebot operations), or as
!fp_target (won't be forward-ported to).
"""
prod, other = make_basic(env, config, make_repo)
project = env['runbot_merge.project'].search([])
bot_name = project.fp_github_name
with prod:
[c] = prod.make_commits('a', Commit('c 0', tree={'0': '0'}), ref='heads/branch0')
pr = prod.make_pr(target='a', head='branch0')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to b' % bot_name, config['role_reviewer']['token'])
[c] = prod.make_commits('a', Commit('c 1', tree={'1': '1'}), ref='heads/branch1')
pr = prod.make_pr(target='a', head='branch1')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
# disable branch b
env['runbot_merge.branch'].search([('name', '=', 'b')]).write({enabled: False})
env.run_crons()
# should have created a single PR (to branch c, for pr 1)
_0, _1, p = env['runbot_merge.pull_requests'].search([], order='number')
assert p.parent_id == _1
assert p.target.name == 'c'
project.fp_github_token = config['role_other']['token']
bot_name = project.fp_github_name
with prod:
[c] = prod.make_commits('a', Commit('c 2', tree={'2': '2'}), ref='heads/branch2')
pr = prod.make_pr(target='a', head='branch2')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to b' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to foo' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to c' % bot_name, config['role_reviewer']['token'])
env.run_crons()
# use a set because git webhooks delays might lead to mis-ordered
# responses and we don't care that much
assert set(pr.comments) == {
(users['reviewer'], "hansen r+\n%s up to" % bot_name),
(users['reviewer'], "%s up to b" % bot_name),
(users['reviewer'], "%s up to foo" % bot_name),
(users['reviewer'], "%s up to c" % bot_name),
(users['other'], "Please provide a branch to forward-port to."),
(users['other'], "Branch 'b' is disabled, it can't be used as a forward port target."),
(users['other'], "There is no branch 'foo', it can't be used as a forward port target."),
(users['other'], "Forward-porting to 'c'."),
}
def test_default_disabled(env, config, make_repo, users):
""" If the default limit is disabled, it should still be the default
limit but the ping message should be set on the actual last FP (to the
last non-deactivated target)
"""
prod, other = make_basic(env, config, make_repo)
branch_c = env['runbot_merge.branch'].search([('name', '=', 'c')])
branch_c.fp_target = False
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/branch0')
pr = prod.make_pr(target='a', head='branch0')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
assert env['runbot_merge.pull_requests'].search([]).limit_id == branch_c
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
p1, p2 = env['runbot_merge.pull_requests'].search([], order='number')
assert p1.number == pr.number
pr2 = prod.get_pr(p2.number)
cs = pr2.comments
assert len(cs) == 1
assert pr2.comments == [
(users['user'], """\
Ping @%s, @%s
This PR targets b and is the last of the forward-port chain.
To merge the full chain, say
> @%s r+
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
""" % (users['user'], users['reviewer'], users['user'])),
]

View File

@ -23,64 +23,6 @@ FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime('%Y-%m-%d %H:%M:%
# - a github user to create a repo with
# - a github owner to create a repo *for*
# - provide ability to create commits, branches, prs, ...
def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproject'):
""" Creates a basic repo with 3 forking branches
0 -- 1 -- 2 -- 3 -- 4 : a
|
`-- 11 -- 22 : b
|
`-- 111 : c
each branch just adds and modifies a file (resp. f, g and h) through the
contents sequence a b c d e
"""
Projects = env['runbot_merge.project']
project = Projects.search([('name', '=', project_name)])
if not project:
project = env['runbot_merge.project'].create({
'name': project_name,
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'fp_github_token': config['github']['token'],
'required_statuses': 'legal/cla,ci/runbot',
'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}),
],
})
prod = make_repo(reponame)
with prod:
a_0, a_1, a_2, a_3, a_4, = prod.make_commits(
None,
Commit("0", tree={'f': 'a'}),
Commit("1", tree={'f': 'b'}),
Commit("2", tree={'f': 'c'}),
Commit("3", tree={'f': 'd'}),
Commit("4", tree={'f': 'e'}),
ref='heads/a',
)
b_1, b_2 = prod.make_commits(
a_2,
Commit('11', tree={'g': 'a'}),
Commit('22', tree={'g': 'b'}),
ref='heads/b',
)
prod.make_commits(
b_1,
Commit('111', tree={'h': 'a'}),
ref='heads/c',
)
other = prod.fork()
project.write({
'repo_ids': [(0, 0, {
'name': prod.name,
'fp_remote_target': other.name,
})],
})
return prod, other
def test_straightforward_flow(env, config, make_repo, users):
# TODO: ~all relevant data in users when creating partners
# get reviewer's name
@ -730,206 +672,6 @@ def test_partially_empty(env, config, make_repo):
'y': '0',
}
Description = collections.namedtuple('Restriction', 'source limit')
def test_limit_configure(env, config, make_repo):
""" Checks that configuring an FP limit on a PR is respected
* limits to not the latest
* limits to the current target (= no FP)
* limits to an earlier branch (???)
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
descriptions = [
Description(source='a', limit='b'),
Description(source='b', limit='b'),
Description(source='b', limit='a'),
]
originals = []
with prod:
for i, descr in enumerate(descriptions):
[c] = prod.make_commits(
descr.source, Commit('c %d' % i, tree={str(i): str(i)}),
ref='heads/branch%d' % i,
)
pr = prod.make_pr(target=descr.source, head='branch%d'%i)
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to %s' % (bot_name, descr.limit), config['role_reviewer']['token'])
originals.append(pr.number)
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
env.run_crons()
# should have created a single FP PR for 0, none for 1 and none for 2
prs = env['runbot_merge.pull_requests'].search([], order='number')
assert len(prs) == 4
assert prs[-1].parent_id == prs[0]
assert prs[0].number == originals[0]
assert prs[1].number == originals[1]
assert prs[2].number == originals[2]
def test_limit_self_disabled(env, config, make_repo):
""" Allow setting target as limit even if it's disabled
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
branch_a.fp_target = False
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head='mybranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to a' % bot_name, config['role_reviewer']['token'])
env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)])
assert pr_id.limit_id == branch_a
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
def test_fp_ignore(env, config, make_repo):
""" Provide an "ignore" command which is equivalent to setting the limit
to target
"""
prod, other = make_basic(env, config, make_repo)
bot_name = env['runbot_merge.project'].search([]).fp_github_name
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head='mybranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s ignore' % bot_name, config['role_reviewer']['token'])
env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)])
assert pr_id.limit_id == branch_a
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
@pytest.mark.parametrize('enabled', ['active', 'fp_target'])
def test_limit_disable(env, config, make_repo, users, enabled):
""" Checks behaviour if the limit target is disabled:
* disable target while FP is ongoing -> skip over (and stop there so no FP)
* forward-port over a disabled branch
* request a disabled target as limit
Disabling (with respect to forward ports) can be performed by marking the
branch as !active (which also affects mergebot operations), or as
!fp_target (won't be forward-ported to).
"""
prod, other = make_basic(env, config, make_repo)
project = env['runbot_merge.project'].search([])
bot_name = project.fp_github_name
with prod:
[c] = prod.make_commits('a', Commit('c 0', tree={'0': '0'}), ref='heads/branch0')
pr = prod.make_pr(target='a', head='branch0')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to b' % bot_name, config['role_reviewer']['token'])
[c] = prod.make_commits('a', Commit('c 1', tree={'1': '1'}), ref='heads/branch1')
pr = prod.make_pr(target='a', head='branch1')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
# disable branch b
env['runbot_merge.branch'].search([('name', '=', 'b')]).write({enabled: False})
env.run_crons()
# should have created a single PR (to branch c, for pr 1)
_0, _1, p = env['runbot_merge.pull_requests'].search([], order='number')
assert p.parent_id == _1
assert p.target.name == 'c'
project.fp_github_token = config['role_other']['token']
bot_name = project.fp_github_name
with prod:
[c] = prod.make_commits('a', Commit('c 2', tree={'2': '2'}), ref='heads/branch2')
pr = prod.make_pr(target='a', head='branch2')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+\n%s up to' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to b' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to foo' % bot_name, config['role_reviewer']['token'])
pr.post_comment('%s up to c' % bot_name, config['role_reviewer']['token'])
env.run_crons()
# use a set because git webhooks delays might lead to mis-ordered
# responses and we don't care that much
assert set(pr.comments) == {
(users['reviewer'], "hansen r+\n%s up to" % bot_name),
(users['reviewer'], "%s up to b" % bot_name),
(users['reviewer'], "%s up to foo" % bot_name),
(users['reviewer'], "%s up to c" % bot_name),
(users['other'], "Please provide a branch to forward-port to."),
(users['other'], "Branch 'b' is disabled, it can't be used as a forward port target."),
(users['other'], "There is no branch 'foo', it can't be used as a forward port target."),
(users['other'], "Forward-porting to 'c'."),
}
def test_default_disabled(env, config, make_repo, users):
""" If the default limit is disabled, it should still be the default
limit but the ping message should be set on the actual last FP (to the
last non-deactivated target)
"""
prod, other = make_basic(env, config, make_repo)
branch_c = env['runbot_merge.branch'].search([('name', '=', 'c')])
branch_c.fp_target = False
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/branch0')
pr = prod.make_pr(target='a', head='branch0')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
assert env['runbot_merge.pull_requests'].search([]).limit_id == branch_c
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
p1, p2 = env['runbot_merge.pull_requests'].search([], order='number')
assert p1.number == pr.number
pr2 = prod.get_pr(p2.number)
cs = pr2.comments
assert len(cs) == 1
assert pr2.comments == [
(users['user'], """\
Ping @%s, @%s
This PR targets b and is the last of the forward-port chain.
To merge the full chain, say
> @%s r+
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
""" % (users['user'], users['reviewer'], users['user'])),
]
# reviewer = of the FP sequence, the original PR is always reviewed by `user`
# set as reviewer
Case = collections.namedtuple('Case', 'author reviewer delegate success')

View File

@ -34,3 +34,63 @@ class re_matches:
def __repr__(self):
return '~' + self._r.pattern + '~'
def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproject'):
""" Creates a basic repo with 3 forking branches
0 -- 1 -- 2 -- 3 -- 4 : a
|
`-- 11 -- 22 : b
|
`-- 111 : c
each branch just adds and modifies a file (resp. f, g and h) through the
contents sequence a b c d e
"""
Projects = env['runbot_merge.project']
project = Projects.search([('name', '=', project_name)])
if not project:
project = env['runbot_merge.project'].create({
'name': project_name,
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'fp_github_token': config['github']['token'],
'required_statuses': 'legal/cla,ci/runbot',
'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}),
],
})
prod = make_repo(reponame)
with prod:
a_0, a_1, a_2, a_3, a_4, = prod.make_commits(
None,
Commit("0", tree={'f': 'a'}),
Commit("1", tree={'f': 'b'}),
Commit("2", tree={'f': 'c'}),
Commit("3", tree={'f': 'd'}),
Commit("4", tree={'f': 'e'}),
ref='heads/a',
)
b_1, b_2 = prod.make_commits(
a_2,
Commit('11', tree={'g': 'a'}),
Commit('22', tree={'g': 'b'}),
ref='heads/b',
)
prod.make_commits(
b_1,
Commit('111', tree={'h': 'a'}),
ref='heads/c',
)
other = prod.fork()
project.write({
'repo_ids': [(0, 0, {
'name': prod.name,
'fp_remote_target': other.name,
})],
})
return prod, other