diff --git a/bert_e/exceptions.py b/bert_e/exceptions.py index 445c410c..fb93bdcf 100644 --- a/bert_e/exceptions.py +++ b/bert_e/exceptions.py @@ -290,6 +290,17 @@ def __init__(self, name): super(BranchNameInvalid, self).__init__(msg) +class ReleaseAlreadyExists(InternalException): + code = 204 + + def __init__(self, branch, tag): + msg = 'Branch %r must be deleted as %r has been created, ' \ + 'you must use a hotfix branch if you really intend ' \ + 'to target this version.' \ + % (branch, tag) + super(ReleaseAlreadyExists, self).__init__(msg) + + class UnrecognizedBranchPattern(InternalException): code = 207 @@ -435,10 +446,21 @@ class MasterQueueMissing(QueueValidationError): code = 'Q001' auto_recovery = False + @staticmethod + def _format_version(version): + if not version: + return '[]' + if len(version) == 1 or (len(version) > 1 and version[1] is None): + return f"{version[0]}" + if len(version) == 2 or (len(version) > 2 and version[2] is None): + return f"{version[0]}.{version[1]}" + return f"{version[0]}.{version[1]}.{version[2]}" + def __init__(self, version): - msg = 'there are integration queues on this version ' \ - 'but q/%s.%s is missing' % version - super(MasterQueueMissing, self).__init__(msg) + version_str = self._format_version(version) + msg = "there are integration queues on " \ + f"this version but q/{version_str} is missing" + super().__init__(msg) class MasterQueueLateVsDev(QueueValidationError): diff --git a/bert_e/tests/test_bert_e.py b/bert_e/tests/test_bert_e.py index 0551077e..828807bf 100644 --- a/bert_e/tests/test_bert_e.py +++ b/bert_e/tests/test_bert_e.py @@ -104,8 +104,9 @@ def initialize_git_repo(repo, username, usermail): repo.cmd('git add a') repo.cmd('git commit -m "Initial commit"') repo.cmd('git remote add origin ' + repo._url) - for major, minor, micro in [(4, 3, 18), (5, 1, 4), (10, 0, 0)]: + for major, minor, micro in [(4, 3, 18), (5, 1, 4), (10, 0, 1)]: major_minor = "%s.%s" % (major, minor) + major_minor_micro = "%s.%s.%s" % (major, minor, micro) create_branch(repo, 'release/' + major_minor, do_push=False) if major != 10: create_branch(repo, 'hotfix/%s.%s.%s' % @@ -119,11 +120,15 @@ def initialize_git_repo(repo, username, usermail): do_push=False) else: create_branch(repo, 'hotfix/10.0.0', do_push=False) + create_branch(repo, f'development/{major}.{minor}.{micro}', + file_=True, do_push=False) create_branch(repo, 'development/' + major_minor, + f'development/{major_minor_micro}', file_=True, do_push=False) create_branch(repo, f'development/{major}', f'development/{major_minor}', file_=True, do_push=False) + if major != 6 and major != 10: repo.cmd('git tag %s.%s.%s', major, minor, micro - 1) @@ -406,7 +411,7 @@ def test_branch_cascade_target_middle_dev(self): branches = OrderedDict({ 1: {'name': 'development/4.3', 'ignore': True}, 2: {'name': 'development/5.1', 'ignore': False}, - 3: {'name': 'development/10.0', 'ignore': False} + 3: {'name': 'development/10.0', 'ignore': False}, }) tags = ['4.3.16', '4.3.17', '5.1.3'] fixver = ['5.1.4', '10.0.0'] @@ -450,10 +455,116 @@ def test_branch_cascade_target_hotfix(self): fixver = ['6.6.6.3'] self.finalize_cascade(branches, tags, destination, fixver) - def test_branch_cascade_invalid_dev_branch(self): + def test_branch_cascade_target_three_digit_dev(self): + """Test cascade targeting three-digit development branch""" + destination = 'development/4.3.17' + branches = OrderedDict({ + 1: {'name': 'development/4.3.17', 'ignore': False}, + 2: {'name': 'development/4.3', 'ignore': False}, + 3: {'name': 'development/5.1', 'ignore': False}, + 4: {'name': 'development/10.0', 'ignore': False} + }) + tags = ['4.3.14', '4.3.16', '5.1.3'] + fixver = ['4.3.17', '4.3.18', '5.1.4', '10.0.0'] + self.finalize_cascade(branches, tags, destination, fixver) + + def test_branch_cascade_with_two_three_digit_dev(self): + """Test cascade targeting three-digit development branch""" + destination = 'development/4.3.17' + branches = OrderedDict({ + 1: {'name': 'development/4.3.15', 'ignore': True}, + 2: {'name': 'development/4.3.17', 'ignore': False}, + 3: {'name': 'development/4.3', 'ignore': False}, + 4: {'name': 'development/5.1.1', 'ignore': False}, + 5: {'name': 'development/5.1', 'ignore': False}, + 6: {'name': 'development/10.0', 'ignore': False} + }) + tags = ['4.3.14', '4.3.16', '5.1.3'] + fixver = ['4.3.17', '4.3.18', '5.1.1', '5.1.4', '10.0.0'] + self.finalize_cascade(branches, tags, destination, fixver) + + def test_four_digit_fix_version(self): + """Test handling dev/x.y.z with existing x.y.z tag""" + destination = 'development/4.3.17' + branches = OrderedDict({ + 1: {'name': 'development/4.3.17', 'ignore': False}, + 2: {'name': 'development/4.3', 'ignore': False}, + 3: {'name': 'development/5.1.8', 'ignore': False}, + 4: {'name': 'development/5.1', 'ignore': False}, + 5: {'name': 'development/10.0', 'ignore': False} + }) + tags = ['4.3.17.0', '4.3.18', '5.1.3', '5.1.7'] + fixver = ['4.3.17', '4.3.19', '5.1.8', '5.1.9', '10.0.0'] + with self.assertRaises(exns.ReleaseAlreadyExists): + self.finalize_cascade(branches, tags, destination, fixver) + + def test_branch_cascade_with_three_digit_dev_and_hf(self): + """Test cascade targeting three-digit development branch""" destination = 'development/4.3.17' branches = OrderedDict({ - 1: {'name': 'development/4.3.17', 'ignore': False} + 1: {'name': 'development/4.3.15', 'ignore': True}, + 2: {'name': 'development/4.3.17', 'ignore': False}, + 3: {'name': 'development/4.3', 'ignore': False}, + 4: {'name': 'development/5.1.1', 'ignore': False}, + 5: {'name': 'hotfix/5.1.3', 'ignore': True}, + 6: {'name': 'development/5.1', 'ignore': False}, + 7: {'name': 'development/10.0', 'ignore': False} + }) + tags = ['4.3.14', '4.3.16', '5.1.3.1', '5.1.5'] + fixver = ['4.3.17', '4.3.18', '5.1.1', '5.1.6', '10.0.0'] + self.finalize_cascade(branches, tags, destination, fixver) + + def test_branch_cascade_hotfix_and_development_three_digit(self): + destination = 'hotfix/4.3.18' + branches = OrderedDict({ + 1: {'name': 'development/4.3.18', 'ignore': True}, + 2: {'name': 'development/4.3', 'ignore': True}, + 5: {'name': 'hotfix/4.3.18', 'ignore': False}, + }) + tags = ['4.3.16', '4.3.17', '4.3.18'] + fixver = ['4.3.18.1'] + with self.assertRaises(exns.ReleaseAlreadyExists): + self.finalize_cascade(branches, tags, destination, fixver) + + destination = 'development/4.3.18' + branches = OrderedDict({ + 1: {'name': 'development/4.3.18', 'ignore': False}, + 2: {'name': 'development/4.3', 'ignore': False}, + 5: {'name': 'hotfix/4.3.18', 'ignore': True}, + }) + tags = ['4.3.16', '4.3.17', '4.3.18'] + fixver = ['4.3.18'] + with self.assertRaises(exns.ReleaseAlreadyExists): + self.finalize_cascade(branches, tags, destination, fixver) + + destination = 'hotfix/4.3.18' + branches = OrderedDict({ + 1: {'name': 'development/4.3.19', 'ignore': True}, + 2: {'name': 'development/4.3', 'ignore': True}, + 5: {'name': 'hotfix/4.3.18', 'ignore': False}, + }) + tags = ['4.3.18'] + fixver = ['4.3.18.1'] + self.finalize_cascade(branches, tags, destination, fixver) + + def test_branch_cascade_mixed_versions(self): + """Test cascade with mix of 2-digit and 3-digit development branches""" + destination = 'development/5.1' + branches = OrderedDict({ + 1: {'name': 'development/4.3.17', 'ignore': True}, + 2: {'name': 'development/4.3', 'ignore': True}, + 3: {'name': 'development/5.1.8', 'ignore': True}, + 4: {'name': 'development/5.1', 'ignore': False}, + 5: {'name': 'development/10.0', 'ignore': False} + }) + tags = ['4.3.16', '4.3.18', '5.1.3', '5.1.7'] + fixver = ['5.1.9', '10.0.0'] + self.finalize_cascade(branches, tags, destination, fixver) + + def test_branch_cascade_invalid_dev_branch(self): + destination = 'development/4.3.17.1' + branches = OrderedDict({ + 1: {'name': 'development/4.3.17.1', 'ignore': False} }) tags = [] fixver = [] @@ -1037,9 +1148,10 @@ def test_create_integration_pr_manually(self): # Ensure that all PRs have been created self.admin_bb.get_pull_request(pull_request_id=pr.id + 1) self.admin_bb.get_pull_request(pull_request_id=pr.id + 2) - # Only two integration PRs should have been created + self.admin_bb.get_pull_request(pull_request_id=pr.id + 3) + # Only three integration PRs should have been created with self.assertRaises(Exception): - self.admin_bb.get_pull_request(pull_request_id=pr.id + 3) + self.admin_bb.get_pull_request(pull_request_id=pr.id + 4) def test_comments_without_integration_pull_requests(self): """Test Bert-E PR comments on the latest development branch. @@ -1370,8 +1482,10 @@ def test_merge_without_integration_prs(self): self.set_build_status_on_pr_id(pr.id, 'SUCCESSFUL') integration_branches = [ f'w/4/{src_branch}', + f'w/5.1.4/{src_branch}', f'w/5.1/{src_branch}', f'w/5/{src_branch}', + f'w/10.0.1/{src_branch}', f'w/10.0/{src_branch}', f'w/10/{src_branch}', ] @@ -1496,14 +1610,15 @@ def test_conflict(self): except exns.Conflict as e: self.assertIn( '`w/10.0/improvement/TEST-0006-other` with contents from ' - '`improvement/TEST-0006-other`\nand `development/10.0`', + '`w/10.0.1/improvement/TEST-0006-other`\n' + 'and `development/10.0`', e.msg) # Bert-E MUST instruct the user to modify the integration # branch with the same target as the original PR self.assertIn( "git checkout -B w/10.0/improvement/TEST-0006", e.msg) self.assertIn( - "git merge origin/improvement/TEST-0006", e.msg) + "git merge origin/w/10.0.1/improvement/TEST-0006", e.msg) self.assertIn( "git push -u origin w/10.0/improvement/TEST-0006", e.msg) else: @@ -1518,14 +1633,15 @@ def test_conflict(self): except exns.Conflict as e: self.assertIn( '`w/10.0/improvement/TEST-0006-last` with contents from ' - '`w/5/improvement/TEST-0006-last`\nand `development/10.0`', + '`w/10.0.1/improvement/TEST-0006-last`\n' + 'and `development/10.0`', e.msg) # Bert-E MUST instruct the user to modify the integration # branch with the same target as the original PR self.assertIn( "git checkout -B w/10.0/improvement/TEST-0006", e.msg) self.assertIn( - "git merge origin/w/5/improvement/TEST-0006", e.msg) + "git merge origin/w/10.0.1/improvement/TEST-0006", e.msg) self.assertIn("git push -u origin w/10.0/improvement/TEST-0006", e.msg) else: @@ -2820,7 +2936,7 @@ def test_bypass_author_options_build_status(self): backtrace=True) except exns.BuildFailed as excp: self.assertIn( - "did not succeed in branch `w/10/bugfix/TEST-00081`", + "did not succeed in branch `w/10.0/bugfix/TEST-00081`", excp.msg, ) else: @@ -2830,6 +2946,7 @@ def test_bypass_author_options_build_status(self): self.set_build_status_on_pr_id(pr.id, 'SUCCESSFUL') self.set_build_status_on_pr_id(pr.id + 1, 'INPROGRESS') self.set_build_status_on_pr_id(pr.id + 2, 'SUCCESSFUL') + self.set_build_status_on_pr_id(pr.id + 3, 'SUCCESSFUL') with self.assertRaises(exns.BuildInProgress): self.handle(pr.id, settings=settings, @@ -3243,7 +3360,7 @@ def test_build_status(self): backtrace=True) except exns.BuildFailed as excp: self.assertIn( - "did not succeed in branch `w/10/bugfix/TEST-00081`", + "did not succeed in branch `w/10.0/bugfix/TEST-00081`", excp.msg, ) else: @@ -3253,6 +3370,7 @@ def test_build_status(self): self.set_build_status_on_pr_id(pr.id, 'SUCCESSFUL') self.set_build_status_on_pr_id(pr.id + 1, 'INPROGRESS') self.set_build_status_on_pr_id(pr.id + 2, 'SUCCESSFUL') + self.set_build_status_on_pr_id(pr.id + 3, 'SUCCESSFUL') with self.assertRaises(exns.BuildInProgress): self.handle(pr.id, options=self.bypass_all_but(['bypass_build_status']), @@ -3263,6 +3381,7 @@ def test_build_status(self): pr_admin = self.admin_bb.get_pull_request(pull_request_id=pr.id) pr_admin.add_comment('@%s bypass_leader_approval' % self.args.robot_username) + print("pr.id", pr.id) with self.assertRaises(exns.SuccessMessage): self.handle(pr.id, options=[ @@ -3283,7 +3402,7 @@ def test_build_status_triggered_by_build_result(self): # github enforces valid build urls url='https://builds.test.com/DEADBEEF' ) - for pr_id in range(pr.id + 1, pr.id + 4): + for pr_id in range(pr.id + 1, pr.id + 5): self.set_build_status_on_pr_id(pr_id, 'SUCCESSFUL') with self.assertRaises(exns.BuildFailed) as err: @@ -3848,8 +3967,10 @@ def test_integration_pr_declined(self): exp_int_branches = [ 'w/4/bugfix/TEST-0001', + 'w/5.1.4/bugfix/TEST-0001', 'w/5.1/bugfix/TEST-0001', 'w/5/bugfix/TEST-0001', + 'w/10.0.1/bugfix/TEST-0001', 'w/10.0/bugfix/TEST-0001', 'w/10/bugfix/TEST-0001' ] @@ -3865,7 +3986,7 @@ def test_integration_pr_declined(self): self.handle(pr.id, options=self.bypass_all, backtrace=True) # Decline integration pull requests - self.assertEqual(len(int_prs), 5) + self.assertEqual(len(int_prs), 7) for ipr in int_prs: ipr.decline() @@ -4128,7 +4249,7 @@ def test_development_branch_removal(self): options=self.bypass_all_but(['bypass_build_status'])) self.gitrepo.cmd('git checkout development/5.1') - self.gitrepo.cmd('git tag 5.1.4') + self.gitrepo.cmd('git tag 5.1.5') self.gitrepo.cmd( 'git push origin :development/5.1 --tags') @@ -4821,6 +4942,10 @@ def empty_solution(self): gwfb.QueueBranch: self.queue_branch('q/4'), gwfb.QueueIntegrationBranch: [] }), + ((5, 1, 4), { + gwfb.QueueBranch: self.queue_branch('q/5.1.4'), + gwfb.QueueIntegrationBranch: [] + }), ((5, 1), { gwfb.QueueBranch: self.queue_branch('q/5.1'), gwfb.QueueIntegrationBranch: [] @@ -4829,6 +4954,10 @@ def empty_solution(self): gwfb.QueueBranch: self.queue_branch('q/5'), gwfb.QueueIntegrationBranch: [] }), + ((10, 0, 1), { + gwfb.QueueBranch: self.queue_branch('q/10.0.1'), + gwfb.QueueIntegrationBranch: [] + }), ((10, 0), { gwfb.QueueBranch: self.queue_branch('q/10.0'), gwfb.QueueIntegrationBranch: [] @@ -4846,48 +4975,63 @@ def standard_solution(self): ((4, 3), { gwfb.QueueBranch: self.queue_branch('q/4.3'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/4.3/improvement/bar2'), + self.qint_branch('q/w/16/4.3/improvement/bar2'), self.qint_branch('q/w/1/4.3/improvement/bar') ] }), ((4, None), { gwfb.QueueBranch: self.queue_branch('q/4'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/4/improvement/bar2'), + self.qint_branch('q/w/16/4/improvement/bar2'), self.qint_branch('q/w/1/4/improvement/bar') ] }), + ((5, 1, 4), { + gwfb.QueueBranch: self.queue_branch('q/5.1.4'), + gwfb.QueueIntegrationBranch: [ + self.qint_branch('q/w/16/5.1.4/improvement/bar2'), + self.qint_branch('q/w/1/5.1.4/improvement/bar') + ] + }), ((5, 1), { gwfb.QueueBranch: self.queue_branch('q/5.1'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/5.1/improvement/bar2'), - self.qint_branch('q/w/9/5.1/bugfix/bar'), + self.qint_branch('q/w/16/5.1/improvement/bar2'), + self.qint_branch('q/w/11/5.1/bugfix/bar'), self.qint_branch('q/w/1/5.1/improvement/bar') ] }), ((5, None), { gwfb.QueueBranch: self.queue_branch('q/5'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/5/improvement/bar2'), - self.qint_branch('q/w/9/5/bugfix/bar'), + self.qint_branch('q/w/16/5/improvement/bar2'), + self.qint_branch('q/w/11/5/bugfix/bar'), self.qint_branch('q/w/1/5/improvement/bar') ] }), + ((10, 0, 1), { + gwfb.QueueBranch: self.queue_branch('q/10.0.1'), + gwfb.QueueIntegrationBranch: [ + self.qint_branch('q/w/16/10.0.1/improvement/bar2'), + self.qint_branch('q/w/11/10.0.1/bugfix/bar'), + self.qint_branch('q/w/1/10.0.1/improvement/bar') + ] + }), ((10, 0), { gwfb.QueueBranch: self.queue_branch('q/10.0'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/10.0/improvement/bar2'), - self.qint_branch('q/w/9/10.0/bugfix/bar'), - self.qint_branch('q/w/7/10.0/feature/foo'), + self.qint_branch('q/w/16/10.0/improvement/bar2'), + self.qint_branch('q/w/11/10.0/bugfix/bar'), + self.qint_branch('q/w/9/10.0/feature/foo'), self.qint_branch('q/w/1/10.0/improvement/bar') ] }), ((10, None), { gwfb.QueueBranch: self.queue_branch('q/10'), gwfb.QueueIntegrationBranch: [ - self.qint_branch('q/w/13/10/improvement/bar2'), - self.qint_branch('q/w/9/10/bugfix/bar'), - self.qint_branch('q/w/7/10/feature/foo'), + self.qint_branch('q/w/16/10/improvement/bar2'), + self.qint_branch('q/w/11/10/bugfix/bar'), + self.qint_branch('q/w/9/10/feature/foo'), self.qint_branch('q/w/1/10/improvement/bar') ] }), @@ -4899,8 +5043,8 @@ def test_queueing_standard_problem(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_queues, self.standard_solution) def test_queueing_standard_problem_reverse(self): @@ -4909,8 +5053,8 @@ def test_queueing_standard_problem_reverse(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_queues, self.standard_solution) def test_queueing_standard_problem_without_octopus(self): @@ -4924,8 +5068,8 @@ def test_queueing_standard_problem_without_octopus(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_queues, self.standard_solution) finally: gwfi.octopus_merge = git_utils.octopus_merge @@ -4937,8 +5081,10 @@ def test_queueing_last_pr_build_not_started(self): solution = deepcopy(self.standard_solution) solution[(4, 3)][gwfb.QueueIntegrationBranch].pop(0) solution[(4, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(5, 1, 4)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(10, 0, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, 0)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, None)][gwfb.QueueIntegrationBranch].pop(0) qbranches = self.submit_problem(problem) @@ -4946,8 +5092,8 @@ def test_queueing_last_pr_build_not_started(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11]) self.assertEqual(qc.mergeable_queues, solution) def test_queueing_last_pr_build_failed(self): @@ -4956,8 +5102,10 @@ def test_queueing_last_pr_build_failed(self): solution = deepcopy(self.standard_solution) solution[(4, 3)][gwfb.QueueIntegrationBranch].pop(0) solution[(4, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(5, 1, 4)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(10, 0, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, 0)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, None)][gwfb.QueueIntegrationBranch].pop(0) qbranches = self.submit_problem(problem) @@ -4965,8 +5113,8 @@ def test_queueing_last_pr_build_failed(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11]) self.assertEqual(qc.mergeable_queues, solution) def test_queueing_last_pr_other_key(self): @@ -4975,8 +5123,10 @@ def test_queueing_last_pr_other_key(self): solution = deepcopy(self.standard_solution) solution[(4, 3)][gwfb.QueueIntegrationBranch].pop(0) solution[(4, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(5, 1, 4)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(5, None)][gwfb.QueueIntegrationBranch].pop(0) + solution[(10, 0, 1)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, 0)][gwfb.QueueIntegrationBranch].pop(0) solution[(10, None)][gwfb.QueueIntegrationBranch].pop(0) qbranches = self.submit_problem(problem) @@ -4984,8 +5134,8 @@ def test_queueing_last_pr_other_key(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11]) self.assertEqual(qc.mergeable_queues, solution) def test_queueing_fail_masked_by_success(self): @@ -4998,8 +5148,8 @@ def test_queueing_fail_masked_by_success(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) - self.assertEqual(qc.mergeable_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) + self.assertEqual(qc.mergeable_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_queues, self.standard_solution) def test_queueing_all_failed(self): @@ -5012,7 +5162,7 @@ def test_queueing_all_failed(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_prs, []) self.assertEqual(qc.mergeable_queues, self.empty_solution) @@ -5026,7 +5176,7 @@ def test_queueing_all_inprogress(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_prs, []) self.assertEqual(qc.mergeable_queues, self.empty_solution) @@ -5040,7 +5190,7 @@ def test_queueing_mixed_fails(self): qc.finalize() qc.validate() self.assertEqual(qc._queues, self.standard_solution) - self.assertEqual(qc.queued_prs, [1, 7, 9, 13]) + self.assertEqual(qc.queued_prs, [1, 9, 11, 16]) self.assertEqual(qc.mergeable_prs, []) self.assertEqual(qc.mergeable_queues, self.empty_solution) @@ -5153,7 +5303,7 @@ def test_validation_masterq_diverged(self): def test_validation_vertical_inclusion(self): qbranches = self.submit_problem(self.standard_problem) - add_file_to_branch(self.gitrepo, 'q/w/13/5.1/improvement/bar2', + add_file_to_branch(self.gitrepo, 'q/w/16/5.1/improvement/bar2', 'file_pushed_without_bert-e.txt', do_push=True) qc = self.feed_queue_collection(qbranches) qc.finalize() @@ -5197,6 +5347,32 @@ def test_notify_pr_on_queue_fail(self): comment = list(pr.get_comments())[-1].text assert "Queue build failed" in comment + def test_pr_and_merge_on_three_digit_branch(self): + """Test that PRs can be created and + merged on three-digit development branches.""" + pr = self.create_pr('bugfix/TEST-01', 'development/10.0.1') + + # First handle should queue the PR + with self.assertRaises(exns.Queued): + self.handle(pr.id, options=self.bypass_all, backtrace=True) + + queue_branches = [ + f'q/w/{pr.id}/10.0.1/{pr.src_branch}', + f'q/w/{pr.id}/10.0/{pr.src_branch}', + f'q/w/{pr.id}/10/{pr.src_branch}', + ] + + # Set build status on all queue branches + for branch in queue_branches: + self.set_build_status_on_branch_tip(branch, 'SUCCESSFUL') + + # Second handle should merge successfully + with self.assertRaises(exns.Merged): + self.handle(pr.id, options=self.bypass_all, backtrace=True) + + # Verify the PR was merged + assert pr.status == 'MERGED' + def test_system_nominal_case(self): pr = self.create_pr('bugfix/TEST-00001', 'development/5') self.handle(pr.id, @@ -5204,19 +5380,25 @@ def test_system_nominal_case(self): # add a commit to w/5.1 branch self.gitrepo.cmd('git fetch') - self.gitrepo.cmd('git checkout w/10.0/bugfix/TEST-00001') + self.gitrepo.cmd('git checkout w/10.0.1/bugfix/TEST-00001') self.gitrepo.cmd('touch abc') self.gitrepo.cmd('git add abc') self.gitrepo.cmd('git commit -m "add new file"') self.gitrepo.cmd('git push origin') - sha1_w_10_0 = self.gitrepo.cmd( - 'git rev-parse w/10.0/bugfix/TEST-00001').rstrip() + sha1_w_10_0_1 = self.gitrepo.cmd( + 'git rev-parse w/10.0.1/bugfix/TEST-00001').rstrip() with self.assertRaises(exns.Queued): self.handle(pr.id, options=self.bypass_all, backtrace=True) # get the new sha1 on w/10.0 (set_build_status_on_pr_id won't # detect the new commit in mocked mode) + self.gitrepo.cmd('git fetch') + self.gitrepo.cmd('git checkout w/10.0/bugfix/TEST-00001') + self.gitrepo.cmd('git pull') + sha1_w_10_0 = self.gitrepo.cmd( + 'git rev-parse w/10.0/bugfix/TEST-00001').rstrip() + self.gitrepo.cmd('git fetch') self.gitrepo.cmd('git checkout w/10/bugfix/TEST-00001') self.gitrepo.cmd('git pull') @@ -5229,14 +5411,14 @@ def test_system_nominal_case(self): 'q/w/1/5/bugfix/TEST-00001', 'q/w/1/10.0/bugfix/TEST-00001', 'q/w/1/10/bugfix/TEST-00001', - 'w/10.0/bugfix/TEST-00001', - 'w/10/bugfix/TEST-00001' ] + for branch in expected_branches: self.assertTrue(self.gitrepo.remote_branch_exists(branch)) # set build status self.set_build_status_on_pr_id(pr.id, 'SUCCESSFUL') + self.set_build_status(sha1=sha1_w_10_0_1, state='SUCCESSFUL') self.set_build_status(sha1=sha1_w_10_0, state='SUCCESSFUL') self.set_build_status(sha1=sha1_w_10, state='FAILED') with self.assertRaises(exns.QueueBuildFailed): @@ -5258,7 +5440,8 @@ def test_system_nominal_case(self): self.assertTrue(self.gitrepo.remote_branch_exists(branch)) for branch in expected_branches: self.assertFalse(self.gitrepo.remote_branch_exists(branch, True)) - for dev in ['development/5', 'development/10.0', 'development/10']: + for dev in ['development/5', 'development/10.0.1', 'development/10.0', + 'development/10']: branch = gwfb.branch_factory(self.gitrepo, dev) branch.checkout() self.gitrepo.cmd('git pull origin %s', dev) @@ -5266,7 +5449,7 @@ def test_system_nominal_case(self): if dev == 'development/5': self.assertFalse(branch.includes_commit(sha1_w_10_0)) else: - self.assertTrue(branch.includes_commit(sha1_w_10_0)) + self.assertTrue(branch.includes_commit(sha1_w_10_0_1)) self.gitrepo.cmd('cat abc') last_comment = pr.comments[-1].text @@ -5729,6 +5912,67 @@ def test_pr_hotfix_alone(self): self.handle(sha1, options=self.bypass_all, backtrace=True) self.assertEqual(self.prs_in_queue(), set()) + def test_pr_hotfix_and_three_digit_dev_branch_together(self): + """Test that a hotfix PR and a three-digit + development branch PR can be queued together.""" + # Set up a tag needed for hotfix branch + self.gitrepo.cmd('git tag 10.0.2.0') + self.gitrepo.cmd('git push --tags') + + # Create the hotfix branch from the tag + self.gitrepo.cmd('git checkout -b hotfix/10.0.2 10.0.2.0') + self.gitrepo.cmd('git push -u origin hotfix/10.0.2') + + # Create a PR targeting a hotfix branch + pr_hotfix = self.create_pr('bugfix/TEST-HOTFIX', 'hotfix/10.0.2') + with self.assertRaises(exns.Queued): + self.handle(pr_hotfix.id, options=self.bypass_all, backtrace=True) + + # Create a PR targeting a three-digit development branch + pr_dev = self.create_pr('feature/TEST-DEV', 'development/5.1.4') + with self.assertRaises(exns.Queued): + self.handle(pr_dev.id, options=self.bypass_all, backtrace=True) + + # Verify both PRs are in the queue + self.assertEqual(self.prs_in_queue(), {pr_hotfix.id, pr_dev.id}) + + # Set build status to successful on all queue branches for hotfix PR + # Hotfix PR should create queue branches for 4.3.19.1 + hotfix_queue_branches = [ + f'q/w/{pr_hotfix.id}/10.0.2.1/{pr_hotfix.src_branch}', + ] + + # Set build status to successful on all queue branches for dev PR + dev_queue_branches = [ + f'q/w/{pr_dev.id}/5.1.4/{pr_dev.src_branch}', + f'q/w/{pr_dev.id}/5.1/{pr_dev.src_branch}', + f'q/w/{pr_dev.id}/5/{pr_dev.src_branch}', + f'q/w/{pr_dev.id}/10.0.1/{pr_dev.src_branch}', + f'q/w/{pr_dev.id}/10.0/{pr_dev.src_branch}', + f'q/w/{pr_dev.id}/10/{pr_dev.src_branch}', + ] + + # Set successful build status on hotfix queue branches + hotfix_sha1 = None + for branch in hotfix_queue_branches: + hotfix_sha1 = self.set_build_status_on_branch_tip( + branch, 'SUCCESSFUL') + + # Set successful build status on dev queue branches + for branch in dev_queue_branches: + self.set_build_status_on_branch_tip( + branch, 'SUCCESSFUL') + + # Both PRs should merge successfully when we handle their SHA1s + # This triggers the queue merge logic + with self.assertRaises(exns.Merged): + self.handle(hotfix_sha1, options=self.bypass_all, backtrace=True) + + # Verify both PRs were merged and queue is empty + self.assertEqual(self.prs_in_queue(), set()) + self.assertEqual(pr_hotfix.status, 'MERGED') + self.assertEqual(pr_dev.status, 'MERGED') + def test_multi_branch_queues_2(self): pr1 = self.create_pr('bugfix/TEST-00001', 'development/5') with self.assertRaises(exns.Queued): @@ -5742,6 +5986,8 @@ def test_multi_branch_queues_2(self): self.set_build_status_on_branch_tip( 'q/w/%d/5/bugfix/TEST-00001' % pr1.id, 'SUCCESSFUL') + self.set_build_status_on_branch_tip( + 'q/w/%d/10.0.1/bugfix/TEST-00001' % pr1.id, 'SUCCESSFUL') self.set_build_status_on_branch_tip( 'q/w/%d/10.0/bugfix/TEST-00001' % pr1.id, 'SUCCESSFUL') self.set_build_status_on_branch_tip( @@ -5909,9 +6155,10 @@ def test_status_with_queue(self): status = self.berte.status.get('merge queue', OrderedDict()) self.assertIn(1, status) - self.assertEqual(len(status[1]), 6) + self.assertEqual(len(status[1]), 8) versions = tuple(version for version, _ in status[1]) - self.assertEqual(versions, ('10', '10.0', '5', '5.1', '4', '4.3')) + self.assertEqual(versions, ('10', '10.0', '10.0.1', '5', + '5.1', '5.1.4', '4', '4.3')) for _, sha1 in status[1]: self.set_build_status(sha1=sha1, state='SUCCESSFUL') self.process_sha1_job(sha1_q_10_0, 'Merged') @@ -5957,7 +6204,8 @@ def test_status_with_queue_without_octopus(self): versions = tuple(version for version, _ in status[1]) for _, sha1 in status[1]: self.set_build_status(sha1=sha1, state='SUCCESSFUL') - self.assertEqual(versions, ('10', '10.0', '5', '5.1', '4', '4.3')) + self.assertEqual(versions, ('10', '10.0', '10.0.1', + '5', '5.1', '5.1.4', '4', '4.3')) self.process_sha1_job(sha1_q_10_0, 'Merged') merged_pr = self.berte.status.get('merged PRs', []) @@ -6189,7 +6437,7 @@ def test_job_create_branch_dev_start(self): self.gitrepo._get_remote_branches(force=True) self.assertEqual( self.gitrepo._remote_branches['development/2.9'], - self.gitrepo._remote_branches['development/4.3'] + self.gitrepo._remote_branches['development/4.3.18'] ) self.gitrepo.cmd('git fetch') @@ -6324,7 +6572,7 @@ def test_job_create_branch_dev_end(self): 'q/w/3/10.0/feature/TEST-03', 'q/w/3/10/feature/TEST-03', 'q/w/3/11.3/feature/TEST-03', - 'q/w/22/11.3/feature/TEST-9997', + 'q/w/28/11.3/feature/TEST-9997', ] self.gitrepo._get_remote_branches(force=True) for branch in expected_branches: @@ -6411,10 +6659,10 @@ def test_job_delete_branch(self): ('q/w/3/5/feature/TEST-03', self.assertTrue), ('q/w/3/10.0/feature/TEST-03', self.assertTrue), ('q/w/3/10/feature/TEST-03', self.assertTrue), - ('q/w/13/5.1/feature/TEST-9998', self.assertTrue), - ('q/w/13/5/feature/TEST-9998', self.assertTrue), - ('q/w/13/10.0/feature/TEST-9998', self.assertTrue), - ('q/w/13/10/feature/TEST-9998', self.assertTrue), + ('q/w/16/5.1/feature/TEST-9998', self.assertTrue), + ('q/w/16/5/feature/TEST-9998', self.assertTrue), + ('q/w/16/10.0/feature/TEST-9998', self.assertTrue), + ('q/w/16/10/feature/TEST-9998', self.assertTrue), ] self.gitrepo._get_remote_branches(force=True) for branch, func in expected_branches: diff --git a/bert_e/tests/unit/test_exceptions_master_queue_missing.py b/bert_e/tests/unit/test_exceptions_master_queue_missing.py new file mode 100644 index 00000000..8e25422c --- /dev/null +++ b/bert_e/tests/unit/test_exceptions_master_queue_missing.py @@ -0,0 +1,31 @@ +from bert_e.exceptions import MasterQueueMissing + + +def test_master_queue_missing_major_only(): + # Covers version with length 1 + exc = MasterQueueMissing((10,)) + assert 'q/10 is missing' in str(exc) + + +def test_master_queue_missing_major_minor(): + # Covers version with length 2 + exc = MasterQueueMissing((10, 0)) + assert 'q/10.0 is missing' in str(exc) + + +def test_master_queue_missing_major_minor_none_micro(): + # Covers version with length >=3 and micro is None + exc = MasterQueueMissing((10, 0, None)) + assert 'q/10.0 is missing' in str(exc) + + +def test_master_queue_missing_major_minor_micro(): + # Covers version with length >=3 and micro is not None + exc = MasterQueueMissing((10, 0, 5)) + assert 'q/10.0.5 is missing' in str(exc) + + +def test_master_queue_missing_fallback(): + # Covers fallback case + exc = MasterQueueMissing([]) + assert 'q/[] is missing' in str(exc) diff --git a/bert_e/tests/unit/test_sorted.py b/bert_e/tests/unit/test_sorted.py index 59773ff7..670b0c39 100644 --- a/bert_e/tests/unit/test_sorted.py +++ b/bert_e/tests/unit/test_sorted.py @@ -51,3 +51,33 @@ def test_sorted_versions(): sorted_versions = sorted(versions, key=version_key, reverse=True) assert sorted_versions == expected + + +def test_compare_branches_major_minor_vs_major_only(): + branch1 = ((4, 3),) + branch2 = ((4, None),) + assert compare_branches(branch1, branch2) == -1 + + +def test_compare_branches_major_only_vs_major_only_returns_0(): + branch1 = ((4, None),) # major-only + branch2 = ((4, None),) # major.only + assert compare_branches(branch1, branch2) == 0 + + +def test_compare_branches_major_only_vs_major_minor(): + branch1 = ((4, None),) + branch2 = ((4, 3),) + assert compare_branches(branch1, branch2) == 1 + + +def test_compare_branches_major_minor_micro_vs_major_minor(): + branch1 = ((4, 3, 2),) + branch2 = ((4, 3),) + assert compare_branches(branch1, branch2) == -2 + + +def test_compare_branches_major_minor_vs_major_minor_micro(): + branch1 = ((4, 3),) + branch2 = ((4, 3, 2),) + assert compare_branches(branch1, branch2) == 2 diff --git a/bert_e/workflow/gitwaterflow/branches.py b/bert_e/workflow/gitwaterflow/branches.py index a1d98f25..f7d615cb 100644 --- a/bert_e/workflow/gitwaterflow/branches.py +++ b/bert_e/workflow/gitwaterflow/branches.py @@ -30,23 +30,57 @@ def compare_branches(branch1, branch2): - """Compare GitWaterflow branches for sorting. - - Important to note that when a branch has a minor version as None, - it will be considered as the latest version. """ - - major1, minor1 = branch1[0][:2] - major2, minor2 = branch2[0][:2] - if major1 == major2: - if minor1 == minor2: - return 0 - if minor1 is None: - return 1 - if minor2 is None: - return -1 + Compare GitWaterflow branches for sorting. + For cascade ordering, we want: 4.3 < 4 < 5.1 < 10.0 < 10 + Specific minor versions come before major-only branches within + the same major. + """ + # Safely extract version components + version1 = branch1[0] + version2 = branch2[0] + # Extract major versions (always present) + major1 = version1[0] if len(version1) > 0 else 0 + major2 = version2[0] if len(version2) > 0 else 0 + + # Compare major versions first + if major1 != major2: + return major1 - major2 + + # Same major version - check if one is major-only vs major.minor + # Major-only branches have None as minor version, e.g., (4, None) + # Major.minor branches have integer minor version, e.g., (4, 3) + is_major_only_1 = len(version1) >= 2 and version1[1] is None + is_major_only_2 = len(version2) >= 2 and version2[1] is None + + if is_major_only_1 and not is_major_only_2: + # version1 is major-only, version2 is major.minor + # major-only comes after + return 1 + elif not is_major_only_1 and is_major_only_2: + # version1 is major.minor, version2 is major-only + # major.minor comes before + return -1 + + # Both are major.minor or longer - compare normally + minor1 = version1[1] if len(version1) > 1 else 0 + minor2 = version2[1] if len(version2) > 1 else 0 + + # Compare minor versions + if minor1 != minor2: return minor1 - minor2 - return major1 - major2 + + # Same major.minor - extract micro versions + # Default to 0 if no micro + micro1 = version1[2] if len(version1) > 2 else 0 + # Default to 0 if no micro + micro2 = version2[2] if len(version2) > 2 else 0 + + # Compare micro versions + if micro1 != micro2: + return micro2 - micro1 + else: + return 0 def compare_queues(version1, version2): @@ -156,30 +190,55 @@ def version_t(self): @total_ordering class DevelopmentBranch(GWFBranch): - pattern = r'^development/(?P(?P\d+)(\.(?P\d+))?)$' + pattern = (r'^development/(?P(?P\d+)' + r'(\.(?P\d+))?(\.(?P\d+))?)$') cascade_producer = True cascade_consumer = True can_be_destination = True allow_prefixes = FeatureBranch.all_prefixes latest_minor = -1 + def __init__(self, repo, name): + super().__init__(repo, name) + # The regex match is already done in parent GWFBranch.__init__ + # self.major, self.minor are already set by parent, add micro handling + + # Handle micro version - it will be None if not present + if not hasattr(self, 'micro') or self.micro is None: + self.micro = None # Keep as None for x.y format branches + + # Update version property to include micro when present + if self.micro is not None: + self.version = f"{self.major}.{self.minor}.{self.micro}" + elif self.minor is not None: + self.version = f"{self.major}.{self.minor}" + else: + self.version = f"{self.major}" + def __eq__(self, other): return (self.__class__ == other.__class__ and self.major == other.major and - self.minor == other.minor) + self.minor == other.minor and + getattr(self, 'micro', None) == getattr(other, 'micro', None)) def __lt__(self, other): if self.__class__ != other.__class__: return NotImplemented + + # Major version comparison if self.major != other.major: return self.major < other.major + if self.minor is None: # development/ is greater than development/. return False - if other.minor is None: - # development/. is less than development/ + # Same major version - apply cascade rules + if self.minor is not None and other.minor is None: return True - return self.minor < other.minor + + # Both have minor versions + if self.minor != other.minor: + return self.minor < other.minor @property def has_minor(self) -> bool: @@ -187,7 +246,12 @@ def has_minor(self) -> bool: @property def version_t(self): - return (self.major, self.minor) + if getattr(self, 'micro', None) is not None: + return (self.major, self.minor, self.micro) + elif self.minor is not None: + return (self.major, self.minor) + else: + return (self.major, None) class IntegrationBranch(GWFBranch): @@ -266,10 +330,6 @@ def __init__(self, repo, name): if self.hfrev is not None: dest = branch_factory(repo, 'hotfix/%d.%d.%d' % (self.major, self.minor, self.micro)) - elif self.micro is not None: - # This is a hotfix queue (has micro version), create hotfix branch - dest = branch_factory(repo, 'hotfix/%d.%d.%d' % (self.major, - self.minor, self.micro)) else: dest = branch_factory(repo, 'development/%s' % self.version) self.dst_branch = dest @@ -784,7 +844,6 @@ def build(self, repo, dst_branch=None): branch) if match_: flat_branches.add(match_.group('name')) - for flat_branch in flat_branches: try: branch = branch_factory(repo, flat_branch) @@ -795,6 +854,11 @@ def build(self, repo, dst_branch=None): for tag in repo.cmd('git tag').split('\n')[:-1]: self.update_versions(tag) + # Re-sort the cascade after update_versions may have changed keys + self._cascade = OrderedDict( + sorted(self._cascade.items(), key=cmp_to_key(compare_branches)) + ) + self._update_major_versions() if dst_branch: self.finalize(dst_branch) @@ -843,18 +907,20 @@ def add_branch(self, branch, dst_branch=None): else: return - (major, minor) = branch.major, branch.minor - if (major, minor) not in self._cascade.keys(): - self._cascade[(major, minor)] = { + # Create key based on full version tuple + key = branch.version_t + + if key not in self._cascade.keys(): + self._cascade[key] = { DevelopmentBranch: None, HotfixBranch: None, } - # Sort the cascade again + # Sort the cascade again using updated comparison self._cascade = OrderedDict( sorted(self._cascade.items(), key=cmp_to_key(compare_branches)) ) - self._cascade[(major, minor)][branch.__class__] = branch + self._cascade[key][branch.__class__] = branch def update_versions(self, tag): """Update expected versions based on repository tags.""" @@ -872,16 +938,30 @@ def update_versions(self, tag): if match.groupdict()['hfrev'] is not None: hfrev = int(match.groupdict()['hfrev']) + # Look for branches that match this version + # Check for exact (major, minor) match and major-only match branches = self._cascade.get((major, minor), {}) major_branches = self._cascade.get((major, None), {}) + micro_branches = self._cascade.get((major, minor, micro), {}) + + # Also look for hotfix branches with matching major.minor.micro + hf_branch = None + for key, branch_set in self._cascade.items(): + if (len(key) >= 3 and key[0] == major and key[1] == minor and + key[2] == micro and branch_set.get(HotfixBranch)): + hf_branch = branch_set[HotfixBranch] + break - if not branches and not major_branches: + if not branches and not major_branches and not hf_branch: LOG.debug("Ignore tag: %s", tag) return - hf_branch: HotfixBranch = branches.get(HotfixBranch) + # If we didn't find hf_branch above, check in branches + if not hf_branch: + hf_branch = branches.get(HotfixBranch) dev_branch: DevelopmentBranch = branches.get(DevelopmentBranch) major_branch: DevelopmentBranch = major_branches.get(DevelopmentBranch) + micro_branch: DevelopmentBranch = micro_branches.get(DevelopmentBranch) if hf_branch: if hf_branch.micro == micro: @@ -891,15 +971,52 @@ def update_versions(self, tag): hf_branch.micro, hf_branch.hfrev) - if dev_branch: - dev_branch.micro = max(micro, dev_branch.micro) + if micro_branch is not None and \ + ([micro_branch.major, micro_branch.minor, + micro_branch.micro] == [major, minor, micro]): + raise errors.ReleaseAlreadyExists(micro_branch, tag) + if dev_branch: + # Only update if tag matches X.Y + # And is not a three-digit dev branch + if dev_branch.major == major and dev_branch.minor == minor: + # Exclude tags whose micro is already + # A three-digit dev branch for this X.Y + three_digit_keys = [ + k for k in self._cascade.keys() + if len(k) == 3 and k[0] == major and k[1] == minor + ] + + three_digit_micros = set(k[2] for k in three_digit_keys) + next_micro = micro + 1 + + while next_micro in three_digit_micros: + next_micro += 1 + if (not hasattr(dev_branch, '_next_micro') or + dev_branch._next_micro is None): + dev_branch._next_micro = next_micro + else: + dev_branch._next_micro = max( + next_micro, + dev_branch._next_micro + ) if major_branch: - major_branch.latest_minor = max(minor, major_branch.latest_minor) + major_branch.latest_minor = max( + minor, + major_branch.latest_minor + ) + + # Re-sort the cascade after any key changes + self._cascade = OrderedDict( + sorted( + self._cascade.items(), + key=cmp_to_key(compare_branches) + ) + ) def validate(self): previous_dev_branch = None - for (major, minor), branch_set in self._cascade.items(): + for key, branch_set in self._cascade.items(): dev_branch = branch_set[DevelopmentBranch] hf_branch = branch_set[HotfixBranch] @@ -908,10 +1025,6 @@ def validate(self): # skip cascade validation for hf continue - if dev_branch is None: - raise errors.DevBranchDoesNotExist( - 'development/%d.%d' % (major, minor)) - if previous_dev_branch: if not dev_branch.includes_commit(previous_dev_branch): raise errors.DevBranchesNotSelfContained( @@ -920,21 +1033,26 @@ def validate(self): previous_dev_branch = dev_branch def _update_major_versions(self): - """For every major dev branch, ensure the latest minor is set. - + """ + For every major dev branch, ensure the latest minor is set. This function is used on the case where we have a dev/1 and dev/1.0 branch but no 1.0.0 tag. In this case, when expecting the next version for dev/1 we should return 1.1.0 instead of 1.0.0. - """ - for (_, minor), branch_set in self._cascade.items(): - if minor is not None: + for version_tuple, branch_set in self._cascade.items(): + # Only process major-only branches (tuples where minor is None) + if len(version_tuple) < 2 or version_tuple[1] is not None: continue major_branch: DevelopmentBranch = branch_set[DevelopmentBranch] + major = version_tuple[0] + + # Find all minor versions for this major minors = [ - minor for (m, minor) in self._cascade.keys() - if m == major_branch.major and minor is not None + version_tuple[1] for version_tuple in self._cascade.keys() + if (len(version_tuple) >= 2 and + version_tuple[0] == major and + version_tuple[1] is not None) ] minors.append(major_branch.latest_minor) @@ -944,35 +1062,58 @@ def _set_target_versions(self, dst_branch): """Compute list of expected Jira FixVersion/s. Must be called after the cascade has been finalised. - """ - for (major, minor), branch_set in self._cascade.items(): + # Clear any existing target versions to avoid duplicates + self.target_versions = [] + for key, branch_set in self._cascade.items(): dev_branch: DevelopmentBranch = branch_set[DevelopmentBranch] hf_branch: HotfixBranch = branch_set[HotfixBranch] if hf_branch and dst_branch.name.startswith('hotfix/'): - self.target_versions.append('%d.%d.%d.%d' % ( - hf_branch.major, hf_branch.minor, hf_branch.micro, - hf_branch.hfrev)) - - if dev_branch and dev_branch.has_minor is True: - offset = 1 - - self.target_versions.append('%d.%d.%d' % ( - major, minor, dev_branch.micro + offset)) - elif dev_branch and dev_branch.has_minor is False: self.target_versions.append( - f"{major}." - f"{dev_branch.latest_minor + 1}." - f"{dev_branch.micro + 1}" + '%d.%d.%d.%d' % ( + hf_branch.major, hf_branch.minor, hf_branch.micro, + hf_branch.hfrev + ) ) + if dev_branch: + # Development branches with major.minor naming + if dev_branch.has_minor: + # Use the tracked next micro version if available, + # otherwise use the micro+1 if this is a three-digit branch + if (hasattr(dev_branch, '_next_micro') and + dev_branch._next_micro is not None): + next_micro = dev_branch._next_micro + elif getattr(dev_branch, 'micro', None) is not None: + version_current = '%d.%d.%d' % ( + dev_branch.major, dev_branch.minor, + dev_branch.micro) + self.target_versions.append(version_current) + next_micro = dev_branch.micro + else: + next_micro = 0 + version_str = '%d.%d.%d' % ( + dev_branch.major, dev_branch.minor, next_micro + ) + if version_str not in self.target_versions: + self.target_versions.append(version_str) + + else: + # For development/x branches, use latest_minor logic + # Target should be X.(latest_minor+1).0, + # not X.(latest_minor+1).1 + self.target_versions.append( + f"{dev_branch.major}." + f"{dev_branch.latest_minor + 1}." + f"0" + ) def finalize(self, dst_branch): - """Finalize cascade considering given destination. + """ + Finalize cascade considering given destination. Assumes the cascade has been populated by calls to add_branch and update_versions. The local lists keeping track - Args: dst_branch: where the pull request wants to merge @@ -984,50 +1125,105 @@ def finalize(self, dst_branch): """ self.get_merge_paths() # populate merge paths before removing data - include_dev_branches = False - dev_branch = None - dst_hf = dst_branch.name.startswith('hotfix/') - for (major, minor), branch_set in list(self._cascade.items()): + # First: Find the destination branch key if it's a development branch + dst_branch_key = None + if not dst_hf: + for key, branch_set in self._cascade.items(): + dev_branch = branch_set[DevelopmentBranch] + if dev_branch and dev_branch.name == dst_branch.name: + dst_branch_key = key + break + + # Process all branches + for key, branch_set in list(self._cascade.items()): dev_branch = branch_set[DevelopmentBranch] hf_branch = branch_set[HotfixBranch] - # we have to target at least a hf or a dev branch - if dev_branch is None and hf_branch is None: - raise errors.DevBranchDoesNotExist( - 'development/%d.%d' % (major, minor)) - - # remove untargetted branches from cascade - if dst_branch == dev_branch: - include_dev_branches = True - - if not include_dev_branches or dst_hf: - if branch_set[DevelopmentBranch]: - branch_set[DevelopmentBranch] = None - self.ignored_branches.append(dev_branch.name) - + # Handle development branches + if dev_branch: if not dst_hf: - del self._cascade[(major, minor)] - continue - - if not hf_branch or hf_branch.name != dst_branch.name: - if branch_set[HotfixBranch]: - branch_set[HotfixBranch] = None - self.ignored_branches.append(hf_branch.name) - del self._cascade[(major, minor)] - - # add to dst_branches in the correct order - if not dst_hf: - if branch_set[DevelopmentBranch]: - self.dst_branches.append(dev_branch) - else: - if branch_set[HotfixBranch]: - if dst_branch.name == hf_branch.name: - self.dst_branches.append(hf_branch) + # For dev destinations, include branches + # with version >= destination version + # Special handling for major-only vs major.minor branches + key_major = key[0] + key_minor = key[1] if len(key) > 1 else None + dst_major = dst_branch_key[0] + dst_minor = (dst_branch_key[1] if len(dst_branch_key) > 1 + else None) + + should_include = False + + if key_major > dst_major: + # Higher major version always included + should_include = True + elif key_major == dst_major: + # Same major version + if key_minor is None: + # Major-only branch (e.g., development/4) - + # always included for same major + should_include = True + elif dst_minor is None: + # Destination is major-only, this is major.minor - + # exclude specific minor versions + should_include = False + else: + # Both have minor versions - + # Compare micro if present + if len(key) > 2 and len(dst_branch_key) > 2: + # Both have micro versions + should_include = key[2] >= dst_branch_key[2] + elif len(key) > 2 and len(dst_branch_key) <= 2: + # This branch has micro, + # Destination does not + should_include = False + elif len(key) <= 2 and len(dst_branch_key) > 2: + # This branch does not have micro, + # Destination does + should_include = True + else: + # Both have no micro + should_include = key_minor >= dst_minor + + if should_include: + self.dst_branches.append(dev_branch) + else: + self.ignored_branches.append(dev_branch.name) + branch_set[DevelopmentBranch] = None + else: + # For hotfix destinations, ignore all dev branches + self.ignored_branches.append(dev_branch.name) + branch_set[DevelopmentBranch] = None + # Handle hotfix branches + if hf_branch: + self.dst_branches.append(hf_branch) + + # Clean up the cascade by removing keys with no branches + keys_to_remove = [] + for key, branch_set in self._cascade.items(): + if ( + not branch_set[DevelopmentBranch] and + not branch_set[HotfixBranch] + ): + keys_to_remove.append(key) + + for key in keys_to_remove: + del self._cascade[key] + + # Sort dst_branches according to cascade order + # (the cascade is already sorted) + # Extract development branches in cascade order + ordered_dst_branches = [] + for key, branch_set in self._cascade.items(): + dev_branch = branch_set[DevelopmentBranch] + hf_branch = branch_set[HotfixBranch] + if dev_branch and dev_branch in self.dst_branches: + ordered_dst_branches.append(dev_branch) + if hf_branch and hf_branch in self.dst_branches: + ordered_dst_branches.append(hf_branch) - if not dev_branch and not dst_hf: - raise errors.NotASingleDevBranch() + self.dst_branches = ordered_dst_branches self._set_target_versions(dst_branch) self.ignored_branches.sort()