mirror of
https://github.com/odoo/runbot.git
synced 2025-03-27 13:25:47 +07:00
[IMP] runbot_merge: show linked PRs during staging and after merging
Previously, a PR's status page would only show the linked / related PRs when `open`. Since the relations between PRs remains useful, also make this information available during staging and after merging. Fixes #463
This commit is contained in:
parent
c1ebe9da52
commit
6a8c13b1ef
@ -2,6 +2,8 @@
|
|||||||
import itertools
|
import itertools
|
||||||
import re
|
import re
|
||||||
|
|
||||||
|
from lxml import html
|
||||||
|
|
||||||
MESSAGE_TEMPLATE = """{message}
|
MESSAGE_TEMPLATE = """{message}
|
||||||
|
|
||||||
closes {repo}#{number}
|
closes {repo}#{number}
|
||||||
@ -123,3 +125,12 @@ def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproje
|
|||||||
})
|
})
|
||||||
|
|
||||||
return prod, other
|
return prod, other
|
||||||
|
|
||||||
|
def pr_page(page, pr):
|
||||||
|
return html.fromstring(page(f'/{pr.repo.name}/pull/{pr.number}'))
|
||||||
|
|
||||||
|
def to_pr(env, pr):
|
||||||
|
return env['runbot_merge.pull_requests'].search([
|
||||||
|
('repository.name', '=', pr.repo.name),
|
||||||
|
('number', '=', pr.number),
|
||||||
|
])
|
||||||
|
@ -756,6 +756,10 @@ class PullRequests(models.Model):
|
|||||||
def _linked_prs(self):
|
def _linked_prs(self):
|
||||||
if re.search(r':patch-\d+', self.label):
|
if re.search(r':patch-\d+', self.label):
|
||||||
return self.browse(())
|
return self.browse(())
|
||||||
|
if self.state == 'merged':
|
||||||
|
return self.with_context(active_test=False).batch_ids\
|
||||||
|
.filtered(lambda b: b.staging_id.state == 'success')\
|
||||||
|
.prs - self
|
||||||
return self.search([
|
return self.search([
|
||||||
('target', '=', self.target.id),
|
('target', '=', self.target.id),
|
||||||
('label', '=', self.label),
|
('label', '=', self.label),
|
||||||
|
@ -9,11 +9,7 @@ import pytest
|
|||||||
from lxml import html
|
from lxml import html
|
||||||
|
|
||||||
import odoo
|
import odoo
|
||||||
from utils import _simple_init, seen, re_matches, get_partner, Commit
|
from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr
|
||||||
|
|
||||||
|
|
||||||
def pr_page(page, pr):
|
|
||||||
return html.fromstring(page(f'/{pr.repo.name}/pull/{pr.number}'))
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def repo(env, project, make_repo, users, setreviewers):
|
def repo(env, project, make_repo, users, setreviewers):
|
||||||
@ -37,10 +33,7 @@ def test_trivial_flow(env, repo, page, users, config):
|
|||||||
c1 = repo.make_commit(c0, 'add file', None, tree={'a': 'some other content', 'b': 'a second file'})
|
c1 = repo.make_commit(c0, 'add file', None, tree={'a': 'some other content', 'b': 'a second file'})
|
||||||
pr = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c1,)
|
pr = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c1,)
|
||||||
|
|
||||||
[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'
|
||||||
env.run_crons()
|
env.run_crons()
|
||||||
assert pr.comments == [seen(env, pr, users)]
|
assert pr.comments == [seen(env, pr, users)]
|
||||||
@ -92,7 +85,7 @@ def test_trivial_flow(env, repo, page, users, config):
|
|||||||
st = pr_id.staging_id
|
st = pr_id.staging_id
|
||||||
env.run_crons()
|
env.run_crons()
|
||||||
|
|
||||||
assert set(tuple(t) for t in st.statuses) == {
|
assert {tuple(t) for t in st.statuses} == {
|
||||||
(repo.name, 'legal/cla', 'success', ''),
|
(repo.name, 'legal/cla', 'success', ''),
|
||||||
(repo.name, 'ci/runbot', 'success', 'http://foo.com/pog'),
|
(repo.name, 'ci/runbot', 'success', 'http://foo.com/pog'),
|
||||||
(repo.name, 'ci/lint', 'failure', 'http://ignored.com/whocares'),
|
(repo.name, 'ci/lint', 'failure', 'http://ignored.com/whocares'),
|
||||||
|
@ -10,8 +10,9 @@ import time
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
import requests
|
import requests
|
||||||
|
from lxml.etree import XPath, tostring
|
||||||
|
|
||||||
from utils import seen, re_matches, get_partner
|
from utils import seen, re_matches, get_partner, pr_page, to_pr
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@ -20,7 +21,8 @@ def repo_a(project, make_repo, setreviewers):
|
|||||||
r = project.env['runbot_merge.repository'].create({
|
r = project.env['runbot_merge.repository'].create({
|
||||||
'project_id': project.id,
|
'project_id': project.id,
|
||||||
'name': repo.name,
|
'name': repo.name,
|
||||||
'required_statuses': 'legal/cla,ci/runbot'
|
'required_statuses': 'legal/cla,ci/runbot',
|
||||||
|
'group_id': False,
|
||||||
})
|
})
|
||||||
setreviewers(r)
|
setreviewers(r)
|
||||||
return repo
|
return repo
|
||||||
@ -31,7 +33,8 @@ def repo_b(project, make_repo, setreviewers):
|
|||||||
r = project.env['runbot_merge.repository'].create({
|
r = project.env['runbot_merge.repository'].create({
|
||||||
'project_id': project.id,
|
'project_id': project.id,
|
||||||
'name': repo.name,
|
'name': repo.name,
|
||||||
'required_statuses': 'legal/cla,ci/runbot'
|
'required_statuses': 'legal/cla,ci/runbot',
|
||||||
|
'group_id': False,
|
||||||
})
|
})
|
||||||
setreviewers(r)
|
setreviewers(r)
|
||||||
return repo
|
return repo
|
||||||
@ -42,7 +45,8 @@ def repo_c(project, make_repo, setreviewers):
|
|||||||
r = project.env['runbot_merge.repository'].create({
|
r = project.env['runbot_merge.repository'].create({
|
||||||
'project_id': project.id,
|
'project_id': project.id,
|
||||||
'name': repo.name,
|
'name': repo.name,
|
||||||
'required_statuses': 'legal/cla,ci/runbot'
|
'required_statuses': 'legal/cla,ci/runbot',
|
||||||
|
'group_id': False,
|
||||||
})
|
})
|
||||||
setreviewers(r)
|
setreviewers(r)
|
||||||
return repo
|
return repo
|
||||||
@ -56,7 +60,6 @@ def make_pr(repo, prefix, trees, *, target='master', user,
|
|||||||
:type trees: list[dict]
|
:type trees: list[dict]
|
||||||
:type target: str
|
:type target: str
|
||||||
:type user: str
|
:type user: str
|
||||||
:type label: str | None
|
|
||||||
:type statuses: list[(str, str)]
|
:type statuses: list[(str, str)]
|
||||||
:type reviewer: str | None
|
:type reviewer: str | None
|
||||||
:rtype: fake_github.PR
|
:rtype: fake_github.PR
|
||||||
@ -76,11 +79,7 @@ def make_pr(repo, prefix, trees, *, target='master', user,
|
|||||||
if reviewer:
|
if reviewer:
|
||||||
pr.post_comment('hansen r+', reviewer)
|
pr.post_comment('hansen r+', reviewer)
|
||||||
return pr
|
return pr
|
||||||
def to_pr(env, pr):
|
|
||||||
return env['runbot_merge.pull_requests'].search([
|
|
||||||
('repository.name', '=', pr.repo.name),
|
|
||||||
('number', '=', pr.number),
|
|
||||||
])
|
|
||||||
def make_branch(repo, name, message, tree, protect=True):
|
def make_branch(repo, name, message, tree, protect=True):
|
||||||
c = repo.make_commit(None, message, None, tree=tree)
|
c = repo.make_commit(None, message, None, tree=tree)
|
||||||
repo.make_ref('heads/%s' % name, c)
|
repo.make_ref('heads/%s' % name, c)
|
||||||
@ -114,28 +113,34 @@ def test_stage_one(env, project, repo_a, repo_b, config):
|
|||||||
assert to_pr(env, pr_b).state == 'ready'
|
assert to_pr(env, pr_b).state == 'ready'
|
||||||
assert not to_pr(env, pr_b).staging_id
|
assert not to_pr(env, pr_b).staging_id
|
||||||
|
|
||||||
def test_stage_match(env, project, repo_a, repo_b, config):
|
get_related_pr_labels = XPath('.//*[normalize-space(text()) = "Linked pull requests"]//a/text()')
|
||||||
|
def test_stage_match(env, project, repo_a, repo_b, config, page):
|
||||||
""" First PR is matched from A, => should select matched PR from B
|
""" First PR is matched from A, => should select matched PR from B
|
||||||
"""
|
"""
|
||||||
project.batch_limit = 1
|
project.batch_limit = 1
|
||||||
|
|
||||||
with repo_a:
|
with repo_a:
|
||||||
make_branch(repo_a, 'master', 'initial', {'a': 'a_0'})
|
make_branch(repo_a, 'master', 'initial', {'a': 'a_0'})
|
||||||
pr_a = make_pr(
|
prx_a = make_pr(
|
||||||
repo_a, 'do-a-thing', [{'a': 'a_1'}],
|
repo_a, 'do-a-thing', [{'a': 'a_1'}],
|
||||||
user=config['role_user']['token'],
|
user=config['role_user']['token'],
|
||||||
reviewer=config['role_reviewer']['token'],
|
reviewer=config['role_reviewer']['token'],
|
||||||
)
|
)
|
||||||
with repo_b:
|
with repo_b:
|
||||||
make_branch(repo_b, 'master', 'initial', {'a': 'b_0'})
|
make_branch(repo_b, 'master', 'initial', {'a': 'b_0'})
|
||||||
pr_b = make_pr(repo_b, 'do-a-thing', [{'a': 'b_1'}],
|
prx_b = make_pr(repo_b, 'do-a-thing', [{'a': 'b_1'}],
|
||||||
user=config['role_user']['token'],
|
user=config['role_user']['token'],
|
||||||
reviewer=config['role_reviewer']['token'],
|
reviewer=config['role_reviewer']['token'],
|
||||||
)
|
)
|
||||||
|
pr_a = to_pr(env, prx_a)
|
||||||
|
pr_b = to_pr(env, prx_b)
|
||||||
|
|
||||||
|
# check that related PRs link to one another
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_a)) == pr_b.mapped('display_name')
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_b)) == pr_a.mapped('display_name')
|
||||||
|
|
||||||
env.run_crons()
|
env.run_crons()
|
||||||
|
|
||||||
pr_a = to_pr(env, pr_a)
|
|
||||||
pr_b = to_pr(env, pr_b)
|
|
||||||
assert pr_a.state == 'ready'
|
assert pr_a.state == 'ready'
|
||||||
assert pr_a.staging_id
|
assert pr_a.staging_id
|
||||||
assert pr_b.state == 'ready'
|
assert pr_b.state == 'ready'
|
||||||
@ -144,6 +149,22 @@ def test_stage_match(env, project, repo_a, repo_b, config):
|
|||||||
assert pr_a.staging_id == pr_b.staging_id, \
|
assert pr_a.staging_id == pr_b.staging_id, \
|
||||||
"branch-matched PRs should be part of the same staging"
|
"branch-matched PRs should be part of the same staging"
|
||||||
|
|
||||||
|
# check that related PRs *still* link to one another during staging
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_a)) == [pr_b.display_name]
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_b)) == [pr_a.display_name]
|
||||||
|
with repo_a:
|
||||||
|
repo_a.post_status('staging.master', 'failure', 'legal/cla')
|
||||||
|
env.run_crons()
|
||||||
|
|
||||||
|
assert pr_a.state == 'error'
|
||||||
|
assert pr_b.state == 'ready'
|
||||||
|
|
||||||
|
with repo_a:
|
||||||
|
prx_a.post_comment('hansen retry', config['role_reviewer']['token'])
|
||||||
|
env.run_crons()
|
||||||
|
|
||||||
|
assert pr_a.state == pr_b.state == 'ready'
|
||||||
|
assert pr_a.staging_id and pr_b.staging_id
|
||||||
for repo in [repo_a, repo_b]:
|
for repo in [repo_a, repo_b]:
|
||||||
with repo:
|
with repo:
|
||||||
repo.post_status('staging.master', 'success', 'legal/cla')
|
repo.post_status('staging.master', 'success', 'legal/cla')
|
||||||
@ -155,6 +176,11 @@ def test_stage_match(env, project, repo_a, repo_b, config):
|
|||||||
assert 'Related: {}'.format(pr_b.display_name) in repo_a.commit('master').message
|
assert 'Related: {}'.format(pr_b.display_name) in repo_a.commit('master').message
|
||||||
assert 'Related: {}'.format(pr_a.display_name) in repo_b.commit('master').message
|
assert 'Related: {}'.format(pr_a.display_name) in repo_b.commit('master').message
|
||||||
|
|
||||||
|
print(pr_a.batch_ids.read(['staging_id', 'prs']))
|
||||||
|
# check that related PRs *still* link to one another after merge
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_a)) == [pr_b.display_name]
|
||||||
|
assert get_related_pr_labels(pr_page(page, prx_b)) == [pr_a.display_name]
|
||||||
|
|
||||||
def test_different_targets(env, project, repo_a, repo_b, config):
|
def test_different_targets(env, project, repo_a, repo_b, config):
|
||||||
""" PRs with different targets should not be matched together
|
""" PRs with different targets should not be matched together
|
||||||
"""
|
"""
|
||||||
|
@ -307,6 +307,16 @@
|
|||||||
<t t-if="merged_head">
|
<t t-if="merged_head">
|
||||||
at <a t-attf-href="https://github.com/{{pr.repository.name}}/commit/{{merged_head}}"><t t-esc="merged_head"/></a>
|
at <a t-attf-href="https://github.com/{{pr.repository.name}}/commit/{{merged_head}}"><t t-esc="merged_head"/></a>
|
||||||
</t>
|
</t>
|
||||||
|
|
||||||
|
<t t-set="linked_prs" t-value="pr._linked_prs"/>
|
||||||
|
<div t-if="linked_prs">
|
||||||
|
Linked pull requests
|
||||||
|
<ul>
|
||||||
|
<li t-foreach="linked_prs" t-as="linked">
|
||||||
|
<a t-att-href="linked.url" t-field="linked.display_name"/>
|
||||||
|
</li>
|
||||||
|
</ul>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</template>
|
</template>
|
||||||
<template id="view_pull_request_info_closed">
|
<template id="view_pull_request_info_closed">
|
||||||
@ -323,6 +333,16 @@
|
|||||||
<template id="view_pull_request_info_staging">
|
<template id="view_pull_request_info_staging">
|
||||||
<div class="alert alert-primary">
|
<div class="alert alert-primary">
|
||||||
Staged <span t-field="pr.staging_id.staged_at" t-options="{'widget': 'relative'}"/>.
|
Staged <span t-field="pr.staging_id.staged_at" t-options="{'widget': 'relative'}"/>.
|
||||||
|
|
||||||
|
<t t-set="linked_prs" t-value="pr._linked_prs"/>
|
||||||
|
<div t-if="linked_prs">
|
||||||
|
Linked pull requests
|
||||||
|
<ul>
|
||||||
|
<li t-foreach="linked_prs" t-as="linked">
|
||||||
|
<a t-att-href="linked.url" t-field="linked.display_name"/>
|
||||||
|
</li>
|
||||||
|
</ul>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</template>
|
</template>
|
||||||
<template id="view_pull_request_info_open">
|
<template id="view_pull_request_info_open">
|
||||||
|
Loading…
Reference in New Issue
Block a user