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] forwardport: correct batching of intermediate PRs

Browse Source 
When inserting a new branch, if there are extant forward ports
sequences spanning over the insertion, new forward ports must be
created for the new branch.

This was the case, *however* one of the cases was handled incorrectly:
PR batches (PRs to different repositories staged together) must be
forward-ported as batches. However when creating the "insert" batches,
these PRs would be split apart, leading to either inconsistent states
or an inability to merge the new forward ports (if they are
co-dependent which is a common case). This is because the handler
would look for all the relevant PRs to forward ports, but it would
fail to group them by label thereafter.

Fix that, create the `insert` batches correctly, and augment the
relevant test with an additional repository to test this case.

No gh issue was created, but this problem was reported on
odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by
hand in the runbot and mergebot, but on github the labels remain
visibly different, one having the slug ZCwE while the other
hasO7Pr).

It's possible that the problem had occurred previously and not been
noticed (or reported), though it's also possible this issue had never
occurred before: for the latest freeze (16.1) there were 18 forward
ports spanning the insertion, but of them only 2 were the same batch.
This commit is contained in:
Xavier Morel 2023-01-19 10:49:16 +01:00
parent 652b1ff9ae
commit 2742fe18fd
2 changed files with 61 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
forwardport/models/project.py
View File

@ -34,7 +34,7 @@ import resource
from odoo import _, models, fields, api
from odoo.osv import expression
from odoo.exceptions import UserError
from odoo.tools import topological_sort, groupby
from odoo.tools.misc import topological_sort, groupby
from odoo.tools.sql import reverse_order
from odoo.tools.appdirs import user_cache_dir
from odoo.addons.runbot_merge import utils
@ -187,13 +187,13 @@ class Project(models.Model):
])
logger.debug("\nPRs spanning new: %s\nto port: %s", leaves, candidates)
# enqueue the creation of a new forward-port based on our candidates
# but it should only create a single step and needs to stitch batch
# but it should only create a single step and needs to stitch back
# the parents linked list, so it has a special type
for c in candidates:
for _, cs in groupby(candidates, key=lambda p: p.label):
self.env['forwardport.batches'].create({
'batch_id': self.env['runbot_merge.batch'].create({
'target': before[-1].id,
'prs': [(4, c.id, 0)],
'prs': [(4, c.id, 0) for c in cs],
'active': False,
}).id,
'source': 'insert',

80
forwardport/tests/test_weird.py
View File

@ -425,30 +425,39 @@ def test_new_intermediate_branch(env, config, make_repo):
1.0, 2.0 and master, if a branch 3.0 is forked off from master and inserted
before it, we need to create a new *intermediate* forward port PR
"""
def validate(commit):
prod.post_status(commit, 'success', 'ci/runbot')
prod.post_status(commit, 'success', 'legal/cla')
def validate(repo, commit):
repo.post_status(commit, 'success', 'ci/runbot')
repo.post_status(commit, 'success', 'legal/cla')
project, prod, _ = make_basic(env, config, make_repo, fp_token=True, fp_remote=True)
_, prod2, _ = make_basic(env, config, make_repo, fp_token=True, fp_remote=True)
assert len(project.repo_ids) == 2
original_c_tree = prod.read_tree(prod.commit('c'))
prs = []
with prod:
with prod, prod2:
for i in ['0', '1']:
prod.make_commits('a', Commit(i, tree={i:i}), ref='heads/branch%s' % i)
pr = prod.make_pr(target='a', head='branch%s' % i)
prs.append(pr)
validate(pr.head)
validate(prod, pr.head)
pr.post_comment('hansen r+', config['role_reviewer']['token'])
# also add a PR targeting b forward-ported to c, in order to check
# for an insertion right after the source
# for an insertion right after the source, as well as have linked PRs in
# two different repos
prod.make_commits('b', Commit('x', tree={'x': 'x'}), ref='heads/branchx')
prod2.make_commits('b', Commit('x2', tree={'x': 'x2'}), ref='heads/branchx')
prx = prod.make_pr(target='b', head='branchx')
validate(prx.head)
prx2 = prod2.make_pr(target='b', head='branchx')
validate(prod, prx.head)
validate(prod2, prx2.head)
prx.post_comment('hansen r+', config['role_reviewer']['token'])
prx2.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
validate('staging.a')
validate('staging.b')
with prod, prod2:
for r in [prod, prod2]:
validate(r, 'staging.a')
validate(r, 'staging.b')
env.run_crons()
# should have merged pr1, pr2 and prx and created their forward ports, now
@ -461,7 +470,7 @@ def test_new_intermediate_branch(env, config, make_repo):
assert pr0_fp_id
assert pr0_fp_id.target.name == 'b'
with prod:
validate(pr0_fp_id.head)
validate(prod, pr0_fp_id.head)
env.run_crons()
original0 = PRs.search([('parent_id', '=', pr0_fp_id.id)])
assert original0, "Could not find FP of PR0 to C"
@ -473,15 +482,26 @@ def test_new_intermediate_branch(env, config, make_repo):
# also check prx's fp
prx_id = to_pr(env, prx)
prx2_id = to_pr(env, prx2)
assert prx_id.label == prx2_id.label
prx_fp_id = PRs.search([('source_id', '=', prx_id.id)])
assert prx_fp_id
assert prx_fp_id.target.name == 'c'
prx2_fp_id = PRs.search([('source_id', '=', prx2_id.id)])
assert prx2_fp_id
assert prx2_fp_id.target.name == 'c'
assert prx_fp_id.label == prx2_fp_id.label,\
"ensure labels of PRs of same batch are the same"
# NOTE: the branch must be created on git(hub) first, probably
# create new branch forked from the "current master" (c)
c = prod.commit('c').id
with prod:
prod.make_ref('heads/new', c)
c2 = prod2.commit('c').id
with prod2:
prod2.make_ref('heads/new', c2)
currents = {branch.name: branch.id for branch in project.branch_ids}
# insert a branch between "b" and "c"
project.write({
@ -514,6 +534,15 @@ def test_new_intermediate_branch(env, config, make_repo):
assert newx.target.name == 'new'
assert prx_fp_id.parent_id == newx
descx2 = PRs.search([('source_id', '=', prx2_id.id)])
newx2 = descx2 - prx2_fp_id
assert len(newx2) == 1
assert newx2.parent_id == prx2_id
assert newx2.target.name == 'new'
assert prx2_fp_id.parent_id == newx2
assert newx.label == newx2.label
# created followups for 1
# earliest followup is followup from deactivating a branch, creates fp in
# n+1 = c (from b), then inserting a new branch between b and c should
@ -527,20 +556,24 @@ def test_new_intermediate_branch(env, config, make_repo):
# ci on pr1/pr2 fp to b
sources = [to_pr(env, pr).id for pr in prs]
sources.append(prx_id.id)
sources.append(prx2_id.id)
def get_repo(pr):
if pr.repository.name == prod.name:
return prod
return prod2
# CI all the forward port PRs (shouldn't hurt to re-ci the forward port of
# prs[0] to b aka pr0_fp_id
for target in ['new', 'c']:
fps = PRs.search([('source_id', 'in', sources), ('target.name', '=', target)])
with prod:
for fp in fps:
validate(fp.head)
env.run_crons()
fps = PRs.search([('source_id', 'in', sources), ('target.name', '=', ['new', 'c'])])
with prod, prod2:
for fp in fps:
validate(get_repo(fp), fp.head)
env.run_crons()
# now fps should be the last PR of each sequence, and thus r+-able (via
# fwbot so preceding PR is also r+'d)
with prod:
for pr in fps:
assert pr.target.name == 'c'
prod.get_pr(pr.number).post_comment(
with prod, prod2:
for pr in fps.filtered(lambda p: p.target.name == 'c'):
get_repo(pr).get_pr(pr.number).post_comment(
'%s r+' % project.fp_github_name,
config['role_reviewer']['token'])
assert all(p.state == 'merged' for p in PRs.browse(sources)),\
@ -551,9 +584,10 @@ def test_new_intermediate_branch(env, config, make_repo):
assert len(env['runbot_merge.stagings'].search([])) == 2,\
"enabled branches should have been staged"
with prod:
with prod, prod2:
for target in ['new', 'c']:
validate(f'staging.{target}')
validate(prod, f'staging.{target}')
validate(prod2, f'staging.{target}')
env.run_crons()
assert all(p.state == 'merged' for p in PRs.search([('target.name', '!=', 'b')])), \
"All PRs except disabled branch should be merged now"