[Concept,2/9] pickman: Add URL constants to improve test readability

Message ID 20251224213045.3010514-3-sjg@u-boot.org
State New
Headers
Series pickman: Provide better ways to check cherry-picks |

Commit Message

Simon Glass Dec. 24, 2025, 9:30 p.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

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 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 tools/pickman/ftest.py | 48 ++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 18 deletions(-)
  

Patch

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]