[IMP] runbot_merge: split staging cron in two

The staging cron was already essentially split between "check if one
of the stagings is successful (and merge it)" and "check if we should
create a staging" as these were two separate loops in the cron.

But it might be useful to disable these two operations separately
e.g. we might want to stop the creation of new staging but let the
existing stagings complete.

The actual splitting is easy but it turns out a bunch of tests were
"optimised" to only run the merge cron. Most of them didn't blow up
but it seems more prudent to fix them all.

fixes odoo/runbot#310

forwardport/tests/conftest.py

@@ -13,6 +13,7 @@ def default_crons():
     return [
         'runbot_merge.process_updated_commits',
         'runbot_merge.merge_cron',
+        'runbot_merge.staging_cron',
         'forwardport.port_forward',
         'forwardport.updates',
         'runbot_merge.check_linked_prs_status',

runbot_merge/data/merge_cron.xml

@@ -1,9 +1,19 @@
 <odoo>
   <record model="ir.cron" id="merge_cron">
-    <field name="name">Check for progress of PRs &amp; Stagings</field>
+    <field name="name">Check for progress of (and merge) stagings</field>
     <field name="model_id" ref="model_runbot_merge_project"/>
     <field name="state">code</field>
-    <field name="code">model._check_progress(True)</field>
+    <field name="code">model._check_stagings(True)</field>
+    <field name="interval_number">1</field>
+    <field name="interval_type">minutes</field>
+    <field name="numbercall">-1</field>
+    <field name="doall" eval="False"/>
+  </record>
+  <record model="ir.cron" id="staging_cron">
+    <field name="name">Check for progress of PRs and create Stagings</field>
+    <field name="model_id" ref="model_runbot_merge_project"/>
+    <field name="state">code</field>
+    <field name="code">model._create_stagings(True)</field>
     <field name="interval_number">1</field>
     <field name="interval_type">minutes</field>
     <field name="numbercall">-1</field>

runbot_merge/models/pull_requests.py

@@ -64,23 +64,22 @@ class Project(models.Model):
              "will lead to webhook rejection. Should only use ASCII."
     )
 
-    def _check_progress(self, commit=False):
-        for project in self.search([]):
-            for staging in project.mapped('branch_ids.active_staging_id'):
-                staging.check_status()
-                if commit:
-                    self.env.cr.commit()
+    def _create_stagings(self, commit=False):
+        pass
 
-            for branch in project.branch_ids:
+    def _check_stagings(self, commit=False):
+        for staging in self.search([]).mapped('branch_ids.active_staging_id'):
+            staging.check_status()
+            if commit:
+                self.env.cr.commit()
+
+    def _create_stagings(self, commit=False):
+        for branch in self.search([]).mapped('branch_ids'):
+            if not branch.active_staging_id:
                 branch.try_staging()
                 if commit:
                     self.env.cr.commit()
 
-        # I have no idea why this is necessary for tests to pass, the only
-        # DB update done not through the ORM is when receiving a notification
-        # that a PR has been closed
-        self.invalidate_cache()
-
     def _send_feedback(self):
         Repos = self.env['runbot_merge.repository']
         ghs = {}

runbot_merge/tests/conftest.py

@@ -21,8 +21,10 @@ def default_crons():
         'runbot_merge.fetch_prs_cron',
         # env['runbot_merge.commit']._notify()
         'runbot_merge.process_updated_commits',
-        # env['runbot_merge.project']._check_progress()
+        # 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()
         'runbot_merge.check_linked_prs_status',
         # env['runbot_merge.project']._send_feedback()

runbot_merge/tests/test_basic.py

@@ -464,7 +464,7 @@ def test_staging_ci_timeout(env, repo, config):
     timeout = env['runbot_merge.project'].search([]).ci_timeout
 
     pr1.staging_id.staged_at = odoo.fields.Datetime.to_string(datetime.datetime.now() - datetime.timedelta(minutes=2*timeout))
-    env.run_crons('runbot_merge.merge_cron')
+    env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron')
     assert pr1.state == 'error', "timeout should fail the PR"
 
 def test_timeout_bump_on_pending(env, repo, config):
@@ -1068,7 +1068,7 @@ class TestRetry:
             ('repository.name', '=', repo.name),
             ('number', '=', prx.number)
         ]).state == 'ready'
-        env.run_crons('runbot_merge.merge_cron')
+        env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron')
 
         staging_head2 = repo.commit('heads/staging.master')
         assert staging_head2 != staging_head
@@ -1496,7 +1496,7 @@ class TestMergeMethod:
 
             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')
+        env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron')
 
         with repo:
             repo.post_status(prx.head, 'success', 'legal/cla')
@@ -1538,7 +1538,7 @@ class TestMergeMethod:
 
             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')
+        env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron')
 
         with repo:
             repo.post_status(prx.head, 'success', 'legal/cla')
@@ -2379,7 +2379,7 @@ class TestBatching(object):
         with repo:
             repo.post_status(h, 'success', 'ci/runbot')
             repo.post_status(h, 'success', 'legal/cla')
-        env.run_crons('runbot_merge.process_updated_commits', 'runbot_merge.merge_cron')
+        env.run_crons('runbot_merge.process_updated_commits', 'runbot_merge.merge_cron', 'runbot_merge.staging_cron')
         assert pr2.state == 'merged'
 
 class TestReviewing(object):
@@ -2630,7 +2630,7 @@ class TestUnknownPR:
         ])
         assert pr.state == 'ready'
 
-        env.run_crons('runbot_merge.merge_cron')
+        env.run_crons('runbot_merge.merge_cron', 'runbot_merge.staging_cron')
         assert pr.staging_id
 
     def test_rplus_unmanaged(self, env, repo, users, config):

runbot_merge/tests/test_multirepo.py

@@ -366,7 +366,7 @@ def test_ff_fail(env, project, repo_a, repo_b, config):
         repo_a.post_status('heads/staging.master', 'success', 'legal/cla')
         repo_b.post_status('heads/staging.master', 'success', 'ci/runbot')
         repo_b.post_status('heads/staging.master', 'success', 'legal/cla')
-    env.run_crons('runbot_merge.merge_cron')
+    env.run_crons('runbot_merge.merge_cron', '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,\