Fix tests

Author: simeg

.claude/settings.local.json

@@ -15,7 +15,8 @@
       "Bash(find:*)",
       "Bash(ls:*)",
       "Bash(timeout 60 cargo tarpaulin --all-features --workspace --exclude-files=tests/* --timeout 30)",
-      "Bash(pkill:*)"
+      "Bash(pkill:*)",
+      "Bash(rg:*)"
     ],
     "deny": []
   }

CLAUDE.md

@@ -14,6 +14,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 - `make test` - Run unit and integration tests
 - `cargo test` - Alternative test command
 - `cargo test <test_name>` - Run specific test
+- `make coverage` - Generate test coverage using tarpaulin
 
 ### Code Quality
 - `make fmt` - Format code with rustfmt
@@ -41,11 +42,16 @@ Each Git workflow command is implemented as a separate module:
 - Error handling uses `expect()` with descriptive messages
 
 ### Key Dependencies
-- `clap` - Command-line parsing with derive macros
-- `clap_complete` - Shell completion generation
-- `console` - Terminal output formatting
+- `clap` - Command-line parsing with derive macros and subcommands
+- `console` - Terminal output formatting with colors and emojis
 - `chrono` - Date/time handling for summary command
 
+### Dev Dependencies
+- `assert_cmd` - CLI testing framework for integration tests
+- `tempfile` - Temporary Git repositories for testing
+- `predicates` - Assertion helpers for test conditions
+- `criterion` - Benchmarking framework
+
 ### Testing
 - Integration tests in `tests/` directory
 - Each command has corresponding `test_<command>.rs` file
@@ -54,5 +60,10 @@ Each Git workflow command is implemented as a separate module:
 ### Git Plugin Integration
 The binary name `git-x` enables Git's plugin system - any executable named `git-<name>` can be invoked as `git <name>`. Commands like `git x info` work automatically once installed.
 
-### Completion System
-Shell completions are generated via `--generate-completions <shell>` flag using `clap_complete`. The README provides setup instructions for bash, zsh, fish, PowerShell, and Elvish.
+### Error Handling and Types
+- Common error type `GitXError` defined in `src/lib.rs` with variants for Git commands, IO, and parsing
+- Most commands use `expect()` with descriptive messages for quick failure feedback
+- Type alias `Result<T>` available for consistent error handling
+
+### Shell Completion Generation
+Shell completions can be generated via `--generate-completions <shell>` flag using `clap_complete`. The README provides setup instructions for bash, zsh, fish, PowerShell, and Elvish.

Makefile

@@ -23,6 +23,10 @@ run: build
 test:
 	$(CARGO) test
 
+## Run tests serially (useful for debugging test interference)
+test-serial:
+	$(CARGO) test -- --test-threads=1
+
 ## Run test coverage analysis using tarpaulin
 coverage:
 	$(CARGO) tarpaulin --workspace --timeout 120 --out Stdout
@@ -67,6 +71,7 @@ help:
 	@echo "  make build     Build release binary"
 	@echo "  make run       Run binary with ARGS=\"xinfo\""
 	@echo "  make test      Run tests"
+	@echo "  make test-serial Run tests serially (for debugging)"
 	@echo "  make coverage  Generate test coverage report"
 	@echo "  make fmt       Format code"
 	@echo "  make fmt-check  Check formatting"

src/health.rs

@@ -1,39 +1,44 @@
+use crate::{GitXError, Result};
 use console::Style;
 use std::process::Command;
 
