diff --git a/forwardport/data/crons.xml b/forwardport/data/crons.xml index 426e5c71..fafce284 100644 --- a/forwardport/data/crons.xml +++ b/forwardport/data/crons.xml @@ -26,20 +26,25 @@ code - for pr in env['runbot_merge.pull_requests'].search([ - # only FP PRs - ('source_id', '!=', False), - # active - ('state', 'not in', ['merged', 'closed']), - ]): - env['runbot_merge.pull_requests.feedback'].create({ - 'repository': pr.repository.id, - 'pull_request': pr.number, - 'message': "This forward port PR is awaiting action", - }) +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", + }) 1 - weeks + days -1 diff --git a/forwardport/tests/conftest.py b/forwardport/tests/conftest.py index 46f16c4f..fdb6659b 100644 --- a/forwardport/tests/conftest.py +++ b/forwardport/tests/conftest.py @@ -566,12 +566,12 @@ class Environment: def __getitem__(self, name): return Model(self, name) - def run_crons(self, *xids): + def run_crons(self, *xids, **kw): crons = xids or DEFAULT_CRONS for xid in crons: _, model, cron_id = self('ir.model.data', 'xmlid_lookup', xid) assert model == 'ir.cron', "Expected {} to be a cron, got {}".format(xid, model) - self('ir.cron', 'method_direct_trigger', [cron_id]) + self('ir.cron', 'method_direct_trigger', [cron_id], **kw) # sleep for some time as a lot of crap may have happened (?) wait_for_hook() diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index d7b3e1a9..6727f9dc 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- import collections -import sys +from datetime import datetime, timedelta import time from operator import itemgetter @@ -8,6 +8,8 @@ import pytest from utils import * +FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime('%Y-%m-%d %H:%M:%S') + # need: # * an odoo server # - connected to a database @@ -163,7 +165,14 @@ def test_straightforward_flow(env, config, make_repo, users): prod.post_status(pr1.head, 'success', 'legal/cla') env.run_crons() - env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron') + env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK}) + + 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'), + ] pr0_, pr1_, pr2 = env['runbot_merge.pull_requests'].search([], order='number') assert pr0_ == pr0 @@ -190,7 +199,6 @@ To merge the full chain, say More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port """ % (users['reviewer'], project.fp_github_name)), - (users['user'], "This forward port PR is awaiting action"), ] with prod: prod.post_status(pr2.head, 'success', 'ci/runbot') @@ -589,28 +597,25 @@ def test_empty(env, config, make_repo, users): assert env['runbot_merge.pull_requests'].search([], order='number') == prs # check reminder - env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron') - env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron') - failed_pr = prod.get_pr(fail_id.number) - # stop there (criminal scum), checking the entire message is a PITA and - # I'd need to look into integrating better diffing / error reporting when - # an re_matches doesn't match - ping_note = re_matches(r"Ping @%s\nCherrypicking [0-9a-z]{40} of source #%d failed" % (users['reviewer'], pr1.number)) - assert failed_pr.comments == [ - (users['user'], ping_note), - (users['user'], "This forward port PR is awaiting action"), - (users['user'], "This forward port PR is awaiting action"), - ] + 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}) + + assert pr1.comments == [ + (users['reviewer'], 'hansen r+'), + (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'), + ], "each cron run should trigger a new message on the ancestor" # check that this stops if we close the PR with prod: - failed_pr.close() - env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron') - # FIXME: carve out git messages as they're not necessarily stable / parseable - assert failed_pr.comments == [ - (users['user'], ping_note), - (users['user'], "This forward port PR is awaiting action"), - (users['user'], "This forward port PR is awaiting action"), - ], "should not have a 4th note" + 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+'), + (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'), + ] def test_partially_empty(env, config, make_repo): """ Check what happens when only some commits of the PR are now empty