Fix git_clone_fork exit code regression

qmk_cli/subcommands/setup.py

@@ -23,7 +23,7 @@ DEFAULT_BRANCH = 'master'
 @cli.argument('-H', '--home', arg_only=True, default=Path(os.environ['QMK_HOME']), type=AbsPath, help='The location for QMK Firmware. Default: %s' % os.environ['QMK_HOME'])
 @cli.argument('fork', arg_only=True, default=DEFAULT_FORK, nargs='?', help='The qmk_firmware fork to clone. Default: %s' % DEFAULT_FORK)
 @cli.subcommand('Setup your computer for qmk_firmware.')
-def setup(cli):
+def setup(cli):  # noqa: C901
     """Guide the user through setting up their QMK environment.
     """
     clone_prompt = 'Would you like to clone {fg_cyan}%s{fg_reset} to {fg_cyan}%s{fg_reset}?' % (cli.args.fork, shlex.quote(str(cli.args.home)))
@@ -32,7 +32,7 @@ def setup(cli):
     # Sanity checks
     if cli.args.yes and cli.args.no:
         cli.log.error("Can't use both --yes and --no at the same time.")
-        exit(1)
+        return False
 
     # Check on qmk_firmware
     # If it exists, ask the user what to do with it
@@ -50,18 +50,20 @@ def setup(cli):
         found_action = choice(found_prompt, options=found_options, default=2)
         if found_action == f"Delete and reclone {cli.args.fork}":
             if not yesno(delete_confirm, default=False):
-                exit(1)
+                return False
 
-            git_clone_fork(cli.args.home, cli.args.baseurl, cli.args.fork, cli.args.branch, force=True)
+            if not git_clone_fork(cli.args.home, cli.args.baseurl, cli.args.fork, cli.args.branch, force=True):
+                return False
 
         elif found_action == "Delete and clone a different fork":
             fork_name = question("Enter the name of the fork:", default=cli.args.fork)
             branch_name = question("Enter the branch name to clone:", default=cli.args.branch)
 
             if not yesno(delete_confirm, default=False):
-                exit(1)
+                return False
 
-            git_clone_fork(cli.args.home, cli.args.baseurl, fork_name, branch_name, force=True)
+            if not git_clone_fork(cli.args.home, cli.args.baseurl, fork_name, branch_name, force=True):
+                return False
 
     # Exists (but not an empty dir)
     elif cli.args.home.exists() and any(cli.args.home.iterdir()):
@@ -71,12 +73,13 @@ def setup(cli):
             cli.log.warning('Warning: %s does not end in "qmk_firmware". Did you mean to use "--home %s/qmk_firmware"?' % (path_str, path_str))
 
         cli.log.error("Path '%s' exists but is not a qmk_firmware clone!", path_str)
-        exit(1)
+        return False
 
     else:
         cli.log.error('Could not find qmk_firmware!')
         if yesno(clone_prompt):
-            git_clone_fork(cli.args.home, cli.args.baseurl, cli.args.fork, cli.args.branch)
+            if not git_clone_fork(cli.args.home, cli.args.baseurl, cli.args.fork, cli.args.branch):
+                return False
         else:
             cli.log.warning('Not cloning qmk_firmware due to user input or --no flag.')
 

test/subcommands/test_setup.py

@@ -49,10 +49,9 @@ def test_setup_both_yes_no_invalid(subcommand, mock_cli):
     type(mock_cli.args).yes = PropertyMock(return_value=True)
     type(mock_cli.args).no = PropertyMock(return_value=True)
 
-    with pytest.raises(SystemExit) as e:
-        subcommand.setup(mock_cli)
+    ret = subcommand.setup(mock_cli)
 
-    assert e.value.code == 1
+    assert ret is False
 
 
 def test_setup_reclone(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, choice, git_clone_fork):
@@ -70,6 +69,23 @@ def test_setup_reclone(subcommand, mock_cli, is_qmk_firmware, temp_directory, ye
     git_clone_fork.assert_called_once()
 
 
+def test_setup_reclone_failed(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, choice, git_clone_fork):
+    type(mock_cli.args).yes = PropertyMock(return_value=False)
+    type(mock_cli.args).no = PropertyMock(return_value=False)
+    type(mock_cli.args).home = PropertyMock(return_value=temp_directory)
+    type(mock_cli.args).fork = 'asdf'
+    choice.return_value = 'Delete and reclone asdf'
+    yesno.return_value = True
+    git_clone_fork.return_value = False
+
+    ret = subcommand.setup(mock_cli)
+
+    yesno.assert_called_once()
+    assert 'This will delete your current qmk_firmware directory.' in yesno.call_args.args[0]
+    git_clone_fork.assert_called_once()
+    assert ret is False
+
+
 def test_setup_reclone_no(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, choice, git_clone_fork):
     type(mock_cli.args).yes = PropertyMock(return_value=False)
     type(mock_cli.args).no = PropertyMock(return_value=False)
@@ -78,10 +94,9 @@ def test_setup_reclone_no(subcommand, mock_cli, is_qmk_firmware, temp_directory,
     choice.return_value = 'Delete and reclone asdf'
     yesno.return_value = False
 
-    with pytest.raises(SystemExit) as e:
-        subcommand.setup(mock_cli)
+    ret = subcommand.setup(mock_cli)
 
-    assert e.value.code == 1
+    assert ret is False
     git_clone_fork.assert_not_called()
 
 
@@ -99,6 +114,22 @@ def test_setup_clone_diff_fork(subcommand, mock_cli, is_qmk_firmware, temp_direc
     git_clone_fork.assert_called_once()
 
 
+def test_setup_clone_diff_fork_failed(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, choice, question, git_clone_fork):
+    type(mock_cli.args).yes = PropertyMock(return_value=False)
+    type(mock_cli.args).no = PropertyMock(return_value=False)
+    type(mock_cli.args).home = PropertyMock(return_value=temp_directory)
+    choice.return_value = 'Delete and clone a different fork'
+    yesno.return_value = True
+    git_clone_fork.return_value = False
+
+    ret = subcommand.setup(mock_cli)
+
+    yesno.assert_called_once()
+    assert 'This will delete your current qmk_firmware directory.' in yesno.call_args.args[0]
+    git_clone_fork.assert_called_once()
+    assert ret is False
+
+
 def test_setup_clone_diff_fork_no(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, choice, question, git_clone_fork):
     type(mock_cli.args).yes = PropertyMock(return_value=False)
     type(mock_cli.args).no = PropertyMock(return_value=False)
@@ -106,10 +137,9 @@ def test_setup_clone_diff_fork_no(subcommand, mock_cli, is_qmk_firmware, temp_di
     choice.return_value = 'Delete and clone a different fork'
     yesno.return_value = False
 
-    with pytest.raises(SystemExit) as e:
-        subcommand.setup(mock_cli)
+    ret = subcommand.setup(mock_cli)
 
-    assert e.value.code == 1
+    assert ret is False
     git_clone_fork.assert_not_called()
 
 
@@ -120,10 +150,9 @@ def test_setup_home_exists_not_empty(subcommand, mock_cli, is_qmk_firmware, temp
     is_qmk_firmware.return_value = False
     (temp_directory / 'asdf').touch()
 
-    with pytest.raises(SystemExit) as e:
-        subcommand.setup(mock_cli)
+    ret = subcommand.setup(mock_cli)
 
-    assert e.value.code == 1
+    assert ret is False
 
 
 def test_setup_missing(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, git_clone_fork):
@@ -140,6 +169,22 @@ def test_setup_missing(subcommand, mock_cli, is_qmk_firmware, temp_directory, ye
     git_clone_fork.assert_called_once()
 
 
+def test_setup_missing_failed(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, git_clone_fork):
+    type(mock_cli.args).yes = PropertyMock(return_value=False)
+    type(mock_cli.args).no = PropertyMock(return_value=False)
+    type(mock_cli.args).home = PropertyMock(return_value=temp_directory)
+    is_qmk_firmware.return_value = False
+    yesno.return_value = True
+    git_clone_fork.return_value = False
+
+    ret = subcommand.setup(mock_cli)
+
+    yesno.assert_called_once()
+    assert 'Would you like to clone' in yesno.call_args.args[0]
+    git_clone_fork.assert_called_once()
+    assert ret is False
+
+
 def test_setup_missing_no(subcommand, mock_cli, is_qmk_firmware, temp_directory, yesno, git_clone_fork):
     type(mock_cli.args).yes = PropertyMock(return_value=False)
     type(mock_cli.args).no = PropertyMock(return_value=False)