From 6d5c539c7760ad00e3c88e3a80703743def82f80 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 6 Feb 2025 12:35:55 +0100 Subject: [PATCH] [IMP] *: fork with main_branch_only Should limit the risk of cases where the fork contains outdated versions of the reference branches and we end up with odd outdated / not up to date basis for branches & updates, which can lead to confusing situations. --- conftest.py | 4 +++- forwardport/tests/test_batches.py | 8 ++++---- forwardport/tests/test_conflicts.py | 2 +- forwardport/tests/test_limit.py | 4 ++-- forwardport/tests/test_overrides.py | 4 ++-- forwardport/tests/test_simple.py | 23 ++++++++++++----------- forwardport/tests/test_updates.py | 6 +++--- forwardport/tests/test_weird.py | 18 +++++++++--------- runbot_merge/tests/test_multirepo.py | 6 +++--- 9 files changed, 39 insertions(+), 36 deletions(-) diff --git a/conftest.py b/conftest.py index 8e21561b..17c21e96 100644 --- a/conftest.py +++ b/conftest.py @@ -911,7 +911,9 @@ class Repo: def fork(self, *, token=None): s = self._get_session(token) - r = s.post(f'https://api.github.com/repos/{self.name}/forks') + r = s.post(f'https://api.github.com/repos/{self.name}/forks', json={ + 'default_branch_only': True, + }) assert r.ok, r.text response = r.json() diff --git a/forwardport/tests/test_batches.py b/forwardport/tests/test_batches.py index 2ecab8b6..56fbf6d9 100644 --- a/forwardport/tests/test_batches.py +++ b/forwardport/tests/test_batches.py @@ -50,7 +50,7 @@ def test_single_updated(env, config, make_repo): repo, ref = r2.get_pr(pr21_id.number).branch with repo: repo.make_commits( - pr21_id.target.name, + r2.commit(pr21_id.target.name).id, Commit('Whops', tree={'2': '1'}), ref='heads/' + ref, make=False @@ -180,7 +180,7 @@ def test_add_pr_during_fp(env, config, make_repo, users): [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}') + 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() @@ -265,7 +265,7 @@ def test_add_to_forward_ported(env, config, make_repo, users): # 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("b", Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}') + 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() @@ -348,7 +348,7 @@ def test_add_to_forward_port_conflict(env, config, make_repo, users): # 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("b", Commit('b', tree={'b': 'b'}), ref=f'heads/{pr1_b_id.refname}') + 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() diff --git a/forwardport/tests/test_conflicts.py b/forwardport/tests/test_conflicts.py index 50993340..e2705839 100644 --- a/forwardport/tests/test_conflicts.py +++ b/forwardport/tests/test_conflicts.py @@ -127,7 +127,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port pr_repo.make_commits( # if just given a branch name, goes and gets it from pr_repo whose # "b" was cloned before that branch got rolled back - 'c', + prod.commit('c').id, Commit('h should indeed be xxx', tree={'h': 'xxx'}), ref='heads/%s' % pr_ref, make=False, diff --git a/forwardport/tests/test_limit.py b/forwardport/tests/test_limit.py index df22365d..7d62911c 100644 --- a/forwardport/tests/test_limit.py +++ b/forwardport/tests/test_limit.py @@ -114,7 +114,7 @@ def test_unignore(env, config, make_repo, users, page): token = config['role_other']['token'] fork = prod.fork(token=token) with prod, fork: - [c] = fork.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch') + [c] = fork.make_commits(prod.commit('a').id, Commit('c', tree={'0': '0'}), ref='heads/mybranch') pr = prod.make_pr(target='a', head=f'{fork.owner}:mybranch', title="title", token=token) prod.post_status(c, 'success') env.run_crons() @@ -302,7 +302,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port # update pr2 to detach it from pr1 with other: other.make_commits( - p2.target.name, + prod.commit(p2.target.name).id, Commit('updated', tree={'1': '1'}), ref=pr2.ref, make=False diff --git a/forwardport/tests/test_overrides.py b/forwardport/tests/test_overrides.py index df8efd02..5fc8b0a9 100644 --- a/forwardport/tests/test_overrides.py +++ b/forwardport/tests/test_overrides.py @@ -54,7 +54,7 @@ def test_override_inherited(env, config, make_repo, users): pr_repo, pr_ref = pr1.branch with pr_repo: pr_repo.make_commits( - pr1_id.target.name, + repo.commit(pr1_id.target.name).id, Commit('wop wop', tree={'a': '1'}), ref=f'heads/{pr_ref}', make=False @@ -117,7 +117,7 @@ def test_override_combination(env, config, make_repo, users): pr_repo, pr_ref = repo.get_pr(pr1_id.number).branch with pr_repo: pr_repo.make_commits( - pr1_id.target.name, + repo.commit(pr1_id.target.name).id, Commit('wop wop', tree={'a': '1'}), ref=f'heads/{pr_ref}', make=False diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index 323faa76..9319102e 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -40,7 +40,7 @@ def test_straightforward_flow(env, config, make_repo, users): with prod, other_user_repo: # create PR as a user with no access to prod (or other) [_, p_1] = other_user_repo.make_commits( - 'a', + prod.commit('a').id, Commit('p_0', tree={'x': '0'}), Commit('p_1', tree={'x': '1'}), ref='heads/hugechange' @@ -515,7 +515,7 @@ def test_access_rights(env, config, make_repo, users, author, reviewer, delegate author_token = config['role_' + author]['token'] fork = prod.fork(token=author_token) with prod, fork: - [c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') + [c] = fork.make_commits(prod.commit('a').id, Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') pr = prod.make_pr( target='a', title='my change', head=users[author] + ':accessrights', @@ -584,7 +584,7 @@ def test_disapproval(env, config, make_repo, users): author_token = config['role_other']['token'] fork = prod.fork(token=author_token) with prod, fork: - [c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') + [c] = fork.make_commits(prod.commit('a').id, Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') pr0 = prod.make_pr( target='a', title='my change', head=users['other'] + ':accessrights', @@ -655,7 +655,7 @@ def test_delegate_fw(env, config, make_repo, users): author_token = config['role_self_reviewer']['token'] fork = prod.fork(token=author_token) with prod, fork: - [c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') + [c] = fork.make_commits(prod.commit('a').id, Commit('c_0', tree={'y': '0'}), ref='heads/accessrights') pr = prod.make_pr( target='a', title='my change', head=users['self_reviewer'] + ':accessrights', @@ -781,7 +781,7 @@ def test_batched(env, config, make_repo, users): with main1, other1: [c1] = other1.make_commits( - 'a', Commit('commit repo 1', tree={'1': 'a'}), + main1.commit('a').id, Commit('commit repo 1', tree={'1': 'a'}), ref='heads/contribution' ) pr1 = main1.make_pr( @@ -795,7 +795,7 @@ def test_batched(env, config, make_repo, users): pr1.post_comment('hansen r+', config['role_reviewer']['token']) with main2, other2: [c2] = other2.make_commits( - 'a', Commit('commit repo 2', tree={'2': 'a'}), + main2.commit('a').id, Commit('commit repo 2', tree={'2': 'a'}), ref='heads/contribution' # use same ref / label as pr1 ) pr2 = main2.make_pr( @@ -1074,7 +1074,7 @@ class TestBranchDeletion: """ prod, other = make_basic(env, config, make_repo) with prod, other: - [c] = other.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/abranch') + [c] = other.make_commits(prod.commit('a').id, Commit('c', tree={'0': '0'}), ref='heads/abranch') pr = prod.make_pr( target='a', head='%s:abranch' % other.owner, title="a pr", @@ -1105,22 +1105,23 @@ class TestBranchDeletion: """ prod, other = make_basic(env, config, make_repo) with prod, other: - [c] = other.make_commits('a', Commit('c1', tree={'1': '0'}), ref='heads/abranch') + a_ref = prod.commit('a').id + [c] = other.make_commits(a_ref, Commit('c1', tree={'1': '0'}), ref='heads/abranch') pr1 = prod.make_pr(target='a', head='%s:abranch' % other.owner, title='a') prod.post_status(c, 'success', 'legal/cla') prod.post_status(c, 'success', 'ci/runbot') pr1.post_comment('hansen r+', config['role_reviewer']['token']) - other.make_commits('a', Commit('c2', tree={'2': '0'}), ref='heads/bbranch') + other.make_commits(a_ref, Commit('c2', tree={'2': '0'}), ref='heads/bbranch') pr2 = prod.make_pr(target='a', head='%s:bbranch' % other.owner, title='b') pr2.close() - [c] = other.make_commits('a', Commit('c3', tree={'3': '0'}), ref='heads/cbranch') + [c] = other.make_commits(a_ref, Commit('c3', tree={'3': '0'}), ref='heads/cbranch') pr3 = prod.make_pr(target='a', head='%s:cbranch' % other.owner, title='c') prod.post_status(c, 'success', 'legal/cla') prod.post_status(c, 'success', 'ci/runbot') - other.make_commits('a', Commit('c3', tree={'4': '0'}), ref='heads/dbranch') + other.make_commits(a_ref, Commit('c3', tree={'4': '0'}), ref='heads/dbranch') pr4 = prod.make_pr(target='a', head='%s:dbranch' % other.owner, title='d') pr4.post_comment('hansen r+', config['role_reviewer']['token']) env.run_crons() diff --git a/forwardport/tests/test_updates.py b/forwardport/tests/test_updates.py index 4bd40bf9..0b3ce02d 100644 --- a/forwardport/tests/test_updates.py +++ b/forwardport/tests/test_updates.py @@ -110,7 +110,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port with pr_repo: # force-push correct commit to PR's branch [new_c] = pr_repo.make_commits( - pr1_id.target.name, + prod.commit(pr1_id.target.name).id, Commit('whop whop', tree={'x': '5'}), ref='heads/%s' % pr_ref, make=False @@ -265,7 +265,7 @@ def test_update_merged(env, make_repo, config, users): repo, ref = prod.get_pr(pr1_id.number).branch with repo: repo.make_commits( - pr1_id.target.name, + prod.commit(pr1_id.target.name).id, Commit('2', tree={'0': '0', '1': '1'}), ref='heads/%s' % ref, make=False @@ -441,7 +441,7 @@ def test_subsequent_conflict(env, make_repo, config, users): pr2 = repo.get_pr(pr2_id.number) t = {**repo.read_tree(repo.commit(pr2_id.head)), 'h': 'conflict!'} with fork: - fork.make_commits(pr2_id.target.name, Commit('newfiles', tree=t), ref=pr2.ref, make=False) + fork.make_commits(repo.commit(pr2_id.target.name).id, Commit('newfiles', tree=t), ref=pr2.ref, make=False) env.run_crons() assert repo.read_tree(repo.commit(pr3_id.head)) == { diff --git a/forwardport/tests/test_weird.py b/forwardport/tests/test_weird.py index 17cb7bef..13316a75 100644 --- a/forwardport/tests/test_weird.py +++ b/forwardport/tests/test_weird.py @@ -141,7 +141,7 @@ def test_fw_retry(env, config, make_repo, users): other_token = config['role_other']['token'] fork = prod.fork(token=other_token) with prod, fork: - fork.make_commits('a', Commit('c', tree={'a': '0'}), ref='heads/abranch') + fork.make_commits(prod.commit('a').id, Commit('c', tree={'a': '0'}), ref='heads/abranch') pr1 = prod.make_pr( title="whatever", target='a', @@ -274,7 +274,7 @@ class TestNotAllBranches: """ project, a, a_dev, b, _ = repos with a, a_dev: - [c] = a_dev.make_commits('a', Commit('pr', tree={'pr': '1'}), ref='heads/change') + [c] = a_dev.make_commits(a.commit('a').id, Commit('pr', tree={'pr': '1'}), ref='heads/change') pr = a.make_pr(target='a', title="a pr", head=a_dev.owner + ':change') a.post_status(c, 'success', 'ci/runbot') pr.post_comment('hansen r+', config['role_reviewer']['token']) @@ -321,7 +321,7 @@ class TestNotAllBranches: """ project, a, _, b, b_dev = repos with b, b_dev: - [c] = b_dev.make_commits('a', Commit('pr', tree={'pr': '1'}), ref='heads/change') + [c] = b_dev.make_commits(b.commit('a').id, Commit('pr', tree={'pr': '1'}), ref='heads/change') pr = b.make_pr(target='a', title="a pr", head=b_dev.owner + ':change') b.post_status(c, 'success', 'ci/runbot') pr.post_comment('hansen r+', config['role_reviewer']['token']) @@ -355,12 +355,12 @@ class TestNotAllBranches: """ project, a, a_dev, b, b_dev = repos with a, a_dev: - [c_a] = a_dev.make_commits('a', Commit('pr a', tree={'pr': 'a'}), ref='heads/change') + [c_a] = a_dev.make_commits(a.commit('a').id, Commit('pr a', tree={'pr': 'a'}), ref='heads/change') pr_a = a.make_pr(target='a', title='a pr', head=a_dev.owner + ':change') a.post_status(c_a, 'success', 'ci/runbot') pr_a.post_comment('hansen r+', config['role_reviewer']['token']) with b, b_dev: - [c_b] = b_dev.make_commits('a', Commit('pr b', tree={'pr': 'b'}), ref='heads/change') + [c_b] = b_dev.make_commits(b.commit('a').id, Commit('pr b', tree={'pr': 'b'}), ref='heads/change') pr_b = b.make_pr(target='a', title='b pr', head=b_dev.owner + ':change') b.post_status(c_b, 'success', 'ci/runbot') pr_b.post_comment('hansen r+', config['role_reviewer']['token']) @@ -592,7 +592,7 @@ def test_author_can_close_via_fwbot(env, config, make_repo): other = prod.fork(token=other_token) with prod, other: - [c] = other.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/change') + [c] = other.make_commits(prod.commit('a').id, Commit('c', tree={'0': '0'}), ref='heads/change') pr = prod.make_pr( target='a', title='my change', head=other_user['user'] + ':change', @@ -1005,17 +1005,17 @@ def test_disable_branch_with_batches(env, config, make_repo, users): # region set up forward ported batches with repo, fork, repo2, fork2: - fork.make_commits("a", Commit("x", tree={"x": "1"}), ref="heads/x") + fork.make_commits(repo.commit("a").id, Commit("x", tree={"x": "1"}), ref="heads/x") pr1_a = repo.make_pr(title="X", target="a", head=f"{fork.owner}:x") pr1_a.post_comment("hansen r+", config['role_reviewer']['token']) repo.post_status(pr1_a.head, "success") - fork2.make_commits("a", Commit("x", tree={"x": "1"}), ref="heads/x") + fork2.make_commits(repo2.commit("a").id, Commit("x", tree={"x": "1"}), ref="heads/x") pr2_a = repo2.make_pr(title="X", target="a", head=f"{fork2.owner}:x") pr2_a.post_comment("hansen r+", config['role_reviewer']['token']) repo2.post_status(pr2_a.head, "success") - fork.make_commits("a", Commit("y", tree={"y": "1"}), ref="heads/y") + fork.make_commits(repo.commit("a").id, Commit("y", tree={"y": "1"}), ref="heads/y") pr3_a = repo.make_pr(title="Y", target="a", head=f"{fork.owner}:y") pr3_a.post_comment("hansen r+", config['role_reviewer']['token']) repo.post_status(pr3_a.head, 'success') diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 601dd4f5..b4d8eb3f 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -1013,7 +1013,7 @@ class TestSubstitutions: repo_a.make_commits('master', repo_a.Commit('bop', tree={'a': '1'}), ref='heads/abranch') pra = repo_a.make_pr(target='master', head='abranch') with repo_b, repo_b.fork() as b_fork: - b_fork.make_commits('master', b_fork.Commit('pob', tree={'b': '1'}), ref='heads/abranch') + b_fork.make_commits(repo_b.commit('master').id, b_fork.Commit('pob', tree={'b': '1'}), ref='heads/abranch') prb = repo_b.make_pr( title="a pr", target='master', head='%s:abranch' % b_fork.owner @@ -1098,7 +1098,7 @@ def test_multi_project(env, make_repo, setreviewers, users, config, assert r1_dev.owner == r2_dev.owner with r1, r1_dev: - r1_dev.make_commits('default', Commit('new', tree={'a': 'b'}), ref='heads/other') + r1_dev.make_commits(r1.commit('default').id, Commit('new', tree={'a': 'b'}), ref='heads/other') # create, validate, and approve pr1 pr1 = r1.make_pr(title='pr 1', target='default', head=r1_dev.owner + ':other') @@ -1106,7 +1106,7 @@ def test_multi_project(env, make_repo, setreviewers, users, config, pr1.post_comment('hansen r+', config['role_reviewer']['token']) with r2, r2_dev: - r2_dev.make_commits('default', Commit('new', tree={'b': 'b'}), ref='heads/other') + r2_dev.make_commits(r2.commit('default').id, Commit('new', tree={'b': 'b'}), ref='heads/other') # create second PR with the same label *in a different project*, don't # approve it