[IMP] *: modernise tests via to_pr

The `to_pr` helper was added a *while* ago to replace the pretty
verbose process of looking a PR with the right number in the right
repository after a `make_pr`. However while I did some ad-hoc
replacement of existing `search` calls as I had to work with existing
tests I never did a full search and replace to try and excise searches
from the test suite.

This is now done. I've undoubtedly missed a few, but a hundred-odd
lines of codes have been simplified.
This commit is contained in:
Xavier Morel 2024-12-02 14:17:28 +01:00
parent 83e588d7fe
commit 509c152156
8 changed files with 84 additions and 245 deletions

View File

@ -1,6 +1,7 @@
import json import json
from utils import Commit, make_basic from utils import Commit, make_basic, to_pr
def statuses(pr): def statuses(pr):
return { return {
@ -25,7 +26,7 @@ def test_override_inherited(env, config, make_repo, users):
pr.post_comment('hansen r+ override=default', config['role_reviewer']['token']) pr.post_comment('hansen r+ override=default', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
original = env['runbot_merge.pull_requests'].search([('repository.name', '=', repo.name), ('number', '=', pr.number)]) original = to_pr(env, pr)
assert original.state == 'ready' assert original.state == 'ready'
assert not original.limit_id assert not original.limit_id
@ -93,7 +94,7 @@ def test_override_combination(env, config, make_repo, users):
pr.post_comment('hansen r+ override=ci/runbot', config['role_reviewer']['token']) pr.post_comment('hansen r+ override=ci/runbot', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr0_id = env['runbot_merge.pull_requests'].search([('repository.name', '=', repo.name), ('number', '=', pr.number)]) pr0_id = to_pr(env, pr)
assert pr0_id.state == 'ready' assert pr0_id.state == 'ready'
assert statuses(pr0_id) == {'ci/runbot': 'success', 'legal/cla': 'success'} assert statuses(pr0_id) == {'ci/runbot': 'success', 'legal/cla': 'success'}

View File

@ -55,10 +55,7 @@ def test_straightforward_flow(env, config, make_repo, users):
# use rebase-ff (instead of rebase-merge) so we don't have to dig in # use rebase-ff (instead of rebase-merge) so we don't have to dig in
# parents of the merge commit to find the cherrypicks # parents of the merge commit to find the cherrypicks
pr.post_comment('hansen r+ rebase-ff', config['role_reviewer']['token']) pr.post_comment('hansen r+ rebase-ff', config['role_reviewer']['token'])
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', prod.name),
('number', '=', pr.number),
])
assert not pr_id.merge_date,\ assert not pr_id.merge_date,\
"PR obviously shouldn't have a merge date before being merged" "PR obviously shouldn't have a merge date before being merged"
@ -1086,10 +1083,7 @@ class TestBranchDeletion:
prod.post_status('staging.a', 'success', 'ci/runbot') prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', prod.name),
('number', '=', pr.number)
])
assert pr_id.state == 'merged' assert pr_id.state == 'merged'
removers = env['forwardport.branch_remover'].search([]) removers = env['forwardport.branch_remover'].search([])
to_delete_branch = removers.mapped('pr_id') to_delete_branch = removers.mapped('pr_id')
@ -1160,10 +1154,7 @@ class TestRecognizeCommands:
r.make_commits('c', Commit('p', tree={'x': '0'}), ref='heads/testbranch') r.make_commits('c', Commit('p', tree={'x': '0'}), ref='heads/testbranch')
pr = r.make_pr(target='a', head='testbranch') pr = r.make_pr(target='a', head='testbranch')
return r, pr, env['runbot_merge.pull_requests'].search([ return r, pr, to_pr(env, pr)
('repository.name', '=', r.name),
('number', '=', pr.number),
])
# FIXME: remove / merge into mergebot tests # FIXME: remove / merge into mergebot tests
def test_botname_casing(self, env, config, make_repo): def test_botname_casing(self, env, config, make_repo):

View File

