diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 72630453..c3d4806d 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -484,6 +484,8 @@ stderr: ('source_id', '!=', False), # active ('state', 'not in', ['merged', 'closed']), + # not ready + ('blocked', '!=', False), ('source_id.merge_date', '<', cutoff), ], order='source_id, id'), lambda p: p.source_id) @@ -498,11 +500,8 @@ stderr: continue source.reminder_backoff_factor += 1 - # only keep the PRs which don't have an attached descendant) - pr_ids = {p.id for p in prs} - for pr in prs: - pr_ids.discard(pr.parent_id.id) - for pr in (p for p in prs if p.id in pr_ids): + # only keep the PRs which don't have an attached descendant + for pr in set(prs).difference(p.parent_id for p in prs): self.env.ref('runbot_merge.forwardport.reminder')._send( repository=pr.repository, pull_request=pr.number, diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index 187a1688..0c4260c2 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -251,7 +251,7 @@ def test_empty(env, config, make_repo, users): """ Cherrypick of an already cherrypicked (or separately implemented) commit -> conflicting pr. """ - prod, other = make_basic(env, config, make_repo) + prod, other = make_basic(env, config, make_repo, statuses="default") # merge change to b with prod: [p_0] = prod.make_commits( @@ -259,13 +259,11 @@ def test_empty(env, config, make_repo, users): 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') + prod.post_status(p_0, 'success') 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') + prod.post_status('staging.b', 'success') # merge same change to a afterwards with prod: @@ -274,15 +272,13 @@ def test_empty(env, config, make_repo, users): 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') + prod.post_status(p_1, 'success') 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') - + prod.post_status('staging.a', 'success') env.run_crons() + assert prod.read_tree(prod.commit('a')) == { 'f': 'e', 'x': '0', @@ -315,7 +311,8 @@ def test_empty(env, config, make_repo, users): } with prod: - validate_all([prod], [fp_id.head, fail_id.head]) + prod.post_status(fp_id.head, 'success') + prod.post_status(fail_id.head, 'success') env.run_crons() # should not have created any new PR @@ -379,12 +376,28 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port (users['reviewer'], 'hansen r+'), seen(env, pr1, users), ] - assert fail_pr.comments == [ - seen(env, fail_pr, users), - conflict, + assert fail_pr.comments[2:] == [awaiting]*2,\ + "message should not be triggered on closed PR" + + with prod: + fail_pr.open() + with prod: + prod.post_status(fail_id.head, 'success') + assert fail_id.state == 'validated' + env.run_crons('forwardport.reminder', context={'forwardport_updated_before': FAKE_PREV_WEEK}) + assert fail_pr.comments[2:] == [awaiting]*3, "check that message triggers again" + + with prod: + fail_pr.post_comment('hansen r+', config['role_reviewer']['token']) + assert fail_id.state == 'ready' + env.run_crons('forwardport.reminder', context={'forwardport_updated_before': FAKE_PREV_WEEK}) + assert fail_pr.comments[2:] == [ awaiting, awaiting, - ], "each cron run should trigger a new message" + awaiting, + (users['reviewer'], "hansen r+"), + ],"if a PR is ready (unblocked), the reminder should not trigger as "\ + "we're likely waiting for the mergebot" def test_partially_empty(env, config, make_repo): """ Check what happens when only some commits of the PR are now empty