[FIX] forwardport: not being properly notified on last FP of a seq

If the default limit of a forward-port sequence is not a valid
target (either disabled or not actually forward-ported to), the last
effective forward port in a sequence will be commented on as any
intermediate PR rather than get a proper ping and r+ instructions.

Also remove a bunch of leftover prints in the tests.

Fixes #192
This commit is contained in:
Xavier Morel 2019-09-13 16:06:34 +02:00
parent 8976f9e310
commit f8da17994a
2 changed files with 43 additions and 10 deletions

View File

@ -462,7 +462,7 @@ Either perform the forward-port manually (and push to this branch, proceeding as
In the former case, you may want to edit this PR message as well. In the former case, you may want to edit this PR message as well.
""" % (h, source.number, sout, serr) """ % (h, source.number, sout, serr)
elif base.limit_id == target: elif base._find_next_target(new_pr) is None:
ancestors = "".join( ancestors = "".join(
"* %s#%d\n" % (p.repository.name, p.number) "* %s#%d\n" % (p.repository.name, p.number)
for p in pr._iter_ancestors() for p in pr._iter_ancestors()

View File

@ -1,6 +1,5 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
import collections import collections
import pprint
import sys import sys
import time import time
from operator import itemgetter from operator import itemgetter
@ -425,8 +424,6 @@ xxx
get_pr.post_comment('hansen r+', config['role_reviewer']['token']) get_pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
print(f"check staging of {pr1} ({pr1.staging_id})", file=sys.stderr)
print(f'\t{pr1.batch_ids.read(["id", "active", "staging_id"])}', file=sys.stderr)
assert pr1.staging_id assert pr1.staging_id
with prod: with prod:
prod.post_status('staging.b', 'success', 'legal/cla') prod.post_status('staging.b', 'success', 'legal/cla')
@ -788,6 +785,48 @@ def test_limit_disable(env, config, make_repo, users, enabled):
(users['user'], "Forward-porting to 'c'."), (users['user'], "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
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['reviewer'], users['user'])),
]
# reviewer = of the FP sequence, the original PR is always reviewed by `user` # reviewer = of the FP sequence, the original PR is always reviewed by `user`
# set as reviewer # set as reviewer
Case = collections.namedtuple('Case', 'author reviewer delegate success') Case = collections.namedtuple('Case', 'author reviewer delegate success')
@ -856,7 +895,6 @@ def test_access_rights(env, config, make_repo, users, author, reviewer, delegate
token=config['role_' + reviewer]['token'] token=config['role_' + reviewer]['token']
) )
env.run_crons() env.run_crons()
print(f"check staging of {pr1} ({pr1.staging_id}), {pr2} ({pr2.staging_id})", file=sys.stderr)
if success: if success:
assert pr1.staging_id and pr2.staging_id,\ assert pr1.staging_id and pr2.staging_id,\
"%s should have approved FP of PRs by %s" % (reviewer, author) "%s should have approved FP of PRs by %s" % (reviewer, author)
@ -958,10 +996,6 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
assert pr2a.number == pr2.number assert pr2a.number == pr2.number
assert pr1a.state == pr2a.state == 'merged' assert pr1a.state == pr2a.state == 'merged'
print('PRA label', (pr1a | pr2a).mapped('label'))
print('PRB label', (pr1b | pr2b).mapped('label'))
print('PRC label', (pr1c | pr2c).mapped('label'))
assert pr1b.label == pr2b.label, "batched source should yield batched FP" assert pr1b.label == pr2b.label, "batched source should yield batched FP"
assert pr1c.label == pr2c.label, "batched source should yield batched FP" assert pr1c.label == pr2c.label, "batched source should yield batched FP"
assert pr1b.label != pr1c.label assert pr1b.label != pr1c.label
@ -1121,7 +1155,6 @@ class TestRecognizeCommands:
sPeNgBaB(botname), sPeNgBaB(botname),
] ]
print([repr(o) for o in [a, c, pr_id, pr_id.limit_id]])
for n in names: for n in names:
assert pr_id.limit_id == c assert pr_id.limit_id == c
with repo: with repo: