diff --git a/runbot/models/build.py b/runbot/models/build.py index d5e388ec..62e27ccb 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -72,6 +72,8 @@ class runbot_build(models.Model): ('exact', 'branch/PR exact name'), ('prefix', 'branch whose name is a prefix of current one'), ('fuzzy', 'Fuzzy - common ancestor found'), + ('exact PR', 'Exact match between two PR'), + ('no PR', 'PR matching a branch without PR'), ('default', 'No match found - defaults to master')], string='Server branch matching') revdep_build_ids = fields.Many2many('runbot.build', 'runbot_rev_dep_builds', @@ -108,25 +110,28 @@ class runbot_build(models.Model): extra_info.update({'job_type': job_type}) context = self.env.context - # detect duplicate - duplicate_id = None - domain = [ - ('repo_id', '=', build_id.repo_id.duplicate_id.id), - ('name', '=', build_id.name), - ('duplicate_id', '=', False), - '|', ('result', '=', False), ('result', '!=', 'skipped') - ] + if not context.get('force_rebuild'): + # detect duplicate + duplicate_id = None + domain = [ + ('repo_id', '=', build_id.repo_id.duplicate_id.id), + ('name', '=', build_id.name), + ('duplicate_id', '=', False), + '|', ('result', '=', False), ('result', '!=', 'skipped') + ] + # TODO xdo extract build_id._get_closest_branch_name(extra_repo.id) here + for duplicate in self.search(domain, limit=10): + duplicate_id = duplicate.id + # Consider the duplicate if its closest branches are the same than the current build closest branches. + for extra_repo in build_id.repo_id.dependency_ids: + build_closest_name = build_id._get_closest_branch_name(extra_repo.id)[1] + duplicate_closest_name = duplicate._get_closest_branch_name(extra_repo.id)[1] + if build_closest_name != duplicate_closest_name: + duplicate_id = None + if duplicate_id: + extra_info.update({'state': 'duplicate', 'duplicate_id': duplicate_id}) + break - for duplicate in self.search(domain, limit=1): - duplicate_id = duplicate.id - # Consider the duplicate if its closest branches are the same than the current build closest branches. - for extra_repo in build_id.repo_id.dependency_ids: - build_closest_name = build_id._get_closest_branch_name(extra_repo.id)[1] - duplicate_closest_name = duplicate._get_closest_branch_name(extra_repo.id)[1] - if build_closest_name != duplicate_closest_name: - duplicate_id = None - if duplicate_id and not context.get('force_rebuild'): - extra_info.update({'state': 'duplicate', 'duplicate_id': duplicate_id}) build_id.write(extra_info) if build_id.state == 'duplicate' and build_id.duplicate_id.state in ('running', 'done'): build_id._github_status() @@ -203,7 +208,11 @@ class runbot_build(models.Model): for pull in Branch.browse([pu['id'] for pu in pulls]): pi = pull._get_pull_info() if pi.get('state') == 'open': - return (pull.repo_id.id, pull.name, 'exact') + if ':' in name: # we assume that branch exists if we got pull info + pr_branch_name = name.split(':')[1] + return (pull.repo_id.duplicate_id.id, 'refs/heads/%s' % pr_branch_name, 'exact PR') + else: + return (pull.repo_id.id, pull.name, 'exact PR') # 3. Match a branch which is the dashed-prefix of current branch name branches = Branch.search_read( @@ -216,8 +225,22 @@ class runbot_build(models.Model): if name.startswith(branch['branch_name'] + '-') and self._branch_exists(branch['id']): return result_for(branch, 'prefix') - # 4. last-resort value - return target_repo_id, target_branch, 'default' + # 4.Match a PR in enterprise without community PR + if build.branch_id.name.startswith('refs/pull') and ':' in name: + pr_branch_name = name.split(':')[1] + duplicate_branch_name = 'refs/heads/%s' % pr_branch_name + domain = [ + ('repo_id', 'in', target_repo_ids), # target_repo_ids should contain the target duplicate repo + ('branch_name', '=', pr_branch_name), + ('pull_head_name', '=', False), + ] + targets = Branch.search_read(domain, fields, order='id DESC') + targets = sorted(targets, key=sort_by_repo) + if targets and self._branch_exists(targets[0]['id']): + return result_for(targets[0], 'no PR') + + # 5. last-resort value + return target_repo_id, 'refs/heads/%s' % target_branch, 'default' @api.depends('name', 'branch_id.name') def _get_dest(self): diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index f28f797a..3168fd13 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -3,6 +3,7 @@ from unittest.mock import patch from odoo.tools.config import configmanager from odoo.tests import common + class Test_Build(common.TransactionCase): def setUp(self): @@ -29,7 +30,7 @@ class Test_Build(common.TransactionCase): build = self.Build.create({ 'branch_id': self.branch.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - 'port' : '1234', + 'port': '1234', }) self.assertEqual(build.id, build.sequence) self.assertEqual(build.dest, '%05d-master-d0d0ca' % build.id) @@ -58,60 +59,6 @@ class Test_Build(common.TransactionCase): cmd = build._cmd()[0] self.assertIn('--log-db=%s' % uri, cmd) - def test_pr_is_duplicate(self): - """ test PR is a duplicate of a dev branch build """ - dup_repo = self.Repo.create({ - 'name': 'bla@example.com:foo-dev/bar', - 'duplicate_id': self.repo.id - }) - self.repo.duplicate_id = dup_repo.id - dev_branch = self.Branch.create({ - 'repo_id': dup_repo.id, - 'name': 'refs/heads/10.0-fix-thing-moc' - }) - dev_build = self.Build.create({ - 'branch_id': dev_branch.id, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) - pr = self.Branch.create({ - 'repo_id': self.repo.id, - 'name': 'refs/pull/12345' - }) - - pr_build = self.Build.create({ - 'branch_id': pr.id, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) - self.assertEqual(pr_build.state, 'duplicate') - self.assertEqual(pr_build.duplicate_id.id, dev_build.id) - - def test_dev_is_duplicate(self): - """ test dev branch build is a duplicate of a PR """ - dup_repo = self.Repo.create({ - 'name': 'bla@example.com:foo-dev/bar', - 'duplicate_id': self.repo.id - }) - self.repo.duplicate_id = dup_repo.id - dev_branch = self.Branch.create({ - 'repo_id': dup_repo.id, - 'name': 'refs/heads/10.0-fix-thing-moc' - }) - pr = self.Branch.create({ - 'repo_id': self.repo.id, - 'name': 'refs/pull/12345' - }) - - pr_build = self.Build.create({ - 'branch_id': pr.id, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) - dev_build = self.Build.create({ - 'branch_id': dev_branch.id, - 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', - }) - self.assertEqual(dev_build.state, 'duplicate') - self.assertEqual(dev_build.duplicate_id.id, pr_build.id) - def test_build_job_type_from_branch_default(self): """test build job_type is computed from branch default job_type""" build = self.Build.create({ @@ -181,109 +128,355 @@ class Test_Build(common.TransactionCase): log_first_part = '%s skip %%s' % (other_build.dest) mock_logger.debug.assert_called_with(log_first_part, 'A good reason') + +class TestClosestBranch(common.TransactionCase): + + def branch_description(self, branch): + branch_type = 'pull' if 'pull' in branch.name else 'branch' + return '%s %s:%s' % (branch_type, branch.repo_id.name.split(':')[-1], branch.name.split('/')[-1]) + + def assertClosest(self, build, closest): + extra_repo = build.repo_id.dependency_ids[0] + self.assertEqual(closest, build._get_closest_branch_name(extra_repo.id), "build on %s didn't had the extected closest branch" % self.branch_description(build.branch_id)) + + def assertDuplicate(self, branch1, branch2, b1_closest=None, b2_closest=None): + """ + Test that the creation of a build on branch1 and branch2 detects duplicate, no matter the order. + Also test that build on branch1 closest_branch_name result is b1_closest if given + Also test that build on branch2 closest_branch_name result is b2_closest if given + """ + closest = { + branch1: b1_closest, + branch2: b2_closest, + } + for b1, b2 in [(branch1, branch2), (branch2, branch1)]: + hash = '%s%s' % (b1.name, b2.name) + build1 = self.Build.create({ + 'branch_id': b1.id, + 'name': hash, + }) + + if b1_closest: + self.assertClosest(build1, closest[b1]) + + build2 = self.Build.create({ + 'branch_id': b2.id, + 'name': hash, + }) + + if b2_closest: + self.assertClosest(build2, closest[b2]) + + self.assertEqual(build2.duplicate_id.id, build1.id, "build on %s wasn't detected as duplicate of build on %s" % (self.branch_description(b2), self.branch_description(b1))) + self.assertEqual(build2.state, 'duplicate') + + def setUp(self): + """ Setup repositories that mimick the Odoo repos """ + super(TestClosestBranch, self).setUp() + self.Repo = self.env['runbot.repo'] + self.community_repo = self.Repo.create({'name': 'bla@example.com:odoo/odoo', 'token': '1'}) + self.enterprise_repo = self.Repo.create({'name': 'bla@example.com:odoo/enterprise', 'token': '1'}) + self.community_dev_repo = self.Repo.create({'name': 'bla@example.com:odoo-dev/odoo', 'token': '1'}) + self.enterprise_dev_repo = self.Repo.create({'name': 'bla@example.com:odoo-dev/enterprise', 'token': '1'}) + + # tweak duplicates links between repos + self.community_repo.duplicate_id = self.community_dev_repo.id + self.community_dev_repo.duplicate_id = self.community_repo.id + self.enterprise_repo.duplicate_id = self.enterprise_dev_repo.id + self.enterprise_dev_repo.duplicate_id = self.enterprise_repo.id + + # create depenedencies to find Odoo server + self.enterprise_repo.dependency_ids = self.community_repo + self.enterprise_dev_repo.dependency_ids = self.community_dev_repo + + # Create some sticky branches + self.Branch = self.env['runbot.branch'] + self.branch_odoo_master = self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/heads/master' + }) + self.branch_odoo_10 = self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/heads/10.0' + }) + self.branch_odoo_11 = self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/heads/11.0' + }) + + self.branch_enterprise_master = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/heads/master' + }) + self.branch_enterprise_10 = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/heads/10.0' + }) + self.branch_enterprise_11 = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/heads/11.0' + }) + + self.Build = self.env['runbot.build'] + + @patch('odoo.addons.runbot.models.repo.runbot_repo._github') + def test_pr_is_duplicate(self, mock_github): + """ test PR is a duplicate of a dev branch build """ + + mock_github.return_value = { + 'head': {'label': 'odoo-dev:10.0-fix-thing-moc'}, + 'base': {'ref': '10.0'}, + 'state': 'open' + } + + dev_branch = self.Branch.create({ + 'repo_id': self.community_dev_repo.id, + 'name': 'refs/heads/10.0-fix-thing-moc' + }) + pr = self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/pull/12345' + }) + self.assertDuplicate(dev_branch, pr) + @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 """ mock_is_on_remote.return_value = True - server_repo = self.Repo.create({'name': 'bla@example.com:foo-dev/bar'}) - addons_repo = self.Repo.create({'name': 'bla@example.com:ent-dev/bar'}) self.Branch.create({ - 'repo_id': server_repo.id, + 'repo_id': self.community_dev_repo.id, 'name': 'refs/heads/10.0-fix-thing-moc' }) addons_branch = self.Branch.create({ - 'repo_id': addons_repo.id, + 'repo_id': self.enterprise_dev_repo.id, 'name': 'refs/heads/10.0-fix-thing-moc' }) addons_build = self.Build.create({ 'branch_id': addons_branch.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', }) - self.assertEqual((server_repo.id, addons_branch.name, 'exact'), addons_build._get_closest_branch_name(server_repo.id)) + self.assertEqual((self.enterprise_dev_repo.id, addons_branch.name, 'exact'), addons_build._get_closest_branch_name(self.enterprise_dev_repo.id)) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') def test_closest_branch_02(self, mock_github): """ test find two matching PR having the same head name """ mock_github.return_value = { - 'head' : {'label': 'foo-dev:bar_branch'}, - 'base' : {'ref': 'master'}, + # "head label" is the repo:branch where the PR comes from + # "base ref" is the target of the PR + 'head': {'label': 'odoo-dev:bar_branch'}, + 'base': {'ref': 'saas-12.2'}, 'state': 'open' } - server_repo = self.Repo.create({'name': 'bla@example.com:foo-dev/bar', 'token': '1'}) - addons_repo = self.Repo.create({'name': 'bla@example.com:ent-dev/bar', 'token': '1'}) - server_pr = self.Branch.create({ - 'repo_id': server_repo.id, + + # Create PR in community + community_pr = self.Branch.create({ + 'repo_id': self.community_repo.id, 'name': 'refs/pull/123456' }) - addons_pr = self.Branch.create({ - 'repo_id': addons_repo.id, + enterprise_pr = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, 'name': 'refs/pull/789101' }) - addons_build = self.Build.create({ - 'branch_id': addons_pr.id, + enterprise_build = self.Build.create({ + 'branch_id': enterprise_pr.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', }) - self.assertEqual((server_repo.id, server_pr.name, 'exact'), addons_build._get_closest_branch_name(server_repo.id)) + + self.assertEqual((self.community_dev_repo.id, 'refs/heads/bar_branch', 'exact PR'), enterprise_build._get_closest_branch_name(self.community_repo.id)) + + @patch('odoo.addons.runbot.models.repo.runbot_repo._github') + @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') + def test_closest_branch_02_improved(self, mock_branch_exists, mock_github): + """ test that a PR in enterprise with a matching PR in Community + uses the matching one""" + mock_branch_exists.return_value = True + + self.Branch.create({ + 'repo_id': self.community_dev_repo.id, + 'name': 'refs/heads/saas-12.2-blabla' + }) + + ent_dev_branch = self.Branch.create({ + 'repo_id': self.enterprise_dev_repo.id, + 'name': 'refs/heads/saas-12.2-blabla' + }) + + def github_side_effect(url, **kwargs): + # "head label" is the repo:branch where the PR comes from + # "base ref" is the target of the PR + if url.endswith('/pulls/3721'): + return { + 'head': {'label': 'odoo-dev:saas-12.2-blabla'}, + 'base': {'ref': 'saas-12.2'}, + 'state': 'open' + } + elif url.endswith('/pulls/32156'): + return { + 'head': {'label': 'odoo-dev:saas-12.2-blabla'}, + 'base': {'ref': 'saas-12.2'}, + 'state': 'open' + } + else: + self.assertTrue(False) + + mock_github.side_effect = github_side_effect + + ent_pr = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/pull/3721' + }) + + self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/pull/32156' + }) + + self.assertDuplicate( + ent_dev_branch, + ent_pr, + (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'), + (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact PR') + ) @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') def test_closest_branch_03(self, mock_branch_exists): """ test find a branch based on dashed prefix""" mock_branch_exists.return_value = True - addons_repo = self.Repo.create({'name': 'bla@example.com:ent-dev/bar', 'token': '1'}) addons_branch = self.Branch.create({ - 'repo_id': addons_repo.id, + 'repo_id': self.enterprise_dev_repo.id, 'name': 'refs/heads/10.0-fix-blah-blah-moc' }) addons_build = self.Build.create({ 'branch_id': addons_branch.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', }) - self.assertEqual((self.repo.id, 'refs/heads/10.0', 'prefix'), addons_build._get_closest_branch_name(self.repo.id)) + self.assertEqual((self.community_repo.id, 'refs/heads/10.0', 'prefix'), addons_build._get_closest_branch_name(self.community_repo.id)) + + @patch('odoo.addons.runbot.models.repo.runbot_repo._github') + @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') + def test_closest_branch_03_05(self, mock_branch_exists, mock_github): + """ test that a PR in enterprise without a matching PR in Community + and no branch in community""" + mock_branch_exists.return_value = True + # comm_repo = self.repo + # self.repo.write({'token': 1}) + + ent_dev_branch = self.Branch.create({ + 'repo_id': self.enterprise_dev_repo.id, + 'name': 'refs/heads/saas-12.2-blabla' + }) + + def github_side_effect(url, **kwargs): + if url.endswith('/pulls/3721'): + return { + 'head': {'label': 'odoo-dev:saas-12.2-blabla'}, + 'base': {'ref': 'saas-12.2'}, + 'state': 'open' + } + elif url.endswith('/pulls/32156'): + return { + 'head': {'label': 'odoo-dev:saas-12.2-blabla'}, + 'base': {'ref': 'saas-12.2'}, + 'state': 'open' + } + else: + self.assertTrue(False) + + mock_github.side_effect = github_side_effect + + self.Branch.create({ + 'repo_id': self.community_repo.id, + 'name': 'refs/heads/saas-12.2' + }) + + ent_pr = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/pull/3721' + }) + + self.assertDuplicate( + ent_pr, + ent_dev_branch, + (self.community_repo.id, 'refs/heads/saas-12.2', 'default'), + (self.community_repo.id, 'refs/heads/saas-12.2', 'prefix'), + ) + + @patch('odoo.addons.runbot.models.repo.runbot_repo._github') + @patch('odoo.addons.runbot.models.build.runbot_build._branch_exists') + def test_closest_branch_04(self, mock_branch_exists, mock_github): + """ test that a PR in enterprise without a matching PR in Community + uses the corresponding exact branch in community""" + mock_branch_exists.return_value = True + + self.Branch.create({ + 'repo_id': self.community_dev_repo.id, + 'name': 'refs/heads/saas-12.2-blabla' + }) + + ent_dev_branch = self.Branch.create({ + 'repo_id': self.enterprise_dev_repo.id, + 'name': 'refs/heads/saas-12.2-blabla' + }) + + def github_side_effect(*args, **kwargs): + return { + 'head': {'label': 'ent-dev:saas-12.2-blabla'}, + 'base': {'ref': 'saas-12.2'}, + 'state': 'open' + } + + mock_github.side_effect = github_side_effect + + ent_pr = self.Branch.create({ + 'repo_id': self.enterprise_repo.id, + 'name': 'refs/pull/3721' + }) + + self.assertDuplicate( + ent_dev_branch, + ent_pr, + (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'exact'), + (self.community_dev_repo.id, 'refs/heads/saas-12.2-blabla', 'no PR') + ) @patch('odoo.addons.runbot.models.repo.runbot_repo._github') def test_closest_branch_05(self, mock_github): """ test last resort value """ mock_github.return_value = { - 'head' : {'label': 'foo-dev:bar_branch'}, - 'base' : {'ref': '10.0'}, + 'head': {'label': 'foo-dev:bar_branch'}, + 'base': {'ref': '10.0'}, 'state': 'open' } - server_repo = self.Repo.create({'name': 'bla@example.com:foo-dev/bar', 'token': '1'}) - addons_repo = self.Repo.create({'name': 'bla@example.com:ent-dev/bar', 'token': '1'}) + server_pr = self.Branch.create({ - 'repo_id': server_repo.id, + 'repo_id': self.community_repo.id, 'name': 'refs/pull/123456' }) mock_github.return_value = { - 'head' : {'label': 'foo-dev:foobar_branch'}, - 'base' : {'ref': '10.0'}, + 'head': {'label': 'foo-dev:foobar_branch'}, + 'base': {'ref': '10.0'}, 'state': 'open' } addons_pr = self.Branch.create({ - 'repo_id': addons_repo.id, + 'repo_id': self.enterprise_repo.id, 'name': 'refs/pull/789101' }) addons_build = self.Build.create({ 'branch_id': addons_pr.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', }) - self.assertEqual((server_repo.id, server_pr.target_branch_name, 'default'), addons_build._get_closest_branch_name(server_repo.id)) + self.assertEqual((self.community_repo.id, 'refs/heads/%s' % server_pr.target_branch_name, 'default'), addons_build._get_closest_branch_name(self.community_repo.id)) def test_closest_branch_05_master(self): """ test last resort value when nothing common can be found""" - server_repo = self.Repo.create({'name': 'bla@example.com:foo-dev/bar', 'token': '1'}) - addons_repo = self.Repo.create({'name': 'bla@example.com:ent-dev/bar', 'token': '1'}) - server_pr = self.Branch.create({ - 'repo_id': server_repo.id, - 'name': 'refs/heads/10.0' - }) - addons_pr = self.Branch.create({ - 'repo_id': addons_repo.id, + + addons_branch = self.Branch.create({ + 'repo_id': self.enterprise_dev_repo.id, 'name': 'refs/head/badref-fix-foo' }) addons_build = self.Build.create({ - 'branch_id': addons_pr.id, + 'branch_id': addons_branch.id, 'name': 'd0d0caca0000ffffffffffffffffffffffffffff', }) - self.assertEqual((server_repo.id, 'master', 'default'), addons_build._get_closest_branch_name(server_repo.id)) + self.assertEqual((self.community_repo.id, 'refs/heads/master', 'default'), addons_build._get_closest_branch_name(self.community_repo.id))