From: Simon Glass <sjg@chromium.org>
push_branch() catches CommandExc and returns False on failure, but
callers like apply_subtree_update() use try/except to detect push
failures rather than checking the return value. This means push failures
are silently ignored and the source is advanced even though the branch
was not pushed.
Change push_branch() to re-raise the CommandExc instead of returning
False, and update all callers to use exception handling consistently.
This ensures apply_subtree_update() properly aborts when a push fails.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/pickman/control.py | 9 ++++++---
tools/pickman/ftest.py | 6 ++++--
tools/pickman/gitlab_api.py | 9 +++++++--
3 files changed, 17 insertions(+), 7 deletions(-)
@@ -2080,9 +2080,12 @@ def do_push_branch(args, dbs): # pylint: disable=unused-argument
int: 0 on success, 1 on failure
"""
skip_ci = not args.run_ci
- success = gitlab_api.push_branch(args.remote, args.branch, args.force,
- skip_ci=skip_ci)
- return 0 if success else 1
+ try:
+ gitlab_api.push_branch(args.remote, args.branch, args.force,
+ skip_ci=skip_ci)
+ except command.CommandExc:
+ return 1
+ return 0
def do_commit_source(args, dbs):
@@ -4956,8 +4956,10 @@ 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, 'push_branch',
- return_value=False):
+ with mock.patch.object(
+ gitlab, 'push_branch',
+ side_effect=command.CommandExc(
+ 'push failed', command.CommandResult())):
with terminal.capture():
ret = control.do_push_branch(args, None)
self.assertEqual(ret, 1)
@@ -196,6 +196,9 @@ def push_branch(remote, branch, force=False, skip_ci=True):
Returns:
bool: True on success
+
+ Raises:
+ command.CommandExc: If the push fails
"""
try:
# Use token-authenticated URL if available
@@ -232,7 +235,7 @@ def push_branch(remote, branch, force=False, skip_ci=True):
return True
except command.CommandExc as exc:
tout.error(f'Failed to push branch: {exc}')
- return False
+ raise
# pylint: disable=too-many-arguments
@@ -630,7 +633,9 @@ def push_and_create_mr(remote, branch, target, title, desc=''):
return None
tout.info(f'Pushing {branch} to {remote}...')
- if not push_branch(remote, branch, force=True):
+ try:
+ push_branch(remote, branch, force=True)
+ except command.CommandExc:
return None
tout.info(f'Creating merge request to {target}...')