diff --git a/runbot_merge/data/merge_cron.xml b/runbot_merge/data/merge_cron.xml
index af230f24..c3e225a3 100644
--- a/runbot_merge/data/merge_cron.xml
+++ b/runbot_merge/data/merge_cron.xml
@@ -29,4 +29,14 @@
-1
+
+ Warn on linked PRs where only one is ready
+
+ code
+ model._check_linked_prs_statuses(True)
+ 1
+ hours
+ -1
+
+
diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index b0adb3db..89597e07 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -352,7 +352,7 @@ class PullRequests(models.Model):
# staged?
('merged', 'Merged'),
('error', 'Error'),
- ], default='opened')
+ ], default='opened', index=True)
number = fields.Integer(required=True, index=True)
author = fields.Many2one('res.partner')
@@ -379,6 +379,11 @@ class PullRequests(models.Model):
batch_ids = fields.Many2many('runbot_merge.batch')
staging_id = fields.Many2one(related='batch_id.staging_id', store=True)
+ link_warned = fields.Boolean(
+ default=False, help="Whether we've already warned that this (ready)"
+ " PR is linked to an other non-ready PR"
+ )
+
@api.depends('head')
def _compute_statuses(self):
Commits = self.env['runbot_merge.commit']
@@ -635,6 +640,50 @@ class PullRequests(models.Model):
})
return super().unlink()
+ def _check_linked_prs_statuses(self, commit=False):
+ """ Looks for linked PRs where at least one of the PRs is in a ready
+ state and the others are not, notifies the other PRs.
+
+ :param bool commit: whether to commit the tnx after each comment
+ """
+ # similar to Branch.try_staging's query as it's a subset of that
+ # other query's behaviour
+ self.env.cr.execute("""
+ SELECT
+ array_agg(pr.id) AS match
+ FROM runbot_merge_pull_requests pr
+ WHERE
+ -- exclude terminal states (so there's no issue when
+ -- deleting branches & reusing labels)
+ pr.state != 'merged'
+ AND pr.state != 'closed'
+ GROUP BY pr.label
+ HAVING
+ -- one of the batch's PRs should be ready & not marked
+ bool_or(pr.state = 'ready' AND NOT pr.link_warned)
+ -- one of the others should be unready
+ AND bool_or(pr.state != 'ready')
+ -- but ignore batches with one of the prs at p-
+ AND bool_and(pr.priority != 0)
+ """)
+ for [ids] in self.env.cr.fetchall():
+ prs = self.browse(ids)
+ ready = prs.filtered(lambda p: p.state == 'ready')
+ unready = prs - ready
+
+ for r in ready:
+ r.repository.github().comment(
+ r.number, "Linked pull request(s) {} not ready. Linked PRs are not staged until all of them are ready.".format(
+ ', '.join(map(
+ '{0.repository.name}#{0.number}'.format,
+ unready
+ ))
+ )
+ )
+ r.link_warned = True
+ if commit:
+ self.env.cr.commit()
+
# state changes on reviews
RPLUS = {
'opened': 'approved',
diff --git a/runbot_merge/tests/remote.py b/runbot_merge/tests/remote.py
index ba50a557..dff70c0b 100644
--- a/runbot_merge/tests/remote.py
+++ b/runbot_merge/tests/remote.py
@@ -348,6 +348,10 @@ class Model:
assert self._model == 'runbot_merge.project'
self._run_cron('runbot_merge.fetch_prs_cron')
+ def _check_linked_prs_statuses(self):
+ assert self._model == 'runbot_merge.pull_requests'
+ self._run_cron('runbot_merge.check_linked_prs_status')
+
def _run_cron(self, xid):
_, model, cron_id = self._env('ir.model.data', 'xmlid_lookup', xid)
assert model == 'ir.cron', "Expected {} to be a cron, got {}".format(xid, model)
diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py
index 37856e72..91ed8e36 100644
--- a/runbot_merge/tests/test_multirepo.py
+++ b/runbot_merge/tests/test_multirepo.py
@@ -216,39 +216,98 @@ def test_ff_fail(env, project, repo_a, repo_b):
assert len(st) == 1
assert len(st.batch_ids.prs) == 2
-def test_one_failed(env, project, repo_a, repo_b, owner):
- """ If the companion of a ready branch-matched PR is not ready,
- they should not get staged
- """
- project.batch_limit = 1
- c_a = repo_a.make_commit(None, 'initial', None, tree={'a': 'a_0'})
- repo_a.make_ref('heads/master', c_a)
- repo_a.protect('master')
- # pr_a is born ready
- pr_a = make_pr(repo_a, 'A', [{'a': 'a_1'}], label='do-a-thing')
+class TestCompanionsNotReady:
+ def test_one_pair(self, env, project, repo_a, repo_b, owner, users):
+ """ If the companion of a ready branch-matched PR is not ready,
+ they should not get staged
+ """
+ project.batch_limit = 1
+ make_branch(repo_a, 'master', 'initial', {'a': 'a_0'})
+ # pr_a is born ready
+ p_a = make_pr(repo_a, 'A', [{'a': 'a_1'}], label='do-a-thing')
- c_b = repo_b.make_commit(None, 'initial', None, tree={'a': 'b_0'})
- repo_b.make_ref('heads/master', c_b)
- repo_b.protect('master')
- c_pr = repo_b.make_commit(c_b, 'pr', None, tree={'a': 'b_1'})
- pr_b = repo_b.make_pr(
- 'title', 'body', target='master', ctid=c_pr,
- user='user', label='do-a-thing',
- )
- repo_b.post_status(c_pr, 'success', 'ci/runbot')
- repo_b.post_status(c_pr, 'success', 'legal/cla')
+ make_branch(repo_b, 'master', 'initial', {'a': 'b_0'})
+ p_b = make_pr(repo_b, 'B', [{'a': 'b_1'}], label='do-a-thing', reviewer=None)
- pr_a = to_pr(env, pr_a)
- pr_b = to_pr(env, pr_b)
- assert pr_a.state == 'ready'
- assert pr_b.state == 'validated'
- assert pr_a.label == pr_b.label == '{}:do-a-thing'.format(owner)
+ pr_a = to_pr(env, p_a)
+ pr_b = to_pr(env, p_b)
+ assert pr_a.state == 'ready'
+ assert pr_b.state == 'validated'
+ assert pr_a.label == pr_b.label == '{}:do-a-thing'.format(owner)
- env['runbot_merge.project']._check_progress()
+ env['runbot_merge.project']._check_progress()
- assert not pr_b.staging_id
- assert not pr_a.staging_id, \
- "pr_a should not have been staged as companion is not ready"
+ assert not pr_b.staging_id
+ assert not pr_a.staging_id, \
+ "pr_a should not have been staged as companion is not ready"
+
+ env['runbot_merge.pull_requests']._check_linked_prs_statuses()
+ assert p_a.comments == [
+ (users['reviewer'], 'hansen r+'),
+ (users['user'], "Linked pull request(s) %s#%d not ready. Linked PRs are not staged until all of them are ready." % (repo_b.name, p_b.number)),
+ ]
+ # ensure the message is only sent once per PR
+ env['runbot_merge.pull_requests']._check_linked_prs_statuses()
+ assert p_a.comments == [
+ (users['reviewer'], 'hansen r+'),
+ (users['user'], "Linked pull request(s) %s#%d not ready. Linked PRs are not staged until all of them are ready." % (repo_b.name, p_b.number)),
+ ]
+ assert p_b.comments == []
+
+ def test_two_of_three_unready(self, env, project, repo_a, repo_b, repo_c, owner, users):
+ """ In a 3-batch, if two of the PRs are not ready both should be
+ linked by the first one
+ """
+ project.batch_limit = 1
+ make_branch(repo_a, 'master', 'initial', {'f': 'a0'})
+ pr_a = make_pr(repo_a, 'A', [{'f': 'a1'}], label='a-thing', reviewer=None)
+
+ make_branch(repo_b, 'master', 'initial', {'f': 'b0'})
+ pr_b = make_pr(repo_b, 'B', [{'f': 'b1'}], label='a-thing')
+
+ make_branch(repo_c, 'master', 'initial', {'f': 'c0'})
+ pr_c = make_pr(repo_c, 'C', [{'f': 'c1'}], label='a-thing', reviewer=None)
+
+ env['runbot_merge.pull_requests']._check_linked_prs_statuses()
+ assert pr_a.comments == []
+ assert pr_b.comments == [
+ (users['reviewer'], 'hansen r+'),
+ (users['user'], "Linked pull request(s) %s#%d, %s#%d not ready. Linked PRs are not staged until all of them are ready." % (
+ repo_a.name, pr_a.number,
+ repo_c.name, pr_c.number
+ ))
+ ]
+ assert pr_c.comments == []
+
+ def test_one_of_three_unready(self, env, project, repo_a, repo_b, repo_c, owner, users):
+ """ In a 3-batch, if one PR is not ready it should be linked on the
+ other two
+ """
+ project.batch_limit = 1
+ make_branch(repo_a, 'master', 'initial', {'f': 'a0'})
+ pr_a = make_pr(repo_a, 'A', [{'f': 'a1'}], label='a-thing', reviewer=None)
+
+ make_branch(repo_b, 'master', 'initial', {'f': 'b0'})
+ pr_b = make_pr(repo_b, 'B', [{'f': 'b1'}], label='a-thing')
+
+ make_branch(repo_c, 'master', 'initial', {'f': 'c0'})
+ pr_c = make_pr(repo_c, 'C', [{'f': 'c1'}], label='a-thing')
+
+ env['runbot_merge.pull_requests']._check_linked_prs_statuses()
+ assert pr_a.comments == []
+ assert pr_b.comments == [
+ (users['reviewer'], 'hansen r+'),
+ (users['user'], "Linked pull request(s) %s#%d not ready. Linked PRs are not staged until all of them are ready." % (
+ repo_a.name, pr_a.number
+ ))
+ ]
+ assert pr_c.comments == [
+ (users['reviewer'], 'hansen r+'),
+ (users['user'],
+ "Linked pull request(s) %s#%d not ready. Linked PRs are not staged until all of them are ready." % (
+ repo_a.name, pr_a.number
+ ))
+ ]
def test_other_failed(env, project, repo_a, repo_b, owner, users):
""" In a non-matched-branch scenario, if the companion staging (copy of