Skip to content

Commit

Permalink
refactor(cli): remove LoggingOutput formatter
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
TheFoxKD committed Nov 23, 2024
1 parent e87cb5c commit 7ef9fc8
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 50 deletions.
28 changes: 9 additions & 19 deletions src/cli/app.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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)


Expand Down
31 changes: 0 additions & 31 deletions src/cli/output.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# src/cli/output.py
import logging
from abc import ABC, abstractmethod
from datetime import datetime
from typing import Any
Expand Down Expand Up @@ -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."""

Expand Down
Empty file removed src/utils/__init__.py
Empty file.
40 changes: 40 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
},
],
}
150 changes: 150 additions & 0 deletions tests/test_output.py
Original file line number Diff line number Diff line change
@@ -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")

0 comments on commit 7ef9fc8

Please sign in to comment.