diff --git a/tools/pickman/control.py b/tools/pickman/control.py index f4d4a43c292..a42927f991d 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -1663,7 +1663,7 @@ def _prepare_get_commits(dbs, source, args): return info, None -def prepare_apply(dbs, source, branch, args=None): +def prepare_apply(dbs, source, branch, args=None, info=None): """Prepare for applying commits from a source branch Get the next commits, set up the branch name and prints info about @@ -1676,15 +1676,18 @@ def prepare_apply(dbs, source, branch, args=None): branch (str): Branch name to use, or None to auto-generate args (Namespace): Parsed arguments with 'push', 'remote', 'target' (needed for subtree updates) + info (NextCommitsInfo): Pre-fetched commit info from + _prepare_get_commits(), or None to fetch it here Returns: tuple: (ApplyInfo, return_code) where ApplyInfo is set if there are commits to apply, or None with return_code indicating the result (0 for no commits, 1 for error) """ - info, ret = _prepare_get_commits(dbs, source, args) - if ret is not None: - return None, ret + if info is None: + info, ret = _prepare_get_commits(dbs, source, args) + if ret is not None: + return None, ret commits = info.commits @@ -1889,18 +1892,20 @@ def execute_apply(dbs, source, commits, branch_name, args, advance_to=None): # return ret, success, conv_log -def do_apply(args, dbs): +def do_apply(args, dbs, info=None): """Apply the next set of commits using Claude agent Args: args (Namespace): Parsed arguments with 'source' and 'branch' attributes dbs (Database): Database instance + info (NextCommitsInfo): Pre-fetched commit info from + _prepare_get_commits(), or None to fetch during prepare Returns: int: 0 on success, 1 on failure """ source = args.source - info, ret = prepare_apply(dbs, source, args.branch, args) + info, ret = prepare_apply(dbs, source, args.branch, args, info=info) if not info: return ret @@ -2851,6 +2856,14 @@ def do_step(args, dbs): process_pipeline_failures(remote, active_mrs, dbs, args.target, args.fix_retries) + # Process subtree updates and advance past fully-processed merges + # regardless of MR count, since these don't create MRs + info, ret = _prepare_get_commits(dbs, source, args) + if ret is not None: + if ret: + return ret + return 0 + # Only block new MR creation if we've reached the max allowed open MRs max_mrs = args.max_mrs if len(active_mrs) >= max_mrs: @@ -2869,7 +2882,7 @@ def do_step(args, dbs): tout.info('No pending pickman MRs, creating new one...') args.push = True args.branch = None # Let do_apply generate branch name - return do_apply(args, dbs) + return do_apply(args, dbs, info=info) def do_poll(args, dbs): diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index af26cbb4229..ced2c79ac87 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -2061,16 +2061,21 @@ class TestStep(unittest.TestCase): source_branch='cherry-test', description='Test', ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=1, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=1, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) self.assertEqual(ret, 0) @@ -2109,18 +2114,23 @@ class TestStep(unittest.TestCase): source_branch='cherry-test', description='Test', ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): - with mock.patch.object(control, 'do_apply', - return_value=0) as mock_apply: - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=5, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object(control, 'do_apply', + return_value=0) as mock_apply: + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=5, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) # With 1 open MR and max_mrs=5, it should try to create a new one self.assertEqual(ret, 0) @@ -2138,17 +2148,23 @@ class TestStep(unittest.TestCase): ) for i in range(3) ] + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=mock_mrs): - with mock.patch.object(control, 'do_apply') as mock_apply: - args = argparse.Namespace(cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=3, fix_retries=3) - with terminal.capture(): - ret = control.do_step(args, None) + with mock.patch.object( + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object(control, 'do_apply') \ + as mock_apply: + args = argparse.Namespace( + cmd='step', source='us/next', remote='ci', + target='master', max_mrs=3, fix_retries=3) + with terminal.capture(): + ret = control.do_step(args, None) # With 3 open MRs and max_mrs=3, should not create new MR self.assertEqual(ret, 0) @@ -6120,19 +6136,25 @@ class TestProcessPipelineFailures(unittest.TestCase): pipeline_status='failed', pipeline_id=100, ) + mock_info = control.NextCommitsInfo( + commits=['fake'], merge_found=True, advance_to='abc123') with mock.patch.object(control, 'run_git'): with mock.patch.object(gitlab, 'get_merged_pickman_mrs', return_value=[]): with mock.patch.object(gitlab, 'get_open_pickman_mrs', return_value=[mock_mr]): with mock.patch.object( - control, 'process_pipeline_failures') as mock_ppf: - args = argparse.Namespace( - cmd='step', source='us/next', - remote='ci', target='master', - max_mrs=1, fix_retries=0) - with terminal.capture(): - control.do_step(args, None) + control, '_prepare_get_commits', + return_value=(mock_info, None)): + with mock.patch.object( + control, + 'process_pipeline_failures') as mock_ppf: + args = argparse.Namespace( + cmd='step', source='us/next', + remote='ci', target='master', + max_mrs=1, fix_retries=0) + with terminal.capture(): + control.do_step(args, None) mock_ppf.assert_not_called()