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

[ADD] runbot_merge: better integration controls

Browse Source 
Closes #48
This commit is contained in:
xmo-odoo 2018-11-26 10:28:13 +01:00 committed by GitHub
parent c814ce8f34
commit 6655e0ea5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 297 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

211
runbot_merge/models/pull_requests.py
View File

@ -286,8 +286,13 @@ class Branch(models.Model):
THEN pr.id::text THEN pr.id::text
ELSE pr.label ELSE pr.label
END END
HAVING (bool_or(pr.priority = 0) AND NOT bool_or(pr.state = 'error')) HAVING
-- all PRs in a group need to specify their merge method
bool_and(pr.squash or pr.merge_method IS NOT NULL)
AND (
(bool_or(pr.priority = 0) AND NOT bool_or(pr.state = 'error'))
OR bool_and(pr.state = 'ready') OR bool_and(pr.state = 'ready')
)
ORDER BY min(pr.priority), min(pr.id) ORDER BY min(pr.priority), min(pr.id)
""", [self.id]) """, [self.id])
# result: [(priority, [(repo_id, pr_id) for repo in repos] # result: [(priority, [(repo_id, pr_id) for repo in repos]
@ -401,7 +406,12 @@ class PullRequests(models.Model):
) )
message = fields.Text(required=True) message = fields.Text(required=True)
squash = fields.Boolean(default=False) squash = fields.Boolean(default=False)
rebase = fields.Boolean(default=True) merge_method = fields.Selection([
('merge', "merge directly, using the PR as merge commit message"),
('rebase-merge', "rebase and merge, using the PR as merge commit message"),
('rebase-ff', "rebase and fast-forward"),
], default=False)
method_warned = fields.Boolean(default=False)
delegates = fields.Many2many('res.partner', help="Delegate reviewers, not intrinsically reviewers but can review this PR") delegates = fields.Many2many('res.partner', help="Delegate reviewers, not intrinsically reviewers but can review this PR")
priority = fields.Selection([ priority = fields.Selection([
@ -463,33 +473,31 @@ class PullRequests(models.Model):
}) })
def _parse_command(self, commandstring): def _parse_command(self, commandstring):
m = re.match(r'(\w+)(?:([+-])|=(.*))?', commandstring) for m in re.finditer(
if not m: r'(\S+?)(?:([+-])|=(\S*))?(?:\s|$)',
return None commandstring,
):
name, flag, param = m.groups() name, flag, param = m.groups()
if name == 'retry': if name == 'retry':
return ('retry', None) yield ('retry', None)
elif name in ('r', 'review'): elif name in ('r', 'review'):
if flag == '+': if flag == '+':
return ('review', True) yield ('review', True)
elif flag == '-': elif flag == '-':
return ('review', False) yield ('review', False)
elif name == 'delegate': elif name == 'delegate':
if flag == '+': if flag == '+':
return ('delegate', True) yield ('delegate', True)
elif param: elif param:
return ('delegate', [ yield ('delegate', [
p.lstrip('#@') p.lstrip('#@')
for p in param.split(',') for p in param.split(',')
]) ])
elif name in ('p', 'priority'): elif name in ('p', 'priority'):
if param in ('0', '1', '2'): if param in ('0', '1', '2'):
return ('priority', int(param)) yield ('priority', int(param))
elif name == 'rebase': elif any(name == k for k, _ in type(self).merge_method.selection):
return ('rebase', flag != '-') yield ('method', name)
return None
def _parse_commands(self, author, comment, login): def _parse_commands(self, author, comment, login):
"""Parses a command string prefixed by Project::github_prefix. """Parses a command string prefixed by Project::github_prefix.
@ -523,9 +531,7 @@ class PullRequests(models.Model):
commands = dict( commands = dict(
ps ps
for m in self.repository.project_id._find_commands(comment) for m in self.repository.project_id._find_commands(comment)
for c in m.strip().split() for ps in self._parse_command(m)
for ps in [self._parse_command(c)]
if ps is not None
) )
if not commands: if not commands:
@ -613,9 +619,9 @@ class PullRequests(models.Model):
self.repository.name, self.number, self.repository.name, self.number,
author.github_login, self.target.name, author.github_login, self.target.name,
) )
elif command == 'rebase': elif command == 'method':
# anyone can rebase- their PR I guess? if is_admin:
self.rebase = param self.merge_method = param
ok = True ok = True
_logger.info( _logger.info(
@ -772,6 +778,95 @@ class PullRequests(models.Model):
if commit: if commit:
self.env.cr.commit() self.env.cr.commit()
# send feedback for multi-commit PRs without a merge_method (which
# we've not warned yet)
for r in self.search([
('state', '=', 'ready'),
('squash', '=', False),
('merge_method', '=', False),
('method_warned', '=', False),
]):
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': r.repository.id,
'pull_request': r.number,
'message': "Because this PR has multiple commits, I need to know how to merge it:\n\n" + ''.join(
'* `%s` to %s\n' % pair
for pair in type(self).merge_method.selection
)
})
r.method_warned = True
if commit:
self.env.cr.commit()
def _build_merge_message(self, message):
m = re.search(r'( |{repository})#{pr.number}\b'.format(
pr=self,
repository=self.repository.name.replace('/', '\\/')
), message)
if m:
return message
return message + '\n\ncloses {pr.repository.name}#{pr.number}'.format(pr=self)
def _stage(self, gh, target):
# nb: pr_commits is oldest to newest so pr.head is pr_commits[-1]
_, prdict = gh.pr(self.number)
commits = prdict['commits']
method = self.merge_method or ('rebase-ff' if commits == 1 else None)
assert commits < 50 or not method.startswith('rebase'), \
"rebasing a PR or more than 50 commits is a tad excessive"
assert commits < 250, "merging PRs of 250+ commits is not supported (https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request)"
pr_commits = gh.commits(self.number)
# 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('-', '_'))(gh, target, pr_commits)
def _stage_rebase_ff(self, gh, target, commits):
# updates head commit with PR number (if necessary) then rebases
# on top of target
msg = self._build_merge_message(commits[-1]['commit']['message'])
commits[-1]['commit']['message'] = msg
return gh.rebase(self.number, target, commits=commits)
def _stage_rebase_merge(self, gh, target, commits):
msg = self._build_merge_message(self.message)
h = gh.rebase(self.number, target, reset=True, commits=commits)
return gh.merge(h, target, msg)['sha']
def _stage_merge(self, gh, target, commits):
pr_head = commits[-1] # oldest to newest
base_commit = None
head_parents = {p['sha'] for p in pr_head['parents']}
if len(head_parents) > 1:
# look for parent(s?) of pr_head not in PR, means it's
# from target (so we merged target in pr)
merge = head_parents - {c['sha'] for c in commits}
assert len(merge) <= 1, \
">1 parent from base in PR's head is not supported"
if len(merge) == 1:
[base_commit] = merge
if base_commit:
# replicate pr_head with base_commit replaced by
# the current head
original_head = gh.head(target)
merge_tree = gh.merge(pr_head['sha'], target, 'temp merge')['tree']['sha']
new_parents = [original_head] + list(head_parents - {base_commit})
msg = self._build_merge_message(pr_head['commit']['message'])
copy = gh('post', 'git/commits', json={
'message': msg,
'tree': merge_tree,
'author': pr_head['commit']['author'],
'committer': pr_head['commit']['committer'],
'parents': new_parents,
}).json()
gh.set_ref(target, copy['sha'])
return copy['sha']
else:
# otherwise do a regular merge
msg = self._build_merge_message(self.message)
return gh.merge(self.head, target, msg)['sha']
# state changes on reviews # state changes on reviews
RPLUS = { RPLUS = {
'opened': 'approved', 'opened': 'approved',
@ -1194,15 +1289,6 @@ class Batch(models.Model):
:return: () or Batch object (if all prs successfully staged) :return: () or Batch object (if all prs successfully staged)
""" """
def build_message(message, pr):
m = re.search(r'( |{repository})#{pr.number}\b'.format(
pr=pr, repository=pr.repository.name.replace('/', '\\/')),
message)
if m:
return message
return message + '\n\ncloses {pr.repository.name}#{pr.number}'.format(pr=pr)
new_heads = {} new_heads = {}
for pr in prs: for pr in prs:
gh = meta[pr.repository]['gh'] gh = meta[pr.repository]['gh']
@ -1215,65 +1301,12 @@ class Batch(models.Model):
target = 'tmp.{}'.format(pr.target.name) target = 'tmp.{}'.format(pr.target.name)
original_head = gh.head(target) original_head = gh.head(target)
try: try:
# nb: pr_commits is oldest to newest so pr.head is pr_commits[-1] method, new_heads[pr] = pr._stage(gh, target)
_, prdict = gh.pr(pr.number)
commits = prdict['commits']
assert commits < 50 or not pr.rebase, \
"rebasing a PR or more than 50 commits is a tad excessive"
assert commits < 250, "merging PRs of 250+ commits is not supported (https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request)"
pr_commits = gh.commits(pr.number)
rebase_and_merge = pr.rebase
squash = rebase_and_merge and commits == 1
if squash:
method = 'squash'
msg = build_message(pr_commits[0]['commit']['message'], pr)
pr_commits[0]['commit']['message'] = msg
new_heads[pr] = gh.rebase(pr.number, target, commits=pr_commits)
elif rebase_and_merge:
method = 'rebase & merge'
msg = build_message(pr.message, pr)
h = gh.rebase(pr.number, target, reset=True, commits=pr_commits)
new_heads[pr] = gh.merge(h, target, msg)['sha']
else:
pr_head = pr_commits[-1] # pr_commits is oldest to newest
base_commit = None
head_parents = {p['sha'] for p in pr_head['parents']}
if len(head_parents) > 1:
# look for parent(s?) of pr_head not in PR, means it's
# from target (so we merged target in pr)
merge = head_parents - {c['sha'] for c in pr_commits}
assert len(merge) <= 1, \
">1 parent from base in PR's head is not supported"
if len(merge) == 1:
[base_commit] = merge
if base_commit:
method = 'merge/copy'
# replicate pr_head with base_commit replaced by
# the current head
original_head = gh.head(target)
merge_tree = gh.merge(pr_head['sha'], target, 'temp merge')['tree']['sha']
new_parents = [original_head] + list(head_parents - {base_commit})
msg = build_message(pr_head['commit']['message'], pr)
copy = gh('post', 'git/commits', json={
'message': msg,
'tree': merge_tree,
'author': pr_head['commit']['author'],
'committer': pr_head['commit']['committer'],
'parents': new_parents,
}).json()
gh.set_ref(target, copy['sha'])
new_heads[pr] = copy['sha']
else:
method = 'merge'
# otherwise do a regular merge
msg = build_message(pr.message, pr)
new_heads[pr] = gh.merge(pr.head, target, msg)['sha']
new_head = gh.head(target)
_logger.info( _logger.info(
"Staged pr %s:%s by %s to %s; %s %s -> %s", "Staged pr %s:%s to %s by %s: %s -> %s",
pr.repository.name, pr.number, method, new_heads[pr], pr.repository.name, pr.number,
target, original_head, new_head pr.target.name, method,
original_head, new_heads[pr]
) )
except (exceptions.MergeError, AssertionError) as e: except (exceptions.MergeError, AssertionError) as e:
_logger.exception("Failed to merge %s:%s into staging branch (error: %s)", pr.repository.name, pr.number, e) _logger.exception("Failed to merge %s:%s into staging branch (error: %s)", pr.repository.name, pr.number, e)

191
runbot_merge/tests/test_basic.py
View File

@ -47,7 +47,7 @@ def test_trivial_flow(env, repo, page):
env['runbot_merge.project']._send_feedback() env['runbot_merge.project']._send_feedback()
assert pr1.labels == {'seen 🙂', 'CI 🤖'} assert pr1.labels == {'seen 🙂', 'CI 🤖'}
pr1.post_comment('hansen r+', 'reviewer') pr1.post_comment('hansen r+ rebase-merge', 'reviewer')
assert pr.state == 'ready' assert pr.state == 'ready'
# can't check labels here as running the cron will stage it # can't check labels here as running the cron will stage it
@ -240,7 +240,7 @@ def test_staging_conflict(env, repo):
pr1 = repo.make_pr("gibberish", "blahblah", target='master', ctid=c1, user='user') pr1 = repo.make_pr("gibberish", "blahblah", target='master', ctid=c1, user='user')
repo.post_status(c1, 'success', 'legal/cla') repo.post_status(c1, 'success', 'legal/cla')
repo.post_status(c1, 'success', 'ci/runbot') repo.post_status(c1, 'success', 'ci/runbot')
pr1.post_comment("hansen r+", "reviewer") pr1.post_comment("hansen r+ rebase-merge", "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
pr1 = env['runbot_merge.pull_requests'].search([ pr1 = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -254,7 +254,7 @@ def test_staging_conflict(env, repo):
pr2 = repo.make_pr('gibberish', 'blahblah', target='master', ctid=c3, user='user') pr2 = repo.make_pr('gibberish', 'blahblah', target='master', ctid=c3, user='user')
repo.post_status(c3, 'success', 'legal/cla') repo.post_status(c3, 'success', 'legal/cla')
repo.post_status(c3, 'success', 'ci/runbot') repo.post_status(c3, 'success', 'ci/runbot')
pr2.post_comment('hansen r+', "reviewer") pr2.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
env['runbot_merge.project']._send_feedback() env['runbot_merge.project']._send_feedback()
p_2 = env['runbot_merge.pull_requests'].search([ p_2 = env['runbot_merge.pull_requests'].search([
@ -292,14 +292,14 @@ def test_staging_concurrent(env, repo):
pr1 = repo.make_pr('t1', 'b1', target='1.0', ctid=c11, user='user') pr1 = repo.make_pr('t1', 'b1', target='1.0', ctid=c11, user='user')
repo.post_status(pr1.head, 'success', 'ci/runbot') repo.post_status(pr1.head, 'success', 'ci/runbot')
repo.post_status(pr1.head, 'success', 'legal/cla') repo.post_status(pr1.head, 'success', 'legal/cla')
pr1.post_comment('hansen r+', "reviewer") pr1.post_comment('hansen r+ rebase-merge', "reviewer")
c20 = repo.make_commit(m, 'CCC', None, tree={'m': 'm', 'c': 'c'}) c20 = repo.make_commit(m, 'CCC', None, tree={'m': 'm', 'c': 'c'})
c21 = repo.make_commit(c20, 'DDD', None, tree={'m': 'm', 'c': 'c', 'd': 'd'}) c21 = repo.make_commit(c20, 'DDD', None, tree={'m': 'm', 'c': 'c', 'd': 'd'})
pr2 = repo.make_pr('t2', 'b2', target='2.0', ctid=c21, user='user') pr2 = repo.make_pr('t2', 'b2', target='2.0', ctid=c21, user='user')
repo.post_status(pr2.head, 'success', 'ci/runbot') repo.post_status(pr2.head, 'success', 'ci/runbot')
repo.post_status(pr2.head, 'success', 'legal/cla') repo.post_status(pr2.head, 'success', 'legal/cla')
pr2.post_comment('hansen r+', "reviewer") pr2.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
pr1 = env['runbot_merge.pull_requests'].search([ pr1 = env['runbot_merge.pull_requests'].search([
@ -325,7 +325,7 @@ def test_staging_merge_fail(env, repo, users):
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+', "reviewer") prx.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
env['runbot_merge.project']._send_feedback() env['runbot_merge.project']._send_feedback()
@ -336,7 +336,7 @@ def test_staging_merge_fail(env, repo, users):
assert pr1.state == 'error' assert pr1.state == 'error'
assert prx.labels == {'seen 🙂', 'error 🙅'} assert prx.labels == {'seen 🙂', 'error 🙅'}
assert prx.comments == [ assert prx.comments == [
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+ rebase-merge'),
(users['user'], re_matches('^Unable to stage PR')), (users['user'], re_matches('^Unable to stage PR')),
] ]
@ -352,7 +352,7 @@ def test_staging_ci_timeout(env, repo, users):
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+', "reviewer") prx.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
pr1 = env['runbot_merge.pull_requests'].search([ pr1 = env['runbot_merge.pull_requests'].search([
@ -377,7 +377,7 @@ def test_staging_ci_failure_single(env, repo, users):
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+', "reviewer") prx.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
assert env['runbot_merge.pull_requests'].search([ assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -394,7 +394,7 @@ def test_staging_ci_failure_single(env, repo, users):
]).state == 'error' ]).state == 'error'
assert prx.comments == [ assert prx.comments == [
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+ rebase-merge'),
(users['user'], 'Staging failed: ci/runbot') (users['user'], 'Staging failed: ci/runbot')
] ]
@ -408,7 +408,7 @@ def test_ff_failure(env, repo, users):
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+', "reviewer") prx.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
assert env['runbot_merge.pull_requests'].search([ assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -440,21 +440,21 @@ def test_ff_failure_batch(env, repo, users):
A = repo.make_pr('A', None, target='master', ctid=a2, user='user', label='A') A = repo.make_pr('A', None, target='master', ctid=a2, user='user', label='A')
repo.post_status(A.head, 'success', 'legal/cla') repo.post_status(A.head, 'success', 'legal/cla')
repo.post_status(A.head, 'success', 'ci/runbot') repo.post_status(A.head, 'success', 'ci/runbot')
A.post_comment('hansen r+', "reviewer") A.post_comment('hansen r+ rebase-merge', "reviewer")
b1 = repo.make_commit(m, 'b1', None, tree={'m': 'm', 'b': '1'}) b1 = repo.make_commit(m, 'b1', None, tree={'m': 'm', 'b': '1'})
b2 = repo.make_commit(b1, 'b2', None, tree={'m': 'm', 'b': '2'}) b2 = repo.make_commit(b1, 'b2', None, tree={'m': 'm', 'b': '2'})
B = repo.make_pr('B', None, target='master', ctid=b2, user='user', label='B') B = repo.make_pr('B', None, target='master', ctid=b2, user='user', label='B')
repo.post_status(B.head, 'success', 'legal/cla') repo.post_status(B.head, 'success', 'legal/cla')
repo.post_status(B.head, 'success', 'ci/runbot') repo.post_status(B.head, 'success', 'ci/runbot')
B.post_comment('hansen r+', "reviewer") B.post_comment('hansen r+ rebase-merge', "reviewer")
c1 = repo.make_commit(m, 'c1', None, tree={'m': 'm', 'c': '1'}) c1 = repo.make_commit(m, 'c1', None, tree={'m': 'm', 'c': '1'})
c2 = repo.make_commit(c1, 'c2', None, tree={'m': 'm', 'c': '2'}) c2 = repo.make_commit(c1, 'c2', None, tree={'m': 'm', 'c': '2'})
C = repo.make_pr('C', None, target='master', ctid=c2, user='user', label='C') C = repo.make_pr('C', None, target='master', ctid=c2, user='user', label='C')
repo.post_status(C.head, 'success', 'legal/cla') repo.post_status(C.head, 'success', 'legal/cla')
repo.post_status(C.head, 'success', 'ci/runbot') repo.post_status(C.head, 'success', 'ci/runbot')
C.post_comment('hansen r+', "reviewer") C.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
messages = [ messages = [
@ -586,7 +586,7 @@ def test_forward_port(env, repo):
pr = repo.make_pr('PR', None, target='master', ctid=head, user='user') pr = repo.make_pr('PR', None, target='master', ctid=head, user='user')
repo.post_status(pr.head, 'success', 'legal/cla') repo.post_status(pr.head, 'success', 'legal/cla')
repo.post_status(pr.head, 'success', 'ci/runbot') repo.post_status(pr.head, 'success', 'ci/runbot')
pr.post_comment('hansen rebase- r+', "reviewer") pr.post_comment('hansen r+ merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
st = repo.commit('heads/staging.master') st = repo.commit('heads/staging.master')
@ -721,7 +721,7 @@ class TestRetry:
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+ delegate=%s' % users['other'], "reviewer") prx.post_comment('hansen r+ delegate=%s rebase-merge' % users['other'], "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
assert env['runbot_merge.pull_requests'].search([ assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -783,7 +783,7 @@ class TestRetry:
prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+ delegate=%s' % users['other'], "reviewer") prx.post_comment('hansen r+ delegate=%s rebase-merge' % users['other'], "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
assert env['runbot_merge.pull_requests'].search([ assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -813,7 +813,9 @@ class TestMergeMethod:
if event['pull_request']['commits'] == 1, "squash" (/rebase); otherwise if event['pull_request']['commits'] == 1, "squash" (/rebase); otherwise
regular merge regular merge
""" """
def test_staging_merge_squash(self, repo, env): def test_pr_single_commit(self, repo, env):
""" If single commit, default to rebase & FF
"""
m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
m2 = repo.make_commit(m, 'second', None, tree={'m': 'm', 'm2': 'm2'}) m2 = repo.make_commit(m, 'second', None, tree={'m': 'm', 'm2': 'm2'})
repo.make_ref('heads/master', m2) repo.make_ref('heads/master', m2)
@ -856,7 +858,7 @@ class TestMergeMethod:
]).state == 'merged' ]).state == 'merged'
assert prx.state == 'closed' assert prx.state == 'closed'
def test_pr_update_unsquash(self, repo, env): def test_pr_update_to_many_commits(self, repo, env):
""" """
If a PR starts with 1 commit and a second commit is added, the PR If a PR starts with 1 commit and a second commit is added, the PR
should be unflagged as squash should be unflagged as squash
@ -876,7 +878,7 @@ class TestMergeMethod:
prx.push(repo.make_commit(c1, 'second2', None, tree={'m': 'c2'})) prx.push(repo.make_commit(c1, 'second2', None, tree={'m': 'c2'}))
assert not pr.squash, "a PR with a single commit should not be squashed" assert not pr.squash, "a PR with a single commit should not be squashed"
def test_pr_reset_squash(self, repo, env): def test_pr_reset_to_single_commit(self, repo, env):
""" """
If a PR starts at >1 commits and is reset back to 1, the PR should be If a PR starts at >1 commits and is reset back to 1, the PR should be
re-flagged as squash re-flagged as squash
@ -897,8 +899,67 @@ class TestMergeMethod:
prx.push(repo.make_commit(m, 'fixup', None, tree={'m': 'c2'})) prx.push(repo.make_commit(m, 'fixup', None, tree={'m': 'c2'}))
assert pr.squash, "a PR with a single commit should be squashed" assert pr.squash, "a PR with a single commit should be squashed"
def test_pr_no_method(self, repo, env, users):
""" a multi-repo PR should not be staged by default, should also get
feedback indicating a merge method is necessary
"""
m0 = repo.make_commit(None, 'M0', None, tree={'m': '0'})
m1 = repo.make_commit(m0, 'M1', None, tree={'m': '1'})
m2 = repo.make_commit(m1, 'M2', None, tree={'m': '2'})
repo.make_ref('heads/master', m2)
b0 = repo.make_commit(m1, 'B0', None, tree={'m': '1', 'b': '0'})
b1 = repo.make_commit(b0, 'B1', None, tree={'m': '1', 'b': '1'})
prx = repo.make_pr('title', 'body', target='master', ctid=b1, user='user')
repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+', "reviewer")
run_crons(env)
assert not env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
]).staging_id
assert prx.comments == [
(users['reviewer'], 'hansen r+'),
(users['user'], """Because this PR has multiple commits, I need to know how to merge it:
* `merge` to merge directly, using the PR as merge commit message
* `rebase-merge` to rebase and merge, using the PR as merge commit message
* `rebase-ff` to rebase and fast-forward
"""),
]
def test_pr_method_no_review(self, repo, env):
""" Configuring the method should be idependent from the review
"""
m0 = repo.make_commit(None, 'M0', None, tree={'m': '0'})
m1 = repo.make_commit(m0, 'M1', None, tree={'m': '1'})
m2 = repo.make_commit(m1, 'M2', None, tree={'m': '2'})
repo.make_ref('heads/master', m2)
b0 = repo.make_commit(m1, 'B0', None, tree={'m': '1', 'b': '0'})
b1 = repo.make_commit(b0, 'B1', None, tree={'m': '1', 'b': '1'})
prx = repo.make_pr('title', 'body', target='master', ctid=b1, user='user')
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen rebase-merge', "reviewer")
assert pr.merge_method == 'rebase-merge'
prx.post_comment('hansen merge', "reviewer")
assert pr.merge_method == 'merge'
prx.post_comment('hansen rebase-ff', "reviewer")
assert pr.merge_method == 'rebase-ff'
def test_pr_rebase_merge(self, repo, env): def test_pr_rebase_merge(self, repo, env):
""" a multi-commit PR should be rebased & merged by default """ test result on rebase-merge
left: PR left: PR
right: post-merge result right: post-merge result
@ -940,7 +1001,7 @@ class TestMergeMethod:
prx = repo.make_pr('title', 'body', target='master', ctid=b1, user='user') prx = repo.make_pr('title', 'body', target='master', ctid=b1, user='user')
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+', "reviewer") prx.post_comment('hansen r+ rebase-merge', "reviewer")
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
@ -971,11 +1032,72 @@ class TestMergeMethod:
final_tree = repo.read_tree(repo.commit('heads/master')) final_tree = repo.read_tree(repo.commit('heads/master'))
assert final_tree == {'m': b'2', 'b': b'1'}, "sanity check of final tree" assert final_tree == {'m': b'2', 'b': b'1'}, "sanity check of final tree"
def test_pr_rebase_ff(self, repo, env):
""" test result on rebase-merge
left: PR
right: post-merge result
+------+ +------+
| M0 | | M0 |
+--^---+ +--^---+
| |
| |
+--+---+ +--+---+
+----> M1 <--+ | M1 <--+
| +------+ | +------+ |
| | |
| | |
+--+---+ +---+---+ +------+ +---+---+
| B0 | | M2 | | B0 +------> M2 |
+--^---+ +-------+ +--^---+ +---^---+
| |
+--+---+ +--+---+
PR | B1 | | B1 |
+------+ +--^---+
"""
m0 = repo.make_commit(None, 'M0', None, tree={'m': '0'})
m1 = repo.make_commit(m0, 'M1', None, tree={'m': '1'})
m2 = repo.make_commit(m1, 'M2', None, tree={'m': '2'})
repo.make_ref('heads/master', m2)
b0 = repo.make_commit(m1, 'B0', None, tree={'m': '1', 'b': '0'})
b1 = repo.make_commit(b0, 'B1', None, tree={'m': '1', 'b': '1'})
prx = repo.make_pr('title', 'body', target='master', ctid=b1, user='user')
repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-ff', "reviewer")
env['runbot_merge.project']._check_progress()
# create a dag (msg:str, parents:set) from the log
staging = log_to_node(repo.log('heads/staging.master'))
# then compare to the dag version of the right graph
nm2 = node('M2', node('M1', node('M0')))
nb1 = node('B1\n\ncloses {}#{}'.format(repo.name, prx.number), node('B0', nm2))
expected = node(re_matches('^force rebuild'), nb1)
assert staging == expected
repo.post_status('heads/staging.master', 'success', 'legal/cla')
repo.post_status('heads/staging.master', 'success', 'ci/runbot')
env['runbot_merge.project']._check_progress()
assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
]).state == 'merged'
# check that the dummy commit is not in the final master
master = log_to_node(repo.log('heads/master'))
assert master == nb1
final_tree = repo.read_tree(repo.commit('heads/master'))
assert final_tree == {'m': b'2', 'b': b'1'}, "sanity check of final tree"
@pytest.mark.skip(reason="what do if the PR contains merge commits???") @pytest.mark.skip(reason="what do if the PR contains merge commits???")
def test_pr_contains_merges(self, repo, env): def test_pr_contains_merges(self, repo, env):
pass pass
def test_pr_unrebase(self, repo, env): def test_pr_force_merge_single_commit(self, repo, env):
""" should be possible to flag a PR as regular-merged, regardless of """ should be possible to flag a PR as regular-merged, regardless of
its commits count its commits count
@ -995,7 +1117,7 @@ class TestMergeMethod:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-', 'reviewer') prx.post_comment('hansen r+ merge', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
@ -1024,7 +1146,7 @@ class TestMergeMethod:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-', 'reviewer') prx.post_comment('hansen r+ merge', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
@ -1063,7 +1185,7 @@ class TestMergeMethod:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-', 'reviewer') prx.post_comment('hansen r+ merge', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
@ -1103,7 +1225,7 @@ class TestMergeMethod:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-', 'reviewer') prx.post_comment('hansen r+ merge', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
@ -1406,7 +1528,10 @@ class TestBatching(object):
for context, result in statuses: for context, result in statuses:
repo.post_status(c, result, context) repo.post_status(c, result, context)
if reviewer: if reviewer:
pr.post_comment('hansen r+', reviewer) pr.post_comment(
'hansen r+%s' % (' rebase-merge' if len(trees) > 1 else ''),
reviewer
)
return pr return pr
def _get(self, env, repo, number): def _get(self, env, repo, number):
@ -1456,16 +1581,16 @@ class TestBatching(object):
repo.make_ref('heads/master', m) repo.make_ref('heads/master', m)
pr1 = self._pr(repo, 'PR1', [{'a': 'AAA'}, {'b': 'BBB'}]) pr1 = self._pr(repo, 'PR1', [{'a': 'AAA'}, {'b': 'BBB'}])
pr1.post_comment('hansen rebase-', 'reviewer') pr1.post_comment('hansen merge', 'reviewer')
pr2 = self._pr(repo, 'PR2', [{'c': 'CCC'}, {'d': 'DDD'}]) pr2 = self._pr(repo, 'PR2', [{'c': 'CCC'}, {'d': 'DDD'}])
pr2.post_comment('hansen rebase-', 'reviewer') pr2.post_comment('hansen merge', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
pr1 = self._get(env, repo, pr1.number) pr1 = self._get(env, repo, pr1.number)
assert pr1.staging_id assert pr1.staging_id
assert not pr1.rebase assert pr1.merge_method == 'merge'
pr2 = self._get(env, repo, pr2.number) pr2 = self._get(env, repo, pr2.number)
assert not pr2.rebase assert pr2.merge_method == 'merge'
assert pr1.staging_id assert pr1.staging_id
assert pr2.staging_id assert pr2.staging_id
assert pr1.staging_id == pr2.staging_id assert pr1.staging_id == pr2.staging_id
@ -1565,7 +1690,7 @@ class TestBatching(object):
# no statuses run on PR0s # no statuses run on PR0s
pr01 = self._pr(repo, 'Urgent1', [{'n': 'n'}, {'o': 'o'}], reviewer=None, statuses=[]) pr01 = self._pr(repo, 'Urgent1', [{'n': 'n'}, {'o': 'o'}], reviewer=None, statuses=[])
pr01.post_comment('hansen priority=0', 'reviewer') pr01.post_comment('hansen priority=0 rebase-merge', 'reviewer')
p_01 = self._get(env, repo, pr01.number) p_01 = self._get(env, repo, pr01.number)
assert p_01.state == 'opened' assert p_01.state == 'opened'
assert p_01.priority == 0 assert p_01.priority == 0

3
runbot_merge/tests/test_multirepo.py
View File

@ -450,7 +450,8 @@ def test_urgent(env, repo_a, repo_b):
pr_b = make_pr(repo_b, 'B', [{'b1': 'b'}, {'b2': 'b'}], label='batch', reviewer=None, statuses=[]) pr_b = make_pr(repo_b, 'B', [{'b1': 'b'}, {'b2': 'b'}], label='batch', reviewer=None, statuses=[])
pr_c = make_pr(repo_a, 'C', [{'c1': 'c', 'c2': 'c'}]) pr_c = make_pr(repo_a, 'C', [{'c1': 'c', 'c2': 'c'}])
pr_b.post_comment('hansen p=0', 'reviewer') pr_a.post_comment('hansen rebase-merge', 'reviewer')
pr_b.post_comment('hansen rebase-merge p=0', 'reviewer')
env['runbot_merge.project']._check_progress() env['runbot_merge.project']._check_progress()
# should have batched pr_a and pr_b despite neither being reviewed or # should have batched pr_a and pr_b despite neither being reviewed or