refac: improve main function

successfully refactored the run function in src/dotcat/cli.py into smaller, more modular functions and updated the tests to make use of this higher modularity.

Here's what I did:

Refactored the run function into several smaller functions, each with a single responsibility:

handle_version_flag: Handles the version flag display
handle_special_case_arguments: Handles the special case for dotted-path arguments
validate_required_arguments: Validates that required arguments are present
process_file: Parses the file and handles errors
lookup_and_format_value: Looks up the value and formats the output
Created a new test file tests/test_cli_functions.py that tests each of these functions individually, providing better test coverage and making it easier to identify issues with specific functionality.

Verified that all existing tests in test_exec.py still pass, ensuring that the refactoring didn't break any existing functionality.

The refactoring has improved the code quality by:

Making each function focused on a single responsibility
Improving readability and maintainability
Making the code more testable
Providing better error handling and separation of concerns
All tests are passing, confirming that the refactored code maintains the same behavior as the original implementation.

Author: arthur-debert

src/dotcat/cli.py

@@ -5,7 +5,7 @@
 import sys
 import os
 import argparse
-from typing import List, Tuple
+from typing import List, Tuple, Any
 
 from .__version__ import __version__
 from .formatting import red
@@ -15,6 +15,142 @@
 from .data_access import from_attr_chain
 
 
