From patchwork Wed Dec 17 02:27:57 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 951 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938562; bh=4Bi/EZTK9dI2Qc1n3vOwDJwgzprYFV2ae4qZxAIAQTM=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=JgX3tCM6I4ILwPfVbkhip1xlPNOHVOorwIPUIoGWFhNj5CWMYj8TseT45livM6ybm 6zXUfjyI4PVdpOV3HN4mI07p7xuJrNSOU7w3gZq25NAF0GJRRfwQpW9n2OsqT4FmTk Cf7kzeqOfn+HG8sptiI4AzAxuEHuQihIJ4Dc6RBRydAgYSSQbx1rFtF1Qke0WRN4b6 ktLlNhKNgW3Ydd5NiISCaYtfkr0mZWFb/3r9sc0pAjjaOylTTSO1vmFG5y81v7Y6Bi RwZ6LQlMgR+kaI7xXp7PlhGKhpHFJvzgTVP0BD9/dI17J7OzfXkDwopSooIqjfbJGJ 0YzUGtBcWV2MQ== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id CB96268BBD for ; Tue, 16 Dec 2025 19:29:22 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id sWPET484DIhf for ; Tue, 16 Dec 2025 19:29:22 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938562; bh=4Bi/EZTK9dI2Qc1n3vOwDJwgzprYFV2ae4qZxAIAQTM=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=JgX3tCM6I4ILwPfVbkhip1xlPNOHVOorwIPUIoGWFhNj5CWMYj8TseT45livM6ybm 6zXUfjyI4PVdpOV3HN4mI07p7xuJrNSOU7w3gZq25NAF0GJRRfwQpW9n2OsqT4FmTk Cf7kzeqOfn+HG8sptiI4AzAxuEHuQihIJ4Dc6RBRydAgYSSQbx1rFtF1Qke0WRN4b6 ktLlNhKNgW3Ydd5NiISCaYtfkr0mZWFb/3r9sc0pAjjaOylTTSO1vmFG5y81v7Y6Bi RwZ6LQlMgR+kaI7xXp7PlhGKhpHFJvzgTVP0BD9/dI17J7OzfXkDwopSooIqjfbJGJ 0YzUGtBcWV2MQ== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id BA39168BA0 for ; Tue, 16 Dec 2025 19:29:22 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938560; bh=VVnlYhTnsPpq7EaaVJQ9Bw/2EPuMvyM7zp/4zOZeobM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tVvmBD7bgN6B+1iwBBQT2/vZ8KSGBLaqw3WLxeWPUVFFIk2j8Agf53vWQg9F3Uhcz 44zcVPMEhHYZije5Of5glkt+OVrLoN29QnPnJEqnRKrL14Jqv1ETSPBy4b2P1rpasT TvdfnrMPsaZkPHyf+Qztn4tAmraAduGFHiL+qNvYpMEpZV5Sz+tKaZTp9R45sAq9p/ /zPTodMuV3I7YKPRLR2tm8/6CbcCx4qTt/RCuDw1Lzm0FLClkNcpWGW2/rN00LFTw8 jWiR9YQhwJvPqrAFq2cyHgkImloqGuAEHGnwdURM1buwdUgh16F1M5XvEEonEsRCUE nTVtETwce2EIw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0AFA368AFF; Tue, 16 Dec 2025 19:29:20 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id R0ZT89k4Tr6S; Tue, 16 Dec 2025 19:29:19 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938555; bh=5NCmHYLULdlaK1npxnTY/07e0Hyn+qlU/hH1gTvGATE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=suuZmJpwUeeHs2M4sPgwE1nHJ83oh3BFnD7Zbp7f2Z+j8EhFsk92RQvlSbVNu0ALL t/b1So0e/XPvyPuDayf0DN/VkFfDg/edpSdNFPSFdoVzleZu4Q0QGCiUIH5XI5WUL6 UyGo4gTjnWfuvw5NfAIK9vvk5BCt8etgOHTWq2kb10pYl1ofQlfbyERUevtCICE7vr 2oP4VULOkajw4SnF3R2lr4qjJtUq9lZFLPY+3D8bx+Ke6WX8DqweHdND5IwJ7ZcJIw ysxeES7XRVae0vUcaiitKOyJcnjBaeu7p5MEOgIPSMOuNqXQbFo9lJLSQqImQMTNto RcJkpQjyfET1Q== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 83AEC6884F; Tue, 16 Dec 2025 19:29:15 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Tue, 16 Dec 2025 19:27:57 -0700 Message-ID: <20251217022823.392557-9-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251217022823.392557-1-sjg@u-boot.org> References: <20251217022823.392557-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: LI4OWPHX55SUP4NRUZB3VPAJADNQT53V X-Message-ID-Hash: LI4OWPHX55SUP4NRUZB3VPAJADNQT53V X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass , "Claude Opus 4 . 5" X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 08/24] pickman: Extract prepare_apply() from do_apply() List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass Extract the setup logic from do_apply() into prepare_apply() which: - Gets the next commits from the source branch - Validates the source exists and has commits - Saves the original branch - Generates or uses provided branch name - Deletes existing branch if needed - Prints info about what will be applied Returns (ApplyInfo, return_code) tuple where ApplyInfo contains commits, branch_name, original_branch, and merge_found. This makes the setup logic testable independently from the agent and git operations. Add tests for prepare_apply(): - test_prepare_apply_error: Test error handling for unknown source - test_prepare_apply_no_commits: Test when no commits to apply - test_prepare_apply_with_commits: Test successful preparation - test_prepare_apply_custom_branch: Test custom branch name Co-developed-by: Claude Opus 4.5 Signed-off-by: Simon Glass --- tools/pickman/control.py | 48 ++++++++++++++--- tools/pickman/ftest.py | 108 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 10 deletions(-) diff --git a/tools/pickman/control.py b/tools/pickman/control.py index 190f92fc57a..aa3fd65ec8e 100644 --- a/tools/pickman/control.py +++ b/tools/pickman/control.py @@ -39,6 +39,11 @@ Commit = namedtuple('Commit', ['hash', 'short_hash', 'subject', 'date']) CommitInfo = namedtuple('CommitInfo', ['hash', 'short_hash', 'subject', 'author']) +# Named tuple for prepare_apply result +ApplyInfo = namedtuple('ApplyInfo', + ['commits', 'branch_name', 'original_branch', + 'merge_found']) + def run_git(args): """Run a git command and return output.""" @@ -323,32 +328,37 @@ def write_history(source, commits, branch_name, conversation_log): tout.info(f'Updated {HISTORY_FILE}') -def do_apply(args, dbs): # pylint: disable=too-many-locals,too-many-branches - """Apply the next set of commits using Claude agent +def prepare_apply(dbs, source, branch): + """Prepare for applying commits from a source branch + + Gets the next commits, sets up the branch name, and prints info about + what will be applied. Args: - args (Namespace): Parsed arguments with 'source' and 'branch' attributes dbs (Database): Database instance + source (str): Source branch name + branch (str): Branch name to use, or None to auto-generate Returns: - int: 0 on success, 1 on failure + 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) """ - source = args.source commits, merge_found, error = get_next_commits(dbs, source) if error: tout.error(error) - return 1 + return None, 1 if not commits: tout.info('No new commits to cherry-pick') - return 0 + return None, 0 # Save current branch to return to later original_branch = run_git(['rev-parse', '--abbrev-ref', 'HEAD']) # Generate branch name if not provided - branch_name = args.branch + branch_name = branch if not branch_name: # Use first commit's short hash as part of branch name branch_name = f'cherry-{commits[0].short_hash}' @@ -372,6 +382,28 @@ def do_apply(args, dbs): # pylint: disable=too-many-locals,too-many-branches tout.info(f' {commit.short_hash} {commit.subject}') tout.info('') + return ApplyInfo(commits, branch_name, original_branch, merge_found), 0 + + +def do_apply(args, dbs): # pylint: disable=too-many-locals,too-many-branches + """Apply the next set of commits using Claude agent + + Args: + args (Namespace): Parsed arguments with 'source' and 'branch' attributes + dbs (Database): Database instance + + Returns: + int: 0 on success, 1 on failure + """ + source = args.source + info, ret = prepare_apply(dbs, source, args.branch) + if not info: + return ret + + commits = info.commits + branch_name = info.branch_name + original_branch = info.original_branch + # Add commits to database with 'pending' status source_id = dbs.source_get_id(source) for commit in commits: diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index d50be16fea7..1418211d4ae 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -976,7 +976,7 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() - args = argparse.Namespace(cmd='apply', source='unknown') + args = argparse.Namespace(cmd='apply', source='unknown', branch=None) with terminal.capture() as (_, stderr): ret = control.do_pickman(args) self.assertEqual(ret, 1) @@ -994,7 +994,7 @@ class TestApply(unittest.TestCase): database.Database.instances.clear() command.TEST_RESULT = command.CommandResult(stdout='') - args = argparse.Namespace(cmd='apply', source='us/next') + args = argparse.Namespace(cmd='apply', source='us/next', branch=None) with terminal.capture() as (stdout, _): ret = control.do_pickman(args) self.assertEqual(ret, 0) @@ -1518,6 +1518,110 @@ Other content self.assertIn('- ccc333c Third commit', commit_msg) +class TestPrepareApply(unittest.TestCase): + """Tests for prepare_apply function.""" + + def setUp(self): + """Set up test fixtures.""" + fd, self.db_path = tempfile.mkstemp(suffix='.db') + os.close(fd) + os.unlink(self.db_path) + self.old_db_fname = control.DB_FNAME + control.DB_FNAME = self.db_path + database.Database.instances.clear() + + def tearDown(self): + """Clean up test fixtures.""" + control.DB_FNAME = self.old_db_fname + if os.path.exists(self.db_path): + os.unlink(self.db_path) + database.Database.instances.clear() + command.TEST_RESULT = None + + def test_prepare_apply_error(self): + """Test prepare_apply returns error code 1 on source not found.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + + info, ret = control.prepare_apply(dbs, 'unknown', None) + + self.assertIsNone(info) + self.assertEqual(ret, 1) + dbs.close() + + def test_prepare_apply_no_commits(self): + """Test prepare_apply returns code 0 when no commits.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + command.TEST_RESULT = command.CommandResult(stdout='') + + info, ret = control.prepare_apply(dbs, 'us/next', None) + + self.assertIsNone(info) + self.assertEqual(ret, 0) + dbs.close() + + def test_prepare_apply_with_commits(self): + """Test prepare_apply returns ApplyInfo with commits.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + log_output = 'aaa111|aaa111a|Author 1|First commit|abc123\n' + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if 'log' in cmd: + return command.CommandResult(stdout=log_output) + if 'rev-parse' in cmd: + return command.CommandResult(stdout='master') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, ret = control.prepare_apply(dbs, 'us/next', None) + + self.assertIsNotNone(info) + self.assertEqual(ret, 0) + self.assertEqual(len(info.commits), 1) + self.assertEqual(info.branch_name, 'cherry-aaa111a') + self.assertEqual(info.original_branch, 'master') + dbs.close() + + def test_prepare_apply_custom_branch(self): + """Test prepare_apply uses custom branch name.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + log_output = 'aaa111|aaa111a|Author 1|First commit|abc123\n' + + def mock_git(pipe_list): + cmd = pipe_list[0] if pipe_list else [] + if 'log' in cmd: + return command.CommandResult(stdout=log_output) + if 'rev-parse' in cmd: + return command.CommandResult(stdout='master') + return command.CommandResult(stdout='') + + command.TEST_RESULT = mock_git + + info, _ = control.prepare_apply(dbs, 'us/next', 'my-branch') + + self.assertIsNotNone(info) + self.assertEqual(info.branch_name, 'my-branch') + dbs.close() + + class TestGetNextCommitsEmptyLine(unittest.TestCase): """Tests for get_next_commits with empty lines."""