From 2e05c56354cd17aa04b1c8a1be140cd0a246a2c8 Mon Sep 17 00:00:00 2001
From: Christophe Monniez <moc@odoo.com>
Date: Wed, 23 Jan 2019 10:36:26 +0100
Subject: [PATCH] [FIX] runbot: prevent build creation on job_type none

Since 8c73e6a it's possible to skip jobs from a build by using the
job_type field on the branch. If a branch job_type is set to 'none', the
builds are created but they stay in 'pending' state.

With this commit, the build is not even created if the 'job_type' is
'none'.
---
 runbot/models/build.py         |  3 +++
 runbot/tests/test_build.py     | 23 +++++++++++++++++++++--
 runbot/tests/test_job_types.py | 10 +++++-----
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/runbot/models/build.py b/runbot/models/build.py
index 748f78e9..5ae5aae7 100644
--- a/runbot/models/build.py
+++ b/runbot/models/build.py
@@ -99,6 +99,9 @@ class runbot_build(models.Model):
         raise UserError("Cannot duplicate build!")
 
     def create(self, vals):
+        branch = self.env['runbot.branch'].search([('id', '=', vals.get('branch_id', False))])
+        if branch.job_type == 'none' or vals.get('job_type', '') == 'none':
+            return self.env['runbot.build']
         build_id = super(runbot_build, self).create(vals)
         extra_info = {'sequence': build_id.id}
         job_type = vals['job_type'] if 'job_type' in vals else build_id.branch_id.job_type
diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py
index 4897f417..c0fef53b 100644
--- a/runbot/tests/test_build.py
+++ b/runbot/tests/test_build.py
@@ -120,14 +120,23 @@ class Test_Build(common.TransactionCase):
         })
         self.assertEqual(build.job_type, 'all', "job_type should be the same as the branch")
 
-    def test_build_job_type_from_branch_none(self):
+    def test_build_job_type_from_branch_testing(self):
         """test build job_type is computed from branch"""
+        self.branch.job_type = 'testing'
+        build = self.Build.create({
+            'branch_id': self.branch.id,
+            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
+        })
+        self.assertEqual(build.job_type, 'testing', "job_type should be the same as the branch")
+
+    def test_build_job_type_from_branch_none(self):
+        """test build is not even created when branch job_type is none"""
         self.branch.job_type = 'none'
         build = self.Build.create({
             'branch_id': self.branch.id,
             'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
         })
-        self.assertEqual(build.job_type, 'none', "job_type should be the same as the branch")
+        self.assertEqual(build, self.Build, "build should be an empty recordset")
 
     def test_build_job_type_can_be_set(self):
         """test build job_type can be set to something different than the one on the branch"""
@@ -139,6 +148,16 @@ class Test_Build(common.TransactionCase):
         })
         self.assertEqual(build.job_type, 'testing', "job_type should be the one set on the build")
 
+    def test_build_job_type_none(self):
+        """test build job_type set to none does not create a build"""
+        self.branch.job_type = 'running'
+        build = self.Build.create({
+            'branch_id': self.branch.id,
+            'name': 'd0d0caca0000ffffffffffffffffffffffffffff',
+            'job_type': 'none'
+        })
+        self.assertEqual(build, self.Build, "build should be an empty recordset")
+
     @patch('odoo.addons.runbot.models.branch.runbot_branch._is_on_remote')
     def test_closest_branch_01(self, mock_is_on_remote):
         """ test find a matching branch in a target repo based on branch name """
diff --git a/runbot/tests/test_job_types.py b/runbot/tests/test_job_types.py
index f25dced9..f71f2e2c 100644
--- a/runbot/tests/test_job_types.py
+++ b/runbot/tests/test_job_types.py
@@ -32,24 +32,24 @@ class Test_Jobs(common.TransactionCase):
         # test that the default job_type value executes the tests
         mock_docker_run.return_value = "Mocked run"
         ret = self.Build._job_10_test_base(self.build, '/tmp/x.log')
-        self.assertEqual("Mocked run", ret, "A branch with default job_type should run job_10")
+        self.assertEqual("Mocked run", ret, "A build with default job_type should run job_10")
 
         # test skip when job_type is none
         self.build.job_type = 'none'
         ret = self.Build._job_10_test_base(self.build, '/tmp/x.log')
-        self.assertEqual(-2, ret, "A branch with job_type 'none' should skip job_10")
+        self.assertEqual(-2, ret, "A build with job_type 'none' should skip job_10")
 
         # test skip when job_type is running
         self.build.job_type = 'running'
         ret = self.Build._job_10_test_base(self.build, '/tmp/x.log')
-        self.assertEqual(-2, ret, "A branch with job_type 'running' should skip job_10")
+        self.assertEqual(-2, ret, "A build with job_type 'running' should skip job_10")
 
         # test run when job_type is testing
         self.build.job_type = 'testing'
         ret = self.Build._job_10_test_base(self.build, '/tmp/x.log')
-        self.assertEqual("Mocked run", ret, "A branch with job_type 'testing' should run job_10")
+        self.assertEqual("Mocked run", ret, "A build with job_type 'testing' should run job_10")
 
         # test run when job_type is all
         self.build.job_type = 'all'
         ret = self.Build._job_10_test_base(self.build, '/tmp/x.log')
-        self.assertEqual("Mocked run", ret, "A branch with job_type 'all' should run job_10")
\ No newline at end of file
+        self.assertEqual("Mocked run", ret, "A build with job_type 'all' should run job_10")
\ No newline at end of file