From 54ecee8c4e1a207ca38cf5d94ee077eb9ab6e8df Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Tue, 12 Nov 2019 15:25:23 +0100 Subject: [PATCH] [IMP] runbot: create build when old HEAD is force-pushed _find_new_commits will check if a build exists with current branch HEAD before creating build. This is crutial to avoid to create a new build at each loop turn. The problem is that in some rare cases, when force-pushing an old head on a branch, the build won't appear and the only way to update the branch is to find the corresponding build that may be hidden in hitory. This may be confusing for the user that will rebuild the created build with a commit that doesn't represent the head of the branch. This commit only search for the last build of each branch, in order to only skip build creation if the last build as the same hash. The new created build should be marked as the duplicate of the first one. --- runbot/models/repo.py | 7 +++---- runbot/tests/test_repo.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/runbot/models/repo.py b/runbot/models/repo.py index f13c5a09..af448227 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -270,10 +270,9 @@ class runbot_repo(models.Model): max_age = int(icp.get_param('runbot.runbot_max_age', default=30)) self.env.cr.execute(""" - WITH t (build, branch_id) AS (SELECT unnest(%s), unnest(%s)) - SELECT b.name, b.branch_id - FROM t LEFT JOIN runbot_build b ON (b.name = t.build) AND (b.branch_id = t.branch_id) - """, ([r[1] for r in refs], [ref_branches[r[0]] for r in refs])) + SELECT DISTINCT ON (branch_id) name, branch_id + FROM runbot_build WHERE branch_id in %s ORDER BY branch_id,id DESC; + """, (tuple([ref_branches[r[0]] for r in refs]),)) # generate a set of tuples (branch_id, sha) builds_candidates = {(r[1], r[0]) for r in self.env.cr.fetchall()} diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py index 54e831b9..91d44276 100644 --- a/runbot/tests/test_repo.py +++ b/runbot/tests/test_repo.py @@ -57,7 +57,7 @@ class Test_Repo(common.TransactionCase): 'name': 'refs/heads/bidon' }) - self.commit_list = [('refs/heads/bidon', + first_commit = [('refs/heads/bidon', 'd0d0caca', datetime.datetime.now().strftime("%Y-%m-%d, %H:%M:%S"), 'Marc Bidule', @@ -65,7 +65,15 @@ class Test_Repo(common.TransactionCase): 'A nice subject', 'Marc Bidule', '')] - mock_fetch_head_time.side_effect = [100000.0, 100001.0, 100002.0] + self.commit_list = first_commit + + def counter(): + i = 100000 + while True: + i += 1 + yield i + + mock_fetch_head_time.side_effect = counter() with patch('odoo.addons.runbot.models.repo.runbot_repo._git', new=self.mock_git_helper()): repo._create_pending_builds() @@ -117,6 +125,7 @@ class Test_Repo(common.TransactionCase): self.assertEqual(branch_count, 1, 'No new branch should have been created') build = self.env['runbot.build'].search([('repo_id', '=', repo.id), ('branch_id', '=', branch.id), ('name', '=', 'b00b')]) + self.assertEqual(len(build), 1) self.assertEqual(build.subject, 'Another subject') self.assertEqual(build.local_state, 'pending') self.assertFalse(build.local_result) @@ -125,6 +134,23 @@ class Test_Repo(common.TransactionCase): self.assertEqual(previous_build.local_state, 'done', 'Previous pending build should be done') self.assertEqual(previous_build.local_result, 'skipped', 'Previous pending build result should be skipped') + self.commit_list = first_commit # branch reseted hard to an old commit + builds = self.env['runbot.build'].search([('repo_id', '=', repo.id), ('branch_id', '=', branch.id), ('name', '=', 'd0d0caca')]) + self.assertEqual(len(builds), 1) + with patch('odoo.addons.runbot.models.repo.runbot_repo._git', new=self.mock_git_helper()): + repo._create_pending_builds() + + last_build = self.env['runbot.build'].search([], limit=1) + self.assertEqual(last_build.name, 'd0d0caca') + builds = self.env['runbot.build'].search([('repo_id', '=', repo.id), ('branch_id', '=', branch.id), ('name', '=', 'd0d0caca')]) + self.assertEqual(len(builds), 2) + # self.assertEqual(last_build.duplicate_id, previous_build) False because previous_build is skipped + with patch('odoo.addons.runbot.models.repo.runbot_repo._git', new=self.mock_git_helper()): + other_repo._create_pending_builds() + builds = self.env['runbot.build'].search([('repo_id', '=', repo.id), ('branch_id', '=', branch.id), ('name', '=', 'd0d0caca')]) + self.assertEqual(len(builds), 2) + + @skip('This test is for performances. It needs a lot of real branches in DB to mean something') @patch('odoo.addons.runbot.models.repo.runbot_repo._root') def test_repo_perf_find_new_commits(self, mock_root):