diff --git a/forwardport/tests/test_limit.py b/forwardport/tests/test_limit.py new file mode 100644 index 00000000..1f2c5978 --- /dev/null +++ b/forwardport/tests/test_limit.py @@ -0,0 +1,210 @@ +# -*- coding: utf-8 -*- +import collections + +import pytest + +from utils import * + +Description = collections.namedtuple('Restriction', 'source limit') +def test_configure(env, config, make_repo): + """ Checks that configuring an FP limit on a PR is respected + + * limits to not the latest + * limits to the current target (= no FP) + * limits to an earlier branch (???) + """ + prod, other = make_basic(env, config, make_repo) + bot_name = env['runbot_merge.project'].search([]).fp_github_name + descriptions = [ + Description(source='a', limit='b'), + Description(source='b', limit='b'), + Description(source='b', limit='a'), + ] + originals = [] + with prod: + for i, descr in enumerate(descriptions): + [c] = prod.make_commits( + descr.source, Commit('c %d' % i, tree={str(i): str(i)}), + ref='heads/branch%d' % i, + ) + pr = prod.make_pr(target=descr.source, head='branch%d'%i) + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+\n%s up to %s' % (bot_name, descr.limit), config['role_reviewer']['token']) + originals.append(pr.number) + env.run_crons() + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + prod.post_status('staging.b', 'success', 'legal/cla') + prod.post_status('staging.b', 'success', 'ci/runbot') + env.run_crons() + + # should have created a single FP PR for 0, none for 1 and none for 2 + prs = env['runbot_merge.pull_requests'].search([], order='number') + assert len(prs) == 4 + assert prs[-1].parent_id == prs[0] + assert prs[0].number == originals[0] + assert prs[1].number == originals[1] + assert prs[2].number == originals[2] + + +def test_self_disabled(env, config, make_repo): + """ Allow setting target as limit even if it's disabled + """ + prod, other = make_basic(env, config, make_repo) + bot_name = env['runbot_merge.project'].search([]).fp_github_name + branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')]) + branch_a.fp_target = False + with prod: + [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch') + pr = prod.make_pr(target='a', head='mybranch') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+\n%s up to a' % bot_name, config['role_reviewer']['token']) + env.run_crons() + pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)]) + assert pr_id.limit_id == branch_a + + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + + assert env['runbot_merge.pull_requests'].search([]) == pr_id,\ + "should not have created a forward port" + + +def test_ignore(env, config, make_repo): + """ Provide an "ignore" command which is equivalent to setting the limit + to target + """ + prod, other = make_basic(env, config, make_repo) + bot_name = env['runbot_merge.project'].search([]).fp_github_name + branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')]) + with prod: + [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch') + pr = prod.make_pr(target='a', head='mybranch') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+\n%s ignore' % bot_name, config['role_reviewer']['token']) + env.run_crons() + pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)]) + assert pr_id.limit_id == branch_a + + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + + assert env['runbot_merge.pull_requests'].search([]) == pr_id,\ + "should not have created a forward port" + + +@pytest.mark.parametrize('enabled', ['active', 'fp_target']) +def test_disable(env, config, make_repo, users, enabled): + """ Checks behaviour if the limit target is disabled: + + * disable target while FP is ongoing -> skip over (and stop there so no FP) + * forward-port over a disabled branch + * request a disabled target as limit + + Disabling (with respect to forward ports) can be performed by marking the + branch as !active (which also affects mergebot operations), or as + !fp_target (won't be forward-ported to). + """ + prod, other = make_basic(env, config, make_repo) + project = env['runbot_merge.project'].search([]) + bot_name = project.fp_github_name + with prod: + [c] = prod.make_commits('a', Commit('c 0', tree={'0': '0'}), ref='heads/branch0') + pr = prod.make_pr(target='a', head='branch0') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+\n%s up to b' % bot_name, config['role_reviewer']['token']) + + [c] = prod.make_commits('a', Commit('c 1', tree={'1': '1'}), ref='heads/branch1') + pr = prod.make_pr(target='a', head='branch1') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+', config['role_reviewer']['token']) + env.run_crons() + + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + # disable branch b + env['runbot_merge.branch'].search([('name', '=', 'b')]).write({enabled: False}) + env.run_crons() + + # should have created a single PR (to branch c, for pr 1) + _0, _1, p = env['runbot_merge.pull_requests'].search([], order='number') + assert p.parent_id == _1 + assert p.target.name == 'c' + + project.fp_github_token = config['role_other']['token'] + bot_name = project.fp_github_name + with prod: + [c] = prod.make_commits('a', Commit('c 2', tree={'2': '2'}), ref='heads/branch2') + pr = prod.make_pr(target='a', head='branch2') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+\n%s up to' % bot_name, config['role_reviewer']['token']) + pr.post_comment('%s up to b' % bot_name, config['role_reviewer']['token']) + pr.post_comment('%s up to foo' % bot_name, config['role_reviewer']['token']) + pr.post_comment('%s up to c' % bot_name, config['role_reviewer']['token']) + env.run_crons() + + # use a set because git webhooks delays might lead to mis-ordered + # responses and we don't care that much + assert set(pr.comments) == { + (users['reviewer'], "hansen r+\n%s up to" % bot_name), + (users['reviewer'], "%s up to b" % bot_name), + (users['reviewer'], "%s up to foo" % bot_name), + (users['reviewer'], "%s up to c" % bot_name), + (users['other'], "Please provide a branch to forward-port to."), + (users['other'], "Branch 'b' is disabled, it can't be used as a forward port target."), + (users['other'], "There is no branch 'foo', it can't be used as a forward port target."), + (users['other'], "Forward-porting to 'c'."), + } + + +def test_default_disabled(env, config, make_repo, users): + """ If the default limit is disabled, it should still be the default + limit but the ping message should be set on the actual last FP (to the + last non-deactivated target) + """ + prod, other = make_basic(env, config, make_repo) + branch_c = env['runbot_merge.branch'].search([('name', '=', 'c')]) + branch_c.fp_target = False + + with prod: + [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/branch0') + pr = prod.make_pr(target='a', head='branch0') + prod.post_status(c, 'success', 'legal/cla') + prod.post_status(c, 'success', 'ci/runbot') + pr.post_comment('hansen r+', config['role_reviewer']['token']) + env.run_crons() + + assert env['runbot_merge.pull_requests'].search([]).limit_id == branch_c + + with prod: + prod.post_status('staging.a', 'success', 'legal/cla') + prod.post_status('staging.a', 'success', 'ci/runbot') + env.run_crons() + + p1, p2 = env['runbot_merge.pull_requests'].search([], order='number') + assert p1.number == pr.number + pr2 = prod.get_pr(p2.number) + + cs = pr2.comments + assert len(cs) == 1 + assert pr2.comments == [ + (users['user'], """\ +Ping @%s, @%s +This PR targets b and is the last of the forward-port chain. + +To merge the full chain, say +> @%s r+ + +More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port +""" % (users['user'], users['reviewer'], users['user'])), + ] diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index 3086e9c6..6069b615 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -23,64 +23,6 @@ FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime('%Y-%m-%d %H:%M:% # - a github user to create a repo with # - a github owner to create a repo *for* # - provide ability to create commits, branches, prs, ... -def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproject'): - """ Creates a basic repo with 3 forking branches - - 0 -- 1 -- 2 -- 3 -- 4 : a - | - `-- 11 -- 22 : b - | - `-- 111 : c - each branch just adds and modifies a file (resp. f, g and h) through the - contents sequence a b c d e - """ - Projects = env['runbot_merge.project'] - project = Projects.search([('name', '=', project_name)]) - if not project: - project = env['runbot_merge.project'].create({ - 'name': project_name, - 'github_token': config['github']['token'], - 'github_prefix': 'hansen', - 'fp_github_token': config['github']['token'], - 'required_statuses': 'legal/cla,ci/runbot', - 'branch_ids': [ - (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}), - (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}), - (0, 0, {'name': 'c', 'fp_sequence': 0, 'fp_target': True}), - ], - }) - - prod = make_repo(reponame) - with prod: - a_0, a_1, a_2, a_3, a_4, = prod.make_commits( - None, - Commit("0", tree={'f': 'a'}), - Commit("1", tree={'f': 'b'}), - Commit("2", tree={'f': 'c'}), - Commit("3", tree={'f': 'd'}), - Commit("4", tree={'f': 'e'}), - ref='heads/a', - ) - b_1, b_2 = prod.make_commits( - a_2, - Commit('11', tree={'g': 'a'}), - Commit('22', tree={'g': 'b'}), - ref='heads/b', - ) - prod.make_commits( - b_1, - Commit('111', tree={'h': 'a'}), - ref='heads/c', - ) - other = prod.fork() - project.write({ - 'repo_ids': [(0, 0, { - 'name': prod.name, - 'fp_remote_target': other.name, - })], - }) - - return prod, other def test_straightforward_flow(env, config, make_repo, users): # TODO: ~all relevant data in users when creating partners # get reviewer's name @@ -730,206 +672,6 @@ def test_partially_empty(env, config, make_repo): 'y': '0', } -Description = collections.namedtuple('Restriction', 'source limit') -def test_limit_configure(env, config, make_repo): - """ Checks that configuring an FP limit on a PR is respected - - * limits to not the latest - * limits to the current target (= no FP) - * limits to an earlier branch (???) - """ - prod, other = make_basic(env, config, make_repo) - bot_name = env['runbot_merge.project'].search([]).fp_github_name - descriptions = [ - Description(source='a', limit='b'), - Description(source='b', limit='b'), - Description(source='b', limit='a'), - ] - originals = [] - with prod: - for i, descr in enumerate(descriptions): - [c] = prod.make_commits( - descr.source, Commit('c %d' % i, tree={str(i): str(i)}), - ref='heads/branch%d' % i, - ) - pr = prod.make_pr(target=descr.source, head='branch%d'%i) - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+\n%s up to %s' % (bot_name, descr.limit), config['role_reviewer']['token']) - originals.append(pr.number) - env.run_crons() - with prod: - prod.post_status('staging.a', 'success', 'legal/cla') - prod.post_status('staging.a', 'success', 'ci/runbot') - prod.post_status('staging.b', 'success', 'legal/cla') - prod.post_status('staging.b', 'success', 'ci/runbot') - env.run_crons() - - # should have created a single FP PR for 0, none for 1 and none for 2 - prs = env['runbot_merge.pull_requests'].search([], order='number') - assert len(prs) == 4 - assert prs[-1].parent_id == prs[0] - assert prs[0].number == originals[0] - assert prs[1].number == originals[1] - assert prs[2].number == originals[2] - -def test_limit_self_disabled(env, config, make_repo): - """ Allow setting target as limit even if it's disabled - """ - prod, other = make_basic(env, config, make_repo) - bot_name = env['runbot_merge.project'].search([]).fp_github_name - branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')]) - branch_a.fp_target = False - with prod: - [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch') - pr = prod.make_pr(target='a', head='mybranch') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+\n%s up to a' % bot_name, config['role_reviewer']['token']) - env.run_crons() - pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)]) - assert pr_id.limit_id == branch_a - - with prod: - prod.post_status('staging.a', 'success', 'legal/cla') - prod.post_status('staging.a', 'success', 'ci/runbot') - - assert env['runbot_merge.pull_requests'].search([]) == pr_id,\ - "should not have created a forward port" - -def test_fp_ignore(env, config, make_repo): - """ Provide an "ignore" command which is equivalent to setting the limit - to target - """ - prod, other = make_basic(env, config, make_repo) - bot_name = env['runbot_merge.project'].search([]).fp_github_name - branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')]) - with prod: - [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch') - pr = prod.make_pr(target='a', head='mybranch') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+\n%s ignore' % bot_name, config['role_reviewer']['token']) - env.run_crons() - pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)]) - assert pr_id.limit_id == branch_a - - with prod: - prod.post_status('staging.a', 'success', 'legal/cla') - prod.post_status('staging.a', 'success', 'ci/runbot') - - assert env['runbot_merge.pull_requests'].search([]) == pr_id,\ - "should not have created a forward port" - -@pytest.mark.parametrize('enabled', ['active', 'fp_target']) -def test_limit_disable(env, config, make_repo, users, enabled): - """ Checks behaviour if the limit target is disabled: - - * disable target while FP is ongoing -> skip over (and stop there so no FP) - * forward-port over a disabled branch - * request a disabled target as limit - - Disabling (with respect to forward ports) can be performed by marking the - branch as !active (which also affects mergebot operations), or as - !fp_target (won't be forward-ported to). - """ - prod, other = make_basic(env, config, make_repo) - project = env['runbot_merge.project'].search([]) - bot_name = project.fp_github_name - with prod: - [c] = prod.make_commits('a', Commit('c 0', tree={'0': '0'}), ref='heads/branch0') - pr = prod.make_pr(target='a', head='branch0') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+\n%s up to b' % bot_name, config['role_reviewer']['token']) - - [c] = prod.make_commits('a', Commit('c 1', tree={'1': '1'}), ref='heads/branch1') - pr = prod.make_pr(target='a', head='branch1') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+', config['role_reviewer']['token']) - env.run_crons() - - with prod: - prod.post_status('staging.a', 'success', 'legal/cla') - prod.post_status('staging.a', 'success', 'ci/runbot') - # disable branch b - env['runbot_merge.branch'].search([('name', '=', 'b')]).write({enabled: False}) - env.run_crons() - - # should have created a single PR (to branch c, for pr 1) - _0, _1, p = env['runbot_merge.pull_requests'].search([], order='number') - assert p.parent_id == _1 - assert p.target.name == 'c' - - project.fp_github_token = config['role_other']['token'] - bot_name = project.fp_github_name - with prod: - [c] = prod.make_commits('a', Commit('c 2', tree={'2': '2'}), ref='heads/branch2') - pr = prod.make_pr(target='a', head='branch2') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+\n%s up to' % bot_name, config['role_reviewer']['token']) - pr.post_comment('%s up to b' % bot_name, config['role_reviewer']['token']) - pr.post_comment('%s up to foo' % bot_name, config['role_reviewer']['token']) - pr.post_comment('%s up to c' % bot_name, config['role_reviewer']['token']) - env.run_crons() - - # use a set because git webhooks delays might lead to mis-ordered - # responses and we don't care that much - assert set(pr.comments) == { - (users['reviewer'], "hansen r+\n%s up to" % bot_name), - (users['reviewer'], "%s up to b" % bot_name), - (users['reviewer'], "%s up to foo" % bot_name), - (users['reviewer'], "%s up to c" % bot_name), - (users['other'], "Please provide a branch to forward-port to."), - (users['other'], "Branch 'b' is disabled, it can't be used as a forward port target."), - (users['other'], "There is no branch 'foo', it can't be used as a forward port target."), - (users['other'], "Forward-porting to 'c'."), - } - -def test_default_disabled(env, config, make_repo, users): - """ If the default limit is disabled, it should still be the default - limit but the ping message should be set on the actual last FP (to the - last non-deactivated target) - """ - prod, other = make_basic(env, config, make_repo) - branch_c = env['runbot_merge.branch'].search([('name', '=', 'c')]) - branch_c.fp_target = False - - with prod: - [c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/branch0') - pr = prod.make_pr(target='a', head='branch0') - prod.post_status(c, 'success', 'legal/cla') - prod.post_status(c, 'success', 'ci/runbot') - pr.post_comment('hansen r+', config['role_reviewer']['token']) - env.run_crons() - - assert env['runbot_merge.pull_requests'].search([]).limit_id == branch_c - - with prod: - prod.post_status('staging.a', 'success', 'legal/cla') - prod.post_status('staging.a', 'success', 'ci/runbot') - env.run_crons() - - p1, p2 = env['runbot_merge.pull_requests'].search([], order='number') - assert p1.number == pr.number - pr2 = prod.get_pr(p2.number) - - cs = pr2.comments - assert len(cs) == 1 - assert pr2.comments == [ - (users['user'], """\ -Ping @%s, @%s -This PR targets b and is the last of the forward-port chain. - -To merge the full chain, say -> @%s r+ - -More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port -""" % (users['user'], users['reviewer'], users['user'])), - ] - # reviewer = of the FP sequence, the original PR is always reviewed by `user` # set as reviewer Case = collections.namedtuple('Case', 'author reviewer delegate success') diff --git a/forwardport/tests/utils.py b/forwardport/tests/utils.py index e68be339..fbe527e2 100644 --- a/forwardport/tests/utils.py +++ b/forwardport/tests/utils.py @@ -34,3 +34,63 @@ class re_matches: def __repr__(self): return '~' + self._r.pattern + '~' + + +def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproject'): + """ Creates a basic repo with 3 forking branches + + 0 -- 1 -- 2 -- 3 -- 4 : a + | + `-- 11 -- 22 : b + | + `-- 111 : c + each branch just adds and modifies a file (resp. f, g and h) through the + contents sequence a b c d e + """ + Projects = env['runbot_merge.project'] + project = Projects.search([('name', '=', project_name)]) + if not project: + project = env['runbot_merge.project'].create({ + 'name': project_name, + 'github_token': config['github']['token'], + 'github_prefix': 'hansen', + 'fp_github_token': config['github']['token'], + 'required_statuses': 'legal/cla,ci/runbot', + 'branch_ids': [ + (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}), + (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}), + (0, 0, {'name': 'c', 'fp_sequence': 0, 'fp_target': True}), + ], + }) + + prod = make_repo(reponame) + with prod: + a_0, a_1, a_2, a_3, a_4, = prod.make_commits( + None, + Commit("0", tree={'f': 'a'}), + Commit("1", tree={'f': 'b'}), + Commit("2", tree={'f': 'c'}), + Commit("3", tree={'f': 'd'}), + Commit("4", tree={'f': 'e'}), + ref='heads/a', + ) + b_1, b_2 = prod.make_commits( + a_2, + Commit('11', tree={'g': 'a'}), + Commit('22', tree={'g': 'b'}), + ref='heads/b', + ) + prod.make_commits( + b_1, + Commit('111', tree={'h': 'a'}), + ref='heads/c', + ) + other = prod.fork() + project.write({ + 'repo_ids': [(0, 0, { + 'name': prod.name, + 'fp_remote_target': other.name, + })], + }) + + return prod, other