runbot/forwardport/tests/test_limit.py
Xavier Morel 679d556c90 [FIX] project creation: handling of mergebot info
- don't *fail* in `_compute_identity`, it causes issues when the token
  is valid but doesn't have `user:email` access as the request is
  aborted and saving doesn't work
- make `github_name` and `github_email` required rather than ad-hoc
  requiring them in `_compute_identity` (which doesn't work correctly)
- force copy of `github_name` and `github_email`, with o2ms being
  !copy this means duplicating projects now works out of the box (or
  should...)

Currently errors in `_compute_identity` are reported via logging which
is not great as it's not UI visible, should probably get moved to
chatter eventually but that's not currently enabled on projects.

Fixes #990
2024-12-02 16:32:53 +01:00

585 lines
23 KiB
Python

import lxml.html
import pytest
from utils import seen, Commit, make_basic, to_pr
@pytest.mark.parametrize('source,limit,count', [
pytest.param('a', 'b', 1, id='not-last'),
pytest.param('b', 'b', 0, id='current'),
pytest.param('b', 'a', 0, id='earlier'),
])
def test_configure_fp_limit(env, config, make_repo, source, limit, count, page):
prod, other = make_basic(env, config, make_repo, statuses="default")
with prod:
[c] = prod.make_commits(
source, Commit('c', tree={'f': 'g'}),
ref='heads/branch',
)
pr = prod.make_pr(target=source, head='branch')
prod.post_status(c, 'success')
pr.post_comment(f'hansen r+ up to {limit}', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status(f'staging.{source}', 'success')
env.run_crons()
pr_id = to_pr(env, pr)
descendants = env['runbot_merge.pull_requests'].search([
('source_id', '=', pr_id.id)
])
assert len(descendants) == count
limit_id = env['runbot_merge.branch'].search([('name', '=', limit)])
assert pr_id.limit_id == limit_id
assert not descendants.limit_id, "descendant should not inherit the limit explicitly"
# check that the basic thingie works
page(f'/{prod.name}/pull/{pr.number}.png')
if descendants:
c = env['runbot_merge.branch'].search([('name', '=', 'c')])
descendants.limit_id = c.id
page(f'/{prod.name}/pull/{pr.number}.png')
def test_ignore(env, config, make_repo, users):
""" Provide an "ignore" command which is equivalent to setting the limit
to target
"""
prod, _ = make_basic(env, config, make_repo, statuses="default")
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')
env.run_crons()
with prod:
pr.post_comment('hansen ignore', config['role_reviewer']['token'])
pr.post_comment('hansen r+ fw=no', 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')
env.run_crons()
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
assert pr.comments == [
seen(env, pr, users),
(users['reviewer'], "hansen ignore"),
(users['reviewer'], "hansen r+ fw=no"),
(users['user'], "'ignore' is deprecated, use 'fw=no' to disable forward porting."),
(users['user'], "Forward-port disabled (via limit)."),
(users['user'], "Disabled forward-porting."),
]
with prod:
pr.post_comment('hansen up to c', config['role_reviewer']['token'])
env.run_crons()
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should still not have created a forward port"
assert pr.comments[6:] == [
(users['reviewer'], 'hansen up to c'),
(users['user'], "@%s can not forward-port, policy is 'no' on %s" % (
users['reviewer'],
pr_id.display_name,
))
]
assert pr_id.limit_id == env['runbot_merge.branch'].search([('name', '=', 'c')])
pr_id.limit_id = False
with prod:
pr.post_comment("hansen fw=default", config['role_reviewer']['token'])
env.run_crons()
assert pr.comments[8:] == [
(users['reviewer'], "hansen fw=default"),
(users['user'], "Starting forward-port. Waiting for CI to create followup forward-ports.")
]
assert env['runbot_merge.pull_requests'].search([('source_id', '=', pr_id.id)]),\
"should finally have created a forward port"
def test_unignore(env, config, make_repo, users, page):
env['res.partner'].create({'name': users['other'], 'github_login': users['other'], 'email': 'other@example.org'})
prod, _ = make_basic(env, config, make_repo, statuses="default")
token = config['role_other']['token']
fork = prod.fork(token=token)
with prod, fork:
[c] = fork.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head=f'{fork.owner}:mybranch', title="title", token=token)
prod.post_status(c, 'success')
env.run_crons()
pr_id = to_pr(env, pr)
assert pr_id.batch_id.fw_policy == 'default'
doc = lxml.html.fromstring(page(f'/{prod.name}/pull/{pr.number}'))
assert len(doc.cssselect("table > tbody > tr")) == 3, \
"the overview table should have as many rows as there are tables"
with prod:
pr.post_comment('hansen fw=no', token)
env.run_crons(None)
assert pr_id.batch_id.fw_policy == 'no'
doc = lxml.html.fromstring(page(f'/{prod.name}/pull/{pr.number}'))
assert len(doc.cssselect("table > tbody > tr")) == 1, \
"if fw=no, there should be just one row"
with prod:
pr.post_comment('hansen fw=default', token)
env.run_crons(None)
assert pr_id.batch_id.fw_policy == 'default'
with prod:
pr.post_comment('hansen fw=no', token)
env.run_crons(None)
with prod:
pr.post_comment('hansen up to c', token)
env.run_crons(None)
assert pr_id.batch_id.fw_policy == 'default'
assert pr.comments == [
seen(env, pr, users),
(users['other'], "hansen fw=no"),
(users['user'], "Disabled forward-porting."),
(users['other'], "hansen fw=default"),
(users['user'], "Waiting for CI to create followup forward-ports."),
(users['other'], "hansen fw=no"),
(users['user'], "Disabled forward-porting."),
(users['other'], "hansen up to c"),
(users['user'], "Forward-porting to 'c'. Re-enabled forward-porting "\
"(you should use `fw=default` to re-enable forward "\
"porting after disabling)."
),
]
def test_disable(env, config, make_repo, users):
""" 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
"""
prod, other = make_basic(env, config, make_repo)
project = env['runbot_merge.project'].search([])
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+ up to b', 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')]).active = 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'
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+ up to', config['role_reviewer']['token'])
pr.post_comment('hansen up to b', config['role_reviewer']['token'])
pr.post_comment('hansen up to foo', config['role_reviewer']['token'])
pr.post_comment('hansen up to c', 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) == {
seen(env, pr, users),
(users['reviewer'], "hansen r+ up to"),
(users['user'], """\
@{reviewer} please provide a branch to forward-port to.
For your own safety I've ignored *everything in your entire comment*.
Currently available commands:
|command||
|-|-|
|`help`|displays this help|
|`r(eview)+`|approves the PR, if it's a forwardport also approves all non-detached parents|
|`r(eview)=<number>`|only approves the specified parents|
|`fw=no`|does not forward-port this PR|
|`fw=default`|forward-ports this PR normally|
|`fw=skipci`|does not wait for a forward-port's statuses to succeed before creating the next one|
|`up to <branch>`|only ports this PR forward to the specified branch (included)|
|`merge`|integrate the PR with a simple merge commit, using the PR description as message|
|`rebase-merge`|rebases the PR on top of the target branch the integrates with a merge commit, using the PR description as message|
|`rebase-ff`|rebases the PR on top of the target branch, then fast-forwards|
|`squash`|squashes the PR as a single commit on the target branch, using the PR description as message|
|`delegate+`|grants approval rights to the PR author|
|`delegate=<...>`|grants approval rights on this PR to the specified github users|
|`default`|stages the PR normally|
|`priority`|tries to stage this PR first, then adds `default` PRs if the staging has room|
|`alone`|stages this PR only with other PRs of the same priority|
|`cancel=staging`|automatically cancels the current staging when this PR becomes ready|
|`check`|fetches or refreshes PR metadata, resets mergebot state|
Note: this help text is dynamic and will change with the state of the PR.
""".format_map(users)),
(users['reviewer'], "hansen up to b"),
(users['user'], "@{reviewer} branch 'b' is disabled, it can't be used as a forward port target.".format_map(users)),
(users['reviewer'], "hansen up to foo"),
(users['user'], "@{reviewer} there is no branch 'foo', it can't be used as a forward port target.".format_map(users)),
(users['reviewer'], "hansen up to c"),
(users['user'], "Forward-porting to 'c'."),
}
def test_limit_after_merge(env, config, make_repo, users):
prod, other = make_basic(env, config, make_repo)
reviewer = config['role_reviewer']['token']
branch_b = env['runbot_merge.branch'].search([('name', '=', 'b')])
branch_c = env['runbot_merge.branch'].search([('name', '=', 'c')])
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/abranch')
pr1 = prod.make_pr(target='a', head='abranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr1.post_comment('hansen r+', reviewer)
env.run_crons()
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.limit_id == p2.limit_id == env['runbot_merge.branch'].browse(())
pr2 = prod.get_pr(p2.number)
with prod:
pr1.post_comment('hansen up to b', reviewer)
pr2.post_comment('hansen up to b', reviewer)
env.run_crons()
assert p1.limit_id == p2.limit_id == branch_b
assert pr1.comments == [
(users['reviewer'], "hansen r+"),
seen(env, pr1, users),
(users['reviewer'], 'hansen up to b'),
(users['user'], "Forward-porting to 'b'."),
(users['user'], f"Forward-porting to 'b' (from {p2.display_name})."),
]
assert pr2.comments == [
seen(env, pr2, users),
(users['user'], """\
This PR targets b and is part of the forward-port chain. Further PRs will be created up to c.
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
"""),
(users['reviewer'], 'hansen up to b'),
(users['user'], f"Forward-porting {p1.display_name} to 'b'."),
]
# update pr2 to detach it from pr1
with other:
other.make_commits(
p2.target.name,
Commit('updated', tree={'1': '1'}),
ref=pr2.ref,
make=False
)
env.run_crons()
assert not p2.parent_id
assert p2.source_id == p1
with prod:
pr2.post_comment('hansen up to c', reviewer)
env.run_crons()
assert pr2.comments[4:] == [
(users['user'], f"@{users['user']} @{users['reviewer']} this PR was modified / updated and has become a normal PR. It must be merged directly."),
(users['reviewer'], 'hansen up to c'),
(users['user'], "Forward-porting to 'c'."),
]
with prod:
prod.post_status(p2.head, 'success', 'legal/cla')
prod.post_status(p2.head, 'success', 'ci/runbot')
pr2.post_comment('hansen r+', reviewer)
env.run_crons()
with prod:
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
env.run_crons()
_, _, p3 = env['runbot_merge.pull_requests'].search([], order='number')
assert p3
pr3 = prod.get_pr(p3.number)
with prod:
pr3.post_comment("hansen up to c", reviewer)
env.run_crons()
assert pr3.comments == [
seen(env, pr3, users),
(users['user'], f"""\
@{users['user']} @{users['reviewer']} this PR targets c and is the last of the forward-port chain.
To merge the full chain, use
> @hansen r+
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
"""),
(users['reviewer'], "hansen up to c"),
(users['user'], f"Forward-porting {p2.display_name} to 'c'."),
]
# 7 of previous check, plus r+
assert pr2.comments[8:] == [
(users['user'], f"Forward-porting to 'c' (from {p3.display_name}).")
]
@pytest.mark.parametrize("update_from", [
pytest.param(lambda source: [('id', '=', source)], id='source'),
pytest.param(lambda source: [('source_id', '=', source), ('target', '=', '2')], id='child'),
pytest.param(lambda source: [('source_id', '=', source), ('target', '=', '3')], id='root'),
pytest.param(lambda source: [('source_id', '=', source), ('target', '=', '4')], id='parent'),
pytest.param(lambda source: [('source_id', '=', source), ('target', '=', '5')], id='current'),
# pytest.param(id='tip'), # doesn't exist
])
@pytest.mark.parametrize("limit", range(1, 6+1))
def test_post_merge(
env, post_merge, users, config, branches,
update_from: callable,
limit: int,
):
PRs = env['runbot_merge.pull_requests']
project, prod, _ = post_merge
reviewer = config['role_reviewer']['token']
# fetch source PR
[source] = PRs.search([('source_id', '=', False)])
# validate the forward ports for "child", "root", and "parent" so "current"
# exists and we have one more target
for branch in map(str, range(2, 4+1)):
setci(source=source, repo=prod, target=branch)
env.run_crons()
# update 3 to make it into a root
root = PRs.search([('source_id', '=', source.id), ('target.name', '=', '3')])
root.write({'parent_id': False, 'detach_reason': 'testing'})
# send detach messages so they're not part of the limit stuff batch
env.run_crons()
# cheat: we know PR numbers are assigned sequentially
prs = list(map(prod.get_pr, range(1, 6)))
before = {p.number: len(p.comments) for p in prs}
from_id = PRs.search(update_from(source.id))
from_ = prod.get_pr(from_id.number)
with prod:
from_.post_comment(f'hansen up to {limit}', reviewer)
env.run_crons()
# there should always be a comment on the source and root indicating how
# far we port
# the PR we post on should have a comment indicating the correction
current_id = PRs.search([('number', '=', '5')])
actual_limit = max(limit, 5)
for p in prs:
# case for the PR on which we posted the comment
if p.number == from_.number:
root_opt = '' if p.number == root.number else f' {root.display_name}'
trailer = '' if actual_limit == limit else f" (instead of the requested '{limit}' because {current_id.display_name} already exists)"
assert p.comments[before[p.number] + 1:] == [
(users['user'], f"Forward-porting{root_opt} to '{actual_limit}'{trailer}.")
]
# case for reference PRs source and root (which get their own notifications)
elif p.number in (source.number, root.number):
assert p.comments[before[p.number]:] == [
(users['user'], f"Forward-porting to '{actual_limit}' (from {from_id.display_name}).")
]
@pytest.mark.parametrize('mode', [
None,
# last forward port should fail ci, and only be validated after target bump
'failbump',
# last forward port should fail ci, then be validated, then target bump
'failsucceed',
# last forward port should be merged before bump
'mergetip',
# every forward port should be merged before bump
'mergeall',
])
def test_resume_fw(env, post_merge, users, config, branches, mode):
"""Singleton version of test_post_merge: completes the forward porting
including validation then tries to increase the limit, which should resume
forward porting
"""
PRs = env['runbot_merge.pull_requests']
project, prod, _ = post_merge
reviewer = config['role_reviewer']['token']
# fetch source PR
[source] = PRs.search([('source_id', '=', False)])
with prod:
prod.get_pr(source.number).post_comment('hansen up to 5', reviewer)
# validate the forward ports for "child", "root", and "parent" so "current"
# exists and we have one more target
for branch in map(str, range(2, 5+1)):
setci(
source=source, repo=prod, target=branch,
status='failure' if branch == '5' and mode in ('failbump', 'failsucceed') else 'success'
)
env.run_crons()
# cheat: we know PR numbers are assigned sequentially
prs = list(map(prod.get_pr, range(1, 6)))
before = {p.number: len(p.comments) for p in prs}
if mode == 'failsucceed':
setci(source=source, repo=prod, target=5)
# sees the success, limit is still 5, considers the porting finished
env.run_crons()
if mode and mode.startswith('merge'):
numbers = range(5 if mode == 'mergetip' else 2, 5 + 1)
with prod:
for number in numbers:
prod.get_pr(number).post_comment('hansen r+', reviewer)
env.run_crons()
with prod:
for target in numbers:
pr = PRs.search([('target.name', '=', str(target))])
prod.post_status(f'staging.{target}', 'success')
env.run_crons()
for number in numbers:
assert PRs.search([('number', '=', number)]).state == 'merged'
from_ = prod.get_pr(source.number)
with prod:
from_.post_comment('hansen up to 6', reviewer)
env.run_crons()
if mode == 'failbump':
setci(source=source, repo=prod, target=5)
# setci moved the PR from opened to validated, so *now* it can be
# forward-ported, but that still needs to actually happen
env.run_crons()
# since PR5 CI succeeded and we've increased the limit there should be a
# new PR
assert PRs.search([('source_id', '=', source.id), ('target.name', '=', 6)])
pr5_id = PRs.search([('source_id', '=', source.id), ('target.name', '=', 5)])
if mode == 'failbump':
# because the initial forward porting was never finished as the PR CI
# failed until *after* we bumped the limit, so it's not *resuming* per se.
assert prs[0].comments[before[1]+1:] == [
(users['user'], f"Forward-porting to '6'.")
]
else:
assert prs[0].comments[before[1]+1:] == [
(users['user'], f"Forward-porting to '6', resuming forward-port stopped at {pr5_id.display_name}.")
]
def setci(*, source, repo, target, status='success'):
"""Validates (CI success) the descendant of ``source`` targeting ``target``
in ``repo``.
"""
pr = source.search([('source_id', '=', source.id), ('target.name', '=', str(target))])
assert pr, f"could not find forward port of {source.display_name} to {target}"
with repo:
repo.post_status(pr.head, status)
@pytest.fixture(scope='session')
def branches():
"""Need enough branches to make space for:
- a source
- an ancestor (before and separated from the root, but not the source)
- a root (break in the parent chain
- a parent (between "current" and root)
- "current"
- the tip branch
"""
return range(1, 6 + 1)
@pytest.fixture
def post_merge(env, config, users, make_repo, branches):
"""Create a setup for the post-merge limits test which is both simpler and
more complicated than the standard test setup(s): it doesn't need more
variety in code, but it needs a lot more "depth" in terms of number of
branches it supports. Branches are fixture-ed to make it easier to share
between this fixture and the actual test.
All the branches are set to the same commit because that basically
shouldn't matter.
"""
prod = make_repo("post-merge-test")
with prod:
[c] = prod.make_commits(None, Commit('base', tree={'f': ''}))
for i in branches:
prod.make_ref(f'heads/{i}', c)
dev = prod.fork()
proj = env['runbot_merge.project'].create({
'name': prod.name,
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [
(0, 0, {'name': str(i), 'sequence': 1000 - (i * 10)})
for i in branches
],
'repo_ids': [
(0, 0, {
'name': prod.name,
'required_statuses': 'default',
'fp_remote_target': dev.name,
})
]
})
env['runbot_merge.events_sources'].create({'repository': prod.name})
env['res.partner'].search([
('github_login', '=', config['role_reviewer']['user'])
]).write({
'review_rights': [(0, 0, {'repository_id': proj.repo_ids.id, 'review': True})]
})
reviewer = config['role_reviewer']['token']
# merge the source PR
source_target = str(branches[0])
with prod:
[c] = prod.make_commits(source_target, Commit('my pr', tree={'x': ''}), ref='heads/mypr')
pr1 = prod.make_pr(target=source_target, head=c, title="a title")
prod.post_status(c, 'success')
pr1.post_comment('hansen r+', reviewer)
env.run_crons()
with prod:
prod.post_status(f'staging.{source_target}', 'success')
env.run_crons()
return proj, prod, dev