[IMP] *: make to_pr more resilient to webhook delays

Github delivery delays keep getting worse. Depending on what comes
before `to_pr`, this leads it to fail more often as it runs before the
PR it's looking for was signaled to the mergebot.

In order to mitigate this issue, add a wait loop in `to_pr`, waiting
up to 4 seconds for the PR it's looking for before aborting.

Also replace manual lookups by `to_pr` in every method of
`TestPRUpdate` while at it since it hit a few of the issues. And
remove the xfail test case since it seems unlikely github will change
tack (maybe? could be worth testing to be sure).

mergebot_test_utils/utils.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 import itertools
 import re
+import time
 
 from lxml import html
 
@@ -128,12 +129,17 @@ def pr_page(page, pr):
     return html.fromstring(page(f'/{pr.repo.name}/pull/{pr.number}'))
 
 def to_pr(env, pr):
-    pr = env['runbot_merge.pull_requests'].search([
+    for _ in range(5):
-        ('repository.name', '=', pr.repo.name),
+        pr_id = env['runbot_merge.pull_requests'].search([
-        ('number', '=', pr.number),
+            ('repository.name', '=', pr.repo.name),
-    ])
+            ('number', '=', pr.number),
-    assert len(pr) == 1, f"Expected to find {pr.repo.name}#{pr.number}, got {pr}."
+        ])
-    return pr
+        if pr_id:
+            assert len(pr_id) == 1, f"Expected to find {pr.repo.name}#{pr.number}, got {pr_id}."
+            return pr_id
+        time.sleep(1)
+
+    raise TimeoutError(f"Unable to find {pr.repo.name}#{pr.number}")
 
 def part_of(label, pr_id, *, separator='\n\n'):
     """ Adds the "part-of" pseudo-header in the footer.

runbot_merge/tests/test_basic.py

@@ -2121,10 +2121,8 @@ class TestPRUpdate(object):
 
             c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
             prx = repo.make_pr(title='title', body='body', target='master', head=c)
-        pr = env['runbot_merge.pull_requests'].search([
+
-            ('repository.name', '=', repo.name),
+        pr = to_pr(env, prx)
-            ('number', '=', prx.number),
-        ])
         assert pr.head == c
         # alter & push force PR entirely
         with repo:
@@ -2139,10 +2137,8 @@ class TestPRUpdate(object):
 
             c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
             prx = repo.make_pr(title='title', body='body', target='master', head=c)
-        pr = env['runbot_merge.pull_requests'].search([
+
-            ('repository.name', '=', repo.name),
+        pr = to_pr(env, prx)
-            ('number', '=', prx.number),
-        ])
         with repo:
             prx.close()
         assert pr.state == 'closed'
@@ -2169,10 +2165,7 @@ class TestPRUpdate(object):
             repo.post_status(prx.head, 'success', 'legal/cla')
             repo.post_status(prx.head, 'success', 'ci/runbot')
         env.run_crons()
-        pr = env['runbot_merge.pull_requests'].search([
+        pr = to_pr(env, prx)
-            ('repository.name', '=', repo.name),
-            ('number', '=', prx.number),
-        ])
         assert pr.head == c
         assert pr.state == 'validated'
 
@@ -2190,10 +2183,8 @@ class TestPRUpdate(object):
             c = repo.make_commit(m, 'fist', None, tree={'m': 'c1'})
             prx = repo.make_pr(title='title', body='body', target='master', head=c)
             prx.post_comment('hansen r+', config['role_reviewer']['token'])
-        pr = env['runbot_merge.pull_requests'].search([
+
-            ('repository.name', '=', repo.name),
+        pr = to_pr(env, prx)
-            ('number', '=', prx.number),
-        ])
         assert pr.head == c
         assert pr.state == 'approved'
 
@@ -2216,10 +2207,7 @@ class TestPRUpdate(object):
             repo.post_status(prx.head, 'success', 'ci/runbot')
             prx.post_comment('hansen r+', config['role_reviewer']['token'])
         env.run_crons()
-        pr = env['runbot_merge.pull_requests'].search([
+        pr = to_pr(env, prx)
-            ('repository.name', '=', repo.name),
-            ('number', '=', prx.number),
-        ])
         assert pr.head == c
         assert pr.state == 'ready'
 
@@ -2241,11 +2229,9 @@ class TestPRUpdate(object):
             repo.post_status(prx.head, 'success', 'legal/cla')
             repo.post_status(prx.head, 'success', 'ci/runbot')
             prx.post_comment('hansen r+', config['role_reviewer']['token'])
-        pr = env['runbot_merge.pull_requests'].search([
+
-            ('repository.name', '=', repo.name),
-            ('number', '=', prx.number),
-        ])
         env.run_crons()
+        pr = to_pr(env, prx)
         assert pr.state == 'ready'
         assert pr.staging_id
 
@@ -2314,11 +2300,8 @@ class TestPRUpdate(object):
             repo.post_status(prx.head, 'success', 'legal/cla')
             repo.post_status(prx.head, 'success', 'ci/runbot')
             prx.post_comment('hansen r+', config['role_reviewer']['token'])
-        pr = env['runbot_merge.pull_requests'].search([
-            ('repository.name', '=', repo.name),
-            ('number', '=', prx.number),
-        ])
         env.run_crons()
+        pr = to_pr(env, prx)
         assert pr.state == 'ready'
         assert pr.staging_id
 
@@ -2371,10 +2354,7 @@ class TestPRUpdate(object):
 
         with repo:
             prx = repo.make_pr(title='title', body='body', 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.head == c
         assert pr.state == 'opened'
 
@@ -2407,10 +2387,9 @@ class TestPRUpdate(object):
             repo.post_status(pr.head, 'success', 'legal/cla')
             repo.post_status(pr.head, 'success', 'ci/runbot')
             pr.post_comment('hansen r+', config['role_reviewer']['token'])
-        pr_id = env['runbot_merge.pull_requests'].search([
+
-            ('repository.name', '=', repo.name),
+        env.run_crons()
-            ('number', '=', pr.number),
+        pr_id = to_pr(env, pr)
-        ])
         env.run_crons('runbot_merge.process_updated_commits')
         assert pr_id.message == 'title\n\nbody'
         assert pr_id.state == 'ready'
@@ -2536,10 +2515,7 @@ Please check and re-approve.
             [c] = repo.make_commits(m, repo.Commit('first', tree={'m': 'm3'}), ref='heads/abranch')
             prx = repo.make_pr(title='title', body='body', target='master', head=c)
         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 == 'opened'
         assert pr.head == c
         assert pr.squash
@@ -2575,10 +2551,8 @@ Please check and re-approve.
             repo.post_status(c, 'success', 'ci/runbot')
             prx = repo.make_pr(title='title', body='body', target='master', head=c)
 
-        pr = env['runbot_merge.pull_requests'].search([
+        env.run_crons()
-            ('repository.name', '=', repo.name),
+        pr = to_pr(env, prx)
-            ('number', '=', prx.number),
-        ])
         assert pr.state == 'validated', \
             "if a PR is created on a CI'd commit, it should be validated immediately"
 
@@ -2589,28 +2563,6 @@ Please check and re-approve.
         assert pr.state == 'validated', \
             "if a PR is reopened and had a CI'd head, it should be validated immediately"
 
-    @pytest.mark.xfail(reason="github doesn't allow reopening force-pushed PRs", strict=True)
-    def test_force_update_closed(self, env, repo):
-        with repo:
-            [m] = repo.make_commits(None, repo.Commit('initial', tree={'m': 'm'}), ref='heads/master')
-
-            [c] = repo.make_commits(m, repo.Commit('first', tree={'m': 'm3'}), ref='heads/abranch')
-            prx = repo.make_pr(title='title', body='body', target='master', head=c)
-        env.run_crons()
-        pr = env['runbot_merge.pull_requests'].search([
-            ('repository.name', '=', repo.name),
-            ('number', '=', prx.number)
-        ])
-        with repo:
-            prx.close()
-
-        with repo:
-            c2 = repo.make_commit(m, 'xxx', None, tree={'m': 'm4'})
-            repo.update_ref(prx.ref, c2, force=True)
-
-        with repo:
-            prx.open()
-        assert pr.head == c2
 
 class TestBatching(object):
     def _pr(self, repo, prefix, trees, *, target='master', user, reviewer,