runbot/forwardport/tests/test_batches.py

399 lines
14 KiB
Python
Raw Permalink Normal View History

from utils import Commit, make_basic, to_pr, seen
def test_single_updated(env, config, make_repo):
""" Given co-dependent PRs getting merged, one of them being modified should
lead to a restart of the merge & forward port process.
See test_update_pr for a simpler (single-PR) version
"""
r1, _ = make_basic(env, config, make_repo, reponame='repo-1', statuses='default')
r2, _ = make_basic(env, config, make_repo, reponame='repo-2', statuses='default')
with r1:
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
pr1 = r1.make_pr(target='a', head='aref')
r1.post_status('aref', 'success')
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
with r2:
r2.make_commits('a', Commit('2', tree={'2': '0'}), ref='heads/aref')
pr2 = r2.make_pr(target='a', head='aref')
r2.post_status('aref', 'success')
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.a', 'success')
r2.post_status('staging.a', 'success')
env.run_crons()
pr1_id, pr11_id, pr2_id, pr21_id = pr_ids = env['runbot_merge.pull_requests'].search([]).sorted('display_name')
assert pr1_id.number == pr1.number
assert pr2_id.number == pr2.number
assert pr1_id.state == pr2_id.state == 'merged'
assert pr11_id.parent_id == pr1_id
assert pr11_id.repository.name == pr1_id.repository.name == r1.name
assert pr21_id.parent_id == pr2_id
assert pr21_id.repository.name == pr2_id.repository.name == r2.name
assert pr11_id.target.name == pr21_id.target.name == 'b'
# don't even bother faking CI failure, straight update pr21_id
repo, ref = r2.get_pr(pr21_id.number).branch
with repo:
repo.make_commits(
r2.commit(pr21_id.target.name).id,
Commit('Whops', tree={'2': '1'}),
ref='heads/' + ref,
make=False
)
env.run_crons()
assert not pr21_id.parent_id
with r1, r2:
r1.post_status(pr11_id.head, 'success')
r1.get_pr(pr11_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
r2.post_status(pr21_id.head, 'success')
r2.get_pr(pr21_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
prs_again = env['runbot_merge.pull_requests'].search([])
assert prs_again == pr_ids,\
"should not have created FP PRs as we're now in a detached (iso new PR) state " \
"(%s)" % prs_again.mapped('display_name')
with r1, r2:
r1.post_status('staging.b', 'success')
r2.post_status('staging.b', 'success')
env.run_crons()
new_prs = env['runbot_merge.pull_requests'].search([]).sorted('display_name') - pr_ids
assert len(new_prs) == 2, "should have created the new FP PRs"
pr12_id, pr22_id = new_prs
assert pr12_id.source_id == pr1_id
assert pr12_id.parent_id == pr11_id
assert pr22_id.source_id == pr2_id
assert pr22_id.parent_id == pr21_id
def test_closing_during_fp(env, config, make_repo, users):
""" Closing a PR after it's been ported once should not port it further, but
the rest of the batch should carry on
"""
r1, _ = make_basic(env, config, make_repo, statuses='default')
r2, _ = make_basic(env, config, make_repo, statuses='default')
with r1, r2:
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
pr1 = r1.make_pr(target='a', head='aref')
r1.post_status('aref', 'success')
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
r2.make_commits('a', Commit('2', tree={'2': '0'}), ref='heads/aref')
pr2 = r2.make_pr(target='a', head='aref')
r2.post_status('aref', 'success')
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.a', 'success')
r2.post_status('staging.a', 'success')
env.run_crons()
pr1_id = to_pr(env, pr1)
[pr1_1_id] = pr1_id.forwardport_ids
pr2_id = to_pr(env, pr2)
[pr2_1_id] = pr2_id.forwardport_ids
with r1:
r1.get_pr(pr1_1_id.number).close(config['role_user']['token'])
with r2:
r2.post_status(pr2_1_id.head, 'success')
env.run_crons()
assert env['runbot_merge.pull_requests'].search_count([]) == 5,\
"only one of the forward ports should be ported"
assert not env['runbot_merge.pull_requests'].search([('parent_id', '=', pr1_1_id.id)]),\
"the closed PR should not be ported"
assert env['runbot_merge.pull_requests'].search([('source_id', '=', pr1_id.id)]) == pr1_1_id,\
"the closed PR should not be ported"
r1_b_head = r1.commit("b")
with r2:
r2.get_pr(pr2_1_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
assert not pr2_1_id.blocked
assert not pr2_1_id.batch_id.blocked
st = pr2_1_id.staging_id
assert st
with r1, r2:
r1.post_status('staging.b', 'success')
r2.post_status('staging.b', 'success')
env.run_crons()
assert st.state == 'success'
assert r1_b_head.id == r1.commit("b").id, \
"r1:b's head should not have been touched"
def test_add_pr_during_fp(env, config, make_repo, users):
""" It should be possible to add new PRs to an FP batch
"""
r1, _ = make_basic(env, config, make_repo, statuses="default")
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
# needs a "d" branch
env['runbot_merge.project'].search([]).write({
'branch_ids': [(0, 0, {'name': 'd', 'sequence': 40})],
})
with r1, r2:
r1.make_ref("heads/d", r1.commit("c").id)
r2.make_ref("heads/d", r2.commit("c").id)
with r1:
r1.make_commits('a', Commit('1', tree={'1': '0'}), ref='heads/aref')
pr1_a = r1.make_pr(target='a', head='aref')
r1.post_status('aref', 'success')
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.a', 'success')
r2.post_status('staging.a', 'success')
env.run_crons()
pr1_a_id = to_pr(env, pr1_a)
[pr1_b_id] = pr1_a_id.forwardport_ids
with r2, fork2:
fork2.make_commits(r2.commit('b').id, Commit('2', tree={'2': '0'}), ref=f'heads/{pr1_b_id.refname}')
pr2_b = r2.make_pr(title="B", target='b', head=f'{fork2.owner}:{pr1_b_id.refname}')
env.run_crons()
pr2_b_id = to_pr(env, pr2_b)
assert not pr1_b_id.staging_id
assert not pr2_b_id.staging_id
assert pr1_b_id.batch_id == pr2_b_id.batch_id
assert pr1_b_id.state == "opened",\
"implicit approval from forward port should have been canceled"
batch = pr2_b_id.batch_id
with r1:
r1.post_status(pr1_b_id.head, 'success')
r1.get_pr(pr1_b_id.number).post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
assert batch.blocked
assert pr1_b_id.blocked
with r2:
r2.post_status(pr2_b.head, "success")
pr2_b.post_comment("hansen r+", config['role_reviewer']['token'])
env.run_crons()
assert not batch.blocked
assert pr1_b_id.staging_id and pr1_b_id.staging_id == pr2_b_id.staging_id
with r1, r2:
r1.post_status('staging.b', 'success')
r2.post_status('staging.b', 'success')
env.run_crons()
def find_child(pr):
return env['runbot_merge.pull_requests'].search([
('parent_id', '=', pr.id),
])
pr1_c_id = find_child(pr1_b_id)
assert pr1_c_id
pr2_c_id = find_child(pr2_b_id)
assert pr2_c_id
with r1, r2:
r1.post_status(pr1_c_id.head, 'success')
r2.post_status(pr2_c_id.head, 'success')
env.run_crons()
assert find_child(pr1_c_id)
assert find_child(pr2_c_id)
def test_add_to_forward_ported(env, config, make_repo, users):
"""Add a new branch to an intermediate step of a fw *sequence*, either
because skipci or because all the intermediate CI succeeded
"""
# region setup
r1, _ = make_basic(env, config, make_repo, statuses="default")
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
project = env['runbot_merge.project'].search([])
project.write({
'branch_ids': [(0, 0, {'name': 'd', 'sequence': 40})],
})
with r1, r2:
r1.make_commits('c', Commit('1111', tree={'g': 'a'}), ref='heads/d')
r2.make_commits('c', Commit('1111', tree={'g': 'a'}), ref='heads/d')
with r1:
r1.make_commits('a', Commit('a', tree={'a': 'a'}), ref="heads/pr1")
pr1_a = r1.make_pr(target="a", head="pr1")
r1.post_status(pr1_a.head, 'success')
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.a', 'success')
r2.post_status('staging.a', 'success')
env.run_crons()
# region port forward
pr1_a_id = to_pr(env, pr1_a)
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
for branch in 'bcd':
next_pr = env['runbot_merge.pull_requests'].search([
('source_id', '=', pr1_a_id.id),
('target.name', '=', branch),
])
assert next_pr
with r1:
r1.post_status(next_pr.head, 'success')
env.run_crons()
# endregion
# endregion
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
pr1_b_id, pr1_c_id, pr1_d_id = pr1_a_id.forwardport_ids[::-1]
# new PR must be in fork for labels to actually match
with r2, fork2:
# branch in fork has no owner prefix, but HEAD for cross-repo PR does
fork2.make_commits(r2.commit("b").id, Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}')
pr2_b = r2.make_pr(title="b", target="b", head=pr1_b_id.label)
env.run_crons()
pr2_b_id = to_pr(env, pr2_b)
assert pr2_b_id.batch_id == pr1_b_id.batch_id
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
assert len(pr2_b_id.forwardport_ids) == 2, \
"since the batch is already forward ported, the new PR should" \
" immediately be forward ported to match"
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
assert pr2_b_id.forwardport_ids.mapped('label') == (pr1_d_id | pr1_c_id).mapped('label')
assert not (b := env['forwardport.batches'].search([])),\
f"there should not be any more forward porting, found {b.read(['source', 'pr_id'])}"
known_set = pr1_a_id | pr1_a_id.forwardport_ids | pr2_b_id | pr2_b_id.forwardport_ids
assert not (p := (env['runbot_merge.pull_requests'].search([]) - known_set)), \
"there should not be any PR outside of the sequences we created, "\
f"found {p.read(['source_id', 'parent_id', 'display_name'])}"
with r2:
for pr_id in pr2_b_id | pr2_b_id.forwardport_ids:
r2.post_status(pr_id.head, 'success')
with r1, r2:
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
r1.get_pr(pr1_a_id.forwardport_ids[0].number)\
.post_comment('hansen r+', config['role_reviewer']['token'])
r2.get_pr(pr2_b_id.forwardport_ids[0].number) \
.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
for branch in 'bcd':
r1.post_status(f'staging.{branch}', 'success')
r2.post_status(f'staging.{branch}', 'success')
env.run_crons()
[FIX] *: double forwardport when adding a PR to an existing batch This is a bit of an odd case which was only noticed because of persistent forwardport.batches, which ended up having a ton of related traceback in the logs (the mergebot kept trying to create forward ports from Jan 27th to Feb 10th, thankfully the errors happened in git so did not seem to eat through our API rate limiting). The issue was triggered by the addition of odoo/enterprise#77876 to odoo/odoo#194818. This triggered a completion job which led to the creation of odoo/enterprise#77877 to odoo/enterprise#77880, so far so good. Except the bit of code responsible for creating completion jobs only checked if the PR was being added to a batch with a descendant. That is the case of odoo/enterprise#77877 to odoo/enterprise#77879 (not odoo/enterprise#77880 because that's the end of the line). As a result, those triggered 3 more completion jobs, which kept failing in a loop because they tried pushing different commits to their next-siblings (without forcing, leading git to reject the non-ff push, hurray). A completion request should only be triggered by the addition of a new *source* (a PR without a source) to an existing batch with descendants, so add that to the check. This requires updating `_from_json` to create PRs in a single step (rather than one step to create based on github's data, and an other one for the hierarchical tracking) as we need the source to be set during `create` not as a post-action. Although there was a test which could have triggered this issue, the test only had 3 branches so was not long enough to trigger the issue: - Initial PR 1 on branch A merged then forward-ported to B and C. - Sibling PR 2 added to the batch in B. - Completed to C. - Ending there as C(1) has no descendant batch, leading to no further completion request. Adding a 4th branch did surface / show the issue by providing space for a new completion request from the creation of C(2). Interestingly even though I the test harness attempts to run all triggered crons to completion there can be misses, so the test can fail in two different ways (being now checked for): - there's a leftover forwardport.batch after we've created all our forwardports - there's an extra PR targeting D, descending from C(2) - in almost every case there's also a traceback in the logs, which does fail the build thanks to the `env` fixture's check
2025-02-11 20:27:53 +07:00
assert not env['runbot_merge.pull_requests'].search([]) - known_set, \
"there should not be any PR outside of the sequences we created"
def test_add_to_forward_port_conflict(env, config, make_repo, users):
"""If a PR is added to an existing forward port sequence, and it causes
conflicts when forward ported, it should be treated similarly to an *update*
causing a conflict: the PR is still created, but it's set in conflict.
"""
# region setup
r1, _ = make_basic(env, config, make_repo, statuses="default")
r2, fork2 = make_basic(env, config, make_repo, statuses="default")
project = env['runbot_merge.project'].search([])
with r2:
r2.make_commits(
"c",
Commit("C-onflict", tree={"b": "X"}),
ref="heads/c"
)
with r1:
r1.make_commits('a', Commit('a', tree={'a': 'a'}), ref="heads/pr1")
pr1_a = r1.make_pr(target="a", head="pr1")
r1.post_status(pr1_a.head, 'success')
pr1_a.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.a', 'success')
r2.post_status('staging.a', 'success')
env.run_crons()
# region port forward
pr1_a_id = to_pr(env, pr1_a)
pr1_b_id = pr1_a_id.forwardport_ids
assert pr1_b_id
with r1:
r1.post_status(pr1_b_id.head, 'success')
env.run_crons()
pr1_c_id = pr1_a_id.forwardport_ids - pr1_b_id
assert pr1_c_id
# endregion
# endregion
# new PR must be in fork for labels to actually match
with r2, fork2:
# branch in fork has no owner prefix, but HEAD for cross-repo PR does
fork2.make_commits(r2.commit("b").id, Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}')
pr2_b = r2.make_pr(title="b", target="b", head=pr1_b_id.label)
r2.post_status(pr2_b.head, 'success')
env.run_crons()
pr2_b_id = to_pr(env, pr2_b)
assert pr2_b_id.batch_id == pr1_b_id.batch_id
pr2_c_id = pr2_b_id.forwardport_ids
assert len(pr2_c_id) == 1, \
"since the batch is already forward ported, the new PR should" \
" immediately be forward ported to match"
assert pr2_c_id.label == pr1_c_id.label
assert not pr2_c_id.parent_id, "conflict -> should be detached"
assert pr2_c_id.detach_reason
pr2_a = r1.get_pr(pr1_b_id.number)
with r1, r2:
pr2_a.post_comment('hansen r+', config['role_reviewer']['token'])
pr2_b.post_comment("hansen r+", config['role_reviewer']['token'])
env.run_crons()
with r1, r2:
r1.post_status('staging.b', 'success')
r2.post_status('staging.b', 'success')
env.run_crons()
assert pr1_b_id.state == 'merged'
assert pr2_b_id.state == 'merged'
pr2_c = r2.get_pr(pr2_c_id.number)
assert pr2_c.comments == [
seen(env, pr2_c, users),
# should have conflicts
[CHG] forwardport: perform forward porting without working copies The goal is to reduce maintenance and odd disk interactions & concurrency issues, by not creating concurrent clones, not having to push forks back in the repository, etc... it also removes the need to cleanup "scratch" working copies though that looks not to have been an issue in a while. The work is done on isolated objects without using or mutating refs, so even concurrent work should not be a problem. This turns out to not be any more verbose (less so if anything) than using `cherry-pick`, as that is not really designed for scripted / non-interactive use, or for squashing commits thereafter. Working directly with trees and commits is quite a bit cleaner even without a ton of helpers. Much of the credit goes to Julia Evans for [their investigation of 3-way merges as the underpinnings of cherry-picking][3-way merge], this would have been a lot more difficult if I'd had to rediscover the merge-base trick independently. A few things have been changed by this: - The old trace/stderr from cherrypick has disappeared as it's generated by cherrypick, but for a non-interactive use it's kinda useless anyway so I probably should have looked into removing it earlier (I think the main use was investigation of the inflateinit issue). - Error on emptied commits has to be hand-rolled as `merge-tree` couldn't care less, this is not hard but is a bit annoying. - `merge-tree`'s conflict information only references raw commits, which makes sense, but requires updating a bunch of tests. Then again so does the fact that it *usually* doesn't send anything to stderr, so that's usually disappearing. Conveniently `merge-tree` merges the conflict marker directly in the files / tree so we don't have to mess about moving them back out of the repository and into the working copy as I assume cherry-pick does, which means we don't have to try and commit them back in ether. That is a huge part of the gain over faffing about with the working copy. Fixes #847 [3-way merge]: https://jvns.ca/blog/2023/11/10/how-cherry-pick-and-revert-work/
2024-07-05 18:32:02 +07:00
(users['user'], """@{user} cherrypicking of pull request {previous.display_name} failed.
stdout:
```
Auto-merging b
CONFLICT (add/add): Merge conflict in b
```
Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
In the former case, you may want to edit this PR message as well.
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}.
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
[CHG] forwardport: perform forward porting without working copies The goal is to reduce maintenance and odd disk interactions & concurrency issues, by not creating concurrent clones, not having to push forks back in the repository, etc... it also removes the need to cleanup "scratch" working copies though that looks not to have been an issue in a while. The work is done on isolated objects without using or mutating refs, so even concurrent work should not be a problem. This turns out to not be any more verbose (less so if anything) than using `cherry-pick`, as that is not really designed for scripted / non-interactive use, or for squashing commits thereafter. Working directly with trees and commits is quite a bit cleaner even without a ton of helpers. Much of the credit goes to Julia Evans for [their investigation of 3-way merges as the underpinnings of cherry-picking][3-way merge], this would have been a lot more difficult if I'd had to rediscover the merge-base trick independently. A few things have been changed by this: - The old trace/stderr from cherrypick has disappeared as it's generated by cherrypick, but for a non-interactive use it's kinda useless anyway so I probably should have looked into removing it earlier (I think the main use was investigation of the inflateinit issue). - Error on emptied commits has to be hand-rolled as `merge-tree` couldn't care less, this is not hard but is a bit annoying. - `merge-tree`'s conflict information only references raw commits, which makes sense, but requires updating a bunch of tests. Then again so does the fact that it *usually* doesn't send anything to stderr, so that's usually disappearing. Conveniently `merge-tree` merges the conflict marker directly in the files / tree so we don't have to mess about moving them back out of the repository and into the working copy as I assume cherry-pick does, which means we don't have to try and commit them back in ether. That is a huge part of the gain over faffing about with the working copy. Fixes #847 [3-way merge]: https://jvns.ca/blog/2023/11/10/how-cherry-pick-and-revert-work/
2024-07-05 18:32:02 +07:00
""".format(project=project, previous=pr2_b_id, **users))
]