[FIX] runbot_merge: update squash flag on reopen

While the head gets updated (properly), the squash flag did not which
could lead to odd results. Since a PR can only be reopened if it was
regular-pushed to (not after a force push) there are two scenarios:

* the PR updated to have 0 commits, closed, pushed to with one commit
  then reopened, after reopening the PR would be marked as !squash and
  would ask for a merge method (that's what happened with
  odoo/odoo#51763)
* the PR has a single commit, is closed, pushed to then reopened,
  after reopening the PR would still be marked a squash and potentially
  straight rebased without asking for a merge method

Nothing would break per-se but both scenarios are undesirable.

Close #373
This commit is contained in:
Xavier Morel 2020-05-28 13:10:55 +02:00 committed by xmo-odoo
parent db9e86f760
commit b2a4da4739
2 changed files with 35 additions and 28 deletions

View File

@ -207,6 +207,7 @@ def handle_pr(env, event):
'state': 'opened',
# updating the head triggers a revalidation
'head': pr['head']['sha'],
'squash': pr['commits'] == 1,
})
return 'Reopened {}'.format(pr_obj.id)

View File

@ -935,34 +935,6 @@ def test_ci_failure_after_review(env, repo, users, config):
(users['user'], "'ci/runbot' failed on this reviewed PR.".format_map(users)),
]
def test_reopen_state(env, repo):
""" The PR should be validated on opening and reopening in case there's
already a CI+ stored (as the CI might never trigger unless explicitly
re-requested)
"""
with repo:
m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
repo.make_ref('heads/master', m)
c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
repo.post_status(c, 'success', 'legal/cla')
repo.post_status(c, 'success', 'ci/runbot')
prx = repo.make_pr(title='title', body='body', target='master', head=c)
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'validated', \
"if a PR is created on a CI'd commit, it should be validated immediately"
with repo: prx.close()
assert pr.state == 'closed'
with repo: prx.open()
assert pr.state == 'validated', \
"if a PR is reopened and had a CI'd head, it should be validated immediately"
def test_reopen_merged_pr(env, repo, config, users):
""" Reopening a *merged* PR should cause us to immediately close it again,
and insult whoever did it
@ -2147,6 +2119,10 @@ class TestPRUpdate(object):
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert pr.state == 'opened'
assert pr.head == c
assert pr.squash
with repo:
prx.close()
@ -2156,11 +2132,41 @@ class TestPRUpdate(object):
assert pr.state == 'closed'
assert pr.head == c
assert pr.squash
with repo:
prx.open()
assert pr.state == 'opened'
assert pr.head == c2
assert not pr.squash
def test_update_closed_revalidate(self, env, repo):
""" The PR should be validated on opening and reopening in case there's
already a CI+ stored (as the CI might never trigger unless explicitly
re-requested)
"""
with repo:
m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
repo.make_ref('heads/master', m)
c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
repo.post_status(c, 'success', 'legal/cla')
repo.post_status(c, 'success', 'ci/runbot')
prx = repo.make_pr(title='title', body='body', target='master', head=c)
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'validated', \
"if a PR is created on a CI'd commit, it should be validated immediately"
with repo: prx.close()
assert pr.state == 'closed'
with repo: prx.open()
assert pr.state == 'validated', \
"if a PR is reopened and had a CI'd head, it should be validated immediately"
@pytest.mark.xfail(reason="github doesn't allow reopening force-pushed PRs", strict=True)
def test_force_update_closed(self, env, repo):