diff --git a/forwardport/models/project.py b/forwardport/models/project.py index a4099d05..1839979d 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -553,18 +553,13 @@ stderr: if source.merge_date > (cutoff_dt - backoff): continue source.reminder_backoff_factor += 1 - self.env.ref('runbot_merge.forwardport.reminder')._send( - repository=source.repository, - pull_request=source.number, - token_field='fp_github_token', - format_args={ - 'pr': source, - 'outstanding': ''.join( - f'\n- {pr.display_name}' - for pr in sorted(prs, key=lambda p: p.number) - ), - } - ) + for pr in prs: + self.env.ref('runbot_merge.forwardport.reminder')._send( + repository=pr.repository, + pull_request=pr.number, + token_field='fp_github_token', + format_args={'pr': pr, 'source': source}, + ) class Stagings(models.Model): _inherit = 'runbot_merge.stagings' diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index c086bd9b..e7214871 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta import pytest -from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr +from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr, re_matches FMT = '%Y-%m-%d %H:%M:%S' FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT) @@ -132,10 +132,20 @@ def test_straightforward_flow(env, config, make_repo, users): (users['reviewer'], 'hansen r+ rebase-ff'), seen(env, pr, users), (users['user'], 'Merge method set to rebase and fast-forward.'), - (users['user'], '@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n- %s' % ( - users['other'], users['reviewer'], - '\n- '.join((pr1 | pr2).mapped('display_name')) - )), + ] + pr1_remote = prod.get_pr(pr1.number) + assert pr1_remote.comments == [ + seen(env, pr1_remote, 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['user'], "@%s @%s this forward port of %s is awaiting action (not merged or closed)." % ( + users['other'], + users['reviewer'], + pr0.display_name, + )) ] assert pr0_ == pr0 @@ -168,6 +178,11 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port users['other'], users['reviewer'], pr1.display_name, )), + (users['user'], "@%s @%s this forward port of %s is awaiting action (not merged or closed)." % ( + users['other'], + users['reviewer'], + pr0.display_name, + )) ] with prod: prod.post_status(pr2.head, 'success', 'ci/runbot') @@ -231,7 +246,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port assert other_user_repo.get_ref(pr.ref) == p_1 # should have deleted all PR branches - pr1_ref = prod.get_pr(pr1.number).ref + pr1_ref = pr1_remote.ref with pytest.raises(AssertionError, match='Not Found'): other.get_ref(pr1_ref) @@ -326,27 +341,57 @@ def test_empty(env, config, make_repo, users): awaiting = ( users['other'], - '@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n- %s' % ( + '@%s @%s this forward port of %s is awaiting action (not merged or closed).' % ( users['user'], users['reviewer'], - fail_id.display_name + pr1_id.display_name ) ) + conflict = (users['user'], re_matches( + fr"""@{users['user']} @{users['reviewer']} cherrypicking of pull request {pr1_id.display_name} failed\. + +stdout: +``` +.* +``` + +stderr: +``` +.* +``` + +Either perform the forward-port manually \(and push to this branch, proceeding as usual\) or close this PR \(maybe\?\)\. + +In the former case, you may want to edit this PR message as well\. + +:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}\. + +More info at https://github\.com/odoo/odoo/wiki/Mergebot#forward-port +""", re.DOTALL)) assert pr1.comments == [ (users['reviewer'], 'hansen r+'), seen(env, pr1, users), + ] + fail_pr = prod.get_pr(fail_id.number) + assert fail_pr.comments == [ + seen(env, fail_pr, users), + conflict, awaiting, awaiting, - ], "each cron run should trigger a new message on the ancestor" + ], "each cron run should trigger a new message" # check that this stops if we close the PR with prod: - prod.get_pr(fail_id.number).close() + fail_pr.close() env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK}) assert pr1.comments == [ (users['reviewer'], 'hansen r+'), seen(env, pr1, users), - awaiting, - awaiting, ] + assert fail_pr.comments == [ + seen(env, fail_pr, users), + conflict, + awaiting, + awaiting, + ], "each cron run should trigger a new message" def test_partially_empty(env, config, make_repo): """ Check what happens when only some commits of the PR are now empty diff --git a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv b/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv index 5fe768ae..eab2a241 100644 --- a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv +++ b/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv @@ -131,10 +131,10 @@ runbot_merge.forwardport.failure.conflict,"{pr.ping}the next pull request ({new. pr: the pr which was just forward ported new: the new forward-port footer: some footer text" -runbot_merge.forwardport.reminder,{pr.ping}this pull request has forward-port PRs awaiting action (not merged or closed):{outstanding},"Comment when a forward port has outstanding (not merged or closed) descendants +runbot_merge.forwardport.reminder,{pr.ping}this forward port of {source.display_name} is awaiting action (not merged or closed).,"Comment when a forward port has outstanding (not merged or closed) descendants -pr: the original (source) PR -outstanding: markdown-formatted list of outstanding PRs" +pr: the forward-port +source: the source PR" runbot_merge.forwardport.failure,"{pr.ping}cherrypicking of pull request {pr.source_id.display_name} failed. {commits}{stdout}{stderr} Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).