From patchwork Wed Dec 17 02:27:55 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 949 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=1765938552; bh=MouFL+Yo/GBQvwRxP3L8qCJ/JPlNdH6VKjwcLFb4Azg=; 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=aaJ0RXWK7XJy/gXNUpLRqrJMTm61+rNfozc5B88tH7a2dXRJL3yQ5Z7mlDay3rKe9 0yzxFX99KF+L7lxoLfsq80uoamqXzPR1fvzj97Hu1i4xN2ynxwN1+MaCcyCHtOSnWn XX3f9XLIhJvXveAc8/ThsAoh8bNaOKR0+oNVZiDmX1/BXOO3UrN+LTw3tIMCHf1a0w jo9PQhadtwgSECH+iJa+gILtlZa4r/8oXE1dKZRo9pPWCTOcbCP6yLxbOCxrqtvIOa 3l3HP9FPShqvfzqioNEHrc7z8ALpWqfl0JwKEknqDFc9hCOghTf+e6pwQJ9xfZqWLa +fXBokTgh7PBA== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7C66B68AFF for ; Tue, 16 Dec 2025 19:29:12 -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 jJgH9X4LVyh4 for ; Tue, 16 Dec 2025 19:29:12 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938552; bh=MouFL+Yo/GBQvwRxP3L8qCJ/JPlNdH6VKjwcLFb4Azg=; 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=aaJ0RXWK7XJy/gXNUpLRqrJMTm61+rNfozc5B88tH7a2dXRJL3yQ5Z7mlDay3rKe9 0yzxFX99KF+L7lxoLfsq80uoamqXzPR1fvzj97Hu1i4xN2ynxwN1+MaCcyCHtOSnWn XX3f9XLIhJvXveAc8/ThsAoh8bNaOKR0+oNVZiDmX1/BXOO3UrN+LTw3tIMCHf1a0w jo9PQhadtwgSECH+iJa+gILtlZa4r/8oXE1dKZRo9pPWCTOcbCP6yLxbOCxrqtvIOa 3l3HP9FPShqvfzqioNEHrc7z8ALpWqfl0JwKEknqDFc9hCOghTf+e6pwQJ9xfZqWLa +fXBokTgh7PBA== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 6435768BA0 for ; Tue, 16 Dec 2025 19:29:12 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938550; bh=b+ude6wIEC+F7dm6nRZxnaoQ2PlqI/km93/Lg2dgAGQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=u7xXjiT7b+ZBX8WLkgsufDaFEB2KvCqGBnyyyw+C5nR6CMCNdGSgHl1ucR47MuN3e pRo2hL+k5DMU00e21YM4rrHnMvLxXsfoTk2/T7FGb0qikzEPgXQcWlitryyVhkqY8t SZlIlQLxBsOxXHUBdW6kmWfiO/nCiN64IZSscLdvWyorpqtHof1UBVfdHEYTJdewQe mFWoj0Go6MfB4cFH6Eihi+AITA9XPzxKnnZa7hZH+iwTbEi9cqTkZNt5gtL4KwQLBr Xk/5YiR/cYveMJZ1+fridHwsXOekK5ay/RBklBmSDpypU8GRIFuAXSvm6nbECo/IV8 CdYz5OKVWGsVw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7C32B68ABD; Tue, 16 Dec 2025 19:29:10 -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 2TFOU0KVj2yB; Tue, 16 Dec 2025 19:29:10 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765938546; bh=JQZY6jpnrTdV1QHW+SwoCxqH6eMbZWrVOoEs4xV0HC4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=M4lAI8nkPbFmsIxS+o2Pbl40rRCVKJ2qI/ee4xSjDkMNbWKbD8snAiNJ7M/Me0z9N qF/7aR/cwnBg+HqiqjMP8t/AvROtZ3EcYNS3llmDflqociEcn9CMmb4oX1S5ks9PoB 7FkIymoAheKgkB8YGx/RZbLHXmfpr6pN814/a2ooFx47OLMqRrj1DMA5ncq/rh3jLw IxKpMpUA+mg2pKquK1Cr3ozAOO4Q4D8yIzJg9YugRSqLsbiqeWuwjACuNC48Bv58qx 9OC4M8qd9sD9GGfmejvgS/+/9i1PFYWy/BGm3YyOwBy4FIn9ZdjbiQOOOE63QJhklY cBus2L1I+RxsA== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 8F46D6884F; Tue, 16 Dec 2025 19:29:06 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Tue, 16 Dec 2025 19:27:55 -0700 Message-ID: <20251217022823.392557-7-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: MU3SE6MOIL4BJQMJXR5DH6BNMDYUP3JS X-Message-ID-Hash: MU3SE6MOIL4BJQMJXR5DH6BNMDYUP3JS 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 06/24] pickman: Add tests to improve control.py coverage 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 Add tests to improve test coverage for control.py: - test_poll_continues_on_step_error: Test poll warning on step error - test_format_history_summary: Test history summary formatting - test_get_next_commits_with_empty_lines: Test empty line handling - test_commit_source_resolve_error: Test commit resolution failure - test_unknown_command: Test unknown command handling - test_review_with_mrs_no_comments: Test review with open MRs This improves control.py coverage from 63% to 67%. Co-developed-by: Claude Opus 4.5 Signed-off-by: Simon Glass --- tools/pickman/ftest.py | 179 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index cc4a5727ac4..8865296ff11 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -20,6 +20,7 @@ sys.path.insert(0, os.path.join(our_path, '..')) # pylint: disable=wrong-import-position,import-error,cyclic-import from u_boot_pylib import command from u_boot_pylib import terminal +from u_boot_pylib import tout from pickman import __main__ as pickman from pickman import control @@ -1363,6 +1364,184 @@ class TestPoll(unittest.TestCase): self.assertEqual(ret, 0) self.assertEqual(call_count[0], 2) + def test_poll_continues_on_step_error(self): + """Test poll continues when step returns non-zero.""" + call_count = [0] + + def mock_step(_args, _dbs): + call_count[0] += 1 + if call_count[0] >= 2: + raise KeyboardInterrupt + return 1 # Return error + + with mock.patch.object(control, 'do_step', mock_step): + with mock.patch('time.sleep'): + args = argparse.Namespace( + cmd='poll', source='us/next', interval=1, + remote='ci', target='master' + ) + with terminal.capture() as (_, stderr): + ret = control.do_poll(args, None) + + self.assertEqual(ret, 0) + self.assertIn('returned 1', stderr.getvalue()) + + +class TestFormatHistorySummary(unittest.TestCase): + """Tests for format_history_summary function.""" + + def test_format_history_summary(self): + """Test formatting history summary.""" + commits = [ + control.CommitInfo('aaa111', 'aaa111a', 'First commit', 'Author 1'), + control.CommitInfo('bbb222', 'bbb222b', 'Second commit', 'Author 2'), + ] + result = control.format_history_summary('us/next', commits, 'cherry-abc') + + self.assertIn('us/next', result) + self.assertIn('Branch: cherry-abc', result) + self.assertIn('- aaa111a First commit', result) + self.assertIn('- bbb222b Second commit', result) + + +class TestGetNextCommitsEmptyLine(unittest.TestCase): + """Tests for get_next_commits with empty lines.""" + + 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_get_next_commits_with_empty_lines(self): + """Test get_next_commits handles empty lines in output.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'abc123') + dbs.commit() + + # Log output with empty lines + log_output = ( + 'aaa111|aaa111a|Author 1|First commit|abc123\n' + '\n' # Empty line + 'bbb222|bbb222b|Author 2|Second commit|aaa111\n' + '\n' # Another empty line + ) + command.TEST_RESULT = command.CommandResult(stdout=log_output) + + commits, merge_found, error = control.get_next_commits(dbs, + 'us/next') + self.assertIsNone(error) + self.assertFalse(merge_found) + self.assertEqual(len(commits), 2) + dbs.close() + + +class TestDoCommitSourceResolveError(unittest.TestCase): + """Tests for do_commit_source error handling.""" + + 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_commit_source_resolve_error(self): + """Test commit-source fails when commit can't be resolved.""" + with terminal.capture(): + dbs = database.Database(self.db_path) + dbs.start() + dbs.source_set('us/next', 'oldcommit12345') + dbs.commit() + + database.Database.instances.clear() + + def mock_git_fail(**_kwargs): + raise command.CommandExc('git error', command.CommandResult()) + + command.TEST_RESULT = mock_git_fail + + args = argparse.Namespace(cmd='commit-source', source='us/next', + commit='badcommit') + with terminal.capture() as (_, stderr): + ret = control.do_commit_source(args, None) + self.assertEqual(ret, 1) + self.assertIn('Could not resolve', stderr.getvalue()) + + +class TestDoPickmanUnknownCommand(unittest.TestCase): + """Tests for do_pickman with unknown command.""" + + 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() + + def test_unknown_command(self): + """Test do_pickman returns 1 for unknown command.""" + args = argparse.Namespace(cmd='unknown-command') + with terminal.capture(): + ret = control.do_pickman(args) + self.assertEqual(ret, 1) + + +class TestDoReviewWithMrs(unittest.TestCase): + """Tests for do_review with open MRs.""" + + def test_review_with_mrs_no_comments(self): + """Test review with open MRs but no comments.""" + tout.init(tout.INFO) + + mock_mr = { + 'iid': 123, + 'title': '[pickman] Test MR', + 'web_url': 'https://gitlab.com/mr/123', + } + with mock.patch.object(gitlab_api, 'get_open_pickman_mrs', + return_value=[mock_mr]): + with mock.patch.object(gitlab_api, 'get_mr_comments', + return_value=[]): + args = argparse.Namespace(cmd='review', remote='ci') + with terminal.capture() as (stdout, _): + ret = control.do_review(args, None) + + self.assertEqual(ret, 0) + self.assertIn('Found 1 open pickman MR', stdout.getvalue()) + if __name__ == '__main__': unittest.main()