mirror of
https://github.com/odoo/runbot.git
synced 2025-03-17 16:35:47 +07:00
a4a067e7e9
Thank god I have a bunch of tests because once again I forgot / missed
a bunch of edge cases in doing the conversion, which the tests
caught (sadly that means I almost certainly broke a few untested edge
cases).
Important notes:
Handling of parent links
------------------------
Unlike PRs, batches don't lose their parent info ever, the link is
permanent, which is convenient to trawl through a forward port
(currently implemented very inefficiently, maybe we'll optimise that
in the future).
However this means the batch having a parent and the batch's PRs
having parents are slightly different informations, one of the edge
cases I missed is that of conflicting PRs, which are deparented and
have to be merged by hand before being forward ported further, I had
originally replaced the checks on a pr and its sibling having parents
by just the batch.
Batches & targets
-----------------
Batches were originally concepted as being fixed to a target and PRs
having that target, a PR being retargeted would move it from one batch
to an other.
As it turns out this does not work in the case where people retarget
forward-port PRs, which I know they do because #551
(2337bd8518). I could not think of a
good way to handle this issue as is, so scrapped the moving PRs thing,
instead one of the coherence checks of a batch being ready is that all
its PRs have the same target, and a batch only has a target if all its
PRs have the same target.
It's possible for somewhat odd effects to arise, notably if a PR is
closed (removed from batch), the other PRs are retargeted, and the new
PR is reopened, it will now be on a separate batch even if it also
gets retargeted. This is weird. I don't quite know how I should handle
it, maybe batches could merge if they have the same target and label?
however batches don't currently have a label so...
Improve limits
--------------
Keep limits on the PRs rather than lift them on the batchL if we can
add/remove PRs of batches having different limits on different PRs of
the same batch is reasonable.
Also leave limit unset by default: previously, the limit was eagerly
set to the tip (accessible) branch. That doesn't really seem
necessary, so stop doing that.
Also remove completely unnecessary `max` when trying to find a PR's
next target: `root` is either `self` or `self.source_id`, so it should
not be possible for that to have a later target.
And for now ensure the limits are consistent per batch: a PR defaults
to the limit of their batch-mate if they don't have one, and if a
limit is set via command it's set on all PRs of a batch.
This commit does not allow differential limits via commands, they are
allowed via the backend but not really tested. The issue is mostly
that it's not clear what the UX should look like to have clear and not
super error prone interactions. So punt on it for now, and hopefully
there's no hole I missed which will create inconsistent batches.
129 lines
4.7 KiB
Python
129 lines
4.7 KiB
Python
import json
|
|
|
|
from utils import Commit, make_basic
|
|
|
|
def statuses(pr):
|
|
return {
|
|
k: v['state']
|
|
for k, v in json.loads(pr.statuses_full).items()
|
|
}
|
|
def test_override_inherited(env, config, make_repo, users):
|
|
""" A forwardport should inherit its parents' overrides, until it's edited.
|
|
"""
|
|
repo, other = make_basic(env, config, make_repo)
|
|
project = env['runbot_merge.project'].search([])
|
|
project.repo_ids.status_ids = [(5, 0, 0), (0, 0, {'context': 'default'})]
|
|
env['res.partner'].search([('github_login', '=', users['reviewer'])])\
|
|
.write({'override_rights': [(0, 0, {
|
|
'repository_id': project.repo_ids.id,
|
|
'context': 'default',
|
|
})]})
|
|
|
|
with repo:
|
|
repo.make_commits('a', Commit('pr 1', tree={'a': '0'}), ref='heads/change')
|
|
pr = repo.make_pr(target='a', head='change')
|
|
pr.post_comment('hansen r+ override=default', config['role_reviewer']['token'])
|
|
env.run_crons()
|
|
|
|
original = env['runbot_merge.pull_requests'].search([('repository.name', '=', repo.name), ('number', '=', pr.number)])
|
|
assert original.state == 'ready'
|
|
assert not original.limit_id
|
|
|
|
with repo:
|
|
repo.post_status('staging.a', 'success')
|
|
env.run_crons()
|
|
|
|
pr0_id, pr1_id, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
|
|
assert pr0_id == original
|
|
assert pr0_id.target.name == 'a'
|
|
|
|
assert pr1_id.parent_id == pr0_id
|
|
assert pr1_id.number == 2
|
|
assert pr1_id.target.name == 'b'
|
|
assert pr1_id.state == 'validated'
|
|
assert statuses(pr1_id) == {'default': 'success'}
|
|
|
|
assert pr2_id.parent_id == pr1_id
|
|
assert pr2_id.target.name == 'c'
|
|
assert pr2_id.state == 'validated'
|
|
assert statuses(pr2_id) == {'default': 'success'}
|
|
|
|
# now we edit the child PR
|
|
pr1 = repo.get_pr(pr1_id.number)
|
|
pr_repo, pr_ref = pr1.branch
|
|
with pr_repo:
|
|
pr_repo.make_commits(
|
|
pr1_id.target.name,
|
|
Commit('wop wop', tree={'a': '1'}),
|
|
ref=f'heads/{pr_ref}',
|
|
make=False
|
|
)
|
|
env.run_crons()
|
|
assert pr1_id.state == 'opened'
|
|
assert not pr1_id.parent_id
|
|
assert statuses(pr1_id) == {}, "should not have any status left"
|
|
assert statuses(pr2_id) == {}
|
|
|
|
with repo:
|
|
pr1.post_comment('hansen override=default', config['role_reviewer']['token'])
|
|
assert statuses(pr1_id) == {'default': 'success'}
|
|
assert statuses(pr2_id) == {'default': 'success'}
|
|
|
|
def test_override_combination(env, config, make_repo, users):
|
|
""" A forwardport should inherit its parents' overrides, until it's edited.
|
|
"""
|
|
repo, other = make_basic(env, config, make_repo)
|
|
project = env['runbot_merge.project'].search([])
|
|
env['res.partner'].search([('github_login', '=', users['reviewer'])]) \
|
|
.write({'override_rights': [
|
|
(0, 0, {
|
|
'repository_id': project.repo_ids.id,
|
|
'context': 'ci/runbot',
|
|
}),
|
|
(0, 0, {
|
|
'repository_id': project.repo_ids.id,
|
|
'context': 'legal/cla',
|
|
})
|
|
]})
|
|
|
|
with repo:
|
|
repo.make_commits('a', Commit('C', tree={'a': '0'}), ref='heads/change')
|
|
pr = repo.make_pr(target='a', head='change')
|
|
repo.post_status('change', 'success', 'legal/cla')
|
|
pr.post_comment('hansen r+ override=ci/runbot', config['role_reviewer']['token'])
|
|
env.run_crons()
|
|
|
|
pr0_id = env['runbot_merge.pull_requests'].search([('repository.name', '=', repo.name), ('number', '=', pr.number)])
|
|
assert pr0_id.state == 'ready'
|
|
assert statuses(pr0_id) == {'ci/runbot': 'success', 'legal/cla': 'success'}
|
|
|
|
with repo:
|
|
repo.post_status('staging.a', 'success', 'legal/cla')
|
|
repo.post_status('staging.a', 'success', 'ci/runbot')
|
|
env.run_crons()
|
|
|
|
# check for combination: ci/runbot is overridden through parent, if we
|
|
# override legal/cla then the PR should be validated
|
|
pr1_id = env['runbot_merge.pull_requests'].search([('parent_id', '=', pr0_id.id)])
|
|
assert pr1_id.state == 'opened'
|
|
assert statuses(pr1_id) == {'ci/runbot': 'success'}
|
|
with repo:
|
|
repo.get_pr(pr1_id.number).post_comment('hansen override=legal/cla', config['role_reviewer']['token'])
|
|
env.run_crons()
|
|
assert pr1_id.state == 'validated'
|
|
|
|
# editing the child should devalidate
|
|
pr_repo, pr_ref = repo.get_pr(pr1_id.number).branch
|
|
with pr_repo:
|
|
pr_repo.make_commits(
|
|
pr1_id.target.name,
|
|
Commit('wop wop', tree={'a': '1'}),
|
|
ref=f'heads/{pr_ref}',
|
|
make=False
|
|
)
|
|
env.run_crons()
|
|
assert pr1_id.state == 'opened'
|
|
assert not pr1_id.parent_id
|
|
assert statuses(pr1_id) == {'legal/cla': 'success'}, \
|
|
"should only have its own status left"
|