From 7ef9fc85504f3cd117cf7fe5dd1aca04463380cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=F0=9F=A6=8A=20=28TheFoxKD=29?= Date: Sun, 24 Nov 2024 00:31:14 +0300 Subject: [PATCH] refactor(cli): remove LoggingOutput formatter - Remove LoggingOutput class and related tests - Logging is now handled at the application level - Simplify output system to focus on ConsoleOutput - Keep MultiOutput for potential future formatters This is a breaking change that moves logging responsibility to the application layer for better separation of concerns. --- src/cli/app.py | 28 +++----- src/cli/output.py | 31 --------- src/utils/__init__.py | 0 tests/conftest.py | 40 +++++++++++ tests/test_output.py | 150 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 50 deletions(-) delete mode 100644 src/utils/__init__.py create mode 100644 tests/test_output.py diff --git a/src/cli/app.py b/src/cli/app.py index 14e4b9a..fee7d8f 100644 --- a/src/cli/app.py +++ b/src/cli/app.py @@ -1,11 +1,11 @@ # src/cli/app.py import argparse -import logging import sys from collections.abc import Sequence from pathlib import Path from src.cli.commands.base import BaseCommand +from src.cli.output import ConsoleOutput from src.storage.abstract import AbstractStorage from src.storage.json_storage import StorageError @@ -18,7 +18,7 @@ def __init__( ) -> None: """Initialize CLI application.""" self.storage = storage - self.logger = logging.getLogger(__name__) + self.output = ConsoleOutput() self.parser = argparse.ArgumentParser( description="Book Manager CLI", @@ -52,34 +52,23 @@ def run(self, args: list[str] | None = None) -> int: command = self.commands[parsed_args.command] result = command.execute(parsed_args) + self.output.display(result) - if not result.success: - self.logger.error(result.message) - return 1 - - self.logger.info(result.message) - return 0 + return 0 if result.success else 1 except StorageError as e: - self.logger.error("Storage error: %s", str(e)) + self.output.error(f"Storage error: {e}") return 2 except Exception as e: - self.logger.error("Unexpected error: %s", str(e)) + self.output.error(f"Unexpected error: {e}") return 3 def main() -> None: """CLI entry point.""" try: - # Configure logging - logging.basicConfig( - level=logging.INFO, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - ) - - # Setup storage - storage_path = Path.home() / ".book-manager" / "books.json" + storage_path = Path("data/books.json") storage_path.parent.mkdir(parents=True, exist_ok=True) from src.cli.commands.add import AddCommand @@ -102,7 +91,8 @@ def main() -> None: sys.exit(app.run(sys.argv[1:])) except Exception as e: - logging.error("Failed to initialize application: %s", str(e)) + console = ConsoleOutput() + console.error(f"Failed to initialize application: {e}") sys.exit(1) diff --git a/src/cli/output.py b/src/cli/output.py index 6dddece..0d55c24 100644 --- a/src/cli/output.py +++ b/src/cli/output.py @@ -1,5 +1,4 @@ # src/cli/output.py -import logging from abc import ABC, abstractmethod from datetime import datetime from typing import Any @@ -134,36 +133,6 @@ def _display_dict(self, data: dict[str, Any]) -> None: self.console.print(table) -class LoggingOutput(OutputFormatter): - """Output formatter that writes to a log file.""" - - def __init__(self, log_file: str) -> None: - """ - Initialize logging output. - - Args: - log_file: Path to the log file - """ - self.logger = logging.getLogger(__name__) - handler = logging.FileHandler(log_file) - handler.setFormatter( - logging.Formatter("%(asctime)s - %(levelname)s - %(message)s") - ) - self.logger.addHandler(handler) - - def display(self, result: CommandResult) -> None: - """Log command result.""" - level = logging.INFO if result.success else logging.ERROR - self.logger.log(level, result.message) - - if result.data: - self.logger.debug("Command data: %s", result.data) - - def error(self, message: str) -> None: - """Log error message.""" - self.logger.error(message) - - class MultiOutput(OutputFormatter): """Output formatter that combines multiple formatters.""" diff --git a/src/utils/__init__.py b/src/utils/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/conftest.py b/tests/conftest.py index 9d11a85..67786e0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,7 @@ from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any +from unittest.mock import Mock import pytest from _pytest.monkeypatch import MonkeyPatch @@ -297,3 +298,42 @@ def cli_commands(storage) -> list[BaseCommand]: def cli_app(storage, cli_commands) -> BookManagerCLI: """Fixture providing a CLI application instance.""" return BookManagerCLI(storage, cli_commands) + + +@pytest.fixture +def mock_console(monkeypatch) -> Mock: + """Fixture providing a mocked Rich console.""" + console = Mock(spec=Console) + monkeypatch.setattr("src.cli.output.Console", lambda: console) + return console + + +@pytest.fixture +def sample_data() -> dict[str, Any]: + """Fixture providing sample test data.""" + now = datetime.now(UTC) + return { + "single": { + "id": "book_1", + "title": "Test Book", + "author": "Test Author", + "year": 2024, + "status": "available", + "created_at": now.isoformat(), + "updated_at": now.isoformat(), + }, + "multiple": [ + { + "id": "book_1", + "title": "Book 1", + "author": "Author 1", + "status": "available", + }, + { + "id": "book_2", + "title": "Book 2", + "author": "Author 2", + "status": "borrowed", + }, + ], + } diff --git a/tests/test_output.py b/tests/test_output.py new file mode 100644 index 0000000..a69e300 --- /dev/null +++ b/tests/test_output.py @@ -0,0 +1,150 @@ +# tests/cli/test_output.py +from unittest.mock import Mock, call + +from rich.table import Table + +from src.cli.commands.base import CommandResult +from src.cli.output import ConsoleOutput, MultiOutput, OutputFormatter + +# Constants for test assertions +EXPECTED_PRINTS_FOR_STATUS_TEST = 4 # 2 messages + 2 tables +EXPECTED_PRINTS_FOR_EMPTY_TEST = 2 # Only message printed + + +class TestConsoleOutput: + """Test cases for ConsoleOutput.""" + + def test_display_success_message(self, mock_console): + """Test displaying success message.""" + output = ConsoleOutput() + result = CommandResult(success=True, message="Test message") + + output.display(result) + + mock_console.print.assert_called_with("\n[green]Test message[/green]\n") + + def test_display_error_message(self, mock_console): + """Test displaying error message.""" + output = ConsoleOutput() + result = CommandResult(success=False, message="Error message") + + output.display(result) + + mock_console.print.assert_called_with("\n[red]Error: Error message[/red]\n") + + def test_display_table_data(self, mock_console, sample_data): + """Test displaying tabular data.""" + output = ConsoleOutput() + result = CommandResult( + success=True, message="Found books", data=sample_data["multiple"] + ) + + output.display(result) + + # Check success message was printed + assert mock_console.print.call_args_list[0] == call( + "\n[green]Found books[/green]\n" + ) + + # Get the table object from the second print call + table_call = mock_console.print.call_args_list[1] + table = table_call[0][0] + + assert isinstance(table, Table) + assert table.show_header is True + assert table.header_style == "bold magenta" + + def test_display_dict_data(self, mock_console, sample_data): + """Test displaying dictionary data.""" + output = ConsoleOutput() + result = CommandResult( + success=True, message="Book details", data=sample_data["single"] + ) + + output.display(result) + + # Check success message was printed + assert mock_console.print.call_args_list[0] == call( + "\n[green]Book details[/green]\n" + ) + + # Get the table object from the second print call + table_call = mock_console.print.call_args_list[1] + table = table_call[0][0] + + assert isinstance(table, Table) + assert table.show_header is False + assert table.box is None + + def test_display_status_formatting(self, mock_console): + """Test status field formatting.""" + output = ConsoleOutput() + + # Test available status + available_data = {"status": "available", "title": "Book 1"} + output.display( + CommandResult(success=True, message="Status test", data=available_data) + ) + + # Test borrowed status + borrowed_data = {"status": "borrowed", "title": "Book 2"} + output.display( + CommandResult(success=True, message="Status test", data=borrowed_data) + ) + + # Verify both tables were printed + assert len(mock_console.print.call_args_list) == EXPECTED_PRINTS_FOR_STATUS_TEST + + def test_empty_data_handling(self, mock_console): + """Test handling empty data.""" + output = ConsoleOutput() + + # Test empty list + result = CommandResult(success=True, message="No data", data=[]) + output.display(result) + assert mock_console.print.call_count == 1 # Only message printed + + # Test None data + result = CommandResult(success=True, message="No data", data=None) + output.display(result) + assert mock_console.print.call_count == EXPECTED_PRINTS_FOR_EMPTY_TEST + + def test_date_formatting(self, mock_console, sample_data): + """Test date formatting in output.""" + output = ConsoleOutput() + data = sample_data["single"] + result = CommandResult(success=True, message="Test", data=data) + + output.display(result) + + table_call = mock_console.print.call_args_list[1] + table = table_call[0][0] + assert isinstance(table, Table) + + +class TestMultiOutput: + """Tests for MultiOutput.""" + + def test_multiple_formatters(self): + """Test multiple formatter handling.""" + formatter1 = Mock(spec=OutputFormatter) + formatter2 = Mock(spec=OutputFormatter) + output = MultiOutput(formatter1, formatter2) + result = CommandResult(success=True, message="Test") + + output.display(result) + output.error("Test error") + + # Check both formatters received the calls + for formatter in (formatter1, formatter2): + formatter.display.assert_called_once_with(result) + formatter.error.assert_called_once_with("Test error") + + def test_no_formatters(self): + """Test behavior with no formatters.""" + output = MultiOutput() + result = CommandResult(success=True, message="Test") + + # Should not raise exceptions + output.display(result) + output.error("Test")