diff --git a/conftest.py b/conftest.py index 1b432a0c..dc3fc561 100644 --- a/conftest.py +++ b/conftest.py @@ -80,6 +80,13 @@ def pytest_addoption(parser): def pytest_report_header(config): return 'Running against database ' + config.getoption('--db') +@pytest.fixture(scope='session', autouse=True) +def _set_socket_timeout(): + """ Avoid unlimited wait on standard sockets during tests, this is mostly + an issue for non-trivial cron calls + """ + socket.setdefaulttimeout(60.0) + @pytest.fixture(scope="session") def config(pytestconfig): """ Flat version of the pytest config file (pytest.ini), parses to a diff --git a/forwardport/data/crons.xml b/forwardport/data/crons.xml index 5a134e7e..39ce9bb8 100644 --- a/forwardport/data/crons.xml +++ b/forwardport/data/crons.xml @@ -23,27 +23,9 @@ Remind open PR - + code - -default_delta = dateutil.relativedelta.relativedelta(days=3) -cutoff = env.context.get('forwardport_updated_before') or (datetime.datetime.now() - default_delta).strftime('%Y-%m-%d %H:%M:%S') - -for pr in env['runbot_merge.pull_requests'].search([ - # only FP PRs - ('source_id', '!=', False), - # active - ('state', 'not in', ['merged', 'closed']), - # last updated more than a week ago - ('write_date', '<', cutoff), -]).mapped('source_id'): - env['runbot_merge.pull_requests.feedback'].create({ - 'repository': pr.repository.id, - 'pull_request': pr.number, - 'message': "This pull request has forward-port PRs awaiting action", - 'token_field': 'fp_github_token', - }) - + model._reminder() 1 days -1 diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 7f7dba84..ae428299 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -13,6 +13,7 @@ it up), ... """ import base64 import contextlib +import datetime import itertools import json import logging @@ -22,15 +23,17 @@ import re import subprocess import tempfile +import dateutil import requests from odoo import _, models, fields, api from odoo.exceptions import UserError -from odoo.tools import topological_sort +from odoo.tools import topological_sort, groupby from odoo.tools.appdirs import user_cache_dir from odoo.addons.runbot_merge import utils from odoo.addons.runbot_merge.models.pull_requests import RPLUS +DEFAULT_DELTA = dateutil.relativedelta.relativedelta(days=3) _logger = logging.getLogger('odoo.addons.forwardport') @@ -228,8 +231,8 @@ class PullRequests(models.Model): ]) if self.parent_id: msg = "Sorry, forward-port limit can only be set on an origin PR" \ - " (#%d here) before it's merged and forward-ported." % ( - self._get_root().number + " (%s here) before it's merged and forward-ported." % ( + self._get_root().display_name ) elif self.state in ['merged', 'closed']: msg = "Sorry, forward-port limit can only be set before the PR is merged." @@ -417,16 +420,16 @@ class PullRequests(models.Model): target = base._find_next_target(ref) if target is None: _logger.info( - "Will not forward-port %s#%s: no next target", - ref.repository.name, ref.number, + "Will not forward-port %s: no next target", + ref.display_name, ) return # QUESTION: do the prs need to be updated? proj = self.mapped('target.project_id') if not proj.fp_github_token: _logger.warning( - "Can not forward-port %s#%s: no token on project %s", - ref.repository.name, ref.number, + "Can not forward-port %s: no token on project %s", + ref.display_name, proj.name ) return @@ -471,7 +474,7 @@ class PullRequests(models.Model): message = '' root = pr._get_root() message += '\n'.join( - "Forward-Port-Of: %s#%s" % (p.repository.name, p.number) + "Forward-Port-Of: %s" % p.display_name for p in root | source ) @@ -539,7 +542,7 @@ In the former case, you may want to edit this PR message as well. """ % (h, source.number, sout, serr) elif base._find_next_target(new_pr) is None: ancestors = "".join( - "* %s#%d\n" % (p.repository.name, p.number) + "* %s\n" % p.display_name for p in pr._iter_ancestors() if p.parent_id ) @@ -767,6 +770,26 @@ stderr: repo.config('--add', 'remote.origin.fetch', '+refs/pull/*/head:refs/heads/pull/*') return repo + def _reminder(self): + cutoff = self.env.context.get('forwardport_updated_before') or fields.Datetime.to_string(datetime.datetime.now() - DEFAULT_DELTA) + + for source, prs in groupby(self.env['runbot_merge.pull_requests'].search([ + # only FP PRs + ('source_id', '!=', False), + # active + ('state', 'not in', ['merged', 'closed']), + # last updated more than ago + ('write_date', '<', cutoff), + ]), lambda p: p.source_id): + self.env['runbot_merge.pull_requests.feedback'].create({ + 'repository': source.repository.id, + 'pull_request': source.number, + 'message': "This pull request has forward-port PRs awaiting action (not merged or closed): %s" % ', '.join( + pr.display_name for pr in sorted(prs, key=lambda p: p.number) + ), + 'token_field': 'fp_github_token', + }) + class Stagings(models.Model): _inherit = 'runbot_merge.stagings' diff --git a/forwardport/tests/test_limit.py b/forwardport/tests/test_limit.py index 5e1d1b33..0e374bf3 100644 --- a/forwardport/tests/test_limit.py +++ b/forwardport/tests/test_limit.py @@ -255,6 +255,6 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port """), (users['reviewer'], bot_name + ' up to b'), (bot_name, "Sorry, forward-port limit can only be set on an origin PR" - " (#%d here) before it's merged and forward-ported." % p1.number + " (%s here) before it's merged and forward-ported." % p1.display_name ), ] \ No newline at end of file diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index 4eb41393..e2188314 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -109,14 +109,15 @@ def test_straightforward_flow(env, config, make_repo, users): env.run_crons() env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK}) + pr0_, pr1_, pr2 = env['runbot_merge.pull_requests'].search([], order='number') + assert pr.comments == [ (users['reviewer'], 'hansen r+ rebase-ff'), (users['user'], 'Merge method set to rebase and fast-forward'), (users['user'], re_matches(r'Merged at [0-9a-f]{40}, thanks!')), - (users['user'], 'This pull request has forward-port PRs awaiting action'), + (users['user'], 'This pull request has forward-port PRs awaiting action (not merged or closed): ' + ', '.join((pr1 | pr2).mapped('display_name'))), ] - pr0_, pr1_, pr2 = env['runbot_merge.pull_requests'].search([], order='number') assert pr0_ == pr0 assert pr1_ == pr1 assert pr1.parent_id == pr1.source_id == pr0 @@ -136,7 +137,7 @@ def test_straightforward_flow(env, config, make_repo, users): (users['user'], """\ Ping @%s, @%s This PR targets c and is the last of the forward-port chain containing: -* %s#%d +* %s To merge the full chain, say > @%s r+ @@ -144,7 +145,7 @@ To merge the full chain, say More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port """ % ( users['other'], users['reviewer'], - pr1.repository.name, pr1.number, + pr1.display_name, project.fp_github_name )), ] @@ -594,8 +595,8 @@ def test_empty(env, config, make_repo, users): assert pr1.comments == [ (users['reviewer'], 'hansen r+'), (users['user'], re_matches(r'Merged at [0-9a-f]{40}, thanks!')), - (users['other'], 'This pull request has forward-port PRs awaiting action'), - (users['other'], 'This pull request has forward-port PRs awaiting action'), + (users['other'], 'This pull request has forward-port PRs awaiting action (not merged or closed): ' + fail_id.display_name), + (users['other'], 'This pull request has forward-port PRs awaiting action (not merged or closed): ' + fail_id.display_name), ], "each cron run should trigger a new message on the ancestor" # check that this stops if we close the PR with prod: @@ -604,8 +605,8 @@ def test_empty(env, config, make_repo, users): assert pr1.comments == [ (users['reviewer'], 'hansen r+'), (users['user'], re_matches(r'Merged at [0-9a-f]{40}, thanks!')), - (users['other'], 'This pull request has forward-port PRs awaiting action'), - (users['other'], 'This pull request has forward-port PRs awaiting action'), + (users['other'], 'This pull request has forward-port PRs awaiting action (not merged or closed): ' + fail_id.display_name), + (users['other'], 'This pull request has forward-port PRs awaiting action (not merged or closed): ' + fail_id.display_name), ] def test_partially_empty(env, config, make_repo): diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 6c151588..ab4a08ae 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -528,7 +528,7 @@ class PullRequests(models.Model): def name_get(self): return { - p.id: '%s:%s' % (p.repository.name, p.number) + p.id: '%s#%s' % (p.repository.name, p.number) for p in self } @@ -540,7 +540,7 @@ class PullRequests(models.Model): else: separator = 's ' return '' % (separator, ' '.join( - '{0.id} ({0.repository.name}:{0.number})'.format(p) + '{0.id} ({0.display_name})'.format(p) for p in self )) @@ -988,7 +988,7 @@ class PullRequests(models.Model): 'pull_request': r.number, 'message': "Linked pull request(s) {} not ready. Linked PRs are not staged until all of them are ready.".format( ', '.join(map( - '{0.repository.name}#{0.number}'.format, + '{0.display_name}'.format, unready )) ) @@ -1032,7 +1032,7 @@ class PullRequests(models.Model): repository=self.repository.name.replace('/', '\\/') ) if not re.search(pattern, m.body): - m.body += '\n\ncloses {pr.repository.name}#{pr.number}'.format(pr=self) + m.body += '\n\ncloses {pr.display_name}'.format(pr=self) if self.reviewed_by: m.headers.add('signed-off-by', self.reviewed_by.formatted_email)