@ -278,7 +278,7 @@ class TestNotAllBranches:
pr = a.make_pr(target='a', title="a pr", head=a_dev.owner + ':change') pr = a.make_pr(target='a', title="a pr", head=a_dev.owner + ':change')
a.post_status(c, 'success', 'ci/runbot') a.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
p = env['runbot_merge.pull_requests'].search([('repository.name', '=', a.name), ('number', '=', pr.number)]) p = to_pr(env, pr)
env.run_crons() env.run_crons()
assert p.staging_id assert p.staging_id
with a, b: with a, b:
@ -371,14 +371,8 @@ class TestNotAllBranches:
repo.post_status('staging.a', 'success', 'ci/runbot') repo.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr_a_id = env['runbot_merge.pull_requests'].search([ pr_a_id = to_pr(env, pr_a)
('repository.name', '=', a.name), pr_b_id = to_pr(env, pr_b)
('number', '=', pr_a.number),
])
pr_b_id = env['runbot_merge.pull_requests'].search([
('repository.name', '=', b.name),
('number', '=', pr_b.number)
])
assert pr_a_id.state == pr_b_id.state == 'merged' assert pr_a_id.state == pr_b_id.state == 'merged'
assert env['runbot_merge.pull_requests'].search([]) == pr_a_id | pr_b_id assert env['runbot_merge.pull_requests'].search([]) == pr_a_id | pr_b_id
# should have refused to create a forward port because the PRs have # should have refused to create a forward port because the PRs have
@ -645,10 +639,7 @@ def test_skip_ci_all(env, config, make_repo):
pr.post_comment('hansen fw=skipci', config['role_reviewer']['token']) pr.post_comment('hansen fw=skipci', config['role_reviewer']['token'])
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, pr).batch_id.fw_policy == 'skipci'
('repository.name', '=', prod.name),
('number', '=', pr.number)
]).batch_id.fw_policy == 'skipci'
with prod: with prod:
prod.post_status('staging.a', 'success', 'legal/cla') prod.post_status('staging.a', 'success', 'legal/cla')

View File

