NextERP/runbot
5
0
Fork 0
mirror of https://github.com/odoo/runbot.git synced 2025-03-17 00:15:47 +07:00
Code Issues Actions Packages Projects Releases Wiki Activity
16.0-fix-button-xdo
runbot/forwardport/tests/test_simple.py

1031 lines
38 KiB
Python
Raw Permalink Normal View History

[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# -*- coding: utf-8 -*-
import collections
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
import re
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
import time
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
from datetime import datetime, timedelta
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
import pytest
[IMP] *: tag all rebased commits with source PRev Although it's possible to find what PR a commit was part of with a bit of `git log` magic (e.g. `--ancestry-path COMMIT.. --reverse`) it's not the most convenient, and many people don't know about it, leading them to various debatable decisions to try and mitigate the issue, such as tagging every commit in a PR with the PR's identity, which then leads github to spam the PR itself with pingbacks from its own commits. Which is great. Add this information to the commits when rebasing them (and *only* when rebasing them), using a `Part-of:` pseudo-header. Fixes #482
2021-08-09 12:55:38 +07:00
from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[IMP] forwardport: reliability of PR reminders The exponential backoff offsets from the write_date of the children PRs, however it doesn't reset, so the offsetting gets bumped up way more than originally expected or designed if the child PRs are under active development for some reason. Fix this by adding a field to specifically record the date of merge of a PR, and check that feature against the backoff offset. This should provide more regular and reliable backoff. Fixes #369
2020-05-20 17:42:45 +07:00
FMT = '%Y-%m-%d %H:%M:%S'
FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT)
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# need:
# * an odoo server
# - connected to a database
# - with relevant modules loaded / installed
# - set up project
# - add repo, branch(es)
# - provide visibility to contents si it can be queried & al
# * a tunnel so the server is visible from the outside (webhooks)
# * the ability to create repos on github
# - repo name
# - a github user to create a repo with
# - a github owner to create a repo *for*
# - provide ability to create commits, branches, prs, ...
def test_straightforward_flow(env, config, make_repo, users):
# TODO: ~all relevant data in users when creating partners
# get reviewer's name
reviewer_name = env['res.partner'].search([
('github_login', '=', users['reviewer'])
]).name
prod, other = make_basic(env, config, make_repo)
other_user = config['role_other']
other_user_repo = prod.fork(token=other_user['token'])
project = env['runbot_merge.project'].search([])
b_head = prod.commit('b')
c_head = prod.commit('c')
with prod, other_user_repo:
# create PR as a user with no access to prod (or other)
[_, p_1] = other_user_repo.make_commits(
'a',
Commit('p_0', tree={'x': '0'}),
Commit('p_1', tree={'x': '1'}),
ref='heads/hugechange'
)
pr = prod.make_pr(
target='a', title="super important change",
head=other_user['user'] + ':hugechange',
token=other_user['token']
)
prod.post_status(p_1, 'success', 'legal/cla')
prod.post_status(p_1, 'success', 'ci/runbot')
# 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'])
[IMP] forwardport: reliability of PR reminders The exponential backoff offsets from the write_date of the children PRs, however it doesn't reset, so the offsetting gets bumped up way more than originally expected or designed if the child PRs are under active development for some reason. Fix this by adding a field to specifically record the date of merge of a PR, and check that feature against the backoff offset. This should provide more regular and reliable backoff. Fixes #369
2020-05-20 17:42:45 +07:00
pr_id = env['runbot_merge.pull_requests'].search([
('repository.name', '=', prod.name),
('number', '=', pr.number),
])
assert not pr_id.merge_date,\
"PR obviously shouldn't have a merge date before being merged"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
# should merge the staging then create the FP PR
env.run_crons()
[FIX] forwardport: fix pinging on forwardport PRs - avoid pinging the author of the fw PR (which is the forward-bot itself) - instead ping the author and reviewer of the source, and possibly the reviewer of the PR if any - might also be a good idea to ping reviewers of intermediate PRs?
2022-11-03 17:46:31 +07:00
assert datetime.utcnow() - datetime.strptime(pr_id.merge_date, FMT) <= timedelta(minutes=1),\
[IMP] forwardport: reliability of PR reminders The exponential backoff offsets from the write_date of the children PRs, however it doesn't reset, so the offsetting gets bumped up way more than originally expected or designed if the child PRs are under active development for some reason. Fix this by adding a field to specifically record the date of merge of a PR, and check that feature against the backoff offset. This should provide more regular and reliable backoff. Fixes #369
2020-05-20 17:42:45 +07:00
"check if merge date was set about now (within a minute as crons and " \
"RPC calls yield various delays before we're back)"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
p_1_merged = prod.commit('a')
assert p_1_merged.id != p_1
assert p_1_merged.message == MESSAGE_TEMPLATE.format(
message='p_1',
repo=prod.name,
number=pr.number,
headers='',
name=reviewer_name,
[IMP] runbot_merge: reject review without email If a reviewer doesn't have an email set, the Signed-Off-By is an `@users.noreply.github.com` address which just looks weird in the final result. Initially the thinking was that emails would be required for users to *be* reviewers or self-reviewers, but since those are now o2ms / m2ms it's a bit of a pain in the ass. Instead, provide an action to easily try and fetch the public email of a user from github. Fixes #531
2021-10-06 18:06:53 +07:00
email=config['role_reviewer']['email'],
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
)
assert prod.read_tree(p_1_merged) == {
'f': 'e',
'x': '1',
}, "ensure p_1_merged has ~ the same contents as p_1 but is a different commit"
[p_0_merged] = p_1_merged.parents
# wait a bit for PR webhook... ?
time.sleep(5)
env.run_crons()
pr0, pr1 = env['runbot_merge.pull_requests'].search([], order='number')
assert pr0.number == pr.number
# 50 lines in, we can start checking the forward port...
assert pr1.parent_id == pr0
assert pr1.source_id == pr0
other_owner = other.name.split('/')[0]
assert re.match(other_owner + ':' + REF_PATTERN.format(target='b', source='hugechange'), pr1.label), \
"check that FP PR was created in FP target repo"
c = prod.commit(pr1.head)
# TODO: add original committer (if !author) as co-author in commit message?
assert c.author['name'] == other_user['user'], "author should still be original's probably"
[IMP] forwardport: better handle authorship information Before this: * the forwardport bot always sets itself as the committer when it creates a forward-port branch * when creating squashed / conflict commits, it becomes both author and committer This is not great as it loses a fair amount of authorship information and doesn't properly give credit where credit is due. Improve this in the following ways: * use the original authorship information as-is when forward-porting commits * the the forward port fails, use the original authorship information as-is if there's a single commit to forward port * otherwise if there's only a single author / committer across the branch being forwardported use that, if there are multiple give up and use the identity of the 'bot, since the branch will probably need to be re-done in full the authorship information of the placeholder commit should not matter overly much Uses git's magic ENV variables as that's pretty much the only way to properly override the COMMITTER date: conf items only provide for author and committer *identity* (name and email), and while `commit` allows overriding the *authorship* date (`--date`) it doesn't provide any option for the *commit* date. Fixes #255
2019-11-20 13:43:56 +07:00
assert c.committer['name'] == other_user['user'], "committer should also still be the original's, really"
[FIX] forwardport: fix pinging on forwardport PRs - avoid pinging the author of the fw PR (which is the forward-bot itself) - instead ping the author and reviewer of the source, and possibly the reviewer of the PR if any - might also be a good idea to ping reviewers of intermediate PRs?
2022-11-03 17:46:31 +07:00
assert pr1.ping() == "@%s @%s " % (
config['role_other']['user'],
config['role_reviewer']['user'],
), "ping of forward-port PR should include author and reviewer of source"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
assert prod.read_tree(c) == {
'f': 'c',
'g': 'b',
'x': '1'
}
with prod:
prod.post_status(pr1.head, 'success', 'ci/runbot')
prod.post_status(pr1.head, 'success', 'legal/cla')
env.run_crons()
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
pr0_, pr1_, pr2 = env['runbot_merge.pull_requests'].search([], order='number')
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
assert pr.comments == [
(users['reviewer'], 'hansen r+ rebase-ff'),
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
seen(env, pr, users),
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
(users['user'], 'Merge method set to rebase and fast-forward.'),
(users['user'], '@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n%s' % (
users['other'], users['reviewer'],
'\n- '.join((pr1 | pr2).mapped('display_name'))
)),
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
assert pr0_ == pr0
assert pr1_ == pr1
[FIX] forwardbot: selection of ancestor in FP tail ping In selecting the parent commits to list on the last PR, we would miss the *first* forward-port of the sequence. Not sure why we added a detrimental check on source_id there. Also add a missing space between "chain" and "containing" in the case where there's at least one forward-port PR other than the final one. Fixes #212
2019-09-27 19:14:07 +07:00
assert pr1.parent_id == pr1.source_id == pr0
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
assert pr2.parent_id == pr1
assert pr2.source_id == pr0
assert not pr0.squash, "original PR has >1 commit"
assert not (pr1.squash or pr2.squash), "forward ports should also have >1 commit"
assert re.match(REF_PATTERN.format(target='c', source='hugechange'), pr2.refname), \
"check that FP PR was created in FP target repo"
assert prod.read_tree(prod.commit(pr2.head)) == {
'f': 'c',
'g': 'a',
'h': 'a',
'x': '1'
}
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr2_remote = prod.get_pr(pr2.number)
assert pr2_remote.comments == [
seen(env, pr2_remote, users),
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
(users['user'], """\
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
@%s @%s this PR targets c and is the last of the forward-port chain containing:
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
* %s
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
To merge the full chain, say
> @%s r+
[FIX] forwardport: fix forward-port Odoo wiki link
2020-03-09 20:20:59 +07:00
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
[FIX] forwardbot: selection of ancestor in FP tail ping In selecting the parent commits to list on the last PR, we would miss the *first* forward-port of the sequence. Not sure why we added a detrimental check on source_id there. Also add a missing space between "chain" and "containing" in the case where there's at least one forward-port PR other than the final one. Fixes #212
2019-09-27 19:14:07 +07:00
""" % (
users['other'], users['reviewer'],
[IMP] forwardbot: show FP PRs in reminder message When posting a reminder that there are open / waiting forward ports on a source PR, also post *which* PRs those are. While at it, move the cron code in a proper python file (so we can use stuff from odoo.tools), and fix display_name so we can straight use display_name as a github ref' ({owner}/{repo}#{number}). This impacts log-grepping but it seems like an improvement nonetheless. Closes odoo/runbot#228
2019-10-10 17:07:57 +07:00
pr1.display_name,
[FIX] forwardbot: selection of ancestor in FP tail ping In selecting the parent commits to list on the last PR, we would miss the *first* forward-port of the sequence. Not sure why we added a detrimental check on source_id there. Also add a missing space between "chain" and "containing" in the case where there's at least one forward-port PR other than the final one. Fixes #212
2019-09-27 19:14:07 +07:00
project.fp_github_name
)),
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
]
with prod:
prod.post_status(pr2.head, 'success', 'ci/runbot')
prod.post_status(pr2.head, 'success', 'legal/cla')
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr2_remote.post_comment('%s r+' % project.fp_github_name, config['role_reviewer']['token'])
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
env.run_crons()
assert pr1.staging_id
assert pr2.staging_id
# two branches so should have two stagings
assert pr1.staging_id != pr2.staging_id
# validate
with prod:
prod.post_status('staging.b', 'success', 'ci/runbot')
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.c', 'success', 'ci/runbot')
prod.post_status('staging.c', 'success', 'legal/cla')
# and trigger merge
env.run_crons()
# apparently github strips out trailing newlines when fetching through the
# API...
message_template = MESSAGE_TEMPLATE.format(
message='p_1',
repo=prod.name,
number='%s',
headers='X-original-commit: {}\n'.format(p_1_merged.id),
name=reviewer_name,
[IMP] runbot_merge: reject review without email If a reviewer doesn't have an email set, the Signed-Off-By is an `@users.noreply.github.com` address which just looks weird in the final result. Initially the thinking was that emails would be required for users to *be* reviewers or self-reviewers, but since those are now o2ms / m2ms it's a bit of a pain in the ass. Instead, provide an action to easily try and fetch the public email of a user from github. Fixes #531
2021-10-06 18:06:53 +07:00
email=config['role_reviewer']['email'],
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
)
old_b = prod.read_tree(b_head)
head_b = prod.commit('b')
assert head_b.message == message_template % pr1.number
[IMP] *: tag all rebased commits with source PRev Although it's possible to find what PR a commit was part of with a bit of `git log` magic (e.g. `--ancestry-path COMMIT.. --reverse`) it's not the most convenient, and many people don't know about it, leading them to various debatable decisions to try and mitigate the issue, such as tagging every commit in a PR with the PR's identity, which then leads github to spam the PR itself with pingbacks from its own commits. Which is great. Add this information to the commits when rebasing them (and *only* when rebasing them), using a `Part-of:` pseudo-header. Fixes #482
2021-08-09 12:55:38 +07:00
assert prod.commit(head_b.parents[0]).message == part_of(f'p_0\n\nX-original-commit: {p_0_merged}', pr1, separator='\n')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
b_tree = prod.read_tree(head_b)
assert b_tree == {
**old_b,
'x': '1',
}
old_c = prod.read_tree(c_head)
head_c = prod.commit('c')
assert head_c.message == message_template % pr2.number
[IMP] *: tag all rebased commits with source PRev Although it's possible to find what PR a commit was part of with a bit of `git log` magic (e.g. `--ancestry-path COMMIT.. --reverse`) it's not the most convenient, and many people don't know about it, leading them to various debatable decisions to try and mitigate the issue, such as tagging every commit in a PR with the PR's identity, which then leads github to spam the PR itself with pingbacks from its own commits. Which is great. Add this information to the commits when rebasing them (and *only* when rebasing them), using a `Part-of:` pseudo-header. Fixes #482
2021-08-09 12:55:38 +07:00
assert prod.commit(head_c.parents[0]).message == part_of(f'p_0\n\nX-original-commit: {p_0_merged}', pr2, separator='\n')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
c_tree = prod.read_tree(head_c)
assert c_tree == {
**old_c,
'x': '1',
}
# check that we didn't just smash the original trees
assert prod.read_tree(prod.commit('a')) != b_tree != c_tree
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
prs = env['forwardport.branch_remover'].search([]).mapped('pr_id')
assert prs == pr0 | pr1 | pr2, "pr1 and pr2 should be slated for branch deletion"
env.run_crons('forwardport.remover', context={'forwardport_merged_before': FAKE_PREV_WEEK})
# should not have deleted the base branch (wrong repo)
assert other_user_repo.get_ref(pr.ref) == p_1
# should have deleted all PR branches
pr1_ref = prod.get_pr(pr1.number).ref
with pytest.raises(AssertionError, match='Not Found'):
other.get_ref(pr1_ref)
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr2_ref = pr2_remote.ref
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
with pytest.raises(AssertionError, match="Not Found"):
other.get_ref(pr2_ref)
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
def test_empty(env, config, make_repo, users):
""" Cherrypick of an already cherrypicked (or separately implemented)
[IMP] *: add draft support to mergebot, kinda * Remove the forwardport creating PRs in draft, that was mostly to avoid codeowners triggering but we've removed the github one and hand-rolled it, so not a concern anymore. * Prevent merging `draft` PRs, the mergebot rejects approval on draft PRs and insults people. TBD (maybe): try to create *conflicting* forward-port PRs in draft so it's clearer they need to be *fixed*? Issue of not being able to do that on all private repositories remains so~~ Fixes #500
2021-08-11 16:36:35 +07:00
commit -> conflicting pr.
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
"""
prod, other = make_basic(env, config, make_repo)
# merge change to b
with prod:
[p_0] = prod.make_commits(
'b', Commit('p', tree={'x': '0'}),
ref='heads/early'
)
pr0 = prod.make_pr(target='b', head='early')
prod.post_status(p_0, 'success', 'legal/cla')
prod.post_status(p_0, 'success', 'ci/runbot')
pr0.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
# merge same change to a afterwards
with prod:
[p_1] = prod.make_commits(
'a', Commit('p_0', tree={'x': '0'}),
ref='heads/late'
)
pr1 = prod.make_pr(target='a', head='late')
prod.post_status(p_1, 'success', 'legal/cla')
prod.post_status(p_1, 'success', 'ci/runbot')
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
assert prod.read_tree(prod.commit('a')) == {
'f': 'e',
'x': '0',
}
assert prod.read_tree(prod.commit('b')) == {
'f': 'c',
'g': 'b',
'x': '0',
}
# should have 4 PRs:
# PR 0
# FP of PR 0 to C
# PR 1
# failed FP of PR1 to B
prs = env['runbot_merge.pull_requests'].search([], order='number')
assert len(prs) == 4
pr0_id = prs.filtered(lambda p: p.number == pr0.number)
pr1_id = prs.filtered(lambda p: p.number == pr1.number)
fp_id = prs.filtered(lambda p: p.parent_id == pr0_id)
fail_id = prs - (pr0_id | pr1_id | fp_id)
assert fp_id
assert fail_id
# unlinked from parent since cherrypick failed
assert not fail_id.parent_id
# the tree should be clean...
assert prod.read_tree(prod.commit(fail_id.head)) == {
'f': 'c',
'g': 'b',
'x': '0',
}
with prod:
validate_all([prod], [fp_id.head, fail_id.head])
env.run_crons()
# should not have created any new PR
assert env['runbot_merge.pull_requests'].search([], order='number') == prs
[IMP] runbot_merge, forwardport: variable-user feedback Having all the feedback be sent by the mergebot user (github_token) is confusing. Add a way to specify which field of project should be used to source the token used when sending feedback. Fixes #190
2019-09-18 20:37:14 +07:00
# change FP token to see if the feedback comes from the proper user
project = env['runbot_merge.project'].search([])
project.fp_github_token = config['role_other']['token']
assert project.fp_github_name == users['other']
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# check reminder
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
awaiting = (
users['other'],
'@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n%s' % (
users['user'], users['reviewer'],
fail_id.display_name
)
)
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
assert pr1.comments == [
(users['reviewer'], 'hansen r+'),
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
seen(env, pr1, users),
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
awaiting,
awaiting,
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
], "each cron run should trigger a new message on the ancestor"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# check that this stops if we close the PR
with prod:
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
prod.get_pr(fail_id.number).close()
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
assert pr1.comments == [
(users['reviewer'], 'hansen r+'),
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
seen(env, pr1, users),
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
awaiting,
awaiting,
[IMP] forwardport: less spammy reminder cron * don't warn on every PR on the dot every week, instead wait for the PRs to be "sufficiently old" (at least 3 days) * after discussion with bugfix, the reminder ping should be sent every day following the PR being "old enough" * run the cron every day instead of every week * add an override context key for test purposes Closes #198
2019-09-16 20:28:03 +07:00
]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
def test_partially_empty(env, config, make_repo):
""" Check what happens when only some commits of the PR are now empty
"""
prod, other = make_basic(env, config, make_repo)
# merge change to b
with prod:
[p_0] = prod.make_commits(
'b', Commit('p', tree={'x': '0'}),
ref='heads/early'
)
pr0 = prod.make_pr(target='b', head='early')
prod.post_status(p_0, 'success', 'legal/cla')
prod.post_status(p_0, 'success', 'ci/runbot')
pr0.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
# merge same change to a afterwards
with prod:
[*_, p_1] = prod.make_commits(
'a',
Commit('p_0', tree={'w': '0'}),
Commit('p_1', tree={'x': '0'}),
Commit('p_2', tree={'y': '0'}),
ref='heads/late'
)
pr1 = prod.make_pr(target='a', head='late')
prod.post_status(p_1, 'success', 'legal/cla')
prod.post_status(p_1, 'success', 'ci/runbot')
pr1.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
assert prod.read_tree(prod.commit('a')) == {
'f': 'e',
'w': '0',
'x': '0',
'y': '0',
}
assert prod.read_tree(prod.commit('b')) == {
'f': 'c',
'g': 'b',
'x': '0',
}
fail_id = env['runbot_merge.pull_requests'].search([
('number', 'not in', [pr0.number, pr1.number]),
('parent_id', '=', False),
], order='number')
assert fail_id
# unlinked from parent since cherrypick failed
assert not fail_id.parent_id
# the tree should be clean...
assert prod.read_tree(prod.commit(fail_id.head)) == {
'f': 'c',
'g': 'b',
'w': '0',
'x': '0',
'y': '0',
}
Case = collections.namedtuple('Case', 'author reviewer delegate success')
ACL = [
[CHG] forwardport: automatically delegate original PR author on FP PRs This way the original author can r+ the forward ports if they succeed (and probably requires no attention). Closes #195
2019-09-17 19:43:21 +07:00
Case('reviewer', 'reviewer', None, True),
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
Case('reviewer', 'self_reviewer', None, False),
Case('reviewer', 'other', None, False),
Case('reviewer', 'other', 'other', True),
Case('self_reviewer', 'reviewer', None, True),
Case('self_reviewer', 'self_reviewer', None, True),
Case('self_reviewer', 'other', None, False),
Case('self_reviewer', 'other', 'other', True),
Case('other', 'reviewer', None, True),
Case('other', 'self_reviewer', None, False),
[CHG] forwardport: automatically delegate original PR author on FP PRs This way the original author can r+ the forward ports if they succeed (and probably requires no attention). Closes #195
2019-09-17 19:43:21 +07:00
Case('other', 'other', None, True),
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
Case('other', 'other', 'other', True),
]
@pytest.mark.parametrize(Case._fields, ACL)
def test_access_rights(env, config, make_repo, users, author, reviewer, delegate, success):
[IMP] runbot_merge: reject review without email If a reviewer doesn't have an email set, the Signed-Off-By is an `@users.noreply.github.com` address which just looks weird in the final result. Initially the thinking was that emails would be required for users to *be* reviewers or self-reviewers, but since those are now o2ms / m2ms it's a bit of a pain in the ass. Instead, provide an action to easily try and fetch the public email of a user from github. Fixes #531
2021-10-06 18:06:53 +07:00
"""Validates the review rights *for the forward-port sequence*, the original
PR is always reviewed by `user`.
"""
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
prod, other = make_basic(env, config, make_repo)
project = env['runbot_merge.project'].search([])
# create a partner for `user`
[IMP] runbot_merge: make review rights repo-dependent As the odds of having more projects or more repos with different requirements in the same project, the need to have different sets of reviewers for different repositories increases. As a result, rather than be trivial boolean flags the review info should probably depend on the user / partner and the repo. Turns out the permission checks had already been extracted into their own function so most of the mess comes from testing utilities which went and configured their review rights as needed. Incidentally it might be that the test suite could just use something like a sequence of commoditized accounts which get configured as needed and not even looked at unless they're used.
2020-02-10 21:05:08 +07:00
c = env['res.partner'].create({
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
'name': users['user'],
'github_login': users['user'],
[IMP] runbot_merge: reject review without email If a reviewer doesn't have an email set, the Signed-Off-By is an `@users.noreply.github.com` address which just looks weird in the final result. Initially the thinking was that emails would be required for users to *be* reviewers or self-reviewers, but since those are now o2ms / m2ms it's a bit of a pain in the ass. Instead, provide an action to easily try and fetch the public email of a user from github. Fixes #531
2021-10-06 18:06:53 +07:00
'email': 'user@example.org',
[IMP] runbot_merge: make review rights repo-dependent As the odds of having more projects or more repos with different requirements in the same project, the need to have different sets of reviewers for different repositories increases. As a result, rather than be trivial boolean flags the review info should probably depend on the user / partner and the repo. Turns out the permission checks had already been extracted into their own function so most of the mess comes from testing utilities which went and configured their review rights as needed. Incidentally it might be that the test suite could just use something like a sequence of commoditized accounts which get configured as needed and not even looked at unless they're used.
2020-02-10 21:05:08 +07:00
})
c.write({
'review_rights': [
(0, 0, {'repository_id': repo.id, 'review': True})
for repo in project.repo_ids
]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
})
[IMP] runbot_merge: reject review without email If a reviewer doesn't have an email set, the Signed-Off-By is an `@users.noreply.github.com` address which just looks weird in the final result. Initially the thinking was that emails would be required for users to *be* reviewers or self-reviewers, but since those are now o2ms / m2ms it's a bit of a pain in the ass. Instead, provide an action to easily try and fetch the public email of a user from github. Fixes #531
2021-10-06 18:06:53 +07:00
# create a partner for `other` so we can put an email on it
env['res.partner'].create({
'name': users['other'],
'github_login': users['other'],
'email': 'other@example.org',
})
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
author_token = config['role_' + author]['token']
fork = prod.fork(token=author_token)
with prod, fork:
[c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights')
pr = prod.make_pr(
target='a', title='my change',
head=users[author] + ':accessrights',
token=author_token,
)
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', token=config['github']['token'])
if delegate:
pr.post_comment('hansen delegate=%s' % users[delegate], token=config['github']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
pr0, pr1 = env['runbot_merge.pull_requests'].search([], order='number')
assert pr0.state == 'merged'
with prod:
prod.post_status(pr1.head, 'success', 'ci/runbot')
prod.post_status(pr1.head, 'success', 'legal/cla')
env.run_crons()
_, _, pr2 = env['runbot_merge.pull_requests'].search([], order='number')
with prod:
prod.post_status(pr2.head, 'success', 'ci/runbot')
prod.post_status(pr2.head, 'success', 'legal/cla')
prod.get_pr(pr2.number).post_comment(
'%s r+' % project.fp_github_name,
token=config['role_' + reviewer]['token']
)
env.run_crons()
if success:
assert pr1.staging_id and pr2.staging_id,\
"%s should have approved FP of PRs by %s" % (reviewer, author)
[IMP] runbot_merge, forwardport: better signed-off-by handling Remove original-signed-off-by, doesn't actually seem useful given the semantics of signed-off-by according to the kernel doc'. Plus it didn't actually work as the intent was to keep the signoff of the original PR in the forward-port, but that signoff is not part of the commit we're cherrypicking (it gets added on the fly when the commit is merged). Therefore explicitly get the ack-chain into the PR: when merging an FP PR, try to integrate the signoff of the original PR, that of the final FP pr, and while at it that of the last explicit update in the commit chain (e.g. in case there's been a conflict or something). Fixes #284
2020-03-02 14:54:58 +07:00
st = prod.commit('staging.b')
# Should be signed-off by both original reviewer and forward port reviewer
[IMP] runbot_merge: remove unnecessary uniquifier dummy commits "Uniquifier" commits were introduced to ensure branches of a staging on which nothing had been staged would still be rebuilt properly. This means technically the branches on which something had been staged never *needed* a uniquifier, strictly speaking. And those lead to extra building, because once the actually staged PRs get pushed from staging to their final destination it's an unknown commit to the runbot, which needs to rebuild it instead of being able to just use the staging it already has. Thus only add the uniquifier where it *might* be necessary: technically the runbot should not manage this use case much better, however there are still issues like an ancillary build working with the same branch tip (e.g. the "current master") and sending a failure result which would fail the entire staging. The uniquifier guards against this issue. Also update rebase semantics to always update the *commit date* of the rebased commits: this ensures the tip commit is always "recent" in the case of a rebase-ff (which is common as that's what single-commit PRs do), as the runbot may skip commits it considers "old". Also update some of the utility methods around repos / commits to be simpler, and avoid assuming the result is JSON-decodable (sometimes it is not). Also update the handling of commit statuses using postgres' ON CONFLICT and jsonb support, hopefully this improves (or even fixes) the serialization errors. Should be compatible with 9.5 onwards which is *ancient* at this point. Fixes #509
2021-08-09 18:21:24 +07:00
original_signoff = signoff(config['role_user'], st.message)
forward_signoff = signoff(config['role_' + reviewer], st.message)
assert st.message.index(original_signoff) <= st.message.index(forward_signoff),\
[IMP] runbot_merge, forwardport: better signed-off-by handling Remove original-signed-off-by, doesn't actually seem useful given the semantics of signed-off-by according to the kernel doc'. Plus it didn't actually work as the intent was to keep the signoff of the original PR in the forward-port, but that signoff is not part of the commit we're cherrypicking (it gets added on the fly when the commit is merged). Therefore explicitly get the ack-chain into the PR: when merging an FP PR, try to integrate the signoff of the original PR, that of the final FP pr, and while at it that of the last explicit update in the commit chain (e.g. in case there's been a conflict or something). Fixes #284
2020-03-02 14:54:58 +07:00
"Check that FP approver is after original PR approver as that's " \
"the review path for the PR"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
else:
assert not (pr1.staging_id or pr2.staging_id),\
"%s should *not* have approved FP of PRs by %s" % (reviewer, author)
[IMP] runbot_merge, forwardport: better signed-off-by handling Remove original-signed-off-by, doesn't actually seem useful given the semantics of signed-off-by according to the kernel doc'. Plus it didn't actually work as the intent was to keep the signoff of the original PR in the forward-port, but that signoff is not part of the commit we're cherrypicking (it gets added on the fly when the commit is merged). Therefore explicitly get the ack-chain into the PR: when merging an FP PR, try to integrate the signoff of the original PR, that of the final FP pr, and while at it that of the last explicit update in the commit chain (e.g. in case there's been a conflict or something). Fixes #284
2020-03-02 14:54:58 +07:00
def signoff(conf, message):
for n in filter(None, [conf.get('name'), conf.get('user')]):
signoff = 'Signed-off-by: ' + n
if signoff in message:
return signoff
raise AssertionError("Failed to find signoff by %s in %s" % (conf, message))
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[FIX] forwardport: don't re-approve approved PRs during an fw-bot r+ When using the forwardport's shortcut, the bot would not skip already-approved PRs leading to a warning from the mergebot that the PR was already approved (out of nowhere which was weird). During the walk to the ancestors, skip any PR which is not approvable (either already approved or in a state where that makes no sense e.g. closed). Fixes #543
2021-10-06 19:42:54 +07:00
[FIX] forwardport: allow delegates on fw PRs to review followups The forward-port process currently automatically adds delegates of a PR as delegates on its forward ports, but that only works for the *source* pull request. If a delegate is added to a forward-port, they were not able to approve the followups to that initial port, which makes little sense. Fixes #548
2021-10-18 16:46:14 +07:00
def test_delegate_fw(env, config, make_repo, users):
"""If a user is delegated *on a forward port* they should be able to approve
*the followup*.
"""
prod, _ = make_basic(env, config, make_repo)
# create a partner for `other` so we can put an email on it
env['res.partner'].create({
'name': users['other'],
'github_login': users['other'],
'email': 'other@example.org',
})
author_token = config['role_self_reviewer']['token']
fork = prod.fork(token=author_token)
with prod, fork:
[c] = fork.make_commits('a', Commit('c_0', tree={'y': '0'}), ref='heads/accessrights')
pr = prod.make_pr(
target='a', title='my change',
head=users['self_reviewer'] + ':accessrights',
token=author_token,
)
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', token=config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
# ensure pr1 has to be approved to be forward-ported
_, pr1_id = env['runbot_merge.pull_requests'].search([], order='number')
# detatch from source
[IMP] forwardport: store and display detachment reason Currently, if a PR forward-port PR gets detached the reason for it is not always obvious, and may have to be hunted in the logs or in "sibling" PRs. By writing a forward port reason (hopefully) ever time we detach a PR, and displaying that reason in the form and dashboard, the justification should be a lot more obvious. Fixes #679
2023-01-19 20:23:41 +07:00
pr1_id.write({
'parent_id': False,
'detach_reason': "Detached for testing.",
})
[FIX] forwardport: allow delegates on fw PRs to review followups The forward-port process currently automatically adds delegates of a PR as delegates on its forward ports, but that only works for the *source* pull request. If a delegate is added to a forward-port, they were not able to approve the followups to that initial port, which makes little sense. Fixes #548
2021-10-18 16:46:14 +07:00
with prod:
prod.post_status(pr1_id.head, 'success', 'legal/cla')
prod.post_status(pr1_id.head, 'success', 'ci/runbot')
env.run_crons()
pr1 = prod.get_pr(pr1_id.number)
# delegate review to "other" consider PR fixed, and have "other" approve it
with prod:
pr1.post_comment('hansen delegate=' + users['other'],
token=config['role_reviewer']['token'])
prod.post_status(pr1_id.head, 'success', 'ci/runbot')
pr1.post_comment('hansen r+', token=config['role_other']['token'])
env.run_crons()
with prod:
prod.post_status('staging.b', 'success', 'legal/cla')
prod.post_status('staging.b', 'success', 'ci/runbot')
env.run_crons()
_, _, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
pr2 = prod.get_pr(pr2_id.number)
# make "other" also approve this one
with prod:
prod.post_status(pr2_id.head, 'success', 'ci/runbot')
prod.post_status(pr2_id.head, 'success', 'legal/cla')
pr2.post_comment('hansen r+', token=config['role_other']['token'])
env.run_crons()
assert pr2.comments == [
seen(env, pr2, users),
[IMP] *: review mergebot & forwardbot messages for pinging Old messages were quite inconsistent in their pinging of the PR author and reviewer. Reviewed messages (probably missed some but...) and try to more consistently ping when the feedback requires some sort of action in order to proceed. Fixes #592
2022-06-23 19:25:07 +07:00
(users['user'], '''@{self_reviewer} @{reviewer} this PR targets c and is the last of the forward-port chain.
[FIX] forwardport: allow delegates on fw PRs to review followups The forward-port process currently automatically adds delegates of a PR as delegates on its forward ports, but that only works for the *source* pull request. If a delegate is added to a forward-port, they were not able to approve the followups to that initial port, which makes little sense. Fixes #548
2021-10-18 16:46:14 +07:00
To merge the full chain, say
> @{user} r+
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
'''.format_map(users)),
(users['other'], 'hansen r+')
]
[FIX] forwardport: don't re-approve approved PRs during an fw-bot r+ When using the forwardport's shortcut, the bot would not skip already-approved PRs leading to a warning from the mergebot that the PR was already approved (out of nowhere which was weird). During the walk to the ancestors, skip any PR which is not approvable (either already approved or in a state where that makes no sense e.g. closed). Fixes #543
2021-10-06 19:42:54 +07:00
def test_redundant_approval(env, config, make_repo, users):
"""If a forward port sequence has been partially approved, fw-bot r+ should
not perform redundant approval as that triggers warning messages.
"""
prod, _ = make_basic(env, config, make_repo)
[project] = env['runbot_merge.project'].search([])
with prod:
prod.make_commits(
'a', Commit('p', tree={'x': '0'}),
ref='heads/early'
)
pr0 = prod.make_pr(target='a', head='early')
prod.post_status('heads/early', 'success', 'legal/cla')
prod.post_status('heads/early', 'success', 'ci/runbot')
pr0.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
env.run_crons()
pr0_id, pr1_id = env['runbot_merge.pull_requests'].search([], order='number asc')
with prod:
prod.post_status(pr1_id.head, 'success', 'legal/cla')
prod.post_status(pr1_id.head, 'success', 'ci/runbot')
env.run_crons()
_, _, pr2_id = env['runbot_merge.pull_requests'].search([], order='number asc')
assert pr2_id.parent_id == pr1_id
assert pr1_id.parent_id == pr0_id
pr1 = prod.get_pr(pr1_id.number)
pr2 = prod.get_pr(pr2_id.number)
with prod:
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
with prod:
pr2.post_comment(f'{project.fp_github_name} r+', config['role_reviewer']['token'])
env.run_crons()
assert pr1.comments == [
seen(env, pr1, users),
(users['user'], 'This PR targets b and is part of the forward-port chain. '
'Further PRs will be created up to c.\n\n'
'More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n'),
(users['reviewer'], 'hansen r+'),
]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
def test_batched(env, config, make_repo, users):
""" Tests for projects with multiple repos & sync'd branches. Batches
should be FP'd to batches
"""
main1, _ = make_basic(env, config, make_repo, reponame='main1')
main2, _ = make_basic(env, config, make_repo, reponame='main2')
main1.unsubscribe(config['role_reviewer']['token'])
main2.unsubscribe(config['role_reviewer']['token'])
friendo = config['role_other']
other1 = main1.fork(token=friendo['token'])
other2 = main2.fork(token=friendo['token'])
with main1, other1:
[c1] = other1.make_commits(
'a', Commit('commit repo 1', tree={'1': 'a'}),
ref='heads/contribution'
)
pr1 = main1.make_pr(
target='a', title="My contribution",
head=friendo['user'] + ':contribution',
token=friendo['token']
)
# we can ack it directly as it should not be taken in account until
# we run crons
validate_all([main1], [c1])
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
with main2, other2:
[c2] = other2.make_commits(
'a', Commit('commit repo 2', tree={'2': 'a'}),
ref='heads/contribution' # use same ref / label as pr1
)
pr2 = main2.make_pr(
target='a', title="Main2 part of my contribution",
head=friendo['user'] + ':contribution',
token=friendo['token']
)
validate_all([main2], [c2])
pr2.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
# sanity check: this should have created a staging with 1 batch with pr1 and pr2
stagings = env['runbot_merge.stagings'].search([])
assert len(stagings) == 1
assert stagings.target.name == 'a'
assert len(stagings.batch_ids) == 1
assert stagings.mapped('batch_ids.prs.number') == [pr1.number, pr2.number]
with main1, main2:
validate_all([main1, main2], ['staging.a'])
env.run_crons()
PullRequests = env['runbot_merge.pull_requests']
# created the first forward port, need to validate it so the second one is
# triggered (FP only goes forward on CI+) (?)
pr1b = PullRequests.search([
('source_id', '!=', False),
('repository.name', '=', main1.name),
])
pr2b = PullRequests.search([
('source_id', '!=', False),
('repository.name', '=', main2.name),
])
# check that relevant users were pinged
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
ping = (users['user'], """\
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
This PR targets b and is part of the forward-port chain. Further PRs will be created up to c.
[FIX] forwardport: fix forward-port Odoo wiki link
2020-03-09 20:20:59 +07:00
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
""")
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
pr_remote_1b = main1.get_pr(pr1b.number)
pr_remote_2b = main2.get_pr(pr2b.number)
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
assert pr_remote_1b.comments == [seen(env, pr_remote_1b, users), ping]
assert pr_remote_2b.comments == [seen(env, pr_remote_2b, users), ping]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with main1, main2:
validate_all([main1], [pr1b.head])
validate_all([main2], [pr2b.head])
env.run_crons() # process updated statuses -> generate followup FP
# should have created two PRs whose source is p1 and two whose source is p2
pr1a, pr1b, pr1c = PullRequests.search([
('repository.name', '=', main1.name),
], order='number')
pr2a, pr2b, pr2c = PullRequests.search([
('repository.name', '=', main2.name),
], order='number')
assert pr1a.number == pr1.number
assert pr2a.number == pr2.number
assert pr1a.state == pr2a.state == 'merged'
assert pr1b.label == pr2b.label, "batched source should yield batched FP"
assert pr1c.label == pr2c.label, "batched source should yield batched FP"
assert pr1b.label != pr1c.label
project = env['runbot_merge.project'].search([])
# ok main1 PRs
with main1:
validate_all([main1], [pr1c.head])
main1.get_pr(pr1c.number).post_comment('%s r+' % project.fp_github_name, config['role_reviewer']['token'])
env.run_crons()
# check that the main1 PRs are ready but blocked on the main2 PRs
assert pr1b.state == 'ready'
assert pr1c.state == 'ready'
assert pr1b.blocked
assert pr1c.blocked
# ok main2 PRs
with main2:
validate_all([main2], [pr2c.head])
main2.get_pr(pr2c.number).post_comment('%s r+' % project.fp_github_name, config['role_reviewer']['token'])
env.run_crons()
[REM] forwardport: fp_sequence field It's almost certainly not useful, save as a minor convenience for tests: decorrelating the branch sequence and the fp sequence seems like it would only be extremely confusing, and on the mergebot all the fp_sequence values are set to the default while the sequence values are set to something useful and sensible (kinda). Fixes #584
2022-12-08 16:42:12 +07:00
env['runbot_merge.stagings'].search([]).mapped('target.display_name')
env['runbot_merge.stagings'].search([], order='target').mapped('target.display_name')
stc, stb = env['runbot_merge.stagings'].search([], order='target')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
assert stb.target.name == 'b'
assert stc.target.name == 'c'
with main1, main2:
validate_all([main1, main2], ['staging.b', 'staging.c'])
class TestClosing:
def test_closing_before_fp(self, env, config, make_repo, users):
""" Closing a PR should preclude its forward port
"""
prod, other = make_basic(env, config, make_repo)
with prod:
[p_1] = prod.make_commits(
'a',
Commit('p_0', tree={'x': '0'}),
ref='heads/hugechange'
)
pr = prod.make_pr(target='a', head='hugechange')
prod.post_status(p_1, 'success', 'legal/cla')
prod.post_status(p_1, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
# should merge the staging then create the FP PR
env.run_crons()
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr0_id, pr1_id = env['runbot_merge.pull_requests'].search([], order='number')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# close the FP PR then have CI validate it
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr1 = prod.get_pr(pr1_id.number)
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with prod:
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
pr1.close()
assert pr1_id.state == 'closed'
assert not pr1_id.parent_id, "closed PR should should be detached from its parent"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with prod:
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
prod.post_status(pr1_id.head, 'success', 'legal/cla')
prod.post_status(pr1_id.head, 'success', 'ci/runbot')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
env.run_crons()
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron')
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
assert env['runbot_merge.pull_requests'].search([], order='number') == pr0_id | pr1_id,\
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
"closing the PR should suppress the FP sequence"
[CHG] runbot_merge, forwardport: remove labelling Because github materialises every labels change in the timeline (interspersed with comments), the increasing labels churn contributes to PRs being difficult to read and review. This change removes the update of labels on PRs, instead the mergebot will automatically send a comment to created PRs serving as a notification that the PR was noticed & providing a link to the mergebot's dashboard for that PR where users should be able to see the PR state in detail in case they wonder what's what. Lots of tests had to be edited to: - remove any check on the labels of the PR - add checks on the PR dashboard (to ensure that they're at least on the correct "view") - add a helper to handle the comment now added to every PR by the 'bot - since that helper is needed by both mergebot and forwardbot, the utils modules were unified and moved out of the odoo modules Probably relevant note: no test was added for the dashboard ACL, though since I had to explicitly unset the group on the repo used for tests for things to work it looks to me like it at least excludes people just fine. Fixes #419
2020-11-17 21:21:21 +07:00
assert pr1.comments == [
seen(env, pr1, users),
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
(users['user'], """\
This PR targets b and is part of the forward-port chain. Further PRs will be created up to c.
[FIX] forwardport: fix forward-port Odoo wiki link
2020-03-09 20:20:59 +07:00
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
[FIX] forwardport: clarify forward port message The ping message was not clear - don't know if anything is needed from the author - don't know if the last step in the chain Ping the authors only when something needs to be done (error or last step). Do not ping non-reviewers as they will not have the rights to r+ or modify the PR anyway Closes #192
2019-09-10 21:29:02 +07:00
""")
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
]
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
def test_closing_after_fp(self, env, config, make_repo, users):
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
""" Closing a PR which has been forward-ported should not touch the
followups
"""
prod, other = make_basic(env, config, make_repo)
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
project = env['runbot_merge.project'].search([])
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with prod:
[p_1] = prod.make_commits(
'a',
Commit('p_0', tree={'x': '0'}),
ref='heads/hugechange'
)
pr = prod.make_pr(target='a', head='hugechange')
prod.post_status(p_1, 'success', 'legal/cla')
prod.post_status(p_1, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
# should merge the staging then create the FP PR
env.run_crons()
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
pr0_id, pr1_id = env['runbot_merge.pull_requests'].search([], order='number')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with prod:
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
prod.post_status(pr1_id.head, 'success', 'legal/cla')
prod.post_status(pr1_id.head, 'success', 'ci/runbot')
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
# should create the second staging
env.run_crons()
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
pr0_id2, pr1_id2, pr2_id = env['runbot_merge.pull_requests'].search([], order='number')
assert pr0_id2 == pr0_id
assert pr1_id2 == pr1_id
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
pr1 = prod.get_pr(pr1_id.number)
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
with prod:
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
pr1.close()
assert pr1_id.state == 'closed'
assert not pr1_id.parent_id
assert pr2_id.state == 'opened'
assert not pr2_id.parent_id, \
"the descendant of a closed PR doesn't really make sense, maybe?"
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[FIX] forwardport: detached / closed PR clarifications - trying to r+ a detached PR *via the forwardbot* should warn, same as a non-forwardport PR - the following sibling of a closed PR should be detached from it (probably) - when a closed forward-port PR is reopened, there should be a notification that it is detached and merged via mergebot Fixes #617
2022-06-29 15:58:13 +07:00
with prod:
pr1.open()
assert pr1_id.state == 'validated'
env.run_crons()
assert pr1.comments[-1] == (
users['user'],
"@{} @{} this PR was closed then reopened. "
"It should be merged the normal way (via @{})".format(
users['user'],
users['reviewer'],
project.github_prefix,
)
)
with prod:
pr1.post_comment(f'{project.fp_github_name} r+', config['role_reviewer']['token'])
env.run_crons()
assert pr1.comments[-1] == (
users['user'],
"@{} I can only do this on unmodified forward-port PRs, ask {}.".format(
users['reviewer'],
project.github_prefix,
),
)
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
class TestBranchDeletion:
def test_delete_normal(self, env, config, make_repo):
""" Regular PRs should get their branch deleted as long as they're
created in the fp repository
"""
prod, other = make_basic(env, config, make_repo)
with prod, other:
[c] = other.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/abranch')
pr = prod.make_pr(
target='a', head='%s:abranch' % other.owner,
title="a pr",
)
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
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)
])
assert pr_id.state == 'merged'
removers = env['forwardport.branch_remover'].search([])
to_delete_branch = removers.mapped('pr_id')
assert to_delete_branch == pr_id
env.run_crons('forwardport.remover', context={'forwardport_merged_before': FAKE_PREV_WEEK})
with pytest.raises(AssertionError, match="Not Found"):
other.get_ref('heads/abranch')
def test_not_merged(self, env, config, make_repo):
""" The branches of PRs which are still open or have been closed (rather
than merged) should not get deleted
"""
prod, other = make_basic(env, config, make_repo)
with prod, other:
[c] = other.make_commits('a', Commit('c1', tree={'1': '0'}), ref='heads/abranch')
pr1 = prod.make_pr(target='a', head='%s:abranch' % other.owner, title='a')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr1.post_comment('hansen r+', config['role_reviewer']['token'])
[IMP] forwardport: unmodified fw automatically inherit overrides Before this change, a CI override would have to be replicated on most / all forward-ports of the base PR. This was intentional to see how it would shake out, the answer being that it's rather annoying. Also add a `statuses_full` computed field on PRs for the aggregate status: the existing `statuses` field is just a copy of the commit statuses which I didn't remember I kept free of the overrides so the commit statuses could be displayed "as-is" in the backend (the overrides are displayed separately). And while at it fix the PR dashboard to use that new field: that was basically the intention but then I went on to use the "wrong" field hence #433. Mebbe the UI part should be displayed using a computed M2M (?) as a table or as tags instead? This m2m could indicate whether the status is an override or an "intrinsic" status. Also removed some dead code: * leftover from the removed tagging feature (removed the tag manipulation but forgot some of the setup / computations) * unused local variables * an empty skipped test case Fixes #439. Fixes #433.
2021-01-13 14:18:17 +07:00
other.make_commits('a', Commit('c2', tree={'2': '0'}), ref='heads/bbranch')
[ADD] forwardport: automatic branch deleter If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay). Mainly to avoid FW branches staying around long after they've been merged (possibly eventually closed?), will also clean up regular merged branches, including historical merges forgotten by their author. Fixes #230
2019-10-16 19:41:26 +07:00
pr2 = prod.make_pr(target='a', head='%s:bbranch' % other.owner, title='b')
pr2.close()
[c] = other.make_commits('a', Commit('c3', tree={'3': '0'}), ref='heads/cbranch')
pr3 = prod.make_pr(target='a', head='%s:cbranch' % other.owner, title='c')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
other.make_commits('a', Commit('c3', tree={'4': '0'}), ref='heads/dbranch')
pr4 = prod.make_pr(target='a', head='%s:dbranch' % other.owner, title='d')
pr4.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
PR = env['runbot_merge.pull_requests']
# check PRs are in states we expect
pr_heads = []
for p, st in [(pr1, 'ready'), (pr2, 'closed'), (pr3, 'validated'), (pr4, 'approved')]:
p_id = PR.search([
('repository.name', '=', prod.name),
('number', '=', p.number),
])
assert p_id.state == st
pr_heads.append(p_id.head)
env.run_crons('forwardport.remover', context={'forwardport_merged_before': FAKE_PREV_WEEK})
# check that the branches still exist
assert other.get_ref('heads/abranch') == pr_heads[0]
assert other.get_ref('heads/bbranch') == pr_heads[1]
assert other.get_ref('heads/cbranch') == pr_heads[2]
assert other.get_ref('heads/dbranch') == pr_heads[3]
[ADD] forwardbot * Cherrypicking is handrolled because there seems to be no easy way to programmatically edit commit messages during the cherrypicking sequence: `-n` basically squashes all commits and `-e` invokes a subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it interrupts the process before each commit), however there doesn't seem to be clean status codes so it's difficult to know if the cherrypick failed or if it's just waiting for a commit of this step. Instead, cherrypick commits individually then edit / rewrite their commit messages: * add a reference to the original commit * convert signed-off-by to something else as the original commit was signed off but not necessarily this one * Can't assign users when creating PRs: only repository collaborators or people who commented on the issue / PR (which we're in the process of creating) can be assigned. PR authors are as likely to be collaborators as not, and we can have non-collaborator reviewers. So pinging via a regular comment seems less fraught as a way to notify users.
2019-08-23 21:16:30 +07:00
def sPeNgBaB(s):
return ''.join(
l if i % 2 == 0 else l.upper()
for i, l in enumerate(s)
)
def test_spengbab():
assert sPeNgBaB("spongebob") == 'sPoNgEbOb'
class TestRecognizeCommands:
def make_pr(self, env, config, make_repo):
r, _ = make_basic(env, config, make_repo)
with r:
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),
])
def test_botname_casing(self, env, config, make_repo):
""" Test that the botname is case-insensitive as people might write
bot names capitalised or titlecased or uppercased or whatever
"""
repo, pr, pr_id = self.make_pr(env, config, make_repo)
assert pr_id.state == 'opened'
botname = env['runbot_merge.project'].search([]).fp_github_name
[a] = env['runbot_merge.branch'].search([
('name', '=', 'a')
])
[c] = env['runbot_merge.branch'].search([
('name', '=', 'c')
])
names = [
botname,
botname.upper(),
botname.capitalize(),
sPeNgBaB(botname),
]
for n in names:
assert pr_id.limit_id == c
with repo:
pr.post_comment('@%s up to a' % n, config['role_reviewer']['token'])
assert pr_id.limit_id == a
# reset state
pr_id.write({'limit_id': c.id})
@pytest.mark.parametrize('indent', ['', '\N{SPACE}', '\N{SPACE}'*4, '\N{TAB}'])
def test_botname_indented(self, env, config, make_repo, indent):
""" matching botname should ignore leading whitespaces
"""
repo, pr, pr_id = self.make_pr(env, config, make_repo)
assert pr_id.state == 'opened'
botname = env['runbot_merge.project'].search([]).fp_github_name
[a] = env['runbot_merge.branch'].search([
('name', '=', 'a')
])
[c] = env['runbot_merge.branch'].search([
('name', '=', 'c')
])
assert pr_id.limit_id == c
with repo:
pr.post_comment('%s@%s up to a' % (indent, botname), config['role_reviewer']['token'])
assert pr_id.limit_id == a
Reference in New Issue Copy Permalink