From patchwork Wed Dec 24 21:30:33 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 1073 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=1766611870; bh=c/i4TlOveaz/7RIxZUO5eUAzxC2qlO/yUbKAV6ck5tY=; 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=HsWbX/U38NzC0444pOtbKUJQCsUtuIu5JgM/56e+VCWFlXH/E5QDwxx/7jDO3eokd gTZv1xJMGnKwWr1434MtZGHISitF34QZn/PP9F4+YlNrcizW7GdEBZWl44evdhTjco zxri0yF6IkaFIx2pBgTKOiWw3+Jb5zd4S/E333RBbRkxFPbdBL29k6Nf+r13xYqDwc 1EAmDQl3HM88B+b0CpqLKLAEz70an679BnYdzhjOUf8cDn1kYzP4cRBtFeeCypbbvO lDHFQwc1uDYs/3O0QyJ7sv1/0LEmHYMOTOBd75imky0/4bxiHXLe2zHHkiTdxjcMqV 1csBymxFa149A== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 61BDF64CA9 for ; Wed, 24 Dec 2025 14:31: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 10024) with ESMTP id M5sAbIT7L4tl for ; Wed, 24 Dec 2025 14:31:10 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1766611870; bh=c/i4TlOveaz/7RIxZUO5eUAzxC2qlO/yUbKAV6ck5tY=; 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=HsWbX/U38NzC0444pOtbKUJQCsUtuIu5JgM/56e+VCWFlXH/E5QDwxx/7jDO3eokd gTZv1xJMGnKwWr1434MtZGHISitF34QZn/PP9F4+YlNrcizW7GdEBZWl44evdhTjco zxri0yF6IkaFIx2pBgTKOiWw3+Jb5zd4S/E333RBbRkxFPbdBL29k6Nf+r13xYqDwc 1EAmDQl3HM88B+b0CpqLKLAEz70an679BnYdzhjOUf8cDn1kYzP4cRBtFeeCypbbvO lDHFQwc1uDYs/3O0QyJ7sv1/0LEmHYMOTOBd75imky0/4bxiHXLe2zHHkiTdxjcMqV 1csBymxFa149A== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4BCAD64DD8 for ; Wed, 24 Dec 2025 14:31:10 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1766611868; bh=6uJ1Z8TQcpekjXTSpdqOzq1Ettw49JofhPTUqcrukT8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Sn+V7vGqdt3SoZ0YRamZzGTb49Ot8d/NK2LbeHpWklB8SG8VlZu3NR8MHUUGclT5O /YKvKmCdu6INiUZPo1dNVV7EUNiRs8I1oHVtfnmOel4HWy5ixxpHiicKQSNQgN4hMN GyquzZGlhA18VdFMlRzw37nhjxi+UCUHpuRxmMugLeYy6OMitGaAv0NAi+y18COJpL 3msv978ullKrnkCGBlp/7z/bf5ABGNziMTFlHI2AxhKO8ZzJSEyM9XzeVh/JONQuTN CGoCDe5+gYb1w4LxL654t+QMeZMFgnkGqv5waZDqDlopJ0EVCRN2ESs1zXL5FOTQGc 59GQlrc/wBmkg== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 4676464CA9; Wed, 24 Dec 2025 14:31:08 -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 tW-aLGdXJEK3; Wed, 24 Dec 2025 14:31:08 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1766611862; bh=9JvJqnVGBLDEqs6WRYDV2LdF48Oh0AvQfuxswOehmB4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Fby+UDn+XSHxzqeeLQGC1jPH60QO6IMe/XIaGMThCnP/ghRsAgAxIyXqi4/FHAgXj fZnQuEDiYt8UdIwnhlWoPgB0Pw1IH6lqVxOf0Zoxk3K9g+H6JaaLYMJdAICsV2juT9 C8jfqLAUxXmRQiydeUnvn86Y/3iYyu7/4dwKhkMVfoevw9Nix220VH4Ku4TgcvfD4t bExL3Z4mzwfTzqJaSTH5JQUuWExiRW7j+DAA2HCZjI4yUNnlNRZzr/oOW2qs/TjZvf feEv0ByCFOZr2TuNzq0zD0R4lmq5h9wUFYrWWM44bBKucB2l0FmRfqKDj9Z7iuFtD7 qapyUx75FtRIw== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 35E8E64DD8; Wed, 24 Dec 2025 14:31:02 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Wed, 24 Dec 2025 14:30:33 -0700 Message-ID: <20251224213045.3010514-3-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251224213045.3010514-1-sjg@u-boot.org> References: <20251224213045.3010514-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: 5HAU2T6ROJL62EGAJRXOXNOHRFYP5YBI X-Message-ID-Hash: 5HAU2T6ROJL62EGAJRXOXNOHRFYP5YBI 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 X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 2/9] pickman: Add URL constants to improve test readability 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 Replace long, hardcoded URLs in tests with named constants defined at the top of the file. This improves code maintainability by providing a single point of change for test URLs and helps with line-length violations. Co-developed-by: Claude Signed-off-by: Simon Glass --- tools/pickman/ftest.py | 48 ++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/tools/pickman/ftest.py b/tools/pickman/ftest.py index 9d075586c76..5f126984c9e 100644 --- a/tools/pickman/ftest.py +++ b/tools/pickman/ftest.py @@ -30,6 +30,15 @@ from pickman import control from pickman import database from pickman import gitlab_api +# Test URL constants +TEST_OAUTH_URL = 'https://oauth2:test-token@gitlab.com/group/project.git' +TEST_HTTPS_URL = 'https://gitlab.com/group/project.git' +TEST_SSH_URL = 'git@gitlab.com:group/project.git' +TEST_MR_URL = 'https://gitlab.com/group/project/-/merge_requests/42' +TEST_MR_42_URL = 'https://gitlab.com/mr/42' +TEST_MR_1_URL = 'https://gitlab.com/mr/1' +TEST_SHORT_OAUTH_URL = 'https://oauth2:token@gitlab.com/g/p.git' + class TestCommit(unittest.TestCase): """Tests for the Commit namedtuple.""" @@ -230,7 +239,8 @@ class TestMain(unittest.TestCase): self.assertEqual(ret, 0) # Filter out database migration messages output_lines = [l for l in stdout.getvalue().splitlines() - if not l.startswith(('Update database', 'Creating'))] + if not l.startswith(('Update database', + 'Creating'))] lines = iter(output_lines) self.assertEqual('Commits in us/next not in ci/master: 10', next(lines)) @@ -562,7 +572,7 @@ class TestDatabaseMergereq(unittest.TestCase): # Add a merge request dbs.mergereq_add(source_id, 'cherry-abc123', 42, 'open', - 'https://gitlab.com/mr/42', '2025-01-15') + TEST_MR_42_URL, '2025-01-15') dbs.commit() # Get the merge request @@ -572,7 +582,7 @@ class TestDatabaseMergereq(unittest.TestCase): self.assertEqual(result[2], 'cherry-abc123') # branch_name self.assertEqual(result[3], 42) # mr_id self.assertEqual(result[4], 'open') # status - self.assertEqual(result[5], 'https://gitlab.com/mr/42') # url + self.assertEqual(result[5], TEST_MR_42_URL) # url self.assertEqual(result[6], '2025-01-15') # created_at dbs.close() @@ -598,7 +608,7 @@ class TestDatabaseMergereq(unittest.TestCase): # Add merge requests dbs.mergereq_add(source_id, 'branch-1', 1, 'open', - 'https://gitlab.com/mr/1', '2025-01-01') + TEST_MR_1_URL, '2025-01-01') dbs.mergereq_add(source_id, 'branch-2', 2, 'merged', 'https://gitlab.com/mr/2', '2025-01-02') dbs.mergereq_add(source_id, 'branch-3', 3, 'open', @@ -630,7 +640,7 @@ class TestDatabaseMergereq(unittest.TestCase): source_id = dbs.source_get_id('us/next') dbs.mergereq_add(source_id, 'branch-1', 42, 'open', - 'https://gitlab.com/mr/42', '2025-01-15') + TEST_MR_42_URL, '2025-01-15') dbs.commit() # Update status @@ -671,7 +681,7 @@ class TestDatabaseCommitMergereq(unittest.TestCase): # Add merge request dbs.mergereq_add(source_id, 'branch-1', 42, 'open', - 'https://gitlab.com/mr/42', '2025-01-15') + TEST_MR_42_URL, '2025-01-15') dbs.commit() mr = dbs.mergereq_get(42) mr_id = mr[0] # id field @@ -701,7 +711,7 @@ class TestDatabaseCommitMergereq(unittest.TestCase): # Add merge request dbs.mergereq_add(source_id, 'branch-1', 42, 'open', - 'https://gitlab.com/mr/42', '2025-01-15') + TEST_MR_42_URL, '2025-01-15') dbs.commit() mr = dbs.mergereq_get(42) mr_id = mr[0] @@ -1438,10 +1448,11 @@ class TestGetPushUrl(unittest.TestCase): """Test successful push URL generation.""" with mock.patch.object(gitlab_api, 'get_token', return_value='test-token'): - with mock.patch.object(gitlab_api, 'get_remote_url', - return_value='git@gitlab.com:group/project.git'): + with mock.patch.object( + gitlab_api, 'get_remote_url', + return_value=TEST_SSH_URL): url = gitlab_api.get_push_url('origin') - self.assertEqual(url, 'https://oauth2:test-token@gitlab.com/group/project.git') + self.assertEqual(url, TEST_OAUTH_URL) def test_get_push_url_no_token(self): """Test returns None when no token available.""" @@ -1463,9 +1474,9 @@ class TestGetPushUrl(unittest.TestCase): with mock.patch.object(gitlab_api, 'get_token', return_value='test-token'): with mock.patch.object(gitlab_api, 'get_remote_url', - return_value='https://gitlab.com/group/project.git'): + return_value=TEST_HTTPS_URL): url = gitlab_api.get_push_url('origin') - self.assertEqual(url, 'https://oauth2:test-token@gitlab.com/group/project.git') + self.assertEqual(url, TEST_OAUTH_URL) class TestPushBranch(unittest.TestCase): @@ -1474,7 +1485,7 @@ class TestPushBranch(unittest.TestCase): def test_push_branch_force_with_remote_ref(self): """Test force push when remote branch exists uses --force-with-lease.""" with mock.patch.object(gitlab_api, 'get_push_url', - return_value='https://oauth2:token@gitlab.com/g/p.git'): + return_value=TEST_SHORT_OAUTH_URL): with mock.patch.object(command, 'output') as mock_output: result = gitlab_api.push_branch('ci', 'test-branch', force=True) @@ -1482,14 +1493,15 @@ class TestPushBranch(unittest.TestCase): # Should fetch first, then push with --force-with-lease calls = mock_output.call_args_list self.assertEqual(len(calls), 2) - self.assertEqual(calls[0], mock.call('git', 'fetch', 'ci', 'test-branch')) + self.assertEqual(calls[0], mock.call('git', 'fetch', 'ci', + 'test-branch')) push_args = calls[1][0] self.assertIn('--force-with-lease=refs/remotes/ci/test-branch', push_args) def test_push_branch_force_no_remote_ref(self): """Test force push when remote branch doesn't exist uses --force.""" with mock.patch.object(gitlab_api, 'get_push_url', - return_value='https://oauth2:token@gitlab.com/g/p.git'): + return_value=TEST_SHORT_OAUTH_URL): with mock.patch.object(command, 'output') as mock_output: # Fetch fails (branch doesn't exist on remote) mock_output.side_effect = [ @@ -1510,7 +1522,7 @@ class TestPushBranch(unittest.TestCase): def test_push_branch_no_force(self): """Test regular push without force doesn't fetch or use force flags.""" with mock.patch.object(gitlab_api, 'get_push_url', - return_value='https://oauth2:token@gitlab.com/g/p.git'): + return_value=TEST_SHORT_OAUTH_URL): with mock.patch.object(command, 'output') as mock_output: result = gitlab_api.push_branch('ci', 'test-branch', force=False) @@ -1682,7 +1694,7 @@ class TestGetPickmanMrs(unittest.TestCase): mock_mr1 = mock.MagicMock() mock_mr1.iid = 1 mock_mr1.title = '[pickman] Older MR' - mock_mr1.web_url = 'https://gitlab.com/mr/1' + mock_mr1.web_url = TEST_MR_1_URL mock_mr1.source_branch = 'cherry-1' mock_mr1.description = 'desc1' mock_mr1.has_conflicts = False @@ -1731,7 +1743,7 @@ class TestCreateMr(unittest.TestCase): # Create mock existing MR mock_existing_mr = mock.MagicMock() - mock_existing_mr.web_url = 'https://gitlab.com/group/project/-/merge_requests/42' + mock_existing_mr.web_url = TEST_MR_URL mock_project = mock.MagicMock() mock_project.mergerequests.list.return_value = [mock_existing_mr]