@ -180,8 +180,8 @@ def make_basic(
def pr_page(page, pr): def pr_page(page, pr):
return html.fromstring(page(f'/{pr.repo.name}/pull/{pr.number}')) return html.fromstring(page(f'/{pr.repo.name}/pull/{pr.number}'))
def to_pr(env, pr): def to_pr(env, pr, *, attempts=5):
for _ in range(5): for _ in range(attempts):
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = env['runbot_merge.pull_requests'].search([
('repository.name', '=', pr.repo.name), ('repository.name', '=', pr.repo.name),
('number', '=', pr.number), ('number', '=', pr.number),

View File

@ -386,10 +386,8 @@ class TestWebhookSecurity:
c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'}) c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'})
pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0) pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0)
assert not env['runbot_merge.pull_requests'].search([ with pytest.raises(TimeoutError):
('repository.name', '=', repo.name), to_pr(env, pr0)
('number', '=', pr0.number),
])
def test_wrong_secret(self, env, project, repo): def test_wrong_secret(self, env, project, repo):
with repo: with repo:
@ -401,10 +399,8 @@ class TestWebhookSecurity:
c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'}) c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'})
pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0) pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0)
assert not env['runbot_merge.pull_requests'].search([ with pytest.raises(TimeoutError):
('repository.name', '=', repo.name), to_pr(env, pr0)
('number', '=', pr0.number),
])
def test_correct_secret(self, env, project, repo): def test_correct_secret(self, env, project, repo):
with repo: with repo:
@ -416,10 +412,7 @@ class TestWebhookSecurity:
c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'}) c0 = repo.make_commit(m, 'replace file contents', None, tree={'a': 'some other content'})
pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0) pr0 = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0)
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, pr0)
('repository.name', '=', repo.name),
('number', '=', pr0.number),
])
def test_staging_ongoing(env, repo, config): def test_staging_ongoing(env, repo, config):
with repo: with repo:
@ -435,10 +428,7 @@ def test_staging_ongoing(env, repo, config):
repo.post_status(c1, 'success', 'ci/runbot') repo.post_status(c1, 'success', 'ci/runbot')
pr1.post_comment("hansen r+ rebase-merge", config['role_reviewer']['token']) pr1.post_comment("hansen r+ rebase-merge", config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr1 = env['runbot_merge.pull_requests'].search([ pr1 = to_pr(env, pr1)
('repository.name', '=', repo.name),
('number', '=', 1)
])
assert pr1.staging_id assert pr1.staging_id
with repo: with repo:
@ -450,10 +440,7 @@ def test_staging_ongoing(env, repo, config):
repo.post_status(c3, 'success', 'ci/runbot') repo.post_status(c3, 'success', 'ci/runbot')
pr2.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token']) pr2.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
p_2 = env['runbot_merge.pull_requests'].search([ p_2 = to_pr(env, pr2)
('repository.name', '=', repo.name),
('number', '=', pr2.number)
])
assert p_2.state == 'ready', "PR2 should not have been staged since there is a pending staging for master" assert p_2.state == 'ready', "PR2 should not have been staged since there is a pending staging for master"
with repo: with repo:
@ -496,15 +483,9 @@ def test_staging_concurrent(env, repo, config):
pr2.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token']) pr2.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr1 = env['runbot_merge.pull_requests'].search([ pr1 = to_pr(env, pr1)
('repository.name', '=', repo.name),
('number', '=', pr1.number)
])
assert pr1.staging_id assert pr1.staging_id
pr2 = env['runbot_merge.pull_requests'].search([ pr2 = to_pr(env, pr2)
('repository.name', '=', repo.name),
('number', '=', pr2.number)
])
assert pr2.staging_id assert pr2.staging_id
@ -700,10 +681,7 @@ def test_ff_failure(env, repo, config, page):
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token']) prx.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
st = env['runbot_merge.pull_requests'].search([ st = to_pr(env, prx).staging_id
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).staging_id
assert st assert st
with repo: with repo:
@ -725,10 +703,7 @@ def test_ff_failure(env, repo, config, page):
assert 'bg-gray-lighter' in prev.classes, "ff failure is ~ cancelling" assert 'bg-gray-lighter' in prev.classes, "ff failure is ~ cancelling"
assert 'fast forward failed (update is not a fast forward)' in prev.get('title') assert 'fast forward failed (update is not a fast forward)' in prev.get('title')
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).staging_id, "merge should not have succeeded"
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).staging_id, "merge should not have succeeded"
assert repo.commit('heads/staging.master').id != staging.id,\ assert repo.commit('heads/staging.master').id != staging.id,\
"PR should be staged to a new commit" "PR should be staged to a new commit"
@ -833,10 +808,7 @@ class TestPREdition:
prx.post_comment('hansen rebase-ff r+', config['role_reviewer']['token']) prx.post_comment('hansen rebase-ff r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'ready' assert pr.state == 'ready'
st = pr.staging_id st = pr.staging_id
assert st assert st
@ -858,10 +830,7 @@ class TestPREdition:
env.run_crons() env.run_crons()
with repo: prx.base = '1.0' with repo: prx.base = '1.0'
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).target == branch_1
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).target == branch_1
def test_retarget_update_commits(self, env, project, repo): def test_retarget_update_commits(self, env, project, repo):
""" Retargeting a PR should update its commits count, as well as follow """ Retargeting a PR should update its commits count, as well as follow
@ -890,10 +859,7 @@ class TestPREdition:
prx = repo.make_pr(title='title', body='body', target='1.0', head='abranch') prx = repo.make_pr(title='title', body='body', target='1.0', head='abranch')
repo.post_status('heads/abranch', 'success', 'a') repo.post_status('heads/abranch', 'success', 'a')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert not pr.squash assert not pr.squash
assert pr.status == 'pending' assert pr.status == 'pending'
assert pr.state == 'opened' assert pr.state == 'opened'
@ -938,10 +904,7 @@ class TestPREdition:
[c] = repo.make_commits(m, repo.Commit('first', tree={'m': 'm3'}), ref='heads/abranch') [c] = repo.make_commits(m, repo.Commit('first', tree={'m': 'm3'}), ref='heads/abranch')
prx = repo.make_pr(title='title', body='body', target='1.0', head=c) prx = repo.make_pr(title='title', body='body', target='1.0', head=c)
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert pr.target == branch_1 assert pr.target == branch_1
with repo: with repo:
prx.close() prx.close()
@ -961,10 +924,7 @@ def test_close_staged(env, repo, config, page):
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+', config['role_reviewer']['token']) prx.post_comment('hansen r+', config['role_reviewer']['token'])
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
env.run_crons() env.run_crons()
assert pr.reviewed_by assert pr.reviewed_by
assert pr.state == 'ready' assert pr.state == 'ready'
@ -1113,10 +1073,7 @@ def test_reopen_merged_pr(env, repo, config, users):
repo.post_status('staging.master', 'success', 'legal/cla') repo.post_status('staging.master', 'success', 'legal/cla')
repo.post_status('staging.master', 'success', 'ci/runbot') repo.post_status('staging.master', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert prx.state == 'closed' assert prx.state == 'closed'
assert pr.state == 'merged' assert pr.state == 'merged'
@ -1199,19 +1156,13 @@ class TestRetry:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
prx.post_comment('hansen r+', config['role_reviewer']['token']) prx.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).staging_id
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).staging_id
staging_head = repo.commit('heads/staging.master') staging_head = repo.commit('heads/staging.master')
repo.post_status('staging.master', 'success', 'legal/cla') repo.post_status('staging.master', 'success', 'legal/cla')
repo.post_status('staging.master', 'failure', 'ci/runbot') repo.post_status('staging.master', 'failure', 'ci/runbot')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert pr.state == 'error' assert pr.state == 'error'
repo.update_ref(prx.ref, repo.make_commit(prx.head, 'third', None, tree={'m': 'c3'}), force=True) repo.update_ref(prx.ref, repo.make_commit(prx.head, 'third', None, tree={'m': 'c3'}), force=True)
@ -1383,16 +1334,10 @@ class TestMergeMethod:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
prx.post_comment('hansen r+', config['role_reviewer']['token']) prx.post_comment('hansen r+', config['role_reviewer']['token'])
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).squash
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).squash
env.run_crons() env.run_crons()
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).staging_id
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).staging_id
staging = repo.commit('heads/staging.master') staging = repo.commit('heads/staging.master')
assert not repo.is_ancestor(prx.head, of=staging.id),\ assert not repo.is_ancestor(prx.head, of=staging.id),\
@ -1407,10 +1352,7 @@ class TestMergeMethod:
repo.post_status('staging.master', 'success', 'legal/cla') repo.post_status('staging.master', 'success', 'legal/cla')
repo.post_status('staging.master', 'success', 'ci/runbot') repo.post_status('staging.master', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert pr.state == 'merged' assert pr.state == 'merged'
assert prx.state == 'closed' assert prx.state == 'closed'
assert json.loads(pr.commits_map) == { assert json.loads(pr.commits_map) == {
@ -1452,10 +1394,7 @@ class TestMergeMethod:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
prx = repo.make_pr(title='title', body='body', target='master', head=c1) prx = repo.make_pr(title='title', body='body', target='master', head=c1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.squash, "a PR with a single commit should be squashed" assert pr.squash, "a PR with a single commit should be squashed"
with repo: with repo:
@ -1475,10 +1414,7 @@ class TestMergeMethod:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
c2 = repo.make_commit(c1, 'second2', None, tree={'m': 'c2'}) c2 = repo.make_commit(c1, 'second2', None, tree={'m': 'c2'})
prx = repo.make_pr(title='title', body='body', target='master', head=c2) prx = repo.make_pr(title='title', body='body', target='master', head=c2)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
pr.merge_method = 'rebase-merge' pr.merge_method = 'rebase-merge'
assert not pr.squash, "a PR with a single commit should not be squashed" assert not pr.squash, "a PR with a single commit should not be squashed"
@ -1542,10 +1478,7 @@ commits, I need to know how to merge it:
b0 = repo.make_commit(m1, 'B0', None, tree={'m': '1', 'b': '0'}) b0 = repo.make_commit(m1, 'B0', None, tree={'m': '1', 'b': '0'})
b1 = repo.make_commit(b0, 'B1', None, tree={'m': '1', 'b': '1'}) b1 = repo.make_commit(b0, 'B1', None, tree={'m': '1', 'b': '1'})
prx = repo.make_pr(title='title', body='body', target='master', head=b1) prx = repo.make_pr(title='title', body='body', target='master', head=b1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
with repo: with repo:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'ci/runbot')
@ -1653,10 +1586,7 @@ commits, I need to know how to merge it:
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'merged' assert pr.state == 'merged'
# check that the dummy commit is not in the final master # check that the dummy commit is not in the final master
@ -1734,10 +1664,7 @@ commits, I need to know how to merge it:
repo.post_status('heads/staging.master', 'success', 'ci/runbot') repo.post_status('heads/staging.master', 'success', 'ci/runbot')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'merged' assert pr.state == 'merged'
# check that the dummy commit is not in the final master # check that the dummy commit is not in the final master
@ -1817,10 +1744,7 @@ commits, I need to know how to merge it:
expected = node('gibberish\n\nblahblah\n\ncloses {}#{}' expected = node('gibberish\n\nblahblah\n\ncloses {}#{}'
'\n\nSigned-off-by: {}'.format(repo.name, prx.number, reviewer), m, c0) '\n\nSigned-off-by: {}'.format(repo.name, prx.number, reviewer), m, c0)
assert log_to_node(repo.log('heads/master')), expected assert log_to_node(repo.log('heads/master')), expected
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert json.loads(pr.commits_map) == { assert json.loads(pr.commits_map) == {
prx.head: prx.head, prx.head: prx.head,
'': master.id '': master.id
@ -2391,7 +2315,8 @@ class TestPRUpdate:
) )
repo.make_ref('heads/1.0', m) repo.make_ref('heads/1.0', m)
prx = repo.make_pr(title='title', body='body', target='1.0', head=c) prx = repo.make_pr(title='title', body='body', target='1.0', head=c)
assert not env['runbot_merge.pull_requests'].search([('number', '=', prx.number)]) with pytest.raises(TimeoutError):
to_pr(env, prx)
env['runbot_merge.project'].search([]).write({ env['runbot_merge.project'].search([]).write({
'branch_ids': [(0, 0, {'name': '1.0'})] 'branch_ids': [(0, 0, {'name': '1.0'})]
@ -2401,7 +2326,8 @@ class TestPRUpdate:
[c2] = repo.make_commits(c, Commit('second', tree={'m': 'c2'})) [c2] = repo.make_commits(c, Commit('second', tree={'m': 'c2'}))
repo.update_ref(prx.ref, c2, force=True) repo.update_ref(prx.ref, c2, force=True)
assert not env['runbot_merge.pull_requests'].search([('number', '=', prx.number)]) with pytest.raises(TimeoutError):
to_pr(env, prx)
@pytest.mark.defaultstatuses @pytest.mark.defaultstatuses
def test_update_to_ci(self, env, repo): def test_update_to_ci(self, env, repo):
@ -3092,8 +3018,8 @@ class TestBatching(object):
repo.post_status('heads/staging.master', 'failure', 'ci/runbot') repo.post_status('heads/staging.master', 'failure', 'ci/runbot')
repo.post_status('heads/staging.master', 'success', 'legal/cla') repo.post_status('heads/staging.master', 'success', 'legal/cla')
pr1 = env['runbot_merge.pull_requests'].search([('number', '=', pr1.number)]) pr1 = to_pr(env, pr1)
pr2 = env['runbot_merge.pull_requests'].search([('number', '=', pr2.number)]) pr2 = to_pr(env, pr2)
env.run_crons() env.run_crons()
# should have split the existing batch into two, with one of the # should have split the existing batch into two, with one of the
@ -3137,16 +3063,10 @@ class TestReviewing:
prx.post_comment('hansen r+', config['role_other']['token']) prx.post_comment('hansen r+', config['role_other']['token'])
env.run_crons() env.run_crons()
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).state == 'validated'
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).state == 'validated'
with repo: with repo:
prx.post_comment('hansen r+', config['role_reviewer']['token']) prx.post_comment('hansen r+', config['role_reviewer']['token'])
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).state == 'ready'
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).state == 'ready'
# second r+ to check warning # second r+ to check warning
with repo: with repo:
prx.post_comment('hansen r+', config['role_reviewer']['token']) prx.post_comment('hansen r+', config['role_reviewer']['token'])
@ -3177,10 +3097,7 @@ class TestReviewing:
env.run_crons() env.run_crons()
assert prx.user == users['reviewer'] assert prx.user == users['reviewer']
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).state == 'validated'
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).state == 'validated'
env.run_crons() env.run_crons()
assert prx.comments == [ assert prx.comments == [
@ -3204,10 +3121,7 @@ class TestReviewing:
env.run_crons() env.run_crons()
assert prx.user == users['self_reviewer'] assert prx.user == users['self_reviewer']
assert env['runbot_merge.pull_requests'].search([ assert to_pr(env, prx).state == 'ready'
('repository.name', '=', repo.name),
('number', '=', prx.number)
]).state == 'ready'
def test_delegate_review(self, env, repo, users, config): def test_delegate_review(self, env, repo, users, config):
"""Users should be able to delegate review to either the creator of """Users should be able to delegate review to either the creator of
@ -3274,10 +3188,7 @@ class TestReviewing:
prx = repo.make_pr(title='title', body=None, target='master', head=c) prx = repo.make_pr(title='title', body=None, target='master', head=c)
prx.post_comment('hansen delegate=foo,@bar,#baz', config['role_reviewer']['token']) prx.post_comment('hansen delegate=foo,@bar,#baz', config['role_reviewer']['token'])
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert {d.github_login for d in pr.delegates} == {'foo', 'bar', 'baz'} assert {d.github_login for d in pr.delegates} == {'foo', 'bar', 'baz'}
@ -3291,10 +3202,7 @@ class TestReviewing:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
prx = repo.make_pr(title='title', body='body', target='master', head=c1) prx = repo.make_pr(title='title', body='body', target='master', head=c1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
with repo: with repo:
prx.post_review('COMMENT', "hansen priority", config['role_reviewer']['token']) prx.post_review('COMMENT', "hansen priority", config['role_reviewer']['token'])
@ -3397,10 +3305,7 @@ class TestUnknownPR:
env.run_crons() env.run_crons()
# assume an unknown but ready PR: we don't know the PR or its head commit # assume an unknown but ready PR: we don't know the PR or its head commit
env['runbot_merge.pull_requests'].search([ to_pr(env, prx).unlink()
('repository.name', '=', repo.name),
('number', '=', prx.number),
]).unlink()
env['runbot_merge.commit'].search([('sha', '=', prx.head)]).unlink() env['runbot_merge.commit'].search([('sha', '=', prx.head)]).unlink()
# reviewer reviewers # reviewer reviewers
@ -3434,10 +3339,7 @@ class TestUnknownPR:
"re-approve it as I didn't see previous commands."), "re-approve it as I didn't see previous commands."),
] ]
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert pr.state == 'validated' assert pr.state == 'validated'
with repo: with repo:
@ -3618,10 +3520,7 @@ class TestRecognizeCommands:
c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) c = repo.make_commit(m, 'first', None, tree={'m': 'c'})
prx = repo.make_pr(title='title', body=None, target='master', head=c) prx = repo.make_pr(title='title', body=None, target='master', head=c)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'opened' assert pr.state == 'opened'
with repo: with repo:
@ -3633,20 +3532,19 @@ class TestRecognizeCommands:
""" matching botname should ignore leading whitespaces """ matching botname should ignore leading whitespaces
""" """
with repo: with repo:
m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) m, c = repo.make_commits(
None,
Commit('initial', tree={'m': 'm'}),
Commit('first', tree={'m': 'c'}),
)
repo.make_ref('heads/master', m) repo.make_ref('heads/master', m)
prx = repo.make_pr(title='title', target='master', head=c)
c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) pr = to_pr(env, prx)
prx = repo.make_pr(title='title', body=None, target='master', head=c)
pr = env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'opened' assert pr.state == 'opened'
with repo: with repo:
prx.post_comment('%shansen r+' % indent, config['role_reviewer']['token']) prx.post_comment(f'{indent}hansen r+', config['role_reviewer']['token'])
assert pr.state == 'approved' assert pr.state == 'approved'
def test_unknown_commands(self, repo, env, config, users): def test_unknown_commands(self, repo, env, config, users):
@ -3737,10 +3635,7 @@ class TestRMinus:
c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) c = repo.make_commit(m, 'first', None, tree={'m': 'c'})
prx = repo.make_pr(title='title', body=None, target='master', head=c) prx = repo.make_pr(title='title', body=None, target='master', head=c)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'opened' assert pr.state == 'opened'
with repo: with repo:
@ -3775,10 +3670,7 @@ class TestRMinus:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
assert pr.state == 'validated' assert pr.state == 'validated'
with repo: with repo:
@ -3813,10 +3705,7 @@ class TestRMinus:
repo.post_status(prx.head, 'success', 'legal/cla') repo.post_status(prx.head, 'success', 'legal/cla')
env.run_crons() env.run_crons()
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number),
])
# if reviewer unreviews, cancel staging & unreview # if reviewer unreviews, cancel staging & unreview
with repo: with repo:
@ -3919,10 +3808,7 @@ class TestComments:
prx.post_comment('@hansen delegate=bar', config['role_reviewer']['token']) prx.post_comment('@hansen delegate=bar', config['role_reviewer']['token'])
prx.post_comment('#hansen delegate=baz', config['role_reviewer']['token']) prx.post_comment('#hansen delegate=baz', config['role_reviewer']['token'])
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
assert {p.github_login for p in pr.delegates} \ assert {p.github_login for p in pr.delegates} \
== {'foo', 'bar', 'baz'} == {'foo', 'bar', 'baz'}
@ -3936,10 +3822,7 @@ class TestComments:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
prx = repo.make_pr(title='title', body='body', target='master', head=c1) prx = repo.make_pr(title='title', body='body', target='master', head=c1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
with repo: with repo:
cid = prx.post_comment('hansen r+', config['role_reviewer']['token']) cid = prx.post_comment('hansen r+', config['role_reviewer']['token'])
@ -3960,10 +3843,7 @@ class TestComments:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
prx = repo.make_pr(title='title', body='body', target='master', head=c1) prx = repo.make_pr(title='title', body='body', target='master', head=c1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
with repo: with repo:
cid = prx.post_comment('hansen r+', config['role_reviewer']['token']) cid = prx.post_comment('hansen r+', config['role_reviewer']['token'])
@ -4017,10 +3897,7 @@ class TestFeedback:
c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
prx = repo.make_pr(title='title', body='body', target='master', head=c1) prx = repo.make_pr(title='title', body='body', target='master', head=c1)
pr = env['runbot_merge.pull_requests'].search([ pr = to_pr(env, prx)
('repository.name', '=', repo.name),
('number', '=', prx.number)
])
with repo: with repo:
repo.post_status(prx.head, 'failure', 'ci/runbot') repo.post_status(prx.head, 'failure', 'ci/runbot')

View File

@ -1,6 +1,7 @@
import pytest import pytest
from utils import Commit from utils import Commit, to_pr
@pytest.fixture @pytest.fixture
def _setup_statuses(project, repo): def _setup_statuses(project, repo):
@ -26,10 +27,7 @@ def test_status_applies(env, repo, config):
[c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change') [c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change')
pr = repo.make_pr(target='master', title="super change", head='change') pr = repo.make_pr(target='master', title="super change", head='change')
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'opened' assert pr_id.state == 'opened'
with repo: with repo:
@ -76,10 +74,7 @@ def test_status_skipped(env, project, repo, config):
[c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change') [c] = repo.make_commits(m, Commit('pr', tree={'a': '2'}), ref='heads/change')
pr = repo.make_pr(target='maintenance', title="super change", head='change') pr = repo.make_pr(target='maintenance', title="super change", head='change')
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'opened' assert pr_id.state == 'opened'
with repo: with repo:
@ -140,10 +135,7 @@ def test_pseudo_version_tag(env, project, make_repo, setreviewers, config):
repo.post_status(pr.ref, 'success', 'ci') repo.post_status(pr.ref, 'success', 'ci')
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() # should create staging env.run_crons() # should create staging
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'ready' assert pr_id.state == 'ready'
assert pr_id.staging_id assert pr_id.staging_id
with repo: with repo:
@ -160,10 +152,7 @@ def test_pseudo_version_tag(env, project, make_repo, setreviewers, config):
repo.post_status(pr.ref, 'success', 'ci') repo.post_status(pr.ref, 'success', 'ci')
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() # should create staging env.run_crons() # should create staging
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'ready', pr.comments assert pr_id.state == 'ready', pr.comments
assert pr_id.staging_id assert pr_id.staging_id
with repo: with repo:

View File

@ -111,10 +111,9 @@ def test_new_pr_no_branch(env, project, repo, users):
pr = repo.make_pr(title="title", body='body', target='other', head=c) pr = repo.make_pr(title="title", body='body', target='other', head=c)
env.run_crons() env.run_crons()
assert not env['runbot_merge.pull_requests'].search([ # the PR should not have been created in the backend
('repository', '=', project.repo_ids.id), with pytest.raises(TimeoutError):
('number', '=', pr.number), to_pr(env, pr, attempts=1)
]), "the PR should not have been created in the backend"
assert pr.comments == [ assert pr.comments == [
(users['user'], "This PR targets the un-managed branch %s:other, it needs to be retargeted before it can be merged." % repo.name), (users['user'], "This PR targets the un-managed branch %s:other, it needs to be retargeted before it can be merged." % repo.name),
] ]

View File

@ -2,7 +2,7 @@ import json
import pytest import pytest
from utils import seen, Commit from utils import seen, Commit, to_pr
def test_no_duplicates(env): def test_no_duplicates(env):
@ -70,10 +70,7 @@ def test_basic(env, project, make_repo, users, setreviewers, config):
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'approved' assert pr_id.state == 'approved'
with repo: with repo:
@ -141,10 +138,7 @@ def test_multiple(env, project, make_repo, users, setreviewers, config):
pr = repo.make_pr(target='master', title=f'super change {i}', head=f'change{i}') pr = repo.make_pr(target='master', title=f'super change {i}', head=f'change{i}')
env.run_crons() env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'opened' assert pr_id.state == 'opened'
with repo: with repo:
@ -193,10 +187,7 @@ def test_no_repository(env, project, make_repo, users, setreviewers, config):
pr.post_comment('hansen r+', config['role_reviewer']['token']) pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([ pr_id = to_pr(env, pr)
('repository.name', '=', repo.name),
('number', '=', pr.number)
])
assert pr_id.state == 'approved' assert pr_id.state == 'approved'
with repo: with repo: