[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.

Author: xmo-odoo

forwardport/tests/test_overrides.py

@@ -1,6 +1,7 @@
 import json
 
-from utils import Commit, make_basic
+from utils import Commit, make_basic, to_pr
+
 
 def statuses(pr):
     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'])
     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 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'])
     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 statuses(pr0_id) == {'ci/runbot': 'success', 'legal/cla': 'success'}
 

forwardport/tests/test_simple.py

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

forwardport/tests/test_weird.py

@@ -278,7 +278,7 @@ def test_single_first(self, env, repos, config):
             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'])
-        p = env['runbot_merge.pull_requests'].search([('repository.name', '=', a.name), ('number', '=', pr.number)])
+        p = to_pr(env, pr)
         env.run_crons()
         assert p.staging_id
         with a, b:
@@ -371,14 +371,8 @@ def test_both_first(self, env, repos, config, users):
                 repo.post_status('staging.a', 'success', 'ci/runbot')
         env.run_crons()
 
-        pr_a_id = env['runbot_merge.pull_requests'].search([
-            ('repository.name', '=', a.name),
-            ('number', '=', pr_a.number),
-        ])
-        pr_b_id = env['runbot_merge.pull_requests'].search([
-            ('repository.name', '=', b.name),
-            ('number', '=', pr_b.number)
-        ])
+        pr_a_id = to_pr(env, pr_a)
+        pr_b_id = to_pr(env, pr_b)
         assert pr_a_id.state == pr_b_id.state == 'merged'
         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
@@ -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 r+', config['role_reviewer']['token'])
     env.run_crons()
-    assert env['runbot_merge.pull_requests'].search([
-        ('repository.name', '=', prod.name),
-        ('number', '=', pr.number)
-    ]).batch_id.fw_policy == 'skipci'
+    assert to_pr(env, pr).batch_id.fw_policy == 'skipci'
 
     with prod:
         prod.post_status('staging.a', 'success', 'legal/cla')

mergebot_test_utils/utils.py

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