diff --git a/forwardport/tests/conftest.py b/forwardport/tests/conftest.py index 5243d96e..c5951502 100644 --- a/forwardport/tests/conftest.py +++ b/forwardport/tests/conftest.py @@ -7,7 +7,6 @@ import requests @pytest.fixture def default_crons(): return [ - 'runbot_merge.merge_cron', 'runbot_merge.staging_cron', 'runbot_merge.check_linked_prs_status', ] diff --git a/runbot_merge/data/merge_cron.xml b/runbot_merge/data/merge_cron.xml index be06078f..f6a4f3da 100644 --- a/runbot_merge/data/merge_cron.xml +++ b/runbot_merge/data/merge_cron.xml @@ -4,8 +4,8 @@ code model._check_stagings(True) - 1 - minutes + 6 + hours -1 30 diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index cd331642..0955c853 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -2062,16 +2062,25 @@ class Stagings(models.Model): for context, status in statuses.get(commit.sha, {}).items() ] + def write(self, vals): + if timeout := vals.get('timeout_limit'): + self.env.ref("runbot_merge.merge_cron")\ + ._trigger(fields.Datetime.to_datetime(timeout)) + + return super().write(vals) + # only depend on staged_at as it should not get modified, but we might # update the CI timeout after the staging have been created and we # *do not* want to update the staging timeouts in that case @api.depends('staged_at') def _compute_timeout_limit(self): + timeouts = set() for st in self: - st.timeout_limit = fields.Datetime.to_string( - fields.Datetime.from_string(st.staged_at) - + datetime.timedelta(minutes=st.target.project_id.ci_timeout) - ) + t = st.timeout_limit = st.staged_at + datetime.timedelta(minutes=st.target.project_id.ci_timeout) + timeouts.add(t) + if timeouts: + # we might have very different limits for each staging so need to schedule them all + self.env.ref("runbot_merge.merge_cron")._trigger_list(timeouts) @api.depends('batch_ids.prs') def _compute_prs(self): @@ -2147,9 +2156,11 @@ class Stagings(models.Model): s.state = st if s.state != 'pending': + self.env.ref("runbot_merge.merge_cron")._trigger() s.staging_end = fields.Datetime.now() if update_timeout_limit: - s.timeout_limit = fields.Datetime.to_string(datetime.datetime.now() + datetime.timedelta(minutes=s.target.project_id.ci_timeout)) + s.timeout_limit = datetime.datetime.now() + datetime.timedelta(minutes=s.target.project_id.ci_timeout) + self.env.ref("runbot_merge.merge_cron")._trigger(s.timeout_limit) _logger.debug("%s got pending status, bumping timeout to %s (%s)", self, s.timeout_limit, cmap) def action_cancel(self): diff --git a/runbot_merge/tests/conftest.py b/runbot_merge/tests/conftest.py index 34d70479..e200434a 100644 --- a/runbot_merge/tests/conftest.py +++ b/runbot_merge/tests/conftest.py @@ -7,8 +7,6 @@ def module(): @pytest.fixture def default_crons(): return [ - # env['runbot_merge.project']._check_stagings() - 'runbot_merge.merge_cron', # env['runbot_merge.project']._create_stagings() 'runbot_merge.staging_cron', # env['runbot_merge.pull_requests']._check_linked_prs_statuses() diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 3208e48f..9026aa02 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -3,6 +3,7 @@ import itertools import json import textwrap import time +from typing import Callable from unittest import mock import pytest @@ -13,8 +14,9 @@ import odoo from utils import _simple_init, seen, matches, get_partner, Commit, pr_page, to_pr, part_of, ensure_one @pytest.fixture(autouse=True) -def _configure_statuses(project, repo): - project.repo_ids.required_statuses = 'legal/cla,ci/runbot' +def _configure_statuses(request, project, repo): + if 'defaultstatuses' not in request.keywords: + project.repo_ids.required_statuses = 'legal/cla,ci/runbot' @pytest.fixture(autouse=True, params=["statuses", "rpc"]) def stagings(request, env, project, repo): @@ -562,19 +564,32 @@ def test_staging_conflict_second(env, repo, users, config): assert pr1_id.state == 'error', "now pr1 should be in error" -def test_staging_ci_timeout(env, repo, config, page): +@pytest.mark.defaultstatuses +@pytest.mark.parametrize('update_op', [ + pytest.param( + lambda _: {'timeout_limit': datetime.datetime.now().isoformat(" ", "seconds")}, + id="set-timeout-limit", + ), + pytest.param( + lambda timeout: {'staged_at': (datetime.datetime.now() - datetime.timedelta(minutes=2*timeout)).isoformat(" ", "seconds")}, + id="set-staged-at", + ), +]) +def test_staging_ci_timeout(env, repo, config, page, update_op: Callable[[int], dict]): """If a staging timeouts (~ delay since staged greater than configured)... requeue? """ with repo: - m = repo.make_commit(None, 'initial', None, tree={'f': 'm'}) + m, _, c2 = repo.make_commits( + None, + Commit('initial', tree={'f': 'm'}), + Commit('first', tree={'f': 'c1'}), + Commit('second', tree={'f': 'c2'}), + ) repo.make_ref('heads/master', m) - c1 = repo.make_commit(m, 'first', None, tree={'f': 'c1'}) - c2 = repo.make_commit(c1, 'second', None, tree={'f': 'c2'}) pr = repo.make_pr(title='title', body='body', target='master', head=c2) - repo.post_status(pr.head, 'success', 'ci/runbot') - repo.post_status(pr.head, 'success', 'legal/cla') + repo.post_status(pr.head, 'success') pr.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token']) env.run_crons() @@ -582,23 +597,26 @@ def test_staging_ci_timeout(env, repo, config, page): assert pr_id.staging_id timeout = env['runbot_merge.project'].search([]).ci_timeout - pr_id.staging_id.staged_at = odoo.fields.Datetime.to_string(datetime.datetime.now() - datetime.timedelta(minutes=2*timeout)) - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + pr_id.staging_id.write(update_op(timeout)) + env.run_crons('runbot_merge.staging_cron') assert pr_id.state == 'error', "timeout should fail the PR" dangerbox = pr_page(page, pr).cssselect('.alert-danger span') assert dangerbox assert dangerbox[0].text == 'timed out (>60 minutes)' +@pytest.mark.defaultstatuses def test_timeout_bump_on_pending(env, repo, config): with repo: - m = repo.make_commit(None, 'initial', None, tree={'f': '0'}) + [m, c] = repo.make_commits( + None, + Commit('initial', tree={'f': '0'}), + Commit('c', tree={'f': '1'}), + ) repo.make_ref('heads/master', m) - c = repo.make_commit(m, 'c', None, tree={'f': '1'}) prx = repo.make_pr(title='title', body='body', target='master', head=c) - repo.post_status(prx.head, 'success', 'ci/runbot') - repo.post_status(prx.head, 'success', 'legal/cla') + repo.post_status(prx.head, 'success') prx.post_comment('hansen r+', config['role_reviewer']['token']) env.run_crons() @@ -606,7 +624,7 @@ def test_timeout_bump_on_pending(env, repo, config): old_timeout = odoo.fields.Datetime.to_string(datetime.datetime.now() - datetime.timedelta(days=15)) st.timeout_limit = old_timeout with repo: - repo.post_status('staging.master', 'pending', 'ci/runbot') + repo.post_status('staging.master', 'pending') env.run_crons(None) assert st.timeout_limit > old_timeout @@ -1092,63 +1110,63 @@ def test_reopen_merged_pr(env, repo, config, users): ] class TestNoRequiredStatus: + @pytest.mark.defaultstatuses def test_basic(self, env, repo, config): """ check that mergebot can work on a repo with no CI at all """ env['runbot_merge.repository'].search([('name', '=', repo.name)]).status_ids = False with repo: - m = repo.make_commit(None, 'initial', None, tree={'0': '0'}) + [m, c] = repo.make_commits( + None, + Commit('initial', tree={'0': '0'}), + Commit('first', tree={'0': '1'}), + ) repo.make_ref('heads/master', m) - c = repo.make_commit(m, 'first', None, tree={'0': '1'}) - prx = repo.make_pr(title='title', body='body', target='master', head=c) - prx.post_comment('hansen r+', config['role_reviewer']['token']) + pr = repo.make_pr(title='title', body='body', target='master', head=c) + pr.post_comment('hansen r+', config['role_reviewer']['token']) env.run_crons() - pr = env['runbot_merge.pull_requests'].search([ - ('repository.name', '=', repo.name), - ('number', '=', prx.number) - ]) - assert pr.state == 'ready' - st = pr.staging_id - assert st - env.run_crons() + pr_id = to_pr(env, pr) + + st = env['runbot_merge.stagings'].search([], context={'active_test': False}) assert st.state == 'success' - assert pr.state == 'merged' + assert pr_id.state == 'merged' + @pytest.mark.defaultstatuses def test_updated(self, env, repo, config): env['runbot_merge.repository'].search([('name', '=', repo.name)]).status_ids = False with repo: - m = repo.make_commit(None, 'initial', None, tree={'0': '0'}) + [m, c] = repo.make_commits( + None, + Commit('initial', tree={'0': '0'}), + Commit('first', tree={'0': '1'}), + ) repo.make_ref('heads/master', m) - c = repo.make_commit(m, 'first', None, tree={'0': '1'}) - prx = repo.make_pr(title='title', body='body', target='master', head=c) + pr = repo.make_pr(title='title', body='body', target='master', head=c) env.run_crons() - pr = env['runbot_merge.pull_requests'].search([ - ('repository.name', '=', repo.name), - ('number', '=', prx.number) - ]) - assert pr.state == 'validated' + pr_id = to_pr(env, pr) + assert pr_id.state == 'validated' # normal push with repo: - repo.make_commits(c, repo.Commit('second', tree={'0': '2'}), ref=prx.ref) + repo.make_commits(c, repo.Commit('second', tree={'0': '2'}), ref=pr.ref) env.run_crons() - assert pr.state == 'validated' + assert pr_id.state == 'validated' with repo: - prx.post_comment('hansen r+', config['role_reviewer']['token']) - assert pr.state == 'ready' + pr.post_comment('hansen r+', config['role_reviewer']['token']) + assert pr_id.state == 'ready' # force push with repo: - repo.make_commits(m, repo.Commit('xxx', tree={'0': 'm'}), ref=prx.ref) + repo.make_commits(m, repo.Commit('xxx', tree={'0': 'm'}), ref=pr.ref) env.run_crons() - assert pr.state == 'validated' + assert pr_id.state == 'validated' with repo: - prx.post_comment('hansen r+', config['role_reviewer']['token']) - assert pr.state == 'ready' + pr.post_comment('hansen r+', config['role_reviewer']['token']) + assert pr_id.state == 'ready' class TestRetry: @pytest.mark.xfail(reason="This may not be a good idea as it could lead to tons of rebuild spam") @@ -1230,7 +1248,7 @@ class TestRetry: with repo: pr.post_comment('hansen retry', config['role_' + retrier]['token']) assert pr_id.state == 'ready' - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') staging_head2 = repo.commit('heads/staging.master') assert staging_head2 != staging_head @@ -1263,7 +1281,7 @@ class TestRetry: with repo: pr.post_comment('hansen retry', config['role_reviewer']['token']) - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') with repo: repo.post_status('staging.master', 'success', 'legal/cla') @@ -1293,42 +1311,36 @@ class TestRetry: (users['user'], "@{reviewer} retry makes no sense when the PR is not in error.".format_map(users)), ] + @pytest.mark.defaultstatuses @pytest.mark.parametrize('disabler', ['user', 'other', 'reviewer']) def test_retry_disable(self, env, repo, disabler, users, config): with repo: prx = _simple_init(repo) - repo.post_status(prx.head, 'success', 'ci/runbot') - repo.post_status(prx.head, 'success', 'legal/cla') + repo.post_status(prx.head, 'success') prx.post_comment('hansen r+ delegate=%s rebase-merge' % users['other'], config["role_reviewer"]['token']) env.run_crons() - assert env['runbot_merge.pull_requests'].search([ - ('repository.name', '=', repo.name), - ('number', '=', prx.number) - ]).staging_id + pr_id = to_pr(env, prx) + staging_id = pr_id.staging_id + assert staging_id - staging_head = repo.commit('heads/staging.master') with repo: - repo.post_status('staging.master', 'success', 'legal/cla') - repo.post_status('staging.master', 'failure', 'ci/runbot') + repo.post_status('staging.master', 'failure') env.run_crons() - pr = env['runbot_merge.pull_requests'].search([ - ('repository.name', '=', repo.name), - ('number', '=', prx.number) - ]) - assert pr.state == 'error' + assert staging_id.state == 'failure' + assert not staging_id.active + assert pr_id.state == 'error' with repo: prx.post_comment('hansen r-', config['role_' + disabler]['token']) - assert pr.state == 'validated' + assert pr_id.state == 'validated' with repo: repo.make_commit(prx.ref, 'third', None, tree={'m': 'c3'}) # just in case, apparently in some case the first post_status uses the old head... with repo: - repo.post_status(prx.head, 'success', 'ci/runbot') - repo.post_status(prx.head, 'success', 'legal/cla') + repo.post_status(prx.head, 'success') env.run_crons() - assert pr.state == 'validated' + assert pr_id.state == 'validated' class TestMergeMethod: """ @@ -1759,7 +1771,7 @@ commits, I need to know how to merge it: c0 = repo.make_commit(m, 'C0', None, tree={'a': 'b'}) prx = repo.make_pr(title="gibberish", body="blahblah", target='master', head=c0) - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') with repo: repo.post_status(prx.head, 'success', 'legal/cla') @@ -1801,7 +1813,7 @@ commits, I need to know how to merge it: c0 = repo.make_commit(m, 'C0', None, tree={'a': 'b'}) prx = repo.make_pr(title="gibberish", body=None, target='master', head=c0) - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') with repo: repo.post_status(prx.head, 'success', 'legal/cla') @@ -3077,7 +3089,7 @@ class TestBatching(object): with repo: repo.post_status('staging.master', 'success', 'ci/runbot') repo.post_status('staging.master', 'success', 'legal/cla') - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') assert pr2.state == 'merged' class TestReviewing: diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 3ac35a8f..3c68bd9e 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -493,7 +493,7 @@ def test_ff_fail(env, project, repo_a, repo_b, config): with repo_a, repo_b: repo_a.post_status('heads/staging.master', 'success') repo_b.post_status('heads/staging.master', 'success') - env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron') + env.run_crons('runbot_merge.staging_cron') assert repo_b.commit('heads/master').id == cn,\ "B should still be at the conflicting commit" assert repo_a.commit('heads/master').id == root_a,\ diff --git a/runbot_merge/tests/test_project_toggles.py b/runbot_merge/tests/test_project_toggles.py index 30ce5f78..0dffdccc 100644 --- a/runbot_merge/tests/test_project_toggles.py +++ b/runbot_merge/tests/test_project_toggles.py @@ -114,7 +114,7 @@ def test_staging_priority(env, project, repo, config, mode, cutoff, second): env[model].browse([cron_id]).write({ 'nextcall': (datetime.datetime.utcnow() + datetime.timedelta(minutes=10)).isoformat(" ", "seconds") }) - env.run_crons('runbot_merge.merge_cron') + env.run_crons(None) assert not staging.active assert not env['runbot_merge.stagings'].search([]).active assert env['runbot_merge.split'].search_count([]) == 2