From 4de1d0ddd589be3bfe2693c6102f9b5464acb8b0 Mon Sep 17 00:00:00 2001 From: diogo464 Date: Thu, 19 Jun 2025 09:04:27 +0100 Subject: Switch to anyhow for improved error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace Box with anyhow::Result throughout - Add comprehensive assert_cmd documentation to CLAUDE.md - Create detailed improvement plan in IMPROVEMENT_PLAN.md - Use anyhow::anyhow\! for custom error messages - Add Context trait for better error context where applicable This provides better error messages and more idiomatic Rust error handling. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- CLAUDE.md | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++ IMPROVEMENT_PLAN.md | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 35 +++++------ 3 files changed, 345 insertions(+), 17 deletions(-) create mode 100644 IMPROVEMENT_PLAN.md diff --git a/CLAUDE.md b/CLAUDE.md index 6b97b70..c940c92 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,3 +8,164 @@ do not add dependencies manually, instead, use the following tools: + for logging, prefer the `tracing` crate with `tracing-subscriber` and fully qualify the log macros (ex: `tracing::info!`) + for cli use the `clap` crate. when implementing subcommands use an `enum` and separate structs for each subcommand's arguments + use the `anyhow` crate for error handling + +## testing guidelines +for testing cli applications, use the `assert_cmd` crate for integration testing + +## assert_cmd crate reference + +### Overview +`assert_cmd` is a Rust testing library designed to simplify integration testing of command-line applications. It provides easy command initialization, simplified configuration, and robust assertion mechanisms. + +### Key Features +- Easy command initialization and execution +- Cargo binary testing support +- Flexible output validation with predicates +- Environment variable and stdin management +- Comprehensive assertion mechanisms + +### Basic Usage Patterns + +#### 1. Basic Command Testing +```rust +use assert_cmd::Command; + +// Run a Cargo binary +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.assert().success(); // Basic success assertion +``` + +#### 2. Command with Arguments +```rust +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.args(&["run", "--id", "test", "sleep", "5"]) + .assert() + .success(); +``` + +#### 3. Output Validation +```rust +use predicates::prelude::*; + +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.args(&["list"]) + .assert() + .success() + .stdout(predicate::str::contains("ID")) + .stderr(predicate::str::is_empty()); +``` + +#### 4. Testing Failures +```rust +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.args(&["run", "--id", "test"]) // Missing command + .assert() + .failure() + .stderr(predicate::str::contains("Command cannot be empty")); +``` + +### Key Methods + +#### Command Configuration +- `Command::cargo_bin("binary_name")`: Find and initialize a Cargo project binary +- `arg(arg)` / `args(&[args])`: Add command arguments +- `env(key, value)` / `envs(vars)`: Set environment variables +- `current_dir(path)`: Set working directory +- `write_stdin(input)`: Provide stdin input + +#### Assertions +- `assert()`: Start assertion chain +- `success()`: Check for successful execution (exit code 0) +- `failure()`: Check for command failure (exit code != 0) +- `code(expected)`: Validate specific exit code +- `stdout(predicate)`: Validate stdout content +- `stderr(predicate)`: Validate stderr content + +### Predicates for Output Validation +```rust +use predicates::prelude::*; + +// Exact match +.stdout("exact output") + +// Contains text +.stdout(predicate::str::contains("partial")) + +// Regex match +.stdout(predicate::str::is_match(r"PID: \d+").unwrap()) + +// Empty output +.stderr(predicate::str::is_empty()) + +// Multiple conditions +.stdout(predicate::str::contains("SUCCESS").and(predicate::str::contains("ID"))) +``` + +### Testing File I/O +For testing CLI tools that create/modify files, combine with `tempfile` and `assert_fs`: + +```rust +use tempfile::TempDir; +use std::fs; + +#[test] +fn test_file_creation() { + let temp_dir = TempDir::new().unwrap(); + + let mut cmd = Command::cargo_bin("demon").unwrap(); + cmd.current_dir(temp_dir.path()) + .args(&["run", "--id", "test", "echo", "hello"]) + .assert() + .success(); + + // Verify files were created + assert!(temp_dir.path().join("test.pid").exists()); + assert!(temp_dir.path().join("test.stdout").exists()); +} +``` + +### Best Practices + +1. **Use `cargo_bin()`**: Automatically locate project binaries +2. **Chain configuration**: Configure all arguments/env before calling `assert()` +3. **Test various scenarios**: Success, failure, edge cases +4. **Use predicates**: More flexible than exact string matching +5. **Isolate tests**: Use temporary directories for file-based tests +6. **Test error conditions**: Verify proper error handling and messages + +### Common Patterns for CLI Testing + +#### Testing Help Output +```rust +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.args(&["--help"]) + .assert() + .success() + .stdout(predicate::str::contains("daemon process management")); +``` + +#### Testing Subcommands +```rust +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.args(&["list"]) + .assert() + .success() + .stdout(predicate::str::contains("ID")); +``` + +#### Testing with Timeouts +```rust +use std::time::Duration; + +let mut cmd = Command::cargo_bin("demon").unwrap(); +cmd.timeout(Duration::from_secs(30)) // Prevent hanging tests + .args(&["run", "--id", "long", "sleep", "10"]) + .assert() + .success(); +``` + +### Integration with Other Test Crates +- **`assert_fs`**: Filesystem testing utilities +- **`predicates`**: Advanced output validation +- **`tempfile`**: Temporary file/directory management +- **`serial_test`**: Serialize tests that can't run concurrently diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md new file mode 100644 index 0000000..a553884 --- /dev/null +++ b/IMPROVEMENT_PLAN.md @@ -0,0 +1,166 @@ +# Demon CLI Improvement Plan + +## Overview +This document outlines the planned improvements to the demon CLI tool based on feedback and best practices. + +## Improvement Tasks + +### 1. Switch to `anyhow` for Error Handling +**Priority**: High +**Status**: Pending + +**Goal**: Replace `Box` with `anyhow::Result` throughout the codebase for better error handling. + +**Tasks**: +- Replace all `Result<(), Box>` with `anyhow::Result<()>` +- Use `anyhow::Context` for better error context +- Simplify error handling code +- Update imports and error propagation + +**Benefits**: +- Better error messages with context +- Simpler error handling +- More idiomatic Rust error handling + +### 2. Implement CLI Testing with `assert_cmd` +**Priority**: High +**Status**: Pending + +**Goal**: Create comprehensive integration tests for all CLI commands using the `assert_cmd` crate. + +**Prerequisites**: +- Research and document `assert_cmd` usage in CLAUDE.md +- Add `assert_cmd` dependency +- Create test infrastructure + +**Test Coverage Required**: +- `demon run`: Process spawning, file creation, duplicate detection +- `demon stop`: Process termination, timeout handling, cleanup +- `demon tail`: File watching behavior (basic scenarios) +- `demon cat`: File content display, flag handling +- `demon list`: Process listing, status detection +- `demon status`: Individual process status checks +- `demon clean`: Orphaned file cleanup +- Error scenarios: missing files, invalid PIDs, etc. + +**Test Structure**: +``` +tests/ +├── cli.rs # Main CLI integration tests +├── fixtures/ # Test data and helper files +└── common/ # Shared test utilities +``` + +### 3. Add Quiet Flag to List Command +**Priority**: Medium +**Status**: Pending + +**Goal**: Add `-q/--quiet` flag to the `demon list` command for machine-readable output. + +**Requirements**: +- Add `quiet` field to `ListArgs` struct (if needed, since `List` currently has no args) +- Convert `List` command to use `ListArgs` struct +- When quiet flag is used: + - No headers + - One line per process: `id:pid:status` + - No "No daemon processes found" message when empty + +**Example Output**: +```bash +# Normal mode +$ demon list +ID PID STATUS COMMAND +-------------------------------------------------- +my-app 12345 RUNNING N/A + +# Quiet mode +$ demon list -q +my-app:12345:RUNNING +``` + +### 4. Add LLM Command +**Priority**: Medium +**Status**: Pending + +**Goal**: Add a `demon llm` command that outputs a comprehensive usage guide for other LLMs. + +**Requirements**: +- Add `Llm` variant to `Commands` enum +- No arguments needed +- Output to stdout (not stderr like other messages) +- Include all commands with examples +- Assume the reader is an LLM that needs to understand how to use the tool + +**Content Structure**: +- Tool overview and purpose +- All available commands with syntax +- Practical examples for each command +- Common workflows +- File structure explanation +- Error handling tips + +### 5. Remove `glob` Dependency +**Priority**: Low +**Status**: Pending + +**Goal**: Replace the `glob` crate with standard library `std::fs` functionality. + +**Implementation**: +- Remove `glob` from Cargo.toml +- Replace `glob("*.pid")` with `std::fs::read_dir()` + filtering +- Update imports +- Ensure same functionality is maintained + +**Functions to Update**: +- `list_daemons()`: Find all .pid files +- `clean_orphaned_files()`: Find all .pid files + +**Implementation Pattern**: +```rust +// Replace glob("*.pid") with: +std::fs::read_dir(".")? + .filter_map(|entry| entry.ok()) + .filter(|entry| { + entry.path().extension() + .and_then(|ext| ext.to_str()) + .map(|ext| ext == "pid") + .unwrap_or(false) + }) +``` + +## Implementation Order + +1. **Document assert_cmd** - Add understanding to CLAUDE.md +2. **Switch to anyhow** - Foundation for better error handling +3. **Implement tests** - Ensure current functionality works correctly +4. **Add quiet flag** - Small feature addition +5. **Add LLM command** - Documentation feature +6. **Remove glob** - Cleanup and reduce dependencies + +## Success Criteria + +- [ ] All existing functionality remains intact +- [ ] Comprehensive test coverage (>80% of CLI scenarios) +- [ ] Better error messages with context +- [ ] Machine-readable list output option +- [ ] LLM-friendly documentation command +- [ ] Reduced dependency footprint +- [ ] All changes committed with proper messages + +## Risk Assessment + +**Low Risk**: +- anyhow migration (straightforward replacement) +- quiet flag addition (additive change) +- LLM command (new, isolated feature) + +**Medium Risk**: +- glob removal (need to ensure exact same behavior) +- CLI testing (need to handle file system interactions carefully) + +## Notes + +- Each improvement should be implemented, tested, and committed separately +- Maintain backward compatibility for all existing commands +- Update IMPLEMENTATION_PLAN.md as work progresses +- Consider adding integration tests that verify the actual daemon functionality \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index e0545e6..12a08b6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ use std::path::Path; use notify::{Config, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; use std::sync::mpsc::channel; use glob::glob; +use anyhow::{Result, Context}; #[derive(Parser)] #[command(name = "demon")] @@ -114,11 +115,11 @@ fn main() { } } -fn run_command(command: Commands) -> Result<(), Box> { +fn run_command(command: Commands) -> Result<()> { match command { Commands::Run(args) => { if args.command.is_empty() { - return Err("Command cannot be empty".into()); + return Err(anyhow::anyhow!("Command cannot be empty")); } run_daemon(&args.id, &args.command) } @@ -147,14 +148,14 @@ fn run_command(command: Commands) -> Result<(), Box> { } } -fn run_daemon(id: &str, command: &[String]) -> Result<(), Box> { +fn run_daemon(id: &str, command: &[String]) -> Result<()> { let pid_file = format!("{}.pid", id); let stdout_file = format!("{}.stdout", id); let stderr_file = format!("{}.stderr", id); // Check if process is already running if is_process_running(&pid_file)? { - return Err(format!("Process '{}' is already running", id).into()); + return Err(anyhow::anyhow!("Process '{}' is already running", id)); } tracing::info!("Starting daemon '{}' with command: {:?}", id, command); @@ -177,7 +178,7 @@ fn run_daemon(id: &str, command: &[String]) -> Result<(), Box Result<(), Box Result> { +fn is_process_running(pid_file: &str) -> Result { // Try to read the PID file let mut file = match File::open(pid_file) { Ok(f) => f, @@ -214,7 +215,7 @@ fn is_process_running(pid_file: &str) -> Result Ok(output.status.success()) } -fn stop_daemon(id: &str, timeout: u64) -> Result<(), Box> { +fn stop_daemon(id: &str, timeout: u64) -> Result<()> { let pid_file = format!("{}.pid", id); // Check if PID file exists @@ -255,7 +256,7 @@ fn stop_daemon(id: &str, timeout: u64) -> Result<(), Box> .output()?; if !output.status.success() { - return Err(format!("Failed to send SIGTERM to PID {}", pid).into()); + return Err(anyhow::anyhow!("Failed to send SIGTERM to PID {}", pid)); } // Wait for the process to terminate @@ -280,14 +281,14 @@ fn stop_daemon(id: &str, timeout: u64) -> Result<(), Box> .output()?; if !output.status.success() { - return Err(format!("Failed to send SIGKILL to PID {}", pid).into()); + return Err(anyhow::anyhow!("Failed to send SIGKILL to PID {}", pid)); } // Wait a bit more for SIGKILL to take effect thread::sleep(Duration::from_secs(1)); if is_process_running_by_pid(pid) { - return Err(format!("Process {} is still running after SIGKILL", pid).into()); + return Err(anyhow::anyhow!("Process {} is still running after SIGKILL", pid)); } println!("Process '{}' (PID: {}) terminated forcefully", id, pid); @@ -307,7 +308,7 @@ fn is_process_running_by_pid(pid: u32) -> bool { } } -fn cat_logs(id: &str, show_stdout: bool, show_stderr: bool) -> Result<(), Box> { +fn cat_logs(id: &str, show_stdout: bool, show_stderr: bool) -> Result<()> { let stdout_file = format!("{}.stdout", id); let stderr_file = format!("{}.stderr", id); @@ -348,7 +349,7 @@ fn cat_logs(id: &str, show_stdout: bool, show_stderr: bool) -> Result<(), Box Result<(), Box> { +fn tail_logs(id: &str, show_stdout: bool, show_stderr: bool) -> Result<()> { let stdout_file = format!("{}.stdout", id); let stderr_file = format!("{}.stderr", id); @@ -464,7 +465,7 @@ fn tail_logs(id: &str, show_stdout: bool, show_stderr: bool) -> Result<(), Box Result> { +fn read_file_content(file: &mut File) -> Result { let mut content = String::new(); file.read_to_string(&mut content)?; Ok(content) @@ -474,7 +475,7 @@ fn handle_file_change( file_path: &str, positions: &mut std::collections::HashMap, show_headers: bool -) -> Result<(), Box> { +) -> Result<()> { let mut file = File::open(file_path)?; let current_pos = positions.get(file_path).copied().unwrap_or(0); @@ -500,7 +501,7 @@ fn handle_file_change( Ok(()) } -fn list_daemons() -> Result<(), Box> { +fn list_daemons() -> Result<()> { println!("{:<20} {:<8} {:<10} {}", "ID", "PID", "STATUS", "COMMAND"); println!("{}", "-".repeat(50)); @@ -557,7 +558,7 @@ fn list_daemons() -> Result<(), Box> { Ok(()) } -fn status_daemon(id: &str) -> Result<(), Box> { +fn status_daemon(id: &str) -> Result<()> { let pid_file = format!("{}.pid", id); let stdout_file = format!("{}.stdout", id); let stderr_file = format!("{}.stderr", id); @@ -614,7 +615,7 @@ fn status_daemon(id: &str) -> Result<(), Box> { Ok(()) } -fn clean_orphaned_files() -> Result<(), Box> { +fn clean_orphaned_files() -> Result<()> { tracing::info!("Scanning for orphaned daemon files..."); let mut cleaned_count = 0; -- cgit