@@ -21,7 +21,7 @@ SIGNAL_FILE = '.pickman-signal'
# Signal status codes
SIGNAL_SUCCESS = 'success'
-SIGNAL_ALREADY_APPLIED = 'already_applied'
+SIGNAL_APPLIED = 'already_applied'
SIGNAL_CONFLICT = 'conflict'
# Check if claude_agent_sdk is available
@@ -537,7 +537,7 @@ def handle_skip_comments(remote, mr_iid, title, unresolved, dbs):
return True
-def format_history_summary(source, commits, branch_name):
+def format_history(source, commits, branch_name):
"""Format a summary of the cherry-pick operation
Args:
@@ -575,7 +575,7 @@ def get_history(fname, source, commits, branch_name, conv_log):
tuple: (content, commit_msg) where content is the updated history
and commit_msg is the git commit message
"""
- summary = format_history_summary(source, commits, branch_name)
+ summary = format_history(source, commits, branch_name)
entry = f"""{summary}
### Conversation log
@@ -735,7 +735,7 @@ def handle_already_applied(dbs, source, commits, branch_name, conv_log, args,
# Use merge commit subject as title with [skip] prefix
title = f'{SKIPPED_TAG} [pickman] {commits[-1].subject}'
- summary = format_history_summary(source, commits, branch_name)
+ summary = format_history(source, commits, branch_name)
description = (f'{summary}\n\n'
f'**Status:** Commits already applied to {target} '
f'with different hashes.\n\n'
@@ -778,7 +778,7 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t
# Check for signal file from agent
signal_status, signal_commit = agent.read_signal_file()
- if signal_status == agent.SIGNAL_ALREADY_APPLIED:
+ if signal_status == agent.SIGNAL_APPLIED:
ret = handle_already_applied(dbs, source, commits, branch_name,
conv_log, args, signal_commit)
return ret, False, conv_log
@@ -809,7 +809,7 @@ def execute_apply(dbs, source, commits, branch_name, args): # pylint: disable=t
title = f'[pickman] {commits[-1].subject}'
# Description matches .pickman-history entry
# (summary + conversation)
- summary = format_history_summary(source, commits, branch_name)
+ summary = format_history(source, commits, branch_name)
description = f'{summary}\n\n### Conversation log\n{conv_log}'
mr_url = gitlab_api.push_and_create_mr(
@@ -1000,10 +1000,10 @@ def process_single_mr(remote, merge_req, dbs, target):
new_desc = (f"{old_desc}\n\n### Review response\n\n"
f"**Comments addressed:**\n{comment_summary}\n\n"
f"**Response:**\n{conversation_log}")
- gitlab_api.update_mr_description(remote, mr_iid, new_desc)
+ gitlab_api.update_mr_desc(remote, mr_iid, new_desc)
# Update .pickman-history
- update_history_with_review(merge_req.source_branch,
+ update_history(merge_req.source_branch,
unresolved, conversation_log)
tout.info(f'Updated MR !{mr_iid} description and history')
@@ -1047,7 +1047,7 @@ def process_mr_reviews(remote, mrs, dbs, target='master'):
return processed
-def update_history_with_review(branch_name, comments, conversation_log):
+def update_history(branch_name, comments, conversation_log):
"""Append review handling to .pickman-history
Args:
@@ -28,7 +28,7 @@ from pickman import __main__ as pickman
from pickman import agent
from pickman import control
from pickman import database
-from pickman import gitlab_api
+from pickman import gitlab_api as gitlab
# Test URL constants
TEST_OAUTH_URL = 'https://oauth2:test-token@gitlab.com/group/project.git'
@@ -1370,55 +1370,55 @@ class TestParseUrl(unittest.TestCase):
def test_parse_ssh_url(self):
"""Test parsing SSH URL."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'git@gitlab.com:group/project.git')
self.assertEqual(host, 'gitlab.com')
self.assertEqual(path, 'group/project')
def test_parse_ssh_url_no_git_suffix(self):
"""Test parsing SSH URL without .git suffix."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'git@gitlab.com:group/project')
self.assertEqual(host, 'gitlab.com')
self.assertEqual(path, 'group/project')
def test_parse_ssh_url_nested_group(self):
"""Test parsing SSH URL with nested group."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'git@gitlab.denx.de:u-boot/custodians/u-boot-dm.git')
self.assertEqual(host, 'gitlab.denx.de')
self.assertEqual(path, 'u-boot/custodians/u-boot-dm')
def test_parse_https_url(self):
"""Test parsing HTTPS URL."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'https://gitlab.com/group/project.git')
self.assertEqual(host, 'gitlab.com')
self.assertEqual(path, 'group/project')
def test_parse_https_url_no_git_suffix(self):
"""Test parsing HTTPS URL without .git suffix."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'https://gitlab.com/group/project')
self.assertEqual(host, 'gitlab.com')
self.assertEqual(path, 'group/project')
def test_parse_http_url(self):
"""Test parsing HTTP URL."""
- host, path = gitlab_api.parse_url(
+ host, path = gitlab.parse_url(
'http://gitlab.example.com/group/project.git')
self.assertEqual(host, 'gitlab.example.com')
self.assertEqual(path, 'group/project')
def test_parse_invalid_url(self):
"""Test parsing invalid URL."""
- host, path = gitlab_api.parse_url('not-a-valid-url')
+ host, path = gitlab.parse_url('not-a-valid-url')
self.assertIsNone(host)
self.assertIsNone(path)
def test_parse_empty_url(self):
"""Test parsing empty URL."""
- host, path = gitlab_api.parse_url('')
+ host, path = gitlab.parse_url('')
self.assertIsNone(host)
self.assertIsNone(path)
@@ -1428,16 +1428,16 @@ class TestCheckAvailable(unittest.TestCase):
def test_check_available_false(self):
"""Test check_available returns False when gitlab not installed."""
- with mock.patch.object(gitlab_api, 'AVAILABLE', False):
+ with mock.patch.object(gitlab, 'AVAILABLE', False):
with terminal.capture():
- result = gitlab_api.check_available()
+ result = gitlab.check_available()
self.assertFalse(result)
def test_check_available_true(self):
"""Test check_available returns True when gitlab is installed."""
- with mock.patch.object(gitlab_api, 'AVAILABLE', True):
+ with mock.patch.object(gitlab, 'AVAILABLE', True):
with terminal.capture():
- result = gitlab_api.check_available()
+ result = gitlab.check_available()
self.assertTrue(result)
@@ -1446,36 +1446,36 @@ class TestGetPushUrl(unittest.TestCase):
def test_get_push_url_success(self):
"""Test successful push URL generation."""
- with mock.patch.object(gitlab_api, 'get_token',
+ with mock.patch.object(gitlab, 'get_token',
return_value='test-token'):
with mock.patch.object(
- gitlab_api, 'get_remote_url',
+ gitlab, 'get_remote_url',
return_value=TEST_SSH_URL):
- url = gitlab_api.get_push_url('origin')
+ url = gitlab.get_push_url('origin')
self.assertEqual(url, TEST_OAUTH_URL)
def test_get_push_url_no_token(self):
"""Test returns None when no token available."""
- with mock.patch.object(gitlab_api, 'get_token', return_value=None):
- url = gitlab_api.get_push_url('origin')
+ with mock.patch.object(gitlab, 'get_token', return_value=None):
+ url = gitlab.get_push_url('origin')
self.assertIsNone(url)
def test_get_push_url_invalid_remote(self):
"""Test returns None for invalid remote URL."""
- with mock.patch.object(gitlab_api, 'get_token',
+ with mock.patch.object(gitlab, 'get_token',
return_value='test-token'):
- with mock.patch.object(gitlab_api, 'get_remote_url',
+ with mock.patch.object(gitlab, 'get_remote_url',
return_value='not-a-valid-url'):
- url = gitlab_api.get_push_url('origin')
+ url = gitlab.get_push_url('origin')
self.assertIsNone(url)
def test_get_push_url_https_remote(self):
"""Test with HTTPS remote URL."""
- with mock.patch.object(gitlab_api, 'get_token',
+ with mock.patch.object(gitlab, 'get_token',
return_value='test-token'):
- with mock.patch.object(gitlab_api, 'get_remote_url',
+ with mock.patch.object(gitlab, 'get_remote_url',
return_value=TEST_HTTPS_URL):
- url = gitlab_api.get_push_url('origin')
+ url = gitlab.get_push_url('origin')
self.assertEqual(url, TEST_OAUTH_URL)
@@ -1484,10 +1484,10 @@ 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',
+ with mock.patch.object(gitlab, 'get_push_url',
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)
+ result = gitlab.push_branch('ci', 'test-branch', force=True)
self.assertTrue(result)
# Should fetch first, then push with --force-with-lease
@@ -1496,11 +1496,12 @@ class TestPushBranch(unittest.TestCase):
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)
+ 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',
+ with mock.patch.object(gitlab, 'get_push_url',
return_value=TEST_SHORT_OAUTH_URL):
with mock.patch.object(command, 'output') as mock_output:
# Fetch fails (branch doesn't exist on remote)
@@ -1509,10 +1510,11 @@ class TestPushBranch(unittest.TestCase):
command.CommandResult()), # fetch fails
None, # push succeeds
]
- result = gitlab_api.push_branch('ci', 'new-branch', force=True)
+ result = gitlab.push_branch('ci', 'new-branch', force=True)
self.assertTrue(result)
- # Should try fetch, fail, then push with --force (not --force-with-lease)
+ # Should try fetch, fail, then push with --force
+ # (not --force-with-lease)
calls = mock_output.call_args_list
self.assertEqual(len(calls), 2)
push_args = calls[1][0]
@@ -1521,10 +1523,10 @@ 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',
+ with mock.patch.object(gitlab, 'get_push_url',
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)
+ result = gitlab.push_branch('ci', 'test-branch', force=False)
self.assertTrue(result)
# Should only push, no fetch, no force flags
@@ -1552,16 +1554,16 @@ class TestConfigFile(unittest.TestCase):
with open(self.config_file, 'w', encoding='utf-8') as fhandle:
fhandle.write('[gitlab]\ntoken = test-config-token\n')
- with mock.patch.object(gitlab_api, 'CONFIG_FILE', self.config_file):
- token = gitlab_api.get_token()
+ with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file):
+ token = gitlab.get_token()
self.assertEqual(token, 'test-config-token')
def test_get_token_fallback_to_env(self):
"""Test falling back to environment variable."""
# Config file doesn't exist
- with mock.patch.object(gitlab_api, 'CONFIG_FILE', '/nonexistent/path'):
+ with mock.patch.object(gitlab, 'CONFIG_FILE', '/nonexistent/path'):
with mock.patch.dict(os.environ, {'GITLAB_TOKEN': 'env-token'}):
- token = gitlab_api.get_token()
+ token = gitlab.get_token()
self.assertEqual(token, 'env-token')
def test_get_token_config_missing_section(self):
@@ -1569,9 +1571,9 @@ class TestConfigFile(unittest.TestCase):
with open(self.config_file, 'w', encoding='utf-8') as fhandle:
fhandle.write('[other]\nkey = value\n')
- with mock.patch.object(gitlab_api, 'CONFIG_FILE', self.config_file):
+ with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file):
with mock.patch.dict(os.environ, {'GITLAB_TOKEN': 'env-token'}):
- token = gitlab_api.get_token()
+ token = gitlab.get_token()
self.assertEqual(token, 'env-token')
def test_get_config_value(self):
@@ -1579,17 +1581,17 @@ class TestConfigFile(unittest.TestCase):
with open(self.config_file, 'w', encoding='utf-8') as fhandle:
fhandle.write('[section1]\nkey1 = value1\n')
- with mock.patch.object(gitlab_api, 'CONFIG_FILE', self.config_file):
- value = gitlab_api.get_config_value('section1', 'key1')
+ with mock.patch.object(gitlab, 'CONFIG_FILE', self.config_file):
+ value = gitlab.get_config_value('section1', 'key1')
self.assertEqual(value, 'value1')
class TestCheckPermissions(unittest.TestCase):
"""Tests for check_permissions function."""
- @mock.patch.object(gitlab_api, 'get_remote_url')
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
+ @mock.patch.object(gitlab, 'get_remote_url')
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
def test_check_permissions_developer(self, mock_token, mock_url):
"""Test checking permissions for a developer."""
mock_token.return_value = 'test-token'
@@ -1610,7 +1612,7 @@ class TestCheckPermissions(unittest.TestCase):
mock_glab.projects.get.return_value = mock_project
with mock.patch('gitlab.Gitlab', return_value=mock_glab):
- perms = gitlab_api.check_permissions('origin')
+ perms = gitlab.check_permissions('origin')
self.assertIsNotNone(perms)
self.assertEqual(perms.user, 'testuser')
@@ -1620,30 +1622,30 @@ class TestCheckPermissions(unittest.TestCase):
self.assertTrue(perms.can_create_mr)
self.assertFalse(perms.can_merge)
- @mock.patch.object(gitlab_api, 'AVAILABLE', False)
+ @mock.patch.object(gitlab, 'AVAILABLE', False)
def test_check_permissions_not_available(self):
"""Test check_permissions when gitlab not available."""
with terminal.capture():
- perms = gitlab_api.check_permissions('origin')
+ perms = gitlab.check_permissions('origin')
self.assertIsNone(perms)
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
def test_check_permissions_no_token(self, mock_token):
"""Test check_permissions when no token set."""
mock_token.return_value = None
with terminal.capture():
- perms = gitlab_api.check_permissions('origin')
+ perms = gitlab.check_permissions('origin')
self.assertIsNone(perms)
class TestUpdateMrDescription(unittest.TestCase):
- """Tests for update_mr_description function."""
+ """Tests for update_mr_desc function."""
- @mock.patch.object(gitlab_api, 'get_remote_url')
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
- def test_update_mr_description_success(self, mock_token, mock_url):
+ @mock.patch.object(gitlab, 'get_remote_url')
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
+ def test_update_mr_desc_success(self, mock_token, mock_url):
"""Test successful MR description update."""
mock_token.return_value = 'test-token'
mock_url.return_value = 'git@gitlab.com:group/project.git'
@@ -1655,36 +1657,36 @@ class TestUpdateMrDescription(unittest.TestCase):
with mock.patch('gitlab.Gitlab') as mock_gitlab:
mock_gitlab.return_value.projects.get.return_value = mock_project
- result = gitlab_api.update_mr_description('origin', 123,
+ result = gitlab.update_mr_desc('origin', 123,
'New description')
self.assertTrue(result)
self.assertEqual(mock_mr.description, 'New description')
mock_mr.save.assert_called_once()
- @mock.patch.object(gitlab_api, 'AVAILABLE', False)
- def test_update_mr_description_not_available(self):
- """Test update_mr_description when gitlab not available."""
+ @mock.patch.object(gitlab, 'AVAILABLE', False)
+ def test_update_mr_desc_not_available(self):
+ """Test update_mr_desc when gitlab not available."""
with terminal.capture():
- result = gitlab_api.update_mr_description('origin', 123, 'desc')
+ result = gitlab.update_mr_desc('origin', 123, 'desc')
self.assertFalse(result)
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
- def test_update_mr_description_no_token(self, mock_token):
- """Test update_mr_description when no token set."""
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
+ def test_update_mr_desc_no_token(self, mock_token):
+ """Test update_mr_desc when no token set."""
mock_token.return_value = None
with terminal.capture():
- result = gitlab_api.update_mr_description('origin', 123, 'desc')
+ result = gitlab.update_mr_desc('origin', 123, 'desc')
self.assertFalse(result)
class TestGetPickmanMrs(unittest.TestCase):
"""Tests for get_pickman_mrs function."""
- @mock.patch.object(gitlab_api, 'get_remote_url')
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
+ @mock.patch.object(gitlab, 'get_remote_url')
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
def test_get_pickman_mrs_sorted_oldest_first(self, mock_token, mock_url):
"""Test that MRs are requested sorted by created_at ascending."""
mock_token.return_value = 'test-token'
@@ -1719,7 +1721,7 @@ class TestGetPickmanMrs(unittest.TestCase):
with mock.patch('gitlab.Gitlab') as mock_gitlab:
mock_gitlab.return_value.projects.get.return_value = mock_project
- result = gitlab_api.get_pickman_mrs('origin', state='opened')
+ result = gitlab.get_pickman_mrs('origin', state='opened')
# Verify the list call includes sorting parameters
mock_project.mergerequests.list.assert_called_once_with(
@@ -1734,8 +1736,8 @@ class TestGetPickmanMrs(unittest.TestCase):
class TestCreateMr(unittest.TestCase):
"""Tests for create_mr function."""
- @mock.patch.object(gitlab_api, 'get_token')
- @mock.patch.object(gitlab_api, 'AVAILABLE', True)
+ @mock.patch.object(gitlab, 'get_token')
+ @mock.patch.object(gitlab, 'AVAILABLE', True)
def test_create_mr_409_returns_existing(self, mock_token):
"""Test that 409 error returns existing MR URL."""
tout.init(tout.INFO)
@@ -1750,13 +1752,13 @@ class TestCreateMr(unittest.TestCase):
# Simulate 409 Conflict error
mock_project.mergerequests.create.side_effect = \
- gitlab_api.MrCreateError(response_code=409)
+ gitlab.MrCreateError(response_code=409)
with mock.patch('gitlab.Gitlab') as mock_gitlab:
mock_gitlab.return_value.projects.get.return_value = mock_project
with terminal.capture():
- result = gitlab_api.create_mr(
+ result = gitlab.create_mr(
'gitlab.com', 'group/project',
'cherry-abc', 'master', 'Test MR')
@@ -1883,7 +1885,7 @@ class TestStep(unittest.TestCase):
def test_step_with_pending_mr(self):
"""Test step does nothing when MR is pending."""
- mock_mr = gitlab_api.PickmanMr(
+ mock_mr = gitlab.PickmanMr(
iid=123,
title='[pickman] Test MR',
web_url='https://gitlab.com/mr/123',
@@ -1891,9 +1893,9 @@ class TestStep(unittest.TestCase):
description='Test',
)
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=[]):
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ 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',
@@ -1905,7 +1907,7 @@ class TestStep(unittest.TestCase):
def test_step_gitlab_error(self):
"""Test step when GitLab API returns error."""
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=None):
args = argparse.Namespace(cmd='step', source='us/next',
remote='ci', target='master',
@@ -1917,9 +1919,9 @@ class TestStep(unittest.TestCase):
def test_step_open_mrs_error(self):
"""Test step when get_open_pickman_mrs returns error."""
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=[]):
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_open_pickman_mrs',
return_value=None):
args = argparse.Namespace(cmd='step', source='us/next',
remote='ci', target='master',
@@ -1931,7 +1933,7 @@ class TestStep(unittest.TestCase):
def test_step_allows_below_max(self):
"""Test step allows new MR when count is below max_mrs."""
- mock_mr = gitlab_api.PickmanMr(
+ mock_mr = gitlab.PickmanMr(
iid=123,
title='[pickman] Test MR',
web_url='https://gitlab.com/mr/123',
@@ -1939,9 +1941,9 @@ class TestStep(unittest.TestCase):
description='Test',
)
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=[]):
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ 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:
@@ -1958,7 +1960,7 @@ class TestStep(unittest.TestCase):
def test_step_blocks_at_max(self):
"""Test step blocks new MR when at max_mrs limit."""
mock_mrs = [
- gitlab_api.PickmanMr(
+ gitlab.PickmanMr(
iid=i,
title=f'[pickman] Test MR {i}',
web_url=f'https://gitlab.com/mr/{i}',
@@ -1968,9 +1970,9 @@ class TestStep(unittest.TestCase):
for i in range(3)
]
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=[]):
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ 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',
@@ -2005,7 +2007,7 @@ class TestReview(unittest.TestCase):
def test_review_no_mrs(self):
"""Test review when no open MRs found."""
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_open_pickman_mrs',
return_value=[]):
args = argparse.Namespace(cmd='review', remote='ci')
with terminal.capture():
@@ -2015,7 +2017,7 @@ class TestReview(unittest.TestCase):
def test_review_gitlab_error(self):
"""Test review when GitLab API returns error."""
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_open_pickman_mrs',
return_value=None):
args = argparse.Namespace(cmd='review', remote='ci')
with terminal.capture():
@@ -2025,7 +2027,7 @@ class TestReview(unittest.TestCase):
class TestUpdateHistoryWithReview(unittest.TestCase):
- """Tests for update_history_with_review function."""
+ """Tests for update_history function."""
def setUp(self):
"""Set up test fixtures."""
@@ -2045,20 +2047,20 @@ class TestUpdateHistoryWithReview(unittest.TestCase):
os.chdir(self.orig_dir)
shutil.rmtree(self.test_dir)
- def test_update_history_with_review(self):
+ def test_update_history(self):
"""Test that review handling is appended to history."""
comments = [
- gitlab_api.MrComment(id=1, author='reviewer1',
+ gitlab.MrComment(id=1, author='reviewer1',
body='Please fix the indentation here',
created_at='2025-01-01', resolvable=True,
resolved=False),
- gitlab_api.MrComment(id=2, author='reviewer2', body='Add a docstring',
+ gitlab.MrComment(id=2, author='reviewer2', body='Add a docstring',
created_at='2025-01-01', resolvable=True,
resolved=False),
]
conversation_log = 'Fixed indentation and added docstring.'
- control.update_history_with_review('cherry-abc123', comments,
+ control.update_history('cherry-abc123', comments,
conversation_log)
# Check history file was created
@@ -2083,10 +2085,10 @@ class TestUpdateHistoryWithReview(unittest.TestCase):
subprocess.run(['git', 'commit', '-m', 'Initial'],
check=True, capture_output=True)
- comments = [gitlab_api.MrComment(id=1, author='reviewer', body='Fix this',
- created_at='2025-01-01', resolvable=True,
- resolved=False)]
- control.update_history_with_review('cherry-xyz', comments, 'Fixed it')
+ comms = [gitlab.MrComment(id=1, author='reviewer', body='Fix this',
+ created_at='2025-01-01', resolvable=True,
+ resolved=False)]
+ control.update_history('cherry-xyz', comms, 'Fixed it')
with open(control.HISTORY_FILE, 'r', encoding='utf-8') as fhandle:
content = fhandle.read()
@@ -2132,7 +2134,7 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
dbs.comment_mark_processed(100, 1)
dbs.commit()
- mrs = [gitlab_api.PickmanMr(
+ mrs = [gitlab.PickmanMr(
iid=100,
title='[pickman] Test MR',
source_branch='cherry-test',
@@ -2142,25 +2144,25 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
# Comment 1 is processed, comment 2 is new
comments = [
- gitlab_api.MrComment(id=1, author='reviewer', body='Old comment',
+ gitlab.MrComment(id=1, author='reviewer', body='Old comment',
created_at='2025-01-01', resolvable=True,
resolved=False),
- gitlab_api.MrComment(id=2, author='reviewer', body='New comment',
+ gitlab.MrComment(id=2, author='reviewer', body='New comment',
created_at='2025-01-01', resolvable=True,
resolved=False),
]
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_mr_comments',
+ with mock.patch.object(gitlab, 'get_mr_comments',
return_value=comments):
with mock.patch.object(agent, 'handle_mr_comments',
- return_value=(True, 'Done')) as mock_agent:
- with mock.patch.object(gitlab_api, 'update_mr_description'):
- with mock.patch.object(control, 'update_history_with_review'):
+ return_value=(True, 'Done')) as moc:
+ with mock.patch.object(gitlab, 'update_mr_desc'):
+ with mock.patch.object(control, 'update_history'):
control.process_mr_reviews('ci', mrs, dbs)
# Agent should only receive the new comment
- call_args = mock_agent.call_args
+ call_args = moc.call_args
passed_comments = call_args[0][2]
self.assertEqual(len(passed_comments), 1)
self.assertEqual(passed_comments[0].id, 2)
@@ -2174,7 +2176,7 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
dbs.start()
# MR needs rebase but has no comments
- mrs = [gitlab_api.PickmanMr(
+ mrs = [gitlab.PickmanMr(
iid=100,
title='[pickman] Test MR',
source_branch='cherry-test',
@@ -2185,17 +2187,17 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
)]
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_mr_comments',
+ with mock.patch.object(gitlab, 'get_mr_comments',
return_value=[]):
with mock.patch.object(agent, 'handle_mr_comments',
- return_value=(True, 'Rebased')) as mock_agent:
- with mock.patch.object(gitlab_api, 'update_mr_description'):
- with mock.patch.object(control, 'update_history_with_review'):
+ return_value=(True, 'Rebased')) as m:
+ with mock.patch.object(gitlab, 'update_mr_desc'):
+ with mock.patch.object(control, 'update_history'):
control.process_mr_reviews('ci', mrs, dbs)
# Agent should be called with needs_rebase=True
- mock_agent.assert_called_once()
- call_kwargs = mock_agent.call_args[1]
+ m.assert_called_once()
+ call_kwargs = m.call_args[1]
self.assertTrue(call_kwargs.get('needs_rebase'))
self.assertFalse(call_kwargs.get('has_conflicts'))
@@ -2208,7 +2210,7 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
dbs.start()
# MR has no comments and doesn't need rebase
- mrs = [gitlab_api.PickmanMr(
+ mrs = [gitlab.PickmanMr(
iid=100,
title='[pickman] Test MR',
source_branch='cherry-test',
@@ -2219,14 +2221,14 @@ class TestProcessMrReviewsCommentTracking(unittest.TestCase):
)]
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_mr_comments',
+ with mock.patch.object(gitlab, 'get_mr_comments',
return_value=[]):
with mock.patch.object(agent, 'handle_mr_comments',
- return_value=(True, 'Done')) as mock_agent:
+ return_value=(True, 'Done')) as moc:
control.process_mr_reviews('ci', mrs, dbs)
# Agent should NOT be called
- mock_agent.assert_not_called()
+ moc.assert_not_called()
dbs.close()
@@ -2360,20 +2362,20 @@ class TestParseInstruction(unittest.TestCase):
class TestFormatHistorySummary(unittest.TestCase):
- """Tests for format_history_summary function."""
+ """Tests for format_history function."""
- def test_format_history_summary(self):
+ def test_format_history(self):
"""Test formatting history summary."""
commits = [
control.CommitInfo('aaa111', 'aaa111a', 'First commit', 'Author 1'),
- control.CommitInfo('bbb222', 'bbb222b', 'Second commit', 'Author 2'),
+ control.CommitInfo('bbb222', 'bbb222b', 'Second one', 'Author 2'),
]
- result = control.format_history_summary('us/next', commits, 'cherry-abc')
+ result = control.format_history('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)
+ self.assertIn('- bbb222b Second one', result)
class TestGetHistory(unittest.TestCase):
@@ -2472,7 +2474,7 @@ Other content
"""Test get_history with multiple commits."""
commits = [
control.CommitInfo('aaa111', 'aaa111a', 'First commit', 'Author 1'),
- control.CommitInfo('bbb222', 'bbb222b', 'Second commit', 'Author 2'),
+ control.CommitInfo('bbb222', 'bbb222b', 'Second one', 'Author 2'),
control.CommitInfo('ccc333', 'ccc333c', 'Third commit', 'Author 3'),
]
content, commit_msg = control.get_history(
@@ -2480,13 +2482,13 @@ Other content
# Verify all commits in content
self.assertIn('- aaa111a First commit', content)
- self.assertIn('- bbb222b Second commit', content)
+ self.assertIn('- bbb222b Second one', content)
self.assertIn('- ccc333c Third commit', content)
# Verify commit message
self.assertIn('pickman: Record cherry-pick of 3 commits', commit_msg)
self.assertIn('- aaa111a First commit', commit_msg)
- self.assertIn('- bbb222b Second commit', commit_msg)
+ self.assertIn('- bbb222b Second one', commit_msg)
self.assertIn('- ccc333c Third commit', commit_msg)
@@ -2684,7 +2686,7 @@ class TestExecuteApply(unittest.TestCase):
return_value=(True, 'log')):
with mock.patch.object(control, 'run_git',
return_value='abc123'):
- with mock.patch.object(gitlab_api, 'push_and_create_mr',
+ with mock.patch.object(gitlab, 'push_and_create_mr',
return_value='https://mr/url'):
ret, success, _ = control.execute_apply(
dbs, 'us/next', commits, 'cherry-branch', args)
@@ -2710,7 +2712,7 @@ class TestExecuteApply(unittest.TestCase):
return_value=(True, 'log')):
with mock.patch.object(control, 'run_git',
return_value='abc123'):
- with mock.patch.object(gitlab_api, 'push_and_create_mr',
+ with mock.patch.object(gitlab, 'push_and_create_mr',
return_value=None):
ret, success, _ = control.execute_apply(
dbs, 'us/next', commits, 'cherry-branch', args)
@@ -2735,7 +2737,7 @@ class TestExecuteApply(unittest.TestCase):
with mock.patch.object(control.agent, 'cherry_pick_commits',
return_value=(True, 'aborted log')):
with mock.patch.object(control, 'run_git',
- side_effect=Exception('branch not found')):
+ side_effect=Exception('not found')):
ret, success, _ = control.execute_apply(
dbs, 'us/next', commits, 'cherry-branch', args)
@@ -2766,7 +2768,7 @@ class TestExecuteApply(unittest.TestCase):
with mock.patch.object(control.agent, 'cherry_pick_commits',
return_value=(True, 'already applied log')):
with mock.patch.object(control.agent, 'read_signal_file',
- return_value=(agent.SIGNAL_ALREADY_APPLIED,
+ return_value=(agent.SIGNAL_APPLIED,
'hhh888')):
ret, success, _ = control.execute_apply(
dbs, 'us/next', commits, 'cherry-branch', args)
@@ -2971,9 +2973,9 @@ class TestGetNextCommitsEmptyLine(unittest.TestCase):
call_count = [0]
+ # pylint: disable=unused-argument
def mock_git(pipe_list):
call_count[0] += 1
- _cmd = pipe_list[0] if pipe_list else [] # pylint: disable=unused-variable
# First call: get first-parent log
if call_count[0] == 1:
return command.CommandResult(stdout=fp_log)
@@ -3047,7 +3049,7 @@ class TestDoPushBranch(unittest.TestCase):
tout.init(tout.INFO)
args = argparse.Namespace(cmd='push-branch', branch='test-branch',
remote='ci', force=False, run_ci=False)
- with mock.patch.object(gitlab_api, 'push_branch',
+ with mock.patch.object(gitlab, 'push_branch',
return_value=True) as mock_push:
with terminal.capture():
ret = control.do_push_branch(args, None)
@@ -3060,7 +3062,7 @@ class TestDoPushBranch(unittest.TestCase):
tout.init(tout.INFO)
args = argparse.Namespace(cmd='push-branch', branch='test-branch',
remote='origin', force=True, run_ci=False)
- with mock.patch.object(gitlab_api, 'push_branch',
+ with mock.patch.object(gitlab, 'push_branch',
return_value=True) as mock_push:
with terminal.capture():
ret = control.do_push_branch(args, None)
@@ -3073,7 +3075,7 @@ class TestDoPushBranch(unittest.TestCase):
tout.init(tout.INFO)
args = argparse.Namespace(cmd='push-branch', branch='test-branch',
remote='ci', force=False)
- with mock.patch.object(gitlab_api, 'push_branch',
+ with mock.patch.object(gitlab, 'push_branch',
return_value=False):
with terminal.capture():
ret = control.do_push_branch(args, None)
@@ -3114,7 +3116,7 @@ class TestDoReviewWithMrs(unittest.TestCase):
"""Test review with open MRs but no comments."""
tout.init(tout.INFO)
- mock_mr = gitlab_api.PickmanMr(
+ mock_mr = gitlab.PickmanMr(
iid=123,
title='[pickman] Test MR',
web_url='https://gitlab.com/mr/123',
@@ -3122,9 +3124,9 @@ class TestDoReviewWithMrs(unittest.TestCase):
description='Test',
)
with mock.patch.object(control, 'run_git'):
- with mock.patch.object(gitlab_api, 'get_open_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_open_pickman_mrs',
return_value=[mock_mr]):
- with mock.patch.object(gitlab_api, 'get_mr_comments',
+ with mock.patch.object(gitlab, 'get_mr_comments',
return_value=[]):
args = argparse.Namespace(cmd='review', remote='ci',
target='master')
@@ -3157,10 +3159,10 @@ class TestProcessMergedMrs(unittest.TestCase):
with terminal.capture():
dbs = database.Database(self.db_path)
dbs.start()
- dbs.source_set('us/next', 'aaa111aaa111aaa111aaa111aaa111aaa111aaa1')
+ dbs.source_set('us/next', 'aaa111aaa111aaa111aaa111aaa111aaa111aaa')
dbs.commit()
- merged_mrs = [gitlab_api.PickmanMr(
+ merged_mrs = [gitlab.PickmanMr(
iid=100,
title='[pickman] Test MR',
description='## 2025-01-01: us/next\n\n- bbb222b Subject',
@@ -3176,7 +3178,7 @@ class TestProcessMergedMrs(unittest.TestCase):
return ''
return ''
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=merged_mrs):
with mock.patch.object(control, 'run_git', mock_git):
processed = control.process_merged_mrs('ci', 'us/next', dbs)
@@ -3196,7 +3198,7 @@ class TestProcessMergedMrs(unittest.TestCase):
dbs.source_set('us/next', 'bbb222bbb222bbb222bbb222bbb222bbb222bbb2')
dbs.commit()
- merged_mrs = [gitlab_api.PickmanMr(
+ merged_mrs = [gitlab.PickmanMr(
iid=100,
title='[pickman] Test MR',
description='## 2025-01-01: us/next\n\n- aaa111a Subject',
@@ -3212,7 +3214,7 @@ class TestProcessMergedMrs(unittest.TestCase):
raise RuntimeError('Not an ancestor')
return ''
- with mock.patch.object(gitlab_api, 'get_merged_pickman_mrs',
+ with mock.patch.object(gitlab, 'get_merged_pickman_mrs',
return_value=merged_mrs):
with mock.patch.object(control, 'run_git', mock_git):
processed = control.process_merged_mrs('ci', 'us/next', dbs)
@@ -455,7 +455,7 @@ def reply_to_mr(remote, mr_iid, message):
return False
-def update_mr_description(remote, mr_iid, desc):
+def update_mr_desc(remote, mr_iid, desc):
"""Update a merge request's description
Args: