Skip to content

Commit e3ae367

Browse files
committed
fix: P2 Issue 20 Implementation Complete: Exit Code Handling
Signed-off-by: habeck <habeck@us.ibm.com>
1 parent 0c03a9a commit e3ae367

2 files changed

Lines changed: 88 additions & 40 deletions

File tree

cpex/tools/cli.py

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
import os
3232
import shutil
3333
import subprocess # nosec B404 # Safe: Used only for git commands with hardcoded args
34-
35-
# import sys
34+
import sys
3635
from pathlib import Path
3736
from typing import List, Optional
3837

@@ -41,6 +40,13 @@
4140
from rich.console import Console
4241
from typing_extensions import Annotated
4342

43+
# Exit codes for CLI commands
44+
EXIT_SUCCESS = 0
45+
EXIT_GENERAL_ERROR = 1
46+
EXIT_INVALID_ARGS = 2
47+
EXIT_NOT_FOUND = 3
48+
EXIT_OPERATION_FAILED = 4
49+
4450
# First-Party
4551
from cpex.framework.loader.config import ConfigLoader, ConfigSaver
4652
from cpex.framework.models import (
@@ -237,11 +243,14 @@ def bootstrap(
237243
extra_context=extra_context,
238244
)
239245
else:
240-
logger.warning("No local templates found and git is not available to fetch remote template.")
246+
logger.error("No local templates found and git is not available to fetch remote template.")
247+
raise typer.Exit(EXIT_OPERATION_FAILED)
241248
except (SystemExit, typer.Exit):
242249
raise
243-
except Exception:
250+
except Exception as e:
244251
logger.exception("An error was caught while copying template.")
252+
console.print(f":x: Failed to create plugin project: {str(e)}")
253+
raise typer.Exit(EXIT_OPERATION_FAILED)
245254

246255

247256
def list(type: str, fmt: str = "text") -> None:
@@ -590,15 +599,20 @@ def install(source: str, install_type: str | None, catalog: PluginCatalog, assum
590599
assume_yes: Skip interactive selection prompt for monorepo installs.
591600
592601
Raises:
593-
ValueError: If install_type is not supported.
594-
NotImplementedError: If the installation type is not yet implemented.
602+
typer.Exit: With EXIT_INVALID_ARGS if install_type is not supported.
603+
typer.Exit: With EXIT_OPERATION_FAILED if installation fails.
595604
"""
596605
if install_type is None:
597606
install_type = "monorepo"
598607

599608
if install_type == "monorepo":
600-
_install_from_monorepo(source, catalog, assume_yes=assume_yes)
601-
return
609+
try:
610+
_install_from_monorepo(source, catalog, assume_yes=assume_yes)
611+
return
612+
except Exception as e:
613+
console.print(f":x: Installation failed: {str(e)}")
614+
logger.error("Install error: %s", str(e), exc_info=True)
615+
raise typer.Exit(EXIT_OPERATION_FAILED)
602616

603617
handlers = {
604618
"git": _install_from_git,
@@ -609,9 +623,15 @@ def install(source: str, install_type: str | None, catalog: PluginCatalog, assum
609623

610624
handler = handlers.get(install_type)
611625
if handler is None:
612-
raise ValueError(f"Unsupported installation type: {install_type}. Must be one of: {', '.join(handlers.keys())}")
626+
console.print(f":x: Unsupported installation type: {install_type}. Must be one of: {', '.join(handlers.keys())}")
627+
raise typer.Exit(EXIT_INVALID_ARGS)
613628

614-
handler(source, catalog, use_test=True if install_type == "test-pypi" else False)
629+
try:
630+
handler(source, catalog, use_test=True if install_type == "test-pypi" else False)
631+
except Exception as e:
632+
console.print(f":x: Installation failed: {str(e)}")
633+
logger.error("Install error: %s", str(e), exc_info=True)
634+
raise typer.Exit(EXIT_OPERATION_FAILED)
615635

616636

617637
def versions(plugin_name: str | None, catalog: PluginCatalog, fmt: str = "text"):
@@ -717,7 +737,7 @@ def uninstall(plugin_name: str, catalog: PluginCatalog, assume_yes: bool = False
717737

718738
if installed_plugin is None:
719739
console.print(f":x: Plugin '{plugin_name}' is not installed.")
720-
return
740+
raise typer.Exit(EXIT_NOT_FOUND)
721741

722742
# Confirm uninstallation
723743
console.print(f"Found plugin: {installed_plugin.name} (version {installed_plugin.version})")
@@ -751,18 +771,27 @@ def uninstall(plugin_name: str, catalog: PluginCatalog, assume_yes: bool = False
751771
plugin_registry.remove(plugin_name)
752772
else:
753773
console.print(f":x: Plugin {plugin_name} not found in catalog.")
754-
return
774+
raise typer.Exit(EXIT_NOT_FOUND)
755775

756776
console.print(f":white_heavy_check_mark: {plugin_name} uninstalled successfully.")
757777

778+
except typer.Exit:
779+
raise
758780
except Exception as e:
759781
console.print(f":x: Failed to uninstall {plugin_name}: {str(e)}")
760782
logger.error("Uninstall error: %s", str(e), exc_info=True)
783+
raise typer.Exit(EXIT_OPERATION_FAILED)
761784

762785

763786
@app.command(
764787
help="List, search, install or uninstall plugins.\n\n"
765-
"\ndefault install type is monorepo\n"
788+
"Exit Codes:\n"
789+
" 0 - Success\n"
790+
" 1 - General error\n"
791+
" 2 - Invalid arguments\n"
792+
" 3 - Plugin not found\n"
793+
" 4 - Operation failed\n\n"
794+
"Default install type is monorepo\n\n"
766795
"Examples:\n"
767796
"python cpex/tools/cli.py plugin info pii\n"
768797
"python cpex/tools/cli.py plugin search pii\n"
@@ -810,7 +839,7 @@ def plugin(
810839
if cmd_action == "uninstall":
811840
if source is None:
812841
console.print(":x: Please specify a plugin name to uninstall.")
813-
return
842+
raise typer.Exit(EXIT_INVALID_ARGS)
814843
pc = PluginCatalog()
815844
return uninstall(source, catalog=pc, assume_yes=assume_yes)
816845
if cmd_action == "install" and source is not None:

tests/unit/cpex/tools/test_cli.py

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -395,10 +395,12 @@ def test_warns_when_no_local_and_no_git(self):
395395
patch("cpex.tools.cli.command_exists", return_value=False),
396396
patch(_CC_PATCH_TARGET) as mock_cc,
397397
patch("cpex.tools.cli.logger") as mock_logger,
398+
patch("cpex.tools.cli.console") as mock_console,
398399
):
399-
runner.invoke(app, ["bootstrap", "-d", "/tmp/test_nogit", "--no_input"])
400+
result = runner.invoke(app, ["bootstrap", "-d", "/tmp/test_nogit", "--no_input"])
401+
assert result.exit_code == 4 # EXIT_OPERATION_FAILED
400402
mock_cc.assert_not_called()
401-
mock_logger.warning.assert_called_once()
403+
mock_logger.error.assert_called_once()
402404

403405

404406
class TestBootstrapErrorHandling:
@@ -408,9 +410,10 @@ def test_logs_exception_on_cookiecutter_error(self):
408410
with (
409411
patch(_CC_PATCH_TARGET, side_effect=RuntimeError("template error")),
410412
patch("cpex.tools.cli.logger") as mock_logger,
413+
patch("cpex.tools.cli.console") as mock_console,
411414
):
412415
result = runner.invoke(app, ["bootstrap", "-d", "/tmp/test_err", "--no_input"])
413-
assert result.exit_code == 0 # error is caught and logged
416+
assert result.exit_code == 4 # EXIT_OPERATION_FAILED
414417
mock_logger.exception.assert_called_once()
415418

416419

@@ -845,10 +848,14 @@ def test_install_monorepo_with_available_plugins(self, temp_registry_dir):
845848
mock_catalog.install_folder_via_pip.assert_called_once()
846849

847850
def test_install_requires_type_parameter(self):
848-
"""Test that install raises ValueError for unsupported type."""
851+
"""Test that install raises typer.Exit for unsupported type."""
849852
mock_catalog = Mock()
850-
with pytest.raises(ValueError, match="Unsupported installation type"):
853+
with (
854+
patch("cpex.tools.cli.console") as mock_console,
855+
pytest.raises(click.exceptions.Exit) as exc_info,
856+
):
851857
install("source", "", mock_catalog)
858+
assert exc_info.value.exit_code == 2 # EXIT_INVALID_ARGS
852859

853860

854861
class TestSearchFunction:
@@ -1110,9 +1117,13 @@ def test_uninstall_plugin_not_found(self, temp_registry_dir):
11101117
"""Test uninstalling a plugin that is not installed."""
11111118
mock_catalog = Mock()
11121119

1113-
with patch("cpex.tools.cli.console") as mock_console:
1120+
with (
1121+
patch("cpex.tools.cli.console") as mock_console,
1122+
pytest.raises(click.exceptions.Exit) as exc_info,
1123+
):
11141124
uninstall("nonexistent_plugin", mock_catalog)
1115-
mock_console.print.assert_called_with(":x: Plugin 'nonexistent_plugin' is not installed.")
1125+
assert exc_info.value.exit_code == 3 # EXIT_NOT_FOUND
1126+
mock_console.print.assert_called_with(":x: Plugin 'nonexistent_plugin' is not installed.")
11161127

11171128
def test_uninstall_cancelled_by_user(self, temp_registry_dir):
11181129
"""Test uninstall cancelled by user."""
@@ -1211,33 +1222,35 @@ def test_uninstall_handles_exception(self, temp_registry_dir):
12111222
]
12121223
}
12131224
registry_file.write_text(json.dumps(registry_data))
1214-
1225+
12151226
mock_catalog = Mock()
1216-
1227+
12171228
# Create a manifest to return from find
12181229
test_manifest = create_test_manifest(name="test_plugin", kind="native")
1219-
1230+
12201231
with (
12211232
patch("cpex.tools.cli.inquirer.prompt", return_value={"confirm": True}),
12221233
patch("cpex.tools.cli.console") as mock_console,
12231234
patch("cpex.tools.cli.logger") as mock_logger,
12241235
patch("cpex.tools.cli.PluginCatalog") as mock_catalog_class,
1236+
pytest.raises(click.exceptions.Exit) as exc_info,
12251237
):
12261238
# Mock the catalog instance created inside uninstall()
12271239
mock_catalog_instance = Mock()
12281240
mock_catalog_instance.find = Mock(return_value=test_manifest)
12291241
mock_catalog_instance.uninstall_package = Mock(side_effect=RuntimeError("Uninstall failed"))
12301242
mock_catalog_class.return_value = mock_catalog_instance
1231-
1243+
12321244
mock_status = Mock()
12331245
mock_status.__enter__ = Mock(return_value=mock_status)
12341246
mock_status.__exit__ = Mock(return_value=False)
12351247
mock_console.status = Mock(return_value=mock_status)
1236-
1248+
12371249
uninstall("test_plugin", mock_catalog)
1238-
1239-
mock_console.print.assert_any_call(":x: Failed to uninstall test_plugin: Uninstall failed")
1240-
mock_logger.error.assert_called_once()
1250+
1251+
assert exc_info.value.exit_code == 4 # EXIT_OPERATION_FAILED
1252+
mock_console.print.assert_any_call(":x: Failed to uninstall test_plugin: Uninstall failed")
1253+
mock_logger.error.assert_called_once()
12411254

12421255

12431256
class TestPluginUninstallCommand:
@@ -1251,9 +1264,9 @@ def test_plugin_uninstall_command_without_plugin_name(self, temp_registry_dir):
12511264
):
12521265
mock_catalog = Mock()
12531266
mock_catalog_class.return_value = mock_catalog
1254-
1267+
12551268
result = runner.invoke(app, ["plugin", "uninstall"])
1256-
assert result.exit_code == 0
1269+
assert result.exit_code == 2 # EXIT_INVALID_ARGS
12571270
mock_console.print.assert_called_with(":x: Please specify a plugin name to uninstall.")
12581271

12591272
def test_plugin_uninstall_command_success(self, temp_registry_dir):
@@ -1308,9 +1321,9 @@ def test_plugin_uninstall_command_not_found(self, temp_registry_dir):
13081321
):
13091322
mock_catalog = Mock()
13101323
mock_catalog_class.return_value = mock_catalog
1311-
1324+
13121325
result = runner.invoke(app, ["plugin", "uninstall", "nonexistent_plugin"])
1313-
assert result.exit_code == 0
1326+
assert result.exit_code == 3 # EXIT_NOT_FOUND
13141327
mock_console.print.assert_called_with(":x: Plugin 'nonexistent_plugin' is not installed.")
13151328

13161329

@@ -1800,14 +1813,18 @@ class TestInstallFunctionAdditional:
18001813
"""Additional tests for install() function."""
18011814

18021815
def test_install_with_unsupported_type_raises_error(self):
1803-
"""Test that install raises ValueError for unsupported installation type."""
1816+
"""Test that install raises typer.Exit for unsupported installation type."""
18041817
from cpex.tools.cli import install
18051818
from cpex.tools.catalog import PluginCatalog
18061819

18071820
mock_catalog = Mock(spec=PluginCatalog)
18081821

1809-
with pytest.raises(ValueError, match="Unsupported installation type"):
1822+
with (
1823+
patch("cpex.tools.cli.console") as mock_console,
1824+
pytest.raises(click.exceptions.Exit) as exc_info,
1825+
):
18101826
install("test_plugin", "unsupported_type", mock_catalog)
1827+
assert exc_info.value.exit_code == 2 # EXIT_INVALID_ARGS
18111828

18121829

18131830
class TestVersionsFunction:
@@ -1934,6 +1951,7 @@ def test_uninstall_when_manifest_not_found(self, temp_registry_dir):
19341951
patch("cpex.tools.cli.inquirer.prompt", return_value={"confirm": True}),
19351952
patch("cpex.tools.cli.console") as mock_console,
19361953
patch("cpex.tools.cli.PluginCatalog") as mock_catalog_class,
1954+
pytest.raises(click.exceptions.Exit) as exc_info,
19371955
):
19381956
# Mock the catalog.find method to return None (manifest not found)
19391957
mock_catalog_instance = Mock()
@@ -1946,11 +1964,12 @@ def test_uninstall_when_manifest_not_found(self, temp_registry_dir):
19461964
mock_console.status = Mock(return_value=mock_status)
19471965

19481966
uninstall("test_plugin", mock_catalog)
1949-
1950-
# When manifest is not found, uninstall should print error and return early
1951-
# So uninstall_package should NOT be called
1952-
mock_catalog_instance.uninstall_package.assert_not_called()
1953-
mock_console.print.assert_any_call(":x: Plugin test_plugin not found in catalog.")
1967+
1968+
assert exc_info.value.exit_code == 3 # EXIT_NOT_FOUND
1969+
# When manifest is not found, uninstall should print error and exit
1970+
# So uninstall_package should NOT be called
1971+
mock_catalog_instance.uninstall_package.assert_not_called()
1972+
mock_console.print.assert_any_call(":x: Plugin test_plugin not found in catalog.")
19541973

19551974

19561975

0 commit comments

Comments
 (0)