+def handle_version_flag(version_flag: bool) -> bool:
+    """
+    Handle the version flag if present.
+
+    Args:
+        version_flag: Whether the version flag was provided.
+
+    Returns:
+        True if the version flag was handled, False otherwise.
+    """
+    if version_flag:
+        print(f"dotcat version {__version__}")
+        return True
+    return False
+
+
+def handle_special_case_arguments(
+    filename: str, lookup_chain: str, args: List[str]
+) -> Tuple[str, str]:
+    """
+    Handle special case where a single argument looks like a dotted-path.
+
+    Args:
+        filename: The filename argument.
+        lookup_chain: The dotted-path argument.
+        args: The original command-line arguments.
+
+    Returns:
+        The updated filename and lookup_chain.
+    """
+    # Special case: If we have only one argument and it looks like a dotted-path,
+    # treat it as the dotted-path rather than the file
+    if filename is not None and lookup_chain is None and len(args) == 1:
+        if is_likely_dot_path(filename):
+            # Swap the arguments
+            lookup_chain = filename
+            filename = None
+
+    return filename, lookup_chain
+
+
+def validate_required_arguments(filename: str, lookup_chain: str) -> None:
+    """
+    Validate that the required arguments are present.
+
+    Args:
+        filename: The filename argument.
+        lookup_chain: The dotted-path argument.
+
+    Raises:
+        SystemExit: If required arguments are missing.
+    """
+    if lookup_chain is None or filename is None:
+        if filename is not None and lookup_chain is None:
+            # Case 1: File is provided but dotted-path is missing
+            try:
+                if os.path.exists(filename):
+                    # File exists, but dotted-path is missing
+                    print(
+                        f"Dotted-path required. Which value do you want me to look up in {filename}?"
+                    )
+                    print(f"\n$dotcat {filename} {red('<dotted-path>')}")
+                    sys.exit(2)  # Invalid usage
+            except Exception:
+                # If there's any error checking the file, fall back to general usage message
+                pass
+        elif filename is None and lookup_chain is not None:
+            # Case 2: Dotted-path is provided but file is missing
+            # Check if the argument looks like a dotted-path (contains dots)
+            if "." in lookup_chain:
+                # It looks like a dotted-path, so assume the file is missing
+                print(
+                    f"File path required. Which file contains the value at {lookup_chain}?"
+                )
+                print(f"\n$dotcat {red('<file>')} {lookup_chain}")
+                sys.exit(2)  # Invalid usage
+            # Otherwise, it might be a file without an extension or something else,
+            # so fall back to the general usage message
+
+        # General usage message for other cases
+        print(USAGE)  # Display usage for invalid arguments
+        sys.exit(2)  # Invalid usage
+
+
+def process_file(filename: str) -> Any:
+    """
+    Parse the file and handle any errors.
+
+    Args:
+        filename: The file to parse.
+
+    Returns:
+        The parsed data.
+
+    Raises:
+        SystemExit: If there's an error parsing the file.
+    """
+    try:
+        return parse_file(filename)
+    except FileNotFoundError as e:
+        print(str(e))
+        sys.exit(3)  # File not found
+    except ValueError as e:
+        if "File is empty" in str(e):
+            print(f"{red('[ERROR]')} {filename}: File is empty")
+        elif "Unable to parse file" in str(e):
+            print(f"Unable to parse file: {red(filename)}")
+        else:
+            print(f"{str(e)}: {red(filename)}")
+        sys.exit(4)  # Parsing error
+
+
+def lookup_and_format_value(
+    data: Any, lookup_chain: str, output_format: str, filename: str
+) -> None:
+    """
+    Look up the value using the dotted-path and format the output.
+
+    Args:
+        data: The parsed data.
+        lookup_chain: The dotted-path to look up.
+        output_format: The output format.
+        filename: The filename (for error messages).
+
+    Raises:
+        SystemExit: If the key is not found.
+    """
+    try:
+        value = from_attr_chain(data, lookup_chain)
+        print(format_output(value, output_format))
+    except KeyError as e:
+        key = e.args[0].split("'")[1] if "'" in e.args[0] else e.args[0]
+        print(f"Key {red(key)} not found in {filename}")
+        sys.exit(5)  # Key not found
+
+
 def parse_args(args: List[str]) -> Tuple[str, str, str, bool]:
     """
     Returns the filename, dotted-path, output format, and version flag.
@@ -91,74 +227,21 @@ def run(args: List[str] = None) -> None:
     # validates arguments
     filename, lookup_chain, output_format, version_flag = parse_args(args)
 
-    if version_flag:
-        print(f"dotcat version {__version__}")
-        return
-
-    # Special case: If we have only one argument and it looks like a dotted-path,
-    # treat it as the dotted-path rather than the file
-    if filename is not None and lookup_chain is None and len(args) == 1:
-        if is_likely_dot_path(filename):
-            # Swap the arguments
-            lookup_chain = filename
-            filename = None
-            # Now filename is None and lookup_chain is not None
+    # Handle version flag
+    if handle_version_flag(version_flag):
+        return  # Exit early if version flag was handled
 
-    # Handle cases where one of the required arguments is missing
-    if lookup_chain is None or filename is None:
-        if filename is not None and lookup_chain is None:
-            # Case 1: File is provided but dotted-path is missing
-            try:
-                if os.path.exists(filename):
-                    # File exists, but dotted-path is missing
-                    print(
-                        f"Dotted-path required. Which value do you want me to look up in {filename}?"
-                    )
-                    print(f"\n$dotcat {filename} {red('<dotted-path>')}")
-                    sys.exit(2)  # Invalid usage
-            except Exception:
-                # If there's any error checking the file, fall back to general usage message
-                pass
-        elif filename is None and lookup_chain is not None:
-            # Case 2: Dotted-path is provided but file is missing
-            # Check if the argument looks like a dotted-path (contains dots)
-            if "." in lookup_chain:
-                # It looks like a dotted-path, so assume the file is missing
-                print(
-                    f"File path required. Which file contains the value at {lookup_chain}?"
-                )
-                print(f"\n$dotcat {red('<file>')} {lookup_chain}")
-                sys.exit(2)  # Invalid usage
-            # Otherwise, it might be a file without an extension or something else,
-            # so fall back to the general usage message
+    # Handle special case arguments
+    filename, lookup_chain = handle_special_case_arguments(filename, lookup_chain, args)
 
-        # General usage message for other cases
-        print(USAGE)  # Display usage for invalid arguments
-        sys.exit(2)  # Invalid usage
+    # Validate required arguments
+    validate_required_arguments(filename, lookup_chain)
 
-    # gets the parsed data
-    try:
-        data = parse_file(filename)
-    except FileNotFoundError as e:
-        print(str(e))
-        sys.exit(3)  # File not found
-    except ValueError as e:
-        if "File is empty" in str(e):
-            print(f"{red('[ERROR]')} {filename}: File is empty")
-        elif "Unable to parse file" in str(e):
-            print(f"Unable to parse file: {red(filename)}")
-        else:
-            print(f"{str(e)}: {red(filename)}")
-        sys.exit(4)  # Parsing error
+    # Parse the file
+    data = process_file(filename)
 
-    # get the value at the specified key
-    try:
-        value = from_attr_chain(data, lookup_chain)
-        print(format_output(value, output_format))
-    except KeyError as e:
-        key = e.args[0].split("'")[1] if "'" in e.args[0] else e.args[0]
-        print(f"Key {red(key)} not found in {filename}")
-        sys.exit(5)  # Key not found
+    # Look up and format the value
+    lookup_and_format_value(data, lookup_chain, output_format, filename)
 
 
 def main() -> None:

tests/test_cli_functions.py

@@ -0,0 +1,145 @@
+"""
+Tests for the individual functions in the cli module.
+"""
+
+import pytest
+from io import StringIO
+from unittest.mock import patch
+
+from dotcat.cli import (
+    handle_version_flag,
+    handle_special_case_arguments,
+    validate_required_arguments,
+    process_file,
+    lookup_and_format_value,
+    is_likely_dot_path,
+)
+from dotcat.__version__ import __version__
+from dotcat.help_text import USAGE
+
+
+def test_handle_version_flag():
+    """Test that the version flag is handled correctly."""
+    with patch("sys.stdout", new=StringIO()) as fake_out:
+        result = handle_version_flag(True)
+        assert result is True
+        assert fake_out.getvalue().strip() == f"dotcat version {__version__}"
+
+    with patch("sys.stdout", new=StringIO()) as fake_out:
+        result = handle_version_flag(False)
+        assert result is False
+        assert fake_out.getvalue().strip() == ""
+
+
+def test_handle_special_case_arguments():
+    """Test that special case arguments are handled correctly."""
+    # Test case where filename looks like a dotted-path
+    filename, lookup_chain = handle_special_case_arguments(
+        "config.settings", None, ["config.settings"]
+    )
+    assert filename is None
+    assert lookup_chain == "config.settings"
+
+    # Test case where filename doesn't look like a dotted-path
+    filename, lookup_chain = handle_special_case_arguments("config", None, ["config"])
+    assert filename == "config"
+    assert lookup_chain is None
+
+    # Test case with both arguments provided
+    filename, lookup_chain = handle_special_case_arguments(
+        "config.json", "settings", ["config.json", "settings"]
+    )
+    assert filename == "config.json"
+    assert lookup_chain == "settings"
+
+
+def test_is_likely_dot_path():
+    """Test that dot paths are correctly identified."""
+    # Test a dot path
+    assert is_likely_dot_path("config.settings") is True
+
+    # Test a file path that exists (mocked)
+    with patch("os.path.exists", return_value=True):
+        assert is_likely_dot_path("config.json") is False
+
+    # Test a string without dots
+    assert is_likely_dot_path("config") is False
+
+
+def test_validate_required_arguments():
+    """Test that required arguments are validated correctly."""
+    # Test case where both arguments are provided
+    validate_required_arguments("config.json", "settings")  # Should not raise
+
+    # Test case where file is provided but dotted-path is missing
+    with patch("os.path.exists", return_value=True):
+        with patch("sys.stdout", new=StringIO()):
+            with pytest.raises(SystemExit) as excinfo:
+                validate_required_arguments("config.json", None)
+            assert excinfo.value.code == 2
+
+    # Test case where dotted-path is provided but file is missing
+    with patch("sys.stdout", new=StringIO()):
+        with pytest.raises(SystemExit) as excinfo:
+            validate_required_arguments(None, "config.settings")
+        assert excinfo.value.code == 2
+
+    # Test case where both arguments are missing
+    with patch("sys.stdout", new=StringIO()) as fake_out:
+        with pytest.raises(SystemExit) as excinfo:
+            validate_required_arguments(None, None)
+        assert excinfo.value.code == 2
+        assert USAGE in fake_out.getvalue()
+
+
+def test_process_file():
+    """Test that files are processed correctly."""
+    # Test with a valid file
+    with patch("dotcat.cli.parse_file", return_value={"key": "value"}):
+        result = process_file("config.json")
+        assert result == {"key": "value"}
+
+    # Test with a file not found error
+    with patch(
+        "dotcat.cli.parse_file", side_effect=FileNotFoundError("File not found")
+    ):
+        with patch("sys.stdout", new=StringIO()):
+            with pytest.raises(SystemExit) as excinfo:
+                process_file("nonexistent.json")
+            assert excinfo.value.code == 3
+
+    # Test with a value error (empty file)
+    with patch("dotcat.cli.parse_file", side_effect=ValueError("File is empty")):
+        with patch("sys.stdout", new=StringIO()):
+            with pytest.raises(SystemExit) as excinfo:
+                process_file("empty.json")
+            assert excinfo.value.code == 4
+
+    # Test with a value error (unable to parse)
+    with patch("dotcat.cli.parse_file", side_effect=ValueError("Unable to parse file")):
+        with patch("sys.stdout", new=StringIO()):
+            with pytest.raises(SystemExit) as excinfo:
+                process_file("malformed.json")
+            assert excinfo.value.code == 4
+
+
+def test_lookup_and_format_value():
+    """Test that values are looked up and formatted correctly."""
+    # Test with a valid key
+    with patch("dotcat.cli.from_attr_chain", return_value="value"):
+        with patch("dotcat.cli.format_output", return_value="value"):
+            with patch("sys.stdout", new=StringIO()) as fake_out:
+                lookup_and_format_value({"key": "value"}, "key", "raw", "config.json")
+                assert fake_out.getvalue().strip() == "value"
+
+    # Test with a key not found error
+    with patch(
+        "dotcat.cli.from_attr_chain",
+        side_effect=KeyError("key 'nonexistent' not found"),
+    ):
+        with patch("sys.stdout", new=StringIO()):
+            with pytest.raises(SystemExit) as excinfo:
+                lookup_and_format_value(
+                    {"key": "value"}, "nonexistent", "raw", "config.json"
+                )
+            assert excinfo.value.code == 5