-pub fn run() {
+pub fn run() -> Result<String> {
     let bold = Style::new().bold();
     let green = Style::new().green().bold();
     let yellow = Style::new().yellow().bold();
     let red = Style::new().red().bold();
 
-    println!("{}", bold.apply_to("Repository Health Check"));
-    println!("{}", bold.apply_to("========================="));
-    println!();
+    let mut output = Vec::new();
+
+    output.push(format!("{}", bold.apply_to("Repository Health Check")));
+    output.push(format!("{}", bold.apply_to("=========================")));
+    output.push(String::new());
 
     // Check if we're in a git repository
     if !is_git_repo(&std::env::current_dir().unwrap_or_else(|_| ".".into())) {
-        println!("{} Not in a Git repository", red.apply_to("✗"));
-        return;
+        output.push(format!("{} Not in a Git repository", red.apply_to("✗")));
+        return Ok(output.join("\n"));
     }
 
     // 1. Check repository status
-    check_repo_status(&green, &yellow, &red);
+    output.push(check_repo_status(&green, &yellow, &red)?);
 
     // 2. Check for untracked files
-    check_untracked_files(&green, &yellow, &red);
+    output.push(check_untracked_files(&green, &yellow, &red)?);
 
     // 3. Check for stale branches
-    check_stale_branches(&green, &yellow, &red);
+    output.push(check_stale_branches(&green, &yellow, &red)?);
 
     // 4. Check repository size
-    check_repo_size(&green, &yellow, &red);
+    output.push(check_repo_size(&green, &yellow, &red)?);
 
     // 5. Check for uncommitted changes
-    check_uncommitted_changes(&green, &yellow, &red);
+    output.push(check_uncommitted_changes(&green, &yellow, &red)?);
+
+    output.push(String::new());
+    output.push(format!("{}", bold.apply_to("Health check complete!")));
 
-    println!();
-    println!("{}", bold.apply_to("Health check complete!"));
+    Ok(output.join("\n"))
 }
 
 pub fn is_git_repo(path: &std::path::Path) -> bool {
@@ -45,50 +50,74 @@ pub fn is_git_repo(path: &std::path::Path) -> bool {
         .unwrap_or(false)
 }
 
-fn check_repo_status(green: &Style, _yellow: &Style, red: &Style) {
+fn check_repo_status(green: &Style, _yellow: &Style, red: &Style) -> Result<String> {
     let output = Command::new("git")
         .args(["status", "--porcelain"])
         .output()
-        .expect("Failed to run git status");
+        .map_err(|_| GitXError::GitCommand("Failed to run git status".to_string()))?;
+
+    if !output.status.success() {
+        return Err(GitXError::GitCommand(
+            "Failed to get repository status".to_string(),
+        ));
+    }
 
     let status_output = String::from_utf8_lossy(&output.stdout);
 
     if status_output.trim().is_empty() {
-        println!("{} Working directory is clean", green.apply_to("✓"));
+        Ok(format!(
+            "{} Working directory is clean",
+            green.apply_to("✓")
+        ))
     } else {
-        println!("{} Working directory has changes", red.apply_to("✗"));
+        Ok(format!(
+            "{} Working directory has changes",
+            red.apply_to("✗")
+        ))
     }
 }
 
-fn check_untracked_files(green: &Style, yellow: &Style, _red: &Style) {
+fn check_untracked_files(green: &Style, yellow: &Style, _red: &Style) -> Result<String> {
     let output = Command::new("git")
         .args(["ls-files", "--others", "--exclude-standard"])
         .output()
-        .expect("Failed to list untracked files");
+        .map_err(|_| GitXError::GitCommand("Failed to list untracked files".to_string()))?;
+
+    if !output.status.success() {
+        return Err(GitXError::GitCommand(
+            "Failed to get untracked files".to_string(),
+        ));
+    }
 
     let untracked = String::from_utf8_lossy(&output.stdout);
     let untracked_files: Vec<&str> = untracked.lines().collect();
 
     if untracked_files.is_empty() {
-        println!("{} No untracked files", green.apply_to("✓"));
+        Ok(format!("{} No untracked files", green.apply_to("✓")))
     } else {
-        println!(
+        Ok(format!(
             "{} {} untracked files found",
             yellow.apply_to("!"),
             untracked_files.len()
-        );
+        ))
     }
 }
 
-fn check_stale_branches(green: &Style, yellow: &Style, _red: &Style) {
+fn check_stale_branches(green: &Style, yellow: &Style, _red: &Style) -> Result<String> {
     let output = Command::new("git")
         .args([
             "for-each-ref",
             "--format=%(refname:short) %(committerdate:relative)",
             "refs/heads/",
         ])
         .output()
-        .expect("Failed to list branches");
+        .map_err(|_| GitXError::GitCommand("Failed to list branches".to_string()))?;
+
+    if !output.status.success() {
+        return Err(GitXError::GitCommand(
+            "Failed to get branch information".to_string(),
+        ));
+    }
 
     let branches = String::from_utf8_lossy(&output.stdout);
     let mut stale_count = 0;
@@ -100,24 +129,30 @@ fn check_stale_branches(green: &Style, yellow: &Style, _red: &Style) {
     }
 
     if stale_count == 0 {
-        println!(
+        Ok(format!(
             "{} No stale branches (older than 1 month)",
             green.apply_to("✓")
-        );
+        ))
     } else {
-        println!(
+        Ok(format!(
             "{} {} potentially stale branches found",
             yellow.apply_to("!"),
             stale_count
-        );
+        ))
     }
 }
 
-fn check_repo_size(green: &Style, yellow: &Style, red: &Style) {
+fn check_repo_size(green: &Style, yellow: &Style, red: &Style) -> Result<String> {
     let output = Command::new("du")
         .args(["-sh", ".git"])
         .output()
-        .expect("Failed to check repository size");
+        .map_err(|_| GitXError::GitCommand("Failed to check repository size".to_string()))?;
+
+    if !output.status.success() {
+        return Err(GitXError::GitCommand(
+            "Failed to get repository size".to_string(),
+        ));
+    }
 
     let size_output = String::from_utf8_lossy(&output.stdout);
     let size = size_output.split_whitespace().next().unwrap_or("unknown");
@@ -126,44 +161,50 @@ fn check_repo_size(green: &Style, yellow: &Style, red: &Style) {
     if size.ends_with('K')
         || (size.ends_with('M') && size.chars().next().unwrap_or('0').to_digit(10).unwrap_or(0) < 5)
     {
-        println!(
+        Ok(format!(
             "{} Repository size: {} (healthy)",
             green.apply_to("✓"),
             size
-        );
+        ))
     } else if size.ends_with('M')
         || (size.ends_with('G') && size.chars().next().unwrap_or('0').to_digit(10).unwrap_or(0) < 1)
     {
-        println!(
+        Ok(format!(
             "{} Repository size: {} (moderate)",
             yellow.apply_to("!"),
             size
-        );
+        ))
     } else {
-        println!(
+        Ok(format!(
             "{} Repository size: {} (large - consider cleanup)",
             red.apply_to("✗"),
             size
-        );
+        ))
     }
 }
 
-fn check_uncommitted_changes(green: &Style, yellow: &Style, _red: &Style) {
+fn check_uncommitted_changes(green: &Style, yellow: &Style, _red: &Style) -> Result<String> {
     let output = Command::new("git")
         .args(["diff", "--cached", "--name-only"])
         .output()
-        .expect("Failed to check staged changes");
+        .map_err(|_| GitXError::GitCommand("Failed to check staged changes".to_string()))?;
+
+    if !output.status.success() {
+        return Err(GitXError::GitCommand(
+            "Failed to get staged changes".to_string(),
+        ));
+    }
 
     let staged = String::from_utf8_lossy(&output.stdout);
     let staged_files: Vec<&str> = staged.lines().filter(|line| !line.is_empty()).collect();
 
     if staged_files.is_empty() {
-        println!("{} No staged changes", green.apply_to("✓"));
+        Ok(format!("{} No staged changes", green.apply_to("✓")))
     } else {
-        println!(
+        Ok(format!(
             "{} {} files staged for commit",
             yellow.apply_to("!"),
             staged_files.len()
-        );
+        ))
     }
 }

src/main.rs

@@ -16,14 +16,23 @@ fn main() {
         Commands::Info => info::run(),
         Commands::Graph => graph::run(),
         Commands::ColorGraph => color_graph::run(),
-        Commands::Health => health::run(),
+        Commands::Health => match health::run() {
+            Ok(output) => println!("{output}"),
+            Err(e) => eprintln!("Error: {e}"),
+        },
         Commands::Since { reference } => since::run(reference),
         Commands::Undo => undo::run(),
         Commands::CleanBranches { dry_run } => clean_branches::run(dry_run),
-        Commands::What { target } => what::run(target),
+        Commands::What { target } => match what::run(target) {
+            Ok(output) => println!("{output}"),
+            Err(e) => eprintln!("Error: {e}"),
+        },
         Commands::Summary { since } => summary::run(since),
         Commands::Sync { merge } => sync::run(merge),
-        Commands::New { branch_name, from } => new_branch::run(branch_name, from),
+        Commands::New { branch_name, from } => match new_branch::run(branch_name, from) {
+            Ok(output) => println!("{output}"),
+            Err(e) => eprintln!("Error: {e}"),
+        },
         Commands::LargeFiles { limit, threshold } => large_files::run(limit, threshold),
         Commands::Fixup {
             commit_hash,