From 124d1212a2c28c7cd36bb7bfaf8af71974ec9a72 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 20 Feb 2024 11:24:35 +0100 Subject: [PATCH] [ADD] forwardport: tests that fw batches can vary This tests that the new setup *does* allow both removing PRs from a forward-ported batch (something which may have worked previously already, anyway) and importantly *adding* PRs to a forward ported batch. The updated batch behaves somewhat like a new batch, but it should retain the history via linking through the batch, and it allows cross-repo fixes which were not necessary earlier (e.g. because we're touching an API of repo A which was not used in repo B in earlier branches, but now is), something which was not really possible before the refactoring of batches & co. --- forwardport/tests/test_batches.py | 126 +++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-) diff --git a/forwardport/tests/test_batches.py b/forwardport/tests/test_batches.py index a637e7a0..4c72f3da 100644 --- a/forwardport/tests/test_batches.py +++ b/forwardport/tests/test_batches.py @@ -1,4 +1,4 @@ -from utils import Commit, make_basic +from utils import Commit, make_basic, to_pr def test_single_updated(env, config, make_repo): @@ -87,3 +87,127 @@ def test_single_updated(env, config, make_repo): 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) + r2, _ = make_basic(env, config, make_repo) + env['runbot_merge.repository'].search([]).required_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" + +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) + r2, fork2 = make_basic(env, config, make_repo) + repos = env['runbot_merge.repository'].search([]) + repos.required_statuses = 'default' + # needs a "d" branch + repos[0].project_id.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, r2: + 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('b', 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)