runbot/forwardport/tests/test_skipmerge.py
Xavier Morel c67bb64537 [ADD] *: fw=skipmerge
Skipmerge creates forward-ports before the source PR is even merged.

- In a break from the norm, skipmerge will create forwardports even in
  the face of conflicts.
- It will also not *detach* pull requests in case of conflicts, this
  is so the source PR can be updated and the update correctly cascades
  through the stack (likewise for any intermediate PR though *that*
  will detach as usual).

Note that this doesn't really look at outstandings, especially as they
were recently updated, so it might need to be fixed up in case of
freakout, but I feel like that should not be too much of an issue, the
authors will just get their FW reminders earlier than usual. If that's
a hassle we can always update the reminder job to ignore forward ports
whose source is not merged I guess.

Fixes 
2025-02-04 08:05:15 +01:00

317 lines
9.4 KiB
Python

import re
from datetime import datetime, timedelta
from utils import make_basic, Commit, to_pr, REF_PATTERN, seen, matches
def test_base(env, config, make_repo, users, page):
prod, other = make_basic(env, config, make_repo, statuses='default')
b_head = prod.commit('b')
c_head = prod.commit('c')
with prod:
# create PR as a user with no access to prod (or other)
prod.make_commits(
'a',
Commit('p_0', tree={'x': '1'}),
ref='heads/hugechange'
)
pr0 = prod.make_pr(target='a', title="super important change", head='hugechange')
pr0.post_comment('hansen fw=skipmerge', config['role_reviewer']['token'])
pr0_id = to_pr(env, pr0)
assert not pr0_id.merge_date, \
"PR obviously shouldn't have a merge date before being merged"
assert pr0_id.batch_id.fw_policy == 'skipmerge'
env.run_crons()
pr0_id, pr1_id, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
pr2_id.reminder_next = datetime.now() - timedelta(days=1)
env.run_crons('forwardport.reminder')
assert pr0_id.number == pr0.number
assert pr0.comments == [
(users['reviewer'], 'hansen fw=skipmerge'),
seen(env, pr0, users),
(users['user'], "Starting forward-port. Not waiting for merge to create followup forward-ports."),
]
assert pr1_id.parent_id == pr0_id
assert pr1_id.source_id == pr0_id
other_owner = other.name.split('/')[0]
assert re.match(other_owner + ':' + REF_PATTERN.format(target='b', source='hugechange'), pr1_id.label), \
"check that FP PR was created in FP target repo"
assert pr1_id.ping == f"@{users['user']} ", "not reviewed yet so ping should only include author"
assert prod.read_tree(prod.commit(pr1_id.head)) == {
'f': 'c',
'g': 'b',
'x': '1'
}
pr1 = prod.get_pr(pr1_id.number)
assert pr1.comments == [
seen(env, pr1, 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
""")]
assert pr2_id.parent_id == pr1_id
assert pr2_id.source_id == pr0_id
assert re.match(REF_PATTERN.format(target='c', source='hugechange'), pr2_id.refname), \
"check that FP PR was created in FP target repo"
assert prod.read_tree(prod.commit(pr2_id.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': '1'
}
pr2 = prod.get_pr(pr2_id.number)
assert pr2.comments == [
seen(env, pr2, users),
(users['user'], """\
@%s this PR targets c and is the last of the forward-port chain containing:
* %s
To merge the full chain, use
> @hansen r+
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
""" % (
users['user'],
pr1_id.display_name,
)),
(users['user'], "@%s this forward port of %s is awaiting action (not merged or closed)." % (
users['user'],
pr0_id.display_name,
))
]
with prod:
prod.post_status(pr0_id.head, 'success')
prod.post_status(pr1_id.head, 'success')
prod.post_status(pr2_id.head, 'success')
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
assert pr0_id.staging_id
assert pr1_id.staging_id
assert pr2_id.staging_id
# three branches so should have three stagings
assert len(pr0_id.staging_id | pr1_id.staging_id | pr2_id.staging_id) == 3
# validate
with prod:
prod.post_status('staging.a', 'success')
prod.post_status('staging.b', 'success')
prod.post_status('staging.c', 'success')
# and trigger merge
env.run_crons()
assert all(p.state == 'merged' for p in [pr0_id, pr1_id, pr2_id])
head_a = prod.commit('a')
assert head_a.message == f"""\
p_0
closes {pr0_id.display_name}
Signed-off-by: {pr0_id.reviewed_by.formatted_email}\
"""
old_b = prod.read_tree(b_head)
head_b = prod.commit('b')
assert head_b.message == f"""\
p_0
closes {pr1_id.display_name}
X-original-commit: {pr0_id.head}
Signed-off-by: {pr1_id.reviewed_by.formatted_email}\
""", "since the previous PR is not merged we don't know what its final commit is (?)"
b_tree = prod.read_tree(head_b)
assert b_tree == {**old_b, 'x': '1'}
old_c = prod.read_tree(c_head)
head_c = prod.commit('c')
assert head_c.message == f"""\
p_0
closes {pr2_id.display_name}
X-original-commit: {pr0_id.head}
Signed-off-by: {pr2_id.reviewed_by.formatted_email}\
"""
c_tree = prod.read_tree(head_c)
assert c_tree == {**old_c, 'x': '1'}
# check that we didn't just smash the original trees
assert prod.read_tree(prod.commit('a')) != b_tree != c_tree
assert pr2_id.parent_id == pr1_id
assert pr1_id.parent_id == pr0_id
def test_conflict_recovery_source(env, config, make_repo, users, page):
""" Source recovery is when a forward port is in conflict and we update
*the source* in such a way that the fp doesn't conflict anymore. This should
resume forward-porting.
"""
prod, _ = make_basic(env, config, make_repo, statuses='default')
with prod:
prod.make_commits("b", Commit("xxx", tree={'x': '1'}), ref='heads/b')
prod.make_commits("c", Commit("xxx", tree={'x': '2'}), ref='heads/c')
prod.make_commits(
'a',
Commit('p_0', tree={'x': '0'}),
ref='heads/hugechange'
)
pr0 = prod.make_pr(target='a', title="super important change", head='hugechange')
pr0.post_comment('hansen fw=skipmerge', config['role_reviewer']['token'])
env.run_crons()
pr0_id, pr1_id, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
assert prod.read_tree(prod.commit(pr1_id.head)) == {
'f': 'c',
'g': 'b',
'x': matches('''\
<<<\x3c<<< b
1
||||||| $$
=======
0
>>>\x3e>>> $$
''')
}
assert prod.read_tree(prod.commit(pr2_id.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': matches('''\
<<<\x3c<<< c
2
||||||| $$
=======
0
>>>\x3e>>> $$
''')
}
assert pr1_id.parent_id and pr2_id.parent_id
pr1 = prod.get_pr(pr1_id.number)
assert "CONFLICT (add/add)" in pr1.comments[1][1]
# Oh no, conflict! But turns out we can implement the fix differently <smort>
with prod:
prod.make_commits("a", Commit("workaround", tree={"y": "1"}), ref="heads/hugechange", make=False)
assert prod.read_tree(prod.commit(pr0_id.head)) == {
'f': 'e',
'y': '1',
}
env.run_crons()
assert env['runbot_merge.pull_requests'].search_count([]) == 3,\
"check that we have not created separate new versions of the prs"
assert prod.read_tree(prod.commit(pr1_id.head)) == {
'f': 'c',
'g': 'b',
'x': '1',
'y': '1',
}
assert prod.read_tree(prod.commit(pr2_id.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': '2',
'y': '1',
}
def test_conflict_recovery_manual(env, config, make_repo, users, page):
""" Manual recover is when a forward port is in conflict and we update
*that forward port*. This should resume forward porting when skipmerge.
"""
prod, _ = make_basic(env, config, make_repo, statuses='default')
with prod:
prod.make_commits("b", Commit("xxx", tree={'x': '1'}), ref='heads/b')
prod.make_commits("c", Commit("xxx", tree={'x': '1'}), ref='heads/c')
prod.make_commits(
'a',
Commit('p_0', tree={'x': '0'}),
ref='heads/hugechange'
)
pr0 = prod.make_pr(target='a', title="super important change", head='hugechange')
pr0.post_comment('hansen fw=skipmerge', config['role_reviewer']['token'])
env.run_crons()
_pr0_id, pr1_id, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
assert prod.read_tree(prod.commit(pr1_id.head)) == {
'f': 'c',
'g': 'b',
'x': matches('''\
<<<\x3c<<< b
1
||||||| $$
=======
0
>>>\x3e>>> $$
''')
}
assert prod.read_tree(prod.commit(pr2_id.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': matches('''\
<<<\x3c<<< c
1
||||||| $$
=======
0
>>>\x3e>>> $$
''')
}
assert pr1_id.parent_id and pr2_id.parent_id, "conflicts on skipmerge should not detach"
pr1 = prod.get_pr(pr1_id.number)
assert "CONFLICT (add/add)" in pr1.comments[1][1]
assert "CONFLICT (add/add)" in prod.get_pr(pr2_id.number).comments[1][1]
prb_repo, prb_ref = pr1.branch
with prb_repo:
prb_repo.make_commits(
prod.commit("b").id,
Commit("thing", tree={'x': 'yyy'}),
ref=f'heads/{prb_ref}',
make=False,
)
assert not pr1_id.parent_id, "manual update should still de-parent even on skipmerge"
pr1_head = pr1_id.head
assert prod.read_tree(prod.commit(pr1_head)) == {
'f': 'c',
'g': 'b',
'x': 'yyy',
}
env.run_crons()
assert env['runbot_merge.pull_requests'].search_count([]) == 3,\
"check that we have not created a separate new version of pr3"
assert prod.read_tree(prod.commit(pr2_id.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': 'yyy',
}
with prod:
prod.make_commits(
'a',
Commit('p_0', tree={'x': '42'}),
ref='heads/hugechange'
)
env.run_crons()
assert pr1.head == pr1_head, "since PR1 is detached the update to pr0 should not propagate"