mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 23:45:44 +07:00
[ADD] runbot_merge: warning that a ready PR is linked to a non-ready one
People get surprised/worried that their ready PR never gets picked up, but it's because there is a non-ready (either unreviewed or failing CI) pull request linked to it. This aims to at least warn them of the issue.
This commit is contained in:
Xavier Morel
parent
63b7484873
commit
0b629a32bc
@ -29,4 +29,14 @@
|
||||
<field name="numbercall">-1</field>
|
||||
<field name="doall" eval="False"/>
|
||||
</record>
|
||||
<record model="ir.cron" id="check_linked_prs_status">
|
||||
<field name="name">Warn on linked PRs where only one is ready</field>
|
||||
<field name="model_id" ref="model_runbot_merge_pull_requests"/>
|
||||
<field name="state">code</field>
|
||||
<field name="code">model._check_linked_prs_statuses(True)</field>
|
||||
<field name="interval_number">1</field>
|
||||
<field name="interval_type">hours</field>
|
||||
<field name="numbercall">-1</field>
|
||||
<field name="doall" eval="False"/>
|
||||
</record>
|
||||
</odoo>
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user