NextERP/runbot
5
0
Fork 0
mirror of https://github.com/odoo/runbot.git synced 2025-03-15 23:45:44 +07:00
Code Issues Actions Packages Projects Releases Wiki Activity

[FIX] runbot_merge: ensure PR description is correct on merge

Browse Source 
Because sometimes github updates are missed (usually because github
never triggers it), it's possible for the mergebot's view of a PR
description to be incorrect. In that case, the PR may get merged with
the wrong merge message entirely, through no fault of the user.

Since we already fetch the PR info when staging it, there's very
little overhead to checking that the PR message we store is correct
then, and update it if it's not. This means the forward-port's
description should also be correct.

While at it, clean the forward port PR's creation a bit:

- there should always be a message since the title is required on
  PRs (only the body can be missing), therefore no need to check that
- as we're adding a bunch of pseudo-headers, there always is a body,
  no need for the condition
- inline the `pr_data` and `URL`: they were extracted for the support
  of draft PRs, since that's been removed it's now unnecessary

Fixes #530
This commit is contained in:
Xavier Morel 2021-09-24 08:03:24 +02:00 committed by xmo-odoo
parent aa7200d1c8
commit bf34e9aa95
3 changed files with 51 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
forwardport/models/project.py
View File

@ -642,36 +642,28 @@ class PullRequests(models.Model):
for pr in self:
owner, _ = pr.repository.fp_remote_target.split('/', 1)
source = pr.source_id or pr
message = source.message
if message:
message += '\n\n'
else:
message = ''
root = pr._get_root()
message += '\n'.join(
message = source.message + '\n\n' + '\n'.join(
"Forward-Port-Of: %s" % p.display_name
for p in root | source
)
title, body = re.match(r'(?P<title>[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups()
title = '[FW]' + title
if not body:
body = None
self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE')
url = 'https://api.github.com/repos/{}/pulls'.format(pr.repository.name)
pr_data = {
'base': target.name, 'head': '%s:%s' % (owner, new_branch),
'title': title, 'body': body
}
r = gh.post(url, json=pr_data)
r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={
'base': target.name,
'head': f'{owner}:{new_branch}',
'title': '[FW]' + title,
'body': body
})
if not r.ok:
_logger.warning("Failed to create forward-port PR for %s, deleting branches", pr.display_name)
# delete all the branches this should automatically close the
# PRs if we've created any. Using the API here is probably
# simpler than going through the working copies
for repo in self.mapped('repository'):
d = gh.delete('https://api.github.com/repos/{}/git/refs/heads/{}'.format(repo.fp_remote_target, new_branch))
d = gh.delete(f'https://api.github.com/repos/{repo.fp_remote_target}/git/refs/heads/{new_branch}')
if d.ok:
_logger.info("Deleting %s:%s=success", repo.fp_remote_target, new_branch)
else:

5
runbot_merge/models/pull_requests.py
View File

@ -1418,6 +1418,11 @@ class PullRequests(models.Model):
if gh_name:
self.reviewed_by.name = gh_name
# update pr message in case an update was missed
msg = f'{prdict["title"]}\n\n{prdict.get("body") or ""}'.strip()
if self.message != msg:
self.message = msg
# NOTE: lost merge v merge/copy distinction (head being
# a merge commit reused instead of being re-merged)
return method, getattr(self, '_stage_' + method.replace('-', '_'))(

37
runbot_merge/tests/test_oddities.py
View File

@ -1,3 +1,6 @@
from utils import Commit, to_pr
def test_partner_merge(env):
p_src = env['res.partner'].create({
'name': 'kfhsf',
@ -60,3 +63,37 @@ def test_name_search(env):
assert PRs.name_search('repo') == [pr2, pr0, pr1]
assert PRs.name_search('repo#1959') == [pr1]
def test_message_desync(env, project, make_repo, users, setreviewers, config):
"""If the PR message gets desync'd (github misses sending an update), the
merge message should still match what's on github rather than what's in the
db
"""
repo = make_repo('repo')
env['runbot_merge.repository'].create({
'project_id': project.id,
'name': repo.name,
'status_ids': [(0, 0, {'context': 'status'})]
})
setreviewers(*project.repo_ids)
with repo:
[m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')
[c] = repo.make_commits('master', Commit('whee', tree={'b': '2'}))
pr = repo.make_pr(title='title', body='body', target='master', head=c)
repo.post_status(c, 'success', 'status')
env.run_crons()
pr_id = to_pr(env, pr)
assert pr_id.message == 'title\n\nbody'
pr_id.message = "xxx"
with repo:
pr.post_comment('hansen merge r+', config['role_reviewer']['token'])
env.run_crons()
st = repo.commit('staging.master')
assert st.message.startswith('title\n\nbody'),\
"the stored PR message should have been ignored when staging"
assert st.parents == [m, c], "check the staging's ancestry is the right one"