diff --git a/src/bootstrap/app.rs b/src/bootstrap/app.rs index 735f3b96..926bcb5b 100644 --- a/src/bootstrap/app.rs +++ b/src/bootstrap/app.rs @@ -27,8 +27,9 @@ use crate::{bootstrap, presentation}; /// This function serves as the application bootstrap, handling: /// 1. CLI argument parsing (delegated to presentation layer) /// 2. Logging initialization using `LoggingConfig` -/// 3. Command execution (delegated to presentation layer) -/// 4. Error handling and exit code management +/// 3. Service container creation for dependency injection +/// 4. Command execution (delegated to presentation layer) +/// 5. Error handling and exit code management /// /// # Panics /// @@ -54,10 +55,15 @@ pub fn run() { "Application started" ); + // Initialize service container for dependency injection + let container = bootstrap::Container::new(); + match cli.command { Some(command) => { - if let Err(e) = presentation::execute(command, &cli.global.working_dir) { - presentation::handle_error(&e); + if let Err(e) = + presentation::execute(command, &cli.global.working_dir, &container.user_output()) + { + presentation::handle_error(&e, &container.user_output()); std::process::exit(1); } } diff --git a/src/bootstrap/container.rs b/src/bootstrap/container.rs new file mode 100644 index 00000000..71c5808b --- /dev/null +++ b/src/bootstrap/container.rs @@ -0,0 +1,103 @@ +//! Application Service Container +//! +//! This module provides centralized initialization of application-wide services +//! that need consistent configuration across the entire application. + +use std::sync::{Arc, Mutex}; + +use crate::presentation::commands::constants::DEFAULT_VERBOSITY; +use crate::presentation::user_output::UserOutput; + +/// Application service container +/// +/// Holds shared services initialized during application bootstrap. +/// Services are wrapped in `Arc>` for thread-safe shared ownership +/// with interior mutability across the application. +/// +/// # Example +/// +/// ```rust +/// use torrust_tracker_deployer_lib::bootstrap::container::Container; +/// +/// let container = Container::new(); +/// let user_output = container.user_output(); +/// user_output.lock().unwrap().success("Operation completed"); +/// ``` +#[derive(Clone)] +pub struct Container { + user_output: Arc>, +} + +impl Container { + /// Create a new container with initialized services + /// + /// Uses `DEFAULT_VERBOSITY` for user output. In the future, this may + /// accept a verbosity parameter from CLI flags. + #[must_use] + pub fn new() -> Self { + let user_output = Arc::new(Mutex::new(UserOutput::new(DEFAULT_VERBOSITY))); + + Self { user_output } + } + + /// Get shared reference to user output service + /// + /// Returns an `Arc>` that can be cheaply cloned and shared + /// across threads and function calls. Lock the mutex to access the user output. + /// + /// # Example + /// + /// ```rust + /// use torrust_tracker_deployer_lib::bootstrap::container::Container; + /// + /// let container = Container::new(); + /// let user_output = container.user_output(); + /// user_output.lock().unwrap().success("Operation completed"); + /// ``` + #[must_use] + pub fn user_output(&self) -> Arc> { + Arc::clone(&self.user_output) + } +} + +impl Default for Container { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_container_with_user_output() { + let container = Container::new(); + let user_output = container.user_output(); + + // Verify we can get the user_output service + assert!(Arc::strong_count(&user_output) >= 1); + } + + #[test] + fn it_should_return_cloned_arc_on_user_output_access() { + let container = Container::new(); + let user_output1 = container.user_output(); + let user_output2 = container.user_output(); + + // Both should point to the same UserOutput instance + assert!(Arc::ptr_eq(&user_output1, &user_output2)); + } + + #[test] + fn it_should_be_clonable() { + let container1 = Container::new(); + let container2 = container1.clone(); + + let user_output1 = container1.user_output(); + let user_output2 = container2.user_output(); + + // Cloned containers should share the same UserOutput + assert!(Arc::ptr_eq(&user_output1, &user_output2)); + } +} diff --git a/src/bootstrap/mod.rs b/src/bootstrap/mod.rs index 00be57ec..4ff428e0 100644 --- a/src/bootstrap/mod.rs +++ b/src/bootstrap/mod.rs @@ -1,17 +1,20 @@ //! Bootstrap Module //! //! This module contains application initialization and bootstrap concerns. -//! It handles application lifecycle, logging setup, and help display. +//! It handles application lifecycle, logging setup, help display, and service container. //! //! ## Modules //! //! - `app` - Main application bootstrap and entry point logic +//! - `container` - Application service container for dependency injection //! - `help` - Help and usage information display //! - `logging` - Logging configuration and initialization pub mod app; +pub mod container; pub mod help; pub mod logging; // Re-export commonly used types for convenience +pub use container::Container; pub use logging::{LogFormat, LogOutput, LoggingBuilder, LoggingConfig}; diff --git a/src/presentation/commands/context.rs b/src/presentation/commands/context.rs index 50898f36..db5a36f7 100644 --- a/src/presentation/commands/context.rs +++ b/src/presentation/commands/context.rs @@ -31,18 +31,21 @@ //! //! ```rust //! use std::path::Path; +//! use std::sync::{Arc, Mutex}; //! use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; +//! use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; //! //! fn handle_command(working_dir: &Path) -> Result<(), Box> { -//! let mut ctx = CommandContext::new(working_dir.to_path_buf()); +//! let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); +//! let mut ctx = CommandContext::new(working_dir.to_path_buf(), output.clone()); //! -//! ctx.output().progress("Starting operation..."); +//! output.lock().unwrap().progress("Starting operation..."); //! //! // Use repository and clock through context //! let repo = ctx.repository(); //! let clock = ctx.clock(); //! -//! ctx.output().success("Operation completed"); +//! output.lock().unwrap().success("Operation completed"); //! Ok(()) //! } //! ``` @@ -55,7 +58,7 @@ use crate::infrastructure::persistence::repository_factory::RepositoryFactory; use crate::presentation::user_output::UserOutput; use crate::shared::{Clock, SystemClock}; -use super::constants::{DEFAULT_LOCK_TIMEOUT, DEFAULT_VERBOSITY}; +use super::constants::DEFAULT_LOCK_TIMEOUT; /// Command execution context containing shared dependencies /// @@ -71,10 +74,13 @@ use super::constants::{DEFAULT_LOCK_TIMEOUT, DEFAULT_VERBOSITY}; /// /// ```rust /// use std::path::PathBuf; +/// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; +/// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let working_dir = PathBuf::from("."); -/// let mut ctx = CommandContext::new(working_dir); +/// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); +/// let ctx = CommandContext::new(working_dir, user_output.clone()); /// /// // Access repository /// let repo = ctx.repository(); @@ -83,13 +89,14 @@ use super::constants::{DEFAULT_LOCK_TIMEOUT, DEFAULT_VERBOSITY}; /// let clock = ctx.clock(); /// /// // Access user output -/// ctx.output().progress("Processing..."); -/// ctx.output().success("Complete!"); +/// let mut output = ctx.user_output().lock().unwrap(); +/// output.progress("Processing..."); +/// output.success("Complete!"); /// ``` pub struct CommandContext { repository: Arc, clock: Arc, - output: UserOutput, + user_output: Arc>, } impl CommandContext { @@ -99,32 +106,35 @@ impl CommandContext { /// and default configuration from constants: /// - Repository with default lock timeout /// - System clock - /// - User output with default verbosity + /// - Injected user output service /// /// # Arguments /// /// * `working_dir` - Root directory for environment data storage + /// * `user_output` - Shared user output service for consistent output formatting /// /// # Examples /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let working_dir = PathBuf::from("./data"); - /// let ctx = CommandContext::new(working_dir); + /// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let ctx = CommandContext::new(working_dir, user_output); /// ``` #[must_use] - pub fn new(working_dir: PathBuf) -> Self { + pub fn new(working_dir: PathBuf, user_output: Arc>) -> Self { let repository_factory = RepositoryFactory::new(DEFAULT_LOCK_TIMEOUT); let repository = repository_factory.create(working_dir); let clock = Arc::new(SystemClock); - let output = UserOutput::new(DEFAULT_VERBOSITY); Self { repository, clock, - output, + user_output, } } @@ -138,29 +148,36 @@ impl CommandContext { /// /// * `repository_factory` - Pre-configured repository factory /// * `working_dir` - Root directory for environment data storage + /// * `user_output` - Shared user output service for consistent output formatting /// /// # Examples /// /// ```rust /// use std::path::PathBuf; /// use std::time::Duration; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; /// use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let factory = RepositoryFactory::new(Duration::from_secs(30)); /// let working_dir = PathBuf::from("./data"); - /// let ctx = CommandContext::new_with_factory(&factory, working_dir); + /// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let ctx = CommandContext::new_with_factory(&factory, working_dir, user_output); /// ``` #[must_use] - pub fn new_with_factory(repository_factory: &RepositoryFactory, working_dir: PathBuf) -> Self { + pub fn new_with_factory( + repository_factory: &RepositoryFactory, + working_dir: PathBuf, + user_output: Arc>, + ) -> Self { let repository = repository_factory.create(working_dir); let clock = Arc::new(SystemClock); - let output = UserOutput::new(DEFAULT_VERBOSITY); Self { repository, clock, - output, + user_output, } } @@ -173,12 +190,12 @@ impl CommandContext { /// /// * `repository` - Repository implementation (can be a mock) /// * `clock` - Clock implementation (can be a mock) - /// * `output` - User output instance (can use custom writers) + /// * `user_output` - Shared user output service for consistent output formatting /// /// # Examples /// /// ```rust - /// use std::sync::Arc; + /// use std::sync::{Arc, Mutex}; /// use std::path::PathBuf; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; @@ -190,20 +207,20 @@ impl CommandContext { /// let factory = RepositoryFactory::new(Duration::from_secs(5)); /// let repository = factory.create(PathBuf::from("/tmp/test")); /// let clock: Arc = Arc::new(SystemClock); - /// let output = UserOutput::new(VerbosityLevel::Quiet); + /// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Quiet))); /// - /// let ctx = CommandContext::new_for_testing(repository, clock, output); + /// let ctx = CommandContext::new_for_testing(repository, clock, user_output); /// ``` #[must_use] pub fn new_for_testing( repository: Arc, clock: Arc, - output: UserOutput, + user_output: Arc>, ) -> Self { Self { repository, clock, - output, + user_output, } } @@ -213,9 +230,12 @@ impl CommandContext { /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let ctx = CommandContext::new(PathBuf::from(".")); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let ctx = CommandContext::new(PathBuf::from("."), output); /// let repo = ctx.repository(); /// ``` #[must_use] @@ -229,9 +249,12 @@ impl CommandContext { /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let ctx = CommandContext::new(PathBuf::from(".")); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let ctx = CommandContext::new(PathBuf::from("."), output); /// let clock = ctx.clock(); /// ``` #[must_use] @@ -239,79 +262,61 @@ impl CommandContext { &self.clock } - /// Get mutable reference to user output - /// - /// # Examples + /// Get reference to the shared user output /// - /// ```rust - /// use std::path::PathBuf; - /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; - /// - /// let mut ctx = CommandContext::new(PathBuf::from(".")); - /// ctx.output().progress("Working..."); - /// ctx.output().success("Done!"); - /// ``` - pub fn output(&mut self) -> &mut UserOutput { - &mut self.output - } - - /// Consume the context and return the user output - /// - /// This method is useful when you want to pass ownership of the output - /// to another component, such as a `ProgressReporter`. + /// Returns the Arc-wrapped Mutex-protected `UserOutput` instance, allowing + /// multiple components to share access to the same output sink. /// /// # Examples /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::context::CommandContext; - /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let ctx = CommandContext::new(PathBuf::from(".")); - /// let progress = ProgressReporter::new(ctx.into_output(), 3); + /// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let ctx = CommandContext::new(PathBuf::from("."), user_output); + /// let output_ref = ctx.user_output(); + /// output_ref.lock().unwrap().progress("Working..."); + /// output_ref.lock().unwrap().success("Done!"); /// ``` #[must_use] - pub fn into_output(self) -> UserOutput { - self.output + pub fn user_output(&self) -> &Arc> { + &self.user_output } } -/// Report an error through user output -/// -/// This utility function provides a consistent way to report errors to users. -/// It outputs the error message through the provided user output instance. -/// -/// # Arguments -/// -/// * `output` - User output instance to use for reporting -/// * `error` - Error to report (any type implementing `std::error::Error`) -/// -/// # Examples -/// -/// ```rust -/// use torrust_tracker_deployer_lib::presentation::commands::context::report_error; -/// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; -/// -/// let mut output = UserOutput::new(VerbosityLevel::Normal); -/// let error = std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"); -/// report_error(&mut output, &error); -/// ``` -pub fn report_error(output: &mut UserOutput, error: &dyn std::error::Error) { - output.error(&error.to_string()); -} - #[cfg(test)] mod tests { use super::*; - use std::io::Cursor; use tempfile::TempDir; - #[test] - fn it_should_create_context_with_production_dependencies() { + use crate::presentation::user_output::VerbosityLevel; + + /// Test helper to create a test user output + fn create_test_user_output() -> Arc> { + Arc::new(std::sync::Mutex::new(UserOutput::new( + VerbosityLevel::Normal, + ))) + } + + /// Test helper to create a test context with temporary directory + /// + /// Returns a tuple of (`TempDir`, `PathBuf`, `Arc>`) + /// The `TempDir` must be kept alive for the duration of the test. + fn create_test_setup() -> (TempDir, PathBuf, Arc>) { let temp_dir = TempDir::new().unwrap(); let working_dir = temp_dir.path().to_path_buf(); + let user_output = create_test_user_output(); + (temp_dir, working_dir, user_output) + } - let ctx = CommandContext::new(working_dir); + #[test] + fn it_should_create_context_with_production_dependencies() { + let (_temp_dir, working_dir, user_output) = create_test_setup(); + + let ctx = CommandContext::new(working_dir, user_output); // Verify dependencies are present and accessible (we can call methods on them) let _ = ctx.repository(); @@ -320,10 +325,9 @@ mod tests { #[test] fn it_should_provide_access_to_repository() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (_temp_dir, working_dir, user_output) = create_test_setup(); - let ctx = CommandContext::new(working_dir); + let ctx = CommandContext::new(working_dir, user_output); // Should be able to access repository let _repo = ctx.repository(); @@ -331,34 +335,32 @@ mod tests { #[test] fn it_should_provide_access_to_clock() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (_temp_dir, working_dir, user_output) = create_test_setup(); - let ctx = CommandContext::new(working_dir); + let ctx = CommandContext::new(working_dir, user_output); // Should be able to access clock let _clock = ctx.clock(); } #[test] - fn it_should_provide_mutable_access_to_output() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + fn it_should_provide_access_to_user_output() { + let (_temp_dir, working_dir, user_output) = create_test_setup(); - let mut ctx = CommandContext::new(working_dir); + let ctx = CommandContext::new(working_dir, user_output); - // Should be able to use output methods - ctx.output().progress("Test progress"); - ctx.output().success("Test success"); + // Should be able to use output methods through Arc> + let output_ref = ctx.user_output(); + output_ref.lock().unwrap().progress("Test progress"); + output_ref.lock().unwrap().success("Test success"); } #[test] fn it_should_create_context_with_factory() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (_temp_dir, working_dir, user_output) = create_test_setup(); let repository_factory = RepositoryFactory::new(DEFAULT_LOCK_TIMEOUT); - let ctx = CommandContext::new_with_factory(&repository_factory, working_dir); + let ctx = CommandContext::new_with_factory(&repository_factory, working_dir, user_output); // Verify we can access all dependencies let _repo = ctx.repository(); @@ -374,10 +376,12 @@ mod tests { let repository_factory = RepositoryFactory::new(DEFAULT_LOCK_TIMEOUT); let repository = repository_factory.create(working_dir); let clock: Arc = Arc::new(SystemClock); - let output = UserOutput::new(DEFAULT_VERBOSITY); + let user_output = Arc::new(std::sync::Mutex::new(UserOutput::new( + VerbosityLevel::Normal, + ))); // Create context with test dependencies - let ctx = CommandContext::new_for_testing(repository, clock, output); + let ctx = CommandContext::new_for_testing(repository, clock, user_output); // Verify we can access all dependencies let _repo = ctx.repository(); @@ -386,42 +390,23 @@ mod tests { #[test] fn it_should_allow_accessing_output_multiple_times() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); - - let mut ctx = CommandContext::new(working_dir); + let (_temp_dir, working_dir, user_output) = create_test_setup(); - // Should be able to call output() multiple times - ctx.output().progress("First message"); - ctx.output().success("Second message"); - ctx.output().error("Third message"); - } - - #[test] - fn it_should_report_errors_through_output() { - // Create output with custom writers for testing - let stderr_buf = Vec::new(); - let stderr_writer = Box::new(Cursor::new(stderr_buf)); - let stdout_writer = Box::new(Cursor::new(Vec::new())); - - let mut output = UserOutput::with_writers(DEFAULT_VERBOSITY, stdout_writer, stderr_writer); + let ctx = CommandContext::new(working_dir, user_output); - // Create an error and report it - let error = std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"); - report_error(&mut output, &error); - - // Note: In a real test, we'd verify the output was written, - // but that requires extracting the buffer from output which isn't directly possible - // without additional helper methods. The important thing is that it compiles and runs. + // Should be able to call user_output() multiple times + let output_ref = ctx.user_output(); + output_ref.lock().unwrap().progress("First message"); + output_ref.lock().unwrap().success("Second message"); + output_ref.lock().unwrap().error("Third message"); } #[test] fn it_should_use_default_constants() { - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (_temp_dir, working_dir, user_output) = create_test_setup(); - // Creating context should use DEFAULT_LOCK_TIMEOUT and DEFAULT_VERBOSITY - let _ctx = CommandContext::new(working_dir); + // Creating context should use DEFAULT_LOCK_TIMEOUT + let _ctx = CommandContext::new(working_dir, user_output); // This test verifies that the code compiles with the constants // The actual values are tested in the constants module diff --git a/src/presentation/commands/create/errors.rs b/src/presentation/commands/create/errors.rs index 1ce73713..c029462a 100644 --- a/src/presentation/commands/create/errors.rs +++ b/src/presentation/commands/create/errors.rs @@ -8,6 +8,7 @@ use std::path::PathBuf; use thiserror::Error; use crate::application::command_handlers::create::CreateCommandHandlerError; +use crate::presentation::progress::ProgressReporterError; /// Format of configuration file #[derive(Debug, Clone, Copy)] @@ -106,6 +107,37 @@ Tip: Check that you have write permissions in the target directory" #[source] source: crate::application::command_handlers::create::config::CreateConfigError, }, + + // ===== User Output Lock Errors ===== + /// User output lock acquisition failed + /// + /// Failed to acquire the mutex lock for user output. This indicates a panic + /// occurred in another thread while holding the lock. + #[error("Failed to acquire user output lock - a panic occurred in another thread")] + UserOutputLockFailed, + + /// Progress reporting failed + /// + /// Failed to report progress to the user due to an internal error. + /// This indicates a critical internal error. + #[error( + "Failed to report progress: {source} +Tip: This is a critical bug - please report it with full logs using --log-output file-and-stderr" + )] + ProgressReportingFailed { + #[source] + source: ProgressReporterError, + }, +} + +// ============================================================================ +// ERROR CONVERSIONS +// ============================================================================ + +impl From for CreateSubcommandError { + fn from(source: ProgressReporterError) -> Self { + Self::ProgressReportingFailed { source } + } } impl CreateSubcommandError { @@ -187,6 +219,56 @@ For more information, see the configuration documentation." source.help() } Self::CommandFailed { source } => source.help(), + Self::UserOutputLockFailed => { + "User Output Lock Failed - Troubleshooting: + +This error indicates that a panic occurred in another thread while it was using +the user output system, leaving the mutex in a \"poisoned\" state. + +1. Check for any error messages that appeared before this one + - The original panic message should appear earlier in the output + - This will indicate what caused the initial failure + +2. This is typically caused by: + - A bug in the application code that caused a panic + - An unhandled error condition that triggered a panic + - Resource exhaustion (memory, file handles, etc.) + +3. If you can reproduce this issue: + - Run with --verbose to see more detailed logging + - Report the issue with the full error output and steps to reproduce + +This is a serious application error that indicates a bug. Please report it to the developers." + } + Self::ProgressReportingFailed { .. } => { + "Progress Reporting Failed - Critical Internal Error: + +This error indicates that the progress reporting system encountered a critical +internal error while trying to update the user interface. This is a BUG in the +application and should NOT occur under normal circumstances. + +Immediate Actions: +1. Save any logs using: --log-output file-and-stderr +2. Note the operation that was in progress when this occurred +3. Record any error messages that appeared before this one +4. Document the current state of your environment + +Report the Issue: +1. Include the full log output (--log-output file-and-stderr) +2. Provide steps to reproduce the error +3. Include your environment configuration file +4. Note your operating system and version +5. Report to: https://github.com/torrust/torrust-tracker-deployer/issues + +Workaround: +1. Restart the application and retry the operation +2. Try the operation again with --verbose for more details +3. Check system resources (memory, disk space) +4. Check file system permissions + +This error means the operation may have PARTIALLY completed or FAILED. +Verify the state of your environment before retrying." + } } } } @@ -251,6 +333,7 @@ mod tests { fn it_should_have_help_for_all_error_variants() { use crate::application::command_handlers::create::config::CreateConfigError; use crate::domain::EnvironmentNameError; + use crate::presentation::progress::ProgressReporterError; let errors: Vec = vec![ CreateSubcommandError::ConfigFileNotFound { @@ -270,6 +353,9 @@ mod tests { }, ), }, + CreateSubcommandError::ProgressReportingFailed { + source: ProgressReporterError::UserOutputMutexPoisoned, + }, ]; for error in errors { diff --git a/src/presentation/commands/create/handler.rs b/src/presentation/commands/create/handler.rs index ce81ba8c..7ad5cc7e 100644 --- a/src/presentation/commands/create/handler.rs +++ b/src/presentation/commands/create/handler.rs @@ -4,8 +4,10 @@ //! routing between different subcommands (environment creation or template generation). use std::path::Path; +use std::sync::{Arc, Mutex}; use crate::presentation::cli::commands::CreateAction; +use crate::presentation::user_output::UserOutput; use super::errors::CreateSubcommandError; use super::subcommands; @@ -18,6 +20,7 @@ use super::subcommands; /// /// * `action` - The create action to perform (environment creation or template generation) /// * `working_dir` - Root directory for environment data storage +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Returns /// @@ -30,14 +33,15 @@ use super::subcommands; pub fn handle_create_command( action: CreateAction, working_dir: &Path, + user_output: &Arc>, ) -> Result<(), CreateSubcommandError> { match action { CreateAction::Environment { env_file } => { - subcommands::handle_environment_creation(&env_file, working_dir) + subcommands::handle_environment_creation(&env_file, working_dir, user_output) } CreateAction::Template { output_path } => { let template_path = output_path.unwrap_or_else(CreateAction::default_template_path); - subcommands::handle_template_generation(&template_path) + subcommands::handle_template_generation(&template_path, user_output) } } } diff --git a/src/presentation/commands/create/mod.rs b/src/presentation/commands/create/mod.rs index 4dd0ee92..1c0a70ef 100644 --- a/src/presentation/commands/create/mod.rs +++ b/src/presentation/commands/create/mod.rs @@ -22,14 +22,17 @@ //! //! ```rust,no_run //! use std::path::{Path, PathBuf}; +//! use std::sync::{Arc, Mutex}; //! use torrust_tracker_deployer_lib::presentation::cli::commands::CreateAction; //! use torrust_tracker_deployer_lib::presentation::commands::create; +//! use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; //! //! let action = CreateAction::Environment { //! env_file: PathBuf::from("config/environment.json") //! }; +//! let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); //! -//! if let Err(e) = create::handle_create_command(action, Path::new(".")) { +//! if let Err(e) = create::handle_create_command(action, Path::new("."), &output) { //! eprintln!("Create failed: {e}"); //! eprintln!("\n{}", e.help()); //! } diff --git a/src/presentation/commands/create/subcommands/environment.rs b/src/presentation/commands/create/subcommands/environment.rs index 9914be27..230b9df0 100644 --- a/src/presentation/commands/create/subcommands/environment.rs +++ b/src/presentation/commands/create/subcommands/environment.rs @@ -4,10 +4,10 @@ //! deployment environments from configuration files. use std::path::Path; +use std::sync::{Arc, Mutex}; use crate::application::command_handlers::create::config::EnvironmentCreationConfig; use crate::domain::Environment; -use crate::presentation::commands::context::report_error; use crate::presentation::commands::factory::CommandHandlerFactory; use crate::presentation::progress::ProgressReporter; use crate::presentation::user_output::UserOutput; @@ -31,6 +31,7 @@ use super::super::errors::CreateSubcommandError; /// /// * `env_file` - Path to the environment configuration file (JSON format) /// * `working_dir` - Root directory for environment data storage +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Returns /// @@ -47,40 +48,40 @@ use super::super::errors::CreateSubcommandError; pub fn handle_environment_creation( env_file: &Path, working_dir: &Path, + user_output: &Arc>, ) -> Result<(), CreateSubcommandError> { let factory = CommandHandlerFactory::new(); - let ctx = factory.create_context(working_dir.to_path_buf()); + let ctx = factory.create_context(working_dir.to_path_buf(), user_output.clone()); // Create progress reporter for 3 main steps - let mut progress = ProgressReporter::new(ctx.into_output(), 3); + let mut progress = ProgressReporter::new(ctx.user_output().clone(), 3); // Step 1: Load configuration - progress.start_step("Loading configuration"); + progress.start_step("Loading configuration")?; let config = load_configuration(progress.output(), env_file)?; progress.complete_step(Some(&format!( "Configuration loaded: {}", config.environment.name - ))); + )))?; // Step 2: Initialize dependencies - progress.start_step("Initializing dependencies"); - let ctx = factory.create_context(working_dir.to_path_buf()); + progress.start_step("Initializing dependencies")?; let command_handler = factory.create_create_handler(&ctx); - progress.complete_step(None); + progress.complete_step(None)?; // Step 3: Execute create command (provision infrastructure) - progress.start_step("Creating environment"); + progress.start_step("Creating environment")?; let environment = execute_create_command(progress.output(), &command_handler, config)?; progress.complete_step(Some(&format!( "Instance created: {}", environment.instance_name().as_str() - ))); + )))?; // Complete with summary progress.complete(&format!( "Environment '{}' created successfully", environment.name().as_str() - )); + ))?; // Display final results display_creation_results(progress.output(), &environment); @@ -111,17 +112,24 @@ pub fn handle_environment_creation( /// - JSON parsing fails /// - Domain validation fails fn load_configuration( - output: &mut UserOutput, + user_output: &Arc>, env_file: &Path, ) -> Result { - output.progress(&format!( - "Loading configuration from '{}'...", - env_file.display() - )); + user_output + .lock() + .map_err(|_| CreateSubcommandError::UserOutputLockFailed)? + .progress(&format!( + "Loading configuration from '{}'...", + env_file.display() + )); let loader = ConfigLoader; + loader.load_from_file(env_file).inspect_err(|err| { - report_error(output, err); + // Attempt to log error, but don't fail if mutex is poisoned + if let Ok(mut output) = user_output.lock() { + output.error(&err.to_string()); + } }) } @@ -145,21 +153,30 @@ fn load_configuration( /// /// Returns an error if command execution fails (e.g., environment already exists). fn execute_create_command( - output: &mut UserOutput, + user_output: &Arc>, command_handler: &crate::application::command_handlers::CreateCommandHandler, config: EnvironmentCreationConfig, ) -> Result { - output.progress(&format!( - "Creating environment '{}'...", - config.environment.name - )); - - output.progress("Validating configuration and creating environment..."); + user_output + .lock() + .map_err(|_| CreateSubcommandError::UserOutputLockFailed)? + .progress(&format!( + "Creating environment '{}'...", + config.environment.name + )); + + user_output + .lock() + .map_err(|_| CreateSubcommandError::UserOutputLockFailed)? + .progress("Validating configuration and creating environment..."); #[allow(clippy::manual_inspect)] command_handler.execute(config).map_err(|source| { let error = CreateSubcommandError::CommandFailed { source }; - report_error(output, &error); + // Attempt to log error, but don't fail if mutex is poisoned + if let Ok(mut output) = user_output.lock() { + output.error(&error.to_string()); + } error }) } @@ -174,9 +191,25 @@ fn execute_create_command( /// /// # Arguments /// -/// * `output` - User output for result messages +/// * `user_output` - Shared user output for result messages /// * `environment` - The successfully created environment -fn display_creation_results(output: &mut UserOutput, environment: &Environment) { +/// +/// # Panics +/// +/// This function will panic if the `UserOutput` mutex is poisoned. Since this is +/// called after successful environment creation (when operation is complete), +/// a poisoned mutex indicates an irrecoverable state and panicking is acceptable. +/// +/// The panic message provides detailed context matching our error handling principles: +/// clear explanation of what happened, why it's critical, and that it indicates a bug. +fn display_creation_results(user_output: &Arc>, environment: &Environment) { + let mut output = user_output.lock().expect( + "CRITICAL: UserOutput mutex poisoned after successful environment creation. \ + This indicates a panic occurred in another thread while holding the output lock. \ + The environment was created successfully, but we cannot display the results. \ + This is a bug - please report it with full logs using --log-output file-and-stderr", + ); + output.success(&format!( "Environment '{}' created successfully", environment.name().as_str() @@ -186,10 +219,12 @@ fn display_creation_results(output: &mut UserOutput, environment: &Environment) "Instance name: {}", environment.instance_name().as_str() )); + output.result(&format!( "Data directory: {}", environment.data_dir().display() )); + output.result(&format!( "Build directory: {}", environment.build_dir().display() @@ -199,9 +234,15 @@ fn display_creation_results(output: &mut UserOutput, environment: &Environment) #[cfg(test)] mod tests { use super::*; + use crate::presentation::user_output::VerbosityLevel; use std::fs; use tempfile::TempDir; + /// Test helper to create a test user output + fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) + } + #[test] fn it_should_create_environment_from_valid_config() { let temp_dir = TempDir::new().unwrap(); @@ -227,7 +268,8 @@ mod tests { fs::write(&config_path, config_json).unwrap(); let working_dir = temp_dir.path(); - let result = handle_environment_creation(&config_path, working_dir); + let user_output = create_test_user_output(); + let result = handle_environment_creation(&config_path, working_dir, &user_output); assert!( result.is_ok(), @@ -250,8 +292,9 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let config_path = temp_dir.path().join("nonexistent.json"); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); - let result = handle_environment_creation(&config_path, working_dir); + let result = handle_environment_creation(&config_path, working_dir, &user_output); assert!(result.is_err()); match result.unwrap_err() { @@ -271,7 +314,8 @@ mod tests { fs::write(&config_path, r#"{"invalid json"#).unwrap(); let working_dir = temp_dir.path(); - let result = handle_environment_creation(&config_path, working_dir); + let user_output = create_test_user_output(); + let result = handle_environment_creation(&config_path, working_dir, &user_output); assert!(result.is_err()); match result.unwrap_err() { @@ -306,13 +350,14 @@ mod tests { fs::write(&config_path, config_json).unwrap(); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); // Create environment first time - let result1 = handle_environment_creation(&config_path, working_dir); + let result1 = handle_environment_creation(&config_path, working_dir, &user_output); assert!(result1.is_ok(), "First create should succeed"); // Try to create same environment again - let result2 = handle_environment_creation(&config_path, working_dir); + let result2 = handle_environment_creation(&config_path, working_dir, &user_output); assert!(result2.is_err(), "Second create should fail"); match result2.unwrap_err() { @@ -349,7 +394,8 @@ mod tests { ); fs::write(&config_path, config_json).unwrap(); - let result = handle_environment_creation(&config_path, &custom_working_dir); + let user_output = create_test_user_output(); + let result = handle_environment_creation(&config_path, &custom_working_dir, &user_output); assert!(result.is_ok(), "Should create in custom working dir"); @@ -369,7 +415,6 @@ mod tests { mod load_configuration_tests { use super::*; - use crate::presentation::user_output::{UserOutput, VerbosityLevel}; #[test] fn it_should_load_valid_configuration() { @@ -393,8 +438,8 @@ mod tests { ); fs::write(&config_path, config_json).unwrap(); - let mut output = UserOutput::new(VerbosityLevel::Quiet); - let result = load_configuration(&mut output, &config_path); + let user_output = create_test_user_output(); + let result = load_configuration(&user_output, &config_path); assert!(result.is_ok(), "Should load valid configuration"); let config = result.unwrap(); @@ -406,8 +451,8 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let config_path = temp_dir.path().join("missing.json"); - let mut output = UserOutput::new(VerbosityLevel::Quiet); - let result = load_configuration(&mut output, &config_path); + let user_output = create_test_user_output(); + let result = load_configuration(&user_output, &config_path); assert!(result.is_err()); match result.unwrap_err() { @@ -424,8 +469,8 @@ mod tests { let config_path = temp_dir.path().join("invalid.json"); fs::write(&config_path, r#"{"broken json"#).unwrap(); - let mut output = UserOutput::new(VerbosityLevel::Quiet); - let result = load_configuration(&mut output, &config_path); + let user_output = create_test_user_output(); + let result = load_configuration(&user_output, &config_path); assert!(result.is_err()); match result.unwrap_err() { @@ -439,7 +484,6 @@ mod tests { mod execute_create_command_tests { use super::*; - use crate::presentation::user_output::{UserOutput, VerbosityLevel}; #[test] fn it_should_execute_command_successfully() { @@ -463,14 +507,14 @@ mod tests { ); fs::write(&config_path, config_json).unwrap(); - let mut output = UserOutput::new(VerbosityLevel::Quiet); + let user_output = create_test_user_output(); let loader = ConfigLoader; let config = loader.load_from_file(&config_path).unwrap(); let factory = CommandHandlerFactory::new(); - let ctx = factory.create_context(temp_dir.path().to_path_buf()); + let ctx = factory.create_context(temp_dir.path().to_path_buf(), user_output.clone()); let command_handler = factory.create_create_handler(&ctx); - let result = execute_create_command(&mut output, &command_handler, config); + let result = execute_create_command(&user_output, &command_handler, config); assert!(result.is_ok(), "Should execute command successfully"); let environment = result.unwrap(); @@ -499,21 +543,21 @@ mod tests { ); fs::write(&config_path, config_json).unwrap(); - let mut output = UserOutput::new(VerbosityLevel::Quiet); + let user_output = create_test_user_output(); let loader = ConfigLoader; let config = loader.load_from_file(&config_path).unwrap(); let factory = CommandHandlerFactory::new(); - let ctx = factory.create_context(temp_dir.path().to_path_buf()); + let ctx = factory.create_context(temp_dir.path().to_path_buf(), user_output.clone()); // Create environment first time let command_handler1 = factory.create_create_handler(&ctx); - let result1 = execute_create_command(&mut output, &command_handler1, config.clone()); + let result1 = execute_create_command(&user_output, &command_handler1, config.clone()); assert!(result1.is_ok(), "First execution should succeed"); // Try to create same environment again let command_handler2 = factory.create_create_handler(&ctx); - let result2 = execute_create_command(&mut output, &command_handler2, config); + let result2 = execute_create_command(&user_output, &command_handler2, config); assert!(result2.is_err(), "Second execution should fail"); match result2.unwrap_err() { @@ -553,14 +597,14 @@ mod tests { fs::write(&config_path, config_json).unwrap(); // Create environment + let user_output = create_test_user_output(); let factory = CommandHandlerFactory::new(); - let ctx = factory.create_context(temp_dir.path().to_path_buf()); + let ctx = factory.create_context(temp_dir.path().to_path_buf(), user_output.clone()); let loader = ConfigLoader; let config = loader.load_from_file(&config_path).unwrap(); - let mut quiet_output = UserOutput::new(VerbosityLevel::Quiet); let command_handler = factory.create_create_handler(&ctx); let environment = - execute_create_command(&mut quiet_output, &command_handler, config).unwrap(); + execute_create_command(&user_output, &command_handler, config).unwrap(); // Test display function with custom output let stderr_buf = Vec::new(); @@ -568,11 +612,12 @@ mod tests { let stdout_buf = Vec::new(); let stdout_writer = Box::new(Cursor::new(stdout_buf)); - let mut output = + let output = UserOutput::with_writers(VerbosityLevel::Normal, stdout_writer, stderr_writer); + let display_output = Arc::new(Mutex::new(output)); // This should not panic and should output messages - display_creation_results(&mut output, &environment); + display_creation_results(&display_output, &environment); // Note: We can't easily verify the exact output without refactoring UserOutput // to expose the buffers, but the important thing is it doesn't panic diff --git a/src/presentation/commands/create/subcommands/template.rs b/src/presentation/commands/create/subcommands/template.rs index 14d98c19..7b86296a 100644 --- a/src/presentation/commands/create/subcommands/template.rs +++ b/src/presentation/commands/create/subcommands/template.rs @@ -4,9 +4,9 @@ //! configuration file templates with placeholder values. use std::path::Path; +use std::sync::{Arc, Mutex}; use crate::application::command_handlers::create::config::EnvironmentCreationConfig; -use crate::presentation::commands::constants::DEFAULT_VERBOSITY; use crate::presentation::user_output::UserOutput; use super::super::errors::CreateSubcommandError; @@ -19,6 +19,7 @@ use super::super::errors::CreateSubcommandError; /// # Arguments /// /// * `output_path` - Path where the template file should be created +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Returns /// @@ -33,9 +34,14 @@ use super::super::errors::CreateSubcommandError; /// Panics if the tokio runtime cannot be created. This is a critical system /// failure that prevents any async operations from running. #[allow(clippy::result_large_err)] // Error contains detailed context for user guidance -pub fn handle_template_generation(output_path: &Path) -> Result<(), CreateSubcommandError> { - // Create user output for progress messages - let mut output = UserOutput::new(DEFAULT_VERBOSITY); +pub fn handle_template_generation( + output_path: &Path, + user_output: &Arc>, +) -> Result<(), CreateSubcommandError> { + // Lock user output for progress messages + let mut output = user_output + .lock() + .map_err(|_| CreateSubcommandError::UserOutputLockFailed)?; output.progress("Generating configuration template..."); diff --git a/src/presentation/commands/create/tests/integration.rs b/src/presentation/commands/create/tests/integration.rs index ec146354..fc7dce18 100644 --- a/src/presentation/commands/create/tests/integration.rs +++ b/src/presentation/commands/create/tests/integration.rs @@ -3,12 +3,20 @@ //! This module tests the complete create command workflow including //! configuration loading, validation, and command execution. +use std::sync::{Arc, Mutex}; + use crate::presentation::cli::CreateAction; use crate::presentation::commands::create; use crate::presentation::commands::tests::{ create_config_with_invalid_name, create_config_with_missing_keys, create_invalid_json_config, create_valid_config, TestContext, }; +use crate::presentation::user_output::{UserOutput, VerbosityLevel}; + +/// Helper to create test `UserOutput` +fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) +} /// Helper function to call the environment creation handler fn handle_environment_creation( @@ -18,7 +26,8 @@ fn handle_environment_creation( let action = CreateAction::Environment { env_file: config_path.to_path_buf(), }; - create::handle_create_command(action, working_dir) + let user_output = create_test_user_output(); + create::handle_create_command(action, working_dir, &user_output) } #[test] diff --git a/src/presentation/commands/create/tests/template.rs b/src/presentation/commands/create/tests/template.rs index c5a51c14..68fe0277 100644 --- a/src/presentation/commands/create/tests/template.rs +++ b/src/presentation/commands/create/tests/template.rs @@ -1,8 +1,16 @@ //! Integration Tests for Template Generation +use std::sync::{Arc, Mutex}; + use crate::presentation::cli::CreateAction; use crate::presentation::commands::create; use crate::presentation::commands::tests::TestContext; +use crate::presentation::user_output::{UserOutput, VerbosityLevel}; + +/// Helper to create test `UserOutput` +fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) +} #[test] fn it_should_generate_template_with_default_path() { @@ -13,8 +21,9 @@ fn it_should_generate_template_with_default_path() { std::env::set_current_dir(test_context.working_dir()).unwrap(); let action = CreateAction::Template { output_path: None }; + let user_output = create_test_user_output(); - let result = create::handle_create_command(action, test_context.working_dir()); + let result = create::handle_create_command(action, test_context.working_dir(), &user_output); // Restore original directory std::env::set_current_dir(original_dir).unwrap(); @@ -56,8 +65,9 @@ fn it_should_generate_template_with_custom_path() { let action = CreateAction::Template { output_path: Some(custom_path.clone()), }; + let user_output = create_test_user_output(); - let result = create::handle_create_command(action, test_context.working_dir()); + let result = create::handle_create_command(action, test_context.working_dir(), &user_output); assert!(result.is_ok(), "Template generation should succeed"); @@ -80,8 +90,9 @@ fn it_should_generate_valid_json_template() { let action = CreateAction::Template { output_path: Some(template_path.clone()), }; + let user_output = create_test_user_output(); - create::handle_create_command(action, test_context.working_dir()).unwrap(); + create::handle_create_command(action, test_context.working_dir(), &user_output).unwrap(); // Read and parse the generated template let file_content = std::fs::read_to_string(&template_path).unwrap(); @@ -124,8 +135,9 @@ fn it_should_create_parent_directories() { let action = CreateAction::Template { output_path: Some(deep_path.clone()), }; + let user_output = create_test_user_output(); - let result = create::handle_create_command(action, test_context.working_dir()); + let result = create::handle_create_command(action, test_context.working_dir(), &user_output); assert!(result.is_ok(), "Should create parent directories"); assert!( diff --git a/src/presentation/commands/destroy/errors.rs b/src/presentation/commands/destroy/errors.rs index 38872fc8..0c75a008 100644 --- a/src/presentation/commands/destroy/errors.rs +++ b/src/presentation/commands/destroy/errors.rs @@ -8,6 +8,7 @@ use thiserror::Error; use crate::application::command_handlers::destroy::DestroyCommandHandlerError; use crate::domain::environment::name::EnvironmentNameError; +use crate::presentation::progress::ProgressReporterError; /// Destroy command specific errors /// @@ -64,6 +65,40 @@ Tip: Check logs and try running with --log-output file-and-stderr for more detai #[source] source: DestroyCommandHandlerError, }, + + // ===== Internal Errors ===== + /// `UserOutput` mutex poisoned + /// + /// The shared `UserOutput` mutex was poisoned by a panic in another thread. + /// This indicates a critical internal error that should be reported. + #[error( + "Internal error: UserOutput mutex was poisoned +Tip: This is a critical bug - please report it with full logs using --log-output file-and-stderr" + )] + UserOutputMutexPoisoned, + + /// Progress reporting failed + /// + /// Failed to report progress to the user due to an internal error. + /// This indicates a critical internal error. + #[error( + "Failed to report progress: {source} +Tip: This is a critical bug - please report it with full logs using --log-output file-and-stderr" + )] + ProgressReportingFailed { + #[source] + source: ProgressReporterError, + }, +} + +// ============================================================================ +// ERROR CONVERSIONS +// ============================================================================ + +impl From for DestroySubcommandError { + fn from(source: ProgressReporterError) -> Self { + Self::ProgressReportingFailed { source } + } } impl DestroySubcommandError { @@ -75,11 +110,14 @@ impl DestroySubcommandError { /// # Example /// /// ```rust - /// use torrust_tracker_deployer_lib::presentation::commands::destroy; /// use std::path::Path; + /// use std::sync::{Arc, Mutex}; + /// use torrust_tracker_deployer_lib::presentation::commands::destroy; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// # fn main() -> Result<(), Box> { - /// if let Err(e) = destroy::handle_destroy_command("test-env", Path::new(".")) { + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// if let Err(e) = destroy::handle_destroy_command("test-env", Path::new("."), &output) { /// eprintln!("Error: {e}"); /// eprintln!("\nTroubleshooting:\n{}", e.help()); /// } @@ -87,6 +125,7 @@ impl DestroySubcommandError { /// # } /// ``` #[must_use] + #[allow(clippy::too_many_lines)] // Help text is comprehensive for user guidance pub fn help(&self) -> &'static str { match self { Self::InvalidEnvironmentName { .. } => { @@ -193,6 +232,58 @@ For persistent issues, check the infrastructure documentation." If the problem persists, check system logs and contact administrator." } + + Self::UserOutputMutexPoisoned => { + "UserOutput Mutex Poisoned - Critical Internal Error: + +This is a critical bug that indicates a panic occurred while holding the UserOutput mutex lock. +This should never happen in normal operation. + +1. Immediate actions: + - Save all relevant logs and error messages + - Note what operation was being performed + - Record the environment state + +2. Report the issue: + - This is a bug that needs to be reported + - Include full logs: --log-output file-and-stderr + - Provide steps to reproduce if possible + - Include system information (OS, versions) + +3. Workaround: + - Restart the application + - Try the operation again + - Check for resource exhaustion (memory, threads) + +This error indicates a serious bug in the application's error handling or concurrency management. +Please report it to the development team with full details." + } + + Self::ProgressReportingFailed { .. } => { + "Progress Reporting Failed - Critical Internal Error: + +This is a critical bug that indicates progress reporting to the user failed. +This should never happen in normal operation. + +1. Immediate actions: + - Save all relevant logs and error messages + - Note what operation was being performed + - Record the environment state + +2. Report the issue: + - This is a bug that needs to be reported + - Include full logs: --log-output file-and-stderr + - Provide steps to reproduce if possible + - Include system information (OS, versions) + +3. Workaround: + - Restart the application + - Try the operation again + - Check for resource exhaustion (memory, threads) + +This error indicates a serious bug in the application's progress reporting system. +Please report it to the development team with full details." + } } } } @@ -257,6 +348,10 @@ mod tests { data_dir: "/tmp".to_string(), reason: "permission denied".to_string(), }, + DestroySubcommandError::UserOutputMutexPoisoned, + DestroySubcommandError::ProgressReportingFailed { + source: ProgressReporterError::UserOutputMutexPoisoned, + }, ]; for error in errors { diff --git a/src/presentation/commands/destroy/handler.rs b/src/presentation/commands/destroy/handler.rs index 970775df..c24299a8 100644 --- a/src/presentation/commands/destroy/handler.rs +++ b/src/presentation/commands/destroy/handler.rs @@ -3,47 +3,215 @@ //! This module handles the destroy command execution at the presentation layer, //! including environment validation, repository initialization, and user interaction. +use std::path::PathBuf; +use std::sync::{Arc, Mutex}; + +use crate::application::command_handlers::DestroyCommandHandler; use crate::domain::environment::name::EnvironmentName; -use crate::presentation::commands::context::report_error; +use crate::domain::environment::state::Destroyed; +use crate::domain::Environment; +use crate::presentation::commands::context::CommandContext; use crate::presentation::commands::factory::CommandHandlerFactory; use crate::presentation::progress::ProgressReporter; +use crate::presentation::user_output::UserOutput; use super::errors::DestroySubcommandError; -/// Handle the destroy command +// ============================================================================ +// CONSTANTS +// ============================================================================ + +/// Number of main steps in the destroy workflow +const DESTROY_WORKFLOW_STEPS: usize = 3; + +// ============================================================================ +// PRESENTATION LAYER CONTROLLER +// ============================================================================ + +/// Presentation layer controller for destroy command workflow +/// +/// Coordinates user interaction, progress reporting, and input validation +/// before delegating to the application layer `DestroyCommandHandler`. /// -/// This function orchestrates the environment destruction workflow with progress reporting: -/// 1. Validating the environment name -/// 2. Tearing down infrastructure -/// 3. Cleaning up resources +/// # Responsibilities /// -/// Each step is tracked and timed using `ProgressReporter` for clear user feedback. +/// - Validate user input (environment name format) +/// - Show progress updates to the user +/// - Format success/error messages for display +/// - Delegate business logic to application layer +/// +/// # Architecture +/// +/// This controller sits in the presentation layer and handles all user-facing +/// concerns. It delegates actual business logic to the application layer's +/// `DestroyCommandHandler`, maintaining clear separation of concerns. +pub struct DestroyCommandController { + factory: CommandHandlerFactory, + ctx: CommandContext, + progress: ProgressReporter, +} + +impl DestroyCommandController { + /// Create a new destroy command controller + /// + /// # Arguments + /// + /// * `working_dir` - Root directory for environment data storage + /// * `user_output` - Shared user output service for consistent formatting + pub fn new(working_dir: PathBuf, user_output: Arc>) -> Self { + let factory = CommandHandlerFactory::new(); + let ctx = factory.create_context(working_dir, user_output.clone()); + let progress = ProgressReporter::new(user_output, DESTROY_WORKFLOW_STEPS); + + Self { + factory, + ctx, + progress, + } + } + + /// Execute the complete destroy workflow + /// + /// Orchestrates all steps of the destroy command: + /// 1. Validate environment name + /// 2. Initialize dependencies + /// 3. Tear down infrastructure + /// 4. Complete with success message + /// + /// # Arguments + /// + /// * `environment_name` - The name of the environment to destroy + /// + /// # Errors + /// + /// Returns an error if: + /// - Environment name is invalid (format validation fails) + /// - Environment cannot be loaded from repository + /// - Infrastructure teardown fails + /// - Progress reporting encounters a poisoned mutex + /// + /// # Returns + /// + /// Returns `Ok(())` on success, or a `DestroySubcommandError` if any step fails. + #[allow(clippy::result_large_err)] + pub fn execute(&mut self, environment_name: &str) -> Result<(), DestroySubcommandError> { + let env_name = self.validate_environment_name(environment_name)?; + let handler = self.initialize_dependencies()?; + let _destroyed = self.tear_down_infrastructure(&handler, &env_name)?; + self.complete_workflow(environment_name)?; + Ok(()) + } + + /// Validate the environment name format + /// + /// Shows progress to user and validates that the environment name + /// meets domain requirements (1-63 chars, alphanumeric + hyphens). + #[allow(clippy::result_large_err)] + fn validate_environment_name( + &mut self, + name: &str, + ) -> Result { + self.progress.start_step("Validating environment")?; + + let env_name = EnvironmentName::new(name.to_string()).map_err(|source| { + DestroySubcommandError::InvalidEnvironmentName { + name: name.to_string(), + source, + } + })?; + + self.progress + .complete_step(Some(&format!("Environment name validated: {name}")))?; + + Ok(env_name) + } + + /// Initialize application layer dependencies + /// + /// Creates the application layer command handler with all required + /// dependencies (repository, clock, etc.). + #[allow(clippy::result_large_err)] + fn initialize_dependencies(&mut self) -> Result { + self.progress.start_step("Initializing dependencies")?; + let handler = self.factory.create_destroy_handler(&self.ctx); + self.progress.complete_step(None)?; + Ok(handler) + } + + /// Execute infrastructure teardown via application layer + /// + /// Delegates to the application layer `DestroyCommandHandler` to + /// orchestrate the actual infrastructure destruction workflow. + #[allow(clippy::result_large_err)] + fn tear_down_infrastructure( + &mut self, + handler: &DestroyCommandHandler, + env_name: &EnvironmentName, + ) -> Result, DestroySubcommandError> { + self.progress.start_step("Tearing down infrastructure")?; + + let destroyed = handler.execute(env_name).map_err(|source| { + DestroySubcommandError::DestroyOperationFailed { + name: env_name.to_string(), + source, + } + })?; + + self.progress + .complete_step(Some("Infrastructure torn down"))?; + Ok(destroyed) + } + + /// Complete the workflow with success message + /// + /// Shows final success message to the user with workflow summary. + #[allow(clippy::result_large_err)] + fn complete_workflow(&mut self, name: &str) -> Result<(), DestroySubcommandError> { + self.progress + .complete(&format!("Environment '{name}' destroyed successfully"))?; + Ok(()) + } +} + +// ============================================================================ +// PUBLIC ENTRY POINT +// ============================================================================ + +/// Handle the destroy command +/// +/// This is a thin wrapper over `DestroyCommandController` that serves as +/// the public entry point for the destroy command. /// /// # Arguments /// /// * `environment_name` - The name of the environment to destroy /// * `working_dir` - Root directory for environment data storage -/// -/// # Returns -/// -/// Returns `Ok(())` on success, or a `DestroySubcommandError` if: -/// - Environment name is invalid -/// - Environment cannot be loaded -/// - Destruction fails +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Errors /// -/// This function will return an error if the environment name is invalid, -/// the environment cannot be loaded, or the destruction process fails. +/// Returns an error if: +/// - Environment name is invalid (format validation fails) +/// - Environment cannot be loaded from repository +/// - Infrastructure teardown fails +/// - Progress reporting encounters a poisoned mutex +/// /// All errors include detailed context and actionable troubleshooting guidance. /// +/// # Returns +/// +/// Returns `Ok(())` on success, or a `DestroySubcommandError` on failure. +/// /// # Example /// /// ```rust -/// use torrust_tracker_deployer_lib::presentation::commands::destroy; /// use std::path::Path; +/// use std::sync::{Arc, Mutex}; +/// use torrust_tracker_deployer_lib::presentation::commands::destroy; +/// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// -/// if let Err(e) = destroy::handle_destroy_command("test-env", Path::new(".")) { +/// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); +/// if let Err(e) = destroy::handle_destroy_command("test-env", Path::new("."), &user_output) { /// eprintln!("Destroy failed: {e}"); /// eprintln!("Help: {}", e.help()); /// } @@ -52,67 +220,36 @@ use super::errors::DestroySubcommandError; pub fn handle_destroy_command( environment_name: &str, working_dir: &std::path::Path, + user_output: &Arc>, ) -> Result<(), DestroySubcommandError> { - // Create factory and context with all shared dependencies - let factory = CommandHandlerFactory::new(); - let ctx = factory.create_context(working_dir.to_path_buf()); - - // Create progress reporter for 3 main steps - let mut progress = ProgressReporter::new(ctx.into_output(), 3); - - // Step 1: Validate environment name - progress.start_step("Validating environment"); - let env_name = EnvironmentName::new(environment_name.to_string()).map_err(|source| { - let error = DestroySubcommandError::InvalidEnvironmentName { - name: environment_name.to_string(), - source, - }; - report_error(progress.output(), &error); - error - })?; - progress.complete_step(Some(&format!( - "Environment name validated: {environment_name}" - ))); - - // Step 2: Initialize dependencies - progress.start_step("Initializing dependencies"); - let ctx = factory.create_context(working_dir.to_path_buf()); - let command_handler = factory.create_destroy_handler(&ctx); - progress.complete_step(None); - - // Step 3: Execute destroy command (tear down infrastructure) - progress.start_step("Tearing down infrastructure"); - let _destroyed_env = command_handler.execute(&env_name).map_err(|source| { - let error = DestroySubcommandError::DestroyOperationFailed { - name: environment_name.to_string(), - source, - }; - report_error(progress.output(), &error); - error - })?; - progress.complete_step(Some("Infrastructure torn down")); - - // Complete with summary - progress.complete(&format!( - "Environment '{environment_name}' destroyed successfully" - )); - - Ok(()) + DestroyCommandController::new(working_dir.to_path_buf(), user_output.clone()) + .execute(environment_name) } +// ============================================================================ +// TESTS +// ============================================================================ + #[cfg(test)] mod tests { use super::*; + use crate::presentation::user_output::VerbosityLevel; use std::fs; use tempfile::TempDir; + /// Test helper to create a test user output + fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) + } + #[test] fn it_should_return_error_for_invalid_environment_name() { let temp_dir = TempDir::new().unwrap(); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); // Test with invalid environment name (contains underscore) - let result = handle_destroy_command("invalid_name", working_dir); + let result = handle_destroy_command("invalid_name", working_dir, &user_output); assert!(result.is_err()); match result.unwrap_err() { @@ -127,8 +264,9 @@ mod tests { fn it_should_return_error_for_empty_environment_name() { let temp_dir = TempDir::new().unwrap(); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); - let result = handle_destroy_command("", working_dir); + let result = handle_destroy_command("", working_dir, &user_output); assert!(result.is_err()); match result.unwrap_err() { @@ -143,9 +281,10 @@ mod tests { fn it_should_return_error_for_nonexistent_environment() { let temp_dir = TempDir::new().unwrap(); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); // Try to destroy an environment that doesn't exist - let result = handle_destroy_command("nonexistent-env", working_dir); + let result = handle_destroy_command("nonexistent-env", working_dir, &user_output); assert!(result.is_err()); // Should get DestroyOperationFailed because environment doesn't exist @@ -161,6 +300,7 @@ mod tests { fn it_should_accept_valid_environment_name() { let temp_dir = TempDir::new().unwrap(); let working_dir = temp_dir.path(); + let user_output = create_test_user_output(); // Create a mock environment directory to test validation let env_dir = working_dir.join("test-env"); @@ -168,7 +308,7 @@ mod tests { // Valid environment name should pass validation, but will fail // at destroy operation since we don't have a real environment setup - let result = handle_destroy_command("test-env", working_dir); + let result = handle_destroy_command("test-env", working_dir, &user_output); // Should fail at operation, not at name validation if let Err(DestroySubcommandError::InvalidEnvironmentName { .. }) = result { diff --git a/src/presentation/commands/destroy/mod.rs b/src/presentation/commands/destroy/mod.rs index 8ed516ed..687f8487 100644 --- a/src/presentation/commands/destroy/mod.rs +++ b/src/presentation/commands/destroy/mod.rs @@ -18,9 +18,12 @@ //! //! ```rust,no_run //! use std::path::Path; +//! use std::sync::{Arc, Mutex}; //! use torrust_tracker_deployer_lib::presentation::commands::destroy; +//! use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; //! -//! if let Err(e) = destroy::handle_destroy_command("test-env", Path::new(".")) { +//! let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); +//! if let Err(e) = destroy::handle_destroy_command("test-env", Path::new("."), &output) { //! eprintln!("Destroy failed: {e}"); //! eprintln!("\n{}", e.help()); //! } diff --git a/src/presentation/commands/destroy/tests/integration.rs b/src/presentation/commands/destroy/tests/integration.rs index 339df844..0a4f9ee5 100644 --- a/src/presentation/commands/destroy/tests/integration.rs +++ b/src/presentation/commands/destroy/tests/integration.rs @@ -3,9 +3,17 @@ //! These tests verify the destroy command behavior at the presentation layer, //! including user interaction, error handling, and command orchestration. +use std::fs; +use std::sync::{Arc, Mutex}; + use crate::presentation::commands::destroy::{handle_destroy_command, DestroySubcommandError}; use crate::presentation::commands::tests::TestContext; -use std::fs; +use crate::presentation::user_output::{UserOutput, VerbosityLevel}; + +/// Helper to create test `UserOutput` +fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) +} #[test] fn it_should_reject_invalid_environment_names() { @@ -19,7 +27,8 @@ fn it_should_reject_invalid_environment_names() { ]; for name in invalid_names { - let result = handle_destroy_command(name, context.working_dir()); + let user_output = create_test_user_output(); + let result = handle_destroy_command(name, context.working_dir(), &user_output); assert!( result.is_err(), "Should reject invalid environment name: {name}", @@ -35,7 +44,8 @@ fn it_should_reject_invalid_environment_names() { // Test too long name separately due to String allocation // The actual max length depends on domain validation rules let too_long_name = "a".repeat(64); - let result = handle_destroy_command(&too_long_name, context.working_dir()); + let user_output = create_test_user_output(); + let result = handle_destroy_command(&too_long_name, context.working_dir(), &user_output); assert!(result.is_err(), "Should get some error for 64-char name"); // Accept either InvalidEnvironmentName OR DestroyOperationFailed // The domain layer determines what length is valid @@ -54,7 +64,8 @@ fn it_should_accept_valid_environment_names() { ]; for name in valid_names { - let result = handle_destroy_command(name, context.working_dir()); + let user_output = create_test_user_output(); + let result = handle_destroy_command(name, context.working_dir(), &user_output); // Will fail at operation since environment doesn't exist, // but should NOT fail at name validation @@ -66,7 +77,8 @@ fn it_should_accept_valid_environment_names() { // Test max length separately due to String allocation let max_length_name = "a".repeat(63); - let result = handle_destroy_command(&max_length_name, context.working_dir()); + let user_output = create_test_user_output(); + let result = handle_destroy_command(&max_length_name, context.working_dir(), &user_output); if let Err(DestroySubcommandError::InvalidEnvironmentName { .. }) = result { panic!("Should not reject valid 63-char environment name"); } @@ -76,8 +88,9 @@ fn it_should_accept_valid_environment_names() { #[test] fn it_should_fail_for_nonexistent_environment() { let context = TestContext::new(); + let user_output = create_test_user_output(); - let result = handle_destroy_command("nonexistent-env", context.working_dir()); + let result = handle_destroy_command("nonexistent-env", context.working_dir(), &user_output); assert!(result.is_err()); match result.unwrap_err() { @@ -92,7 +105,11 @@ fn it_should_fail_for_nonexistent_environment() { fn it_should_provide_help_for_errors() { let context = TestContext::new(); - let result = handle_destroy_command("invalid_name", context.working_dir()); + let result = handle_destroy_command( + "invalid_name", + context.working_dir(), + &context.user_output(), + ); assert!(result.is_err()); let error = result.unwrap_err(); @@ -112,7 +129,7 @@ fn it_should_work_with_custom_working_directory() { fs::create_dir(&custom_working_dir).unwrap(); // Try to destroy from custom directory - let result = handle_destroy_command("test-env", &custom_working_dir); + let result = handle_destroy_command("test-env", &custom_working_dir, &context.user_output()); // Should fail at operation (environment doesn't exist) but not at path validation assert!(result.is_err()); diff --git a/src/presentation/commands/factory.rs b/src/presentation/commands/factory.rs index ea54343d..2db34b5b 100644 --- a/src/presentation/commands/factory.rs +++ b/src/presentation/commands/factory.rs @@ -32,14 +32,17 @@ //! //! ```rust //! use std::path::Path; +//! use std::sync::{Arc, Mutex}; //! use torrust_tracker_deployer_lib::presentation::commands::factory::CommandHandlerFactory; +//! use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; //! //! fn handle_command(working_dir: &Path) -> Result<(), Box> { //! // Create factory with default configuration //! let factory = CommandHandlerFactory::new(); +//! let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); //! //! // Create command context -//! let context = factory.create_context(working_dir.to_path_buf()); +//! let context = factory.create_context(working_dir.to_path_buf(), output); //! //! // Create command handler //! let handler = factory.create_create_handler(&context); @@ -50,9 +53,11 @@ //! ``` use std::path::PathBuf; +use std::sync::{Arc, Mutex}; use crate::application::command_handlers::{CreateCommandHandler, DestroyCommandHandler}; use crate::infrastructure::persistence::repository_factory::RepositoryFactory; +use crate::presentation::user_output::UserOutput; use super::constants::DEFAULT_LOCK_TIMEOUT; use super::context::CommandContext; @@ -70,10 +75,13 @@ use super::context::CommandContext; /// /// ```rust /// use std::path::PathBuf; +/// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::factory::CommandHandlerFactory; +/// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let factory = CommandHandlerFactory::new(); -/// let context = factory.create_context(PathBuf::from(".")); +/// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); +/// let context = factory.create_context(PathBuf::from("."), output); /// let handler = factory.create_create_handler(&context); /// ``` pub struct CommandHandlerFactory { @@ -119,14 +127,21 @@ impl CommandHandlerFactory { /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::factory::CommandHandlerFactory; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let factory = CommandHandlerFactory::new(); - /// let context = factory.create_context(PathBuf::from("./data")); + /// let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let context = factory.create_context(PathBuf::from("./data"), user_output); /// ``` #[must_use] - pub fn create_context(&self, working_dir: PathBuf) -> CommandContext { - CommandContext::new_with_factory(&self.repository_factory, working_dir) + pub fn create_context( + &self, + working_dir: PathBuf, + user_output: Arc>, + ) -> CommandContext { + CommandContext::new_with_factory(&self.repository_factory, working_dir, user_output) } /// Create a create command handler @@ -145,10 +160,13 @@ impl CommandHandlerFactory { /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::factory::CommandHandlerFactory; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let factory = CommandHandlerFactory::new(); - /// let context = factory.create_context(PathBuf::from(".")); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let context = factory.create_context(PathBuf::from("."), output); /// let handler = factory.create_create_handler(&context); /// ``` #[must_use] @@ -172,10 +190,13 @@ impl CommandHandlerFactory { /// /// ```rust /// use std::path::PathBuf; + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::commands::factory::CommandHandlerFactory; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let factory = CommandHandlerFactory::new(); - /// let context = factory.create_context(PathBuf::from(".")); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let context = factory.create_context(PathBuf::from("."), output); /// let handler = factory.create_destroy_handler(&context); /// ``` #[must_use] @@ -221,8 +242,31 @@ impl CommandHandlerFactory { #[cfg(test)] mod tests { use super::*; + use crate::presentation::user_output::VerbosityLevel; use tempfile::TempDir; + /// Test helper to create a test user output + fn create_test_user_output() -> Arc> { + Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))) + } + + /// Test helper to create a test setup with factory, temp directory, and user output + /// + /// Returns a tuple of (`CommandHandlerFactory`, `TempDir`, `PathBuf`, `Arc>`) + /// The `TempDir` must be kept alive for the duration of the test. + fn create_test_setup() -> ( + CommandHandlerFactory, + TempDir, + PathBuf, + Arc>, + ) { + let factory = CommandHandlerFactory::new(); + let temp_dir = TempDir::new().unwrap(); + let working_dir = temp_dir.path().to_path_buf(); + let user_output = create_test_user_output(); + (factory, temp_dir, working_dir, user_output) + } + #[test] fn it_should_create_factory_with_default_configuration() { let factory = CommandHandlerFactory::new(); @@ -243,11 +287,9 @@ mod tests { #[test] fn it_should_create_context_with_factory() { - let factory = CommandHandlerFactory::new(); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (factory, _temp_dir, working_dir, user_output) = create_test_setup(); - let context = factory.create_context(working_dir); + let context = factory.create_context(working_dir, user_output); // Verify context is created with dependencies let _ = context.repository(); @@ -256,11 +298,9 @@ mod tests { #[test] fn it_should_create_create_handler() { - let factory = CommandHandlerFactory::new(); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (factory, _temp_dir, working_dir, user_output) = create_test_setup(); - let context = factory.create_context(working_dir); + let context = factory.create_context(working_dir, user_output); let _handler = factory.create_create_handler(&context); // Verify handler is created (basic structure test) @@ -268,11 +308,9 @@ mod tests { #[test] fn it_should_create_destroy_handler() { - let factory = CommandHandlerFactory::new(); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (factory, _temp_dir, working_dir, user_output) = create_test_setup(); - let context = factory.create_context(working_dir); + let context = factory.create_context(working_dir, user_output); let _handler = factory.create_destroy_handler(&context); // Verify handler is created (basic structure test) @@ -280,13 +318,11 @@ mod tests { #[test] fn it_should_create_multiple_contexts_from_same_factory() { - let factory = CommandHandlerFactory::new(); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (factory, _temp_dir, working_dir, _user_output) = create_test_setup(); // Should be able to create multiple contexts - let context1 = factory.create_context(working_dir.clone()); - let context2 = factory.create_context(working_dir); + let context1 = factory.create_context(working_dir.clone(), create_test_user_output()); + let context2 = factory.create_context(working_dir, create_test_user_output()); // Both contexts should be functional let _ = context1.repository(); @@ -295,11 +331,9 @@ mod tests { #[test] fn it_should_create_multiple_handlers_from_same_context() { - let factory = CommandHandlerFactory::new(); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (factory, _temp_dir, working_dir, user_output) = create_test_setup(); - let context = factory.create_context(working_dir); + let context = factory.create_context(working_dir, user_output); // Should be able to create multiple handlers from same context let _create_handler = factory.create_create_handler(&context); @@ -315,11 +349,10 @@ mod tests { let repository_factory = RepositoryFactory::new(Duration::from_millis(100)); let factory = CommandHandlerFactory::new_for_testing(repository_factory); - let temp_dir = TempDir::new().unwrap(); - let working_dir = temp_dir.path().to_path_buf(); + let (_factory, _temp_dir, working_dir, user_output) = create_test_setup(); // Should be able to create context with custom factory - let context = factory.create_context(working_dir); + let context = factory.create_context(working_dir, user_output); let _ = context.repository(); } } diff --git a/src/presentation/commands/mod.rs b/src/presentation/commands/mod.rs index f0132327..ba57610b 100644 --- a/src/presentation/commands/mod.rs +++ b/src/presentation/commands/mod.rs @@ -4,8 +4,11 @@ //! It serves as the central dispatch point for command execution and provides consistent //! error handling across all commands. +use std::sync::{Arc, Mutex}; + use crate::presentation::cli::Commands; use crate::presentation::errors::CommandError; +use crate::presentation::user_output::UserOutput; // Re-export command modules pub mod constants; @@ -33,6 +36,7 @@ pub mod tests; /// /// * `command` - The parsed CLI command to execute /// * `working_dir` - Working directory for environment data storage +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Returns /// @@ -47,27 +51,32 @@ pub mod tests; /// /// ```rust /// use clap::Parser; -/// use torrust_tracker_deployer_lib::presentation::{cli, commands}; -/// use std::path::Path; +/// use torrust_tracker_deployer_lib::presentation::{cli, commands, user_output}; +/// use std::{path::Path, sync::{Arc, Mutex}}; /// /// let cli = cli::Cli::parse(); /// if let Some(command) = cli.command { /// let working_dir = Path::new("."); -/// let result = commands::execute(command, working_dir); +/// let user_output = Arc::new(Mutex::new(user_output::UserOutput::new(user_output::VerbosityLevel::Normal))); +/// let result = commands::execute(command, working_dir, &user_output); /// match result { /// Ok(_) => println!("Command executed successfully"), -/// Err(e) => commands::handle_error(&e), +/// Err(e) => commands::handle_error(&e, &user_output), /// } /// } /// ``` -pub fn execute(command: Commands, working_dir: &std::path::Path) -> Result<(), CommandError> { +pub fn execute( + command: Commands, + working_dir: &std::path::Path, + user_output: &Arc>, +) -> Result<(), CommandError> { match command { Commands::Create { action } => { - create::handle_create_command(action, working_dir)?; + create::handle_create_command(action, working_dir, user_output)?; Ok(()) } Commands::Destroy { environment } => { - destroy::handle_destroy_command(&environment, working_dir)?; + destroy::handle_destroy_command(&environment, working_dir, user_output)?; Ok(()) } // Future commands will be added here: // @@ -97,14 +106,16 @@ pub fn execute(command: Commands, working_dir: &std::path::Path) -> Result<(), C /// # Arguments /// /// * `error` - The command error to handle and display +/// * `user_output` - Shared user output service for consistent output formatting /// /// # Example /// /// ```rust /// use clap::Parser; -/// use torrust_tracker_deployer_lib::presentation::{commands, cli, errors}; +/// use torrust_tracker_deployer_lib::presentation::{commands, cli, errors, user_output}; /// use torrust_tracker_deployer_lib::presentation::commands::destroy::DestroySubcommandError; /// use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentNameError; +/// use std::sync::{Arc, Mutex}; /// /// # fn main() -> Result<(), Box> { /// // Example of handling a command error (simulated for testing) @@ -119,17 +130,33 @@ pub fn execute(command: Commands, working_dir: &std::path::Path) -> Result<(), C /// source: name_error, /// }) /// ); -/// commands::handle_error(&sample_error); +/// let user_output = Arc::new(Mutex::new(user_output::UserOutput::new(user_output::VerbosityLevel::Normal))); +/// commands::handle_error(&sample_error, &user_output); /// # Ok(()) /// # } /// ``` -pub fn handle_error(error: &CommandError) { - use crate::presentation::user_output::{UserOutput, VerbosityLevel}; - - let mut output = UserOutput::new(VerbosityLevel::Normal); +pub fn handle_error(error: &CommandError, user_output: &Arc>) { let help_text = error.help(); - output.error(&format!("{error}")); - output.blank_line(); - output.info_block("For detailed troubleshooting:", &[help_text]); + if let Ok(mut output) = user_output.lock() { + output.error(&format!("{error}")); + output.blank_line(); + output.info_block("For detailed troubleshooting:", &[help_text]); + } else { + // Cannot acquire lock - print to stderr directly as fallback + // + // RATIONALE: Plain text formatting without emojis/styling is intentional. + // When the mutex is poisoned, we're in a degraded error state where another + // thread has panicked. Using plain eprintln! ensures maximum compatibility + // and avoids any additional complexity that could fail in this critical path. + // The goal here is reliability over aesthetics - get the error message to + // the user no matter what, even if it's not pretty. + eprintln!("ERROR: {error}"); + eprintln!(); + eprintln!("CRITICAL: Failed to acquire user output lock."); + eprintln!("This indicates a panic occurred in another thread."); + eprintln!(); + eprintln!("For detailed troubleshooting:"); + eprintln!("{help_text}"); + } } diff --git a/src/presentation/commands/tests/mod.rs b/src/presentation/commands/tests/mod.rs index 0f30889a..2c059545 100644 --- a/src/presentation/commands/tests/mod.rs +++ b/src/presentation/commands/tests/mod.rs @@ -27,8 +27,11 @@ use std::fs; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; use tempfile::TempDir; +use crate::presentation::user_output::{UserOutput, VerbosityLevel}; + // ============================================================================ // PUBLIC API - Test Context // ============================================================================ @@ -57,6 +60,7 @@ use tempfile::TempDir; pub struct TestContext { _temp_dir: TempDir, working_dir: PathBuf, + user_output: Arc>, } impl TestContext { @@ -69,10 +73,12 @@ impl TestContext { pub fn new() -> Self { let temp_dir = TempDir::new().expect("Failed to create temporary directory"); let working_dir = temp_dir.path().to_path_buf(); + let user_output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); Self { _temp_dir: temp_dir, working_dir, + user_output, } } @@ -84,6 +90,12 @@ impl TestContext { pub fn working_dir(&self) -> &Path { &self.working_dir } + + /// Get a reference to the shared user output + #[must_use] + pub fn user_output(&self) -> Arc> { + Arc::clone(&self.user_output) + } } impl Default for TestContext { diff --git a/src/presentation/errors.rs b/src/presentation/errors.rs index 4a241705..ca8c9ae8 100644 --- a/src/presentation/errors.rs +++ b/src/presentation/errors.rs @@ -42,6 +42,13 @@ pub enum CommandError { /// Use `.help()` for detailed troubleshooting steps. #[error("Destroy command failed: {0}")] Destroy(Box), + + /// User output lock acquisition failed + /// + /// Failed to acquire the mutex lock for user output. This typically indicates + /// a panic occurred in another thread while holding the lock. + #[error("Failed to acquire user output lock - a panic occurred in another thread while displaying output")] + UserOutputLockFailed, } impl From for CommandError { @@ -94,6 +101,27 @@ impl CommandError { match self { Self::Create(e) => e.help(), Self::Destroy(e) => e.help(), + Self::UserOutputLockFailed => { + "User Output Lock Failed - Detailed Troubleshooting: + +This error indicates that a panic occurred in another thread while it was using +the user output system, leaving the mutex in a \"poisoned\" state. + +1. Check for any error messages that appeared before this one + - The original panic message should appear earlier in the output + - This will indicate what caused the initial failure + +2. This is typically caused by: + - A bug in the application code that caused a panic + - An unhandled error condition that triggered a panic + - Resource exhaustion (memory, file handles, etc.) + +3. If you can reproduce this issue: + - Run with --verbose to see more detailed logging + - Report the issue with the full error output and steps to reproduce + +This is a serious application error that indicates a bug. Please report it to the developers." + } } } } diff --git a/src/presentation/progress.rs b/src/presentation/progress.rs index 79933f3d..863f2fa3 100644 --- a/src/presentation/progress.rs +++ b/src/presentation/progress.rs @@ -15,31 +15,35 @@ //! ## Example Usage //! //! ```rust +//! use std::sync::{Arc, Mutex}; //! use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; //! use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; //! -//! let mut output = UserOutput::new(VerbosityLevel::Normal); +//! # fn main() -> Result<(), Box> { +//! let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); //! let mut progress = ProgressReporter::new(output, 3); //! //! // Step 1: Load configuration -//! progress.start_step("Loading configuration"); +//! progress.start_step("Loading configuration")?; //! // ... perform operation ... -//! progress.complete_step(Some("Configuration loaded: test-env")); +//! progress.complete_step(Some("Configuration loaded: test-env"))?; //! //! // Step 2: Provision with sub-steps -//! progress.start_step("Provisioning infrastructure"); -//! progress.sub_step("Creating virtual machine"); -//! progress.sub_step("Configuring network"); +//! progress.start_step("Provisioning infrastructure")?; +//! progress.sub_step("Creating virtual machine")?; +//! progress.sub_step("Configuring network")?; //! // ... perform operations ... -//! progress.complete_step(Some("Instance created: test-instance")); +//! progress.complete_step(Some("Instance created: test-instance"))?; //! //! // Step 3: Finalize -//! progress.start_step("Finalizing environment"); +//! progress.start_step("Finalizing environment")?; //! // ... perform operation ... -//! progress.complete_step(None); +//! progress.complete_step(None)?; //! //! // Complete with summary -//! progress.complete("Environment 'test-env' created successfully"); +//! progress.complete("Environment 'test-env' created successfully")?; +//! # Ok(()) +//! # } //! ``` //! //! ## Output Format @@ -58,10 +62,27 @@ //! ✅ Environment 'test-env' created successfully //! ``` +use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; +use thiserror::Error; + use crate::presentation::user_output::UserOutput; +/// Errors that can occur during progress reporting +#[derive(Debug, Error)] +pub enum ProgressReporterError { + /// `UserOutput` mutex was poisoned + /// + /// The shared `UserOutput` mutex was poisoned by a panic in another thread. + /// This indicates a critical internal error. + #[error( + "Internal error: UserOutput mutex was poisoned +Tip: This is a critical bug - please report it with full logs using --log-output file-and-stderr" + )] + UserOutputMutexPoisoned, +} + /// Progress reporter for multi-step operations /// /// Tracks progress through multiple steps of a long-running operation, @@ -70,22 +91,26 @@ use crate::presentation::user_output::UserOutput; /// # Examples /// /// ```rust +/// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// -/// let output = UserOutput::new(VerbosityLevel::Normal); +/// # fn main() -> Result<(), Box> { +/// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); /// let mut progress = ProgressReporter::new(output, 2); /// -/// progress.start_step("Step 1"); -/// progress.complete_step(Some("Step 1 done")); +/// progress.start_step("Step 1")?; +/// progress.complete_step(Some("Step 1 done"))?; /// -/// progress.start_step("Step 2"); -/// progress.complete_step(None); +/// progress.start_step("Step 2")?; +/// progress.complete_step(None)?; /// -/// progress.complete("All done!"); +/// progress.complete("All done!")?; +/// # Ok(()) +/// # } /// ``` pub struct ProgressReporter { - output: UserOutput, + output: Arc>, total_steps: usize, current_step: usize, step_start: Option, @@ -96,20 +121,21 @@ impl ProgressReporter { /// /// # Arguments /// - /// * `output` - User output handler for displaying messages + /// * `output` - Shared user output handler for displaying messages /// * `total_steps` - Total number of steps in the operation /// /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); /// let progress = ProgressReporter::new(output, 5); /// ``` #[must_use] - pub fn new(output: UserOutput, total_steps: usize) -> Self { + pub fn new(output: Arc>, total_steps: usize) -> Self { Self { output, total_steps, @@ -127,26 +153,39 @@ impl ProgressReporter { /// /// * `description` - Human-readable description of what this step does /// + /// # Errors + /// + /// Returns `ProgressReporterError::UserOutputMutexPoisoned` if the mutex is poisoned. + /// /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); + /// # fn main() -> Result<(), Box> { + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); /// let mut progress = ProgressReporter::new(output, 3); /// - /// progress.start_step("Loading configuration"); + /// progress.start_step("Loading configuration")?; /// // Output: ⏳ [1/3] Loading configuration... + /// # Ok(()) + /// # } /// ``` - pub fn start_step(&mut self, description: &str) { + pub fn start_step(&mut self, description: &str) -> Result<(), ProgressReporterError> { self.current_step += 1; self.step_start = Some(Instant::now()); - self.output.progress(&format!( - "[{}/{}] {}...", - self.current_step, self.total_steps, description - )); + self.output + .lock() + .map_err(|_| ProgressReporterError::UserOutputMutexPoisoned)? + .progress(&format!( + "[{}/{}] {}...", + self.current_step, self.total_steps, description + )); + + Ok(()) } /// Complete the current step with optional result message @@ -158,37 +197,48 @@ impl ProgressReporter { /// /// * `result` - Optional description of what was accomplished /// + /// # Errors + /// + /// Returns `ProgressReporterError::UserOutputMutexPoisoned` if the mutex is poisoned. + /// /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); + /// # fn main() -> Result<(), Box> { + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); /// let mut progress = ProgressReporter::new(output, 2); /// - /// progress.start_step("Loading data"); - /// progress.complete_step(Some("Data loaded successfully")); + /// progress.start_step("Loading data")?; + /// progress.complete_step(Some("Data loaded successfully"))?; /// // Output: ✓ Data loaded successfully (took 150ms) /// - /// progress.start_step("Processing"); - /// progress.complete_step(None); + /// progress.start_step("Processing")?; + /// progress.complete_step(None)?; /// // Output: ✓ Done (took 2.3s) + /// # Ok(()) + /// # } /// ``` - pub fn complete_step(&mut self, result: Option<&str>) { + pub fn complete_step(&mut self, result: Option<&str>) -> Result<(), ProgressReporterError> { if let Some(start) = self.step_start { let duration = start.elapsed(); + let mut output = self + .output + .lock() + .map_err(|_| ProgressReporterError::UserOutputMutexPoisoned)?; if let Some(msg) = result { - self.output - .result(&format!(" ✓ {} (took {})", msg, format_duration(duration))); + output.result(&format!(" ✓ {} (took {})", msg, format_duration(duration))); } else { - self.output - .result(&format!(" ✓ Done (took {})", format_duration(duration))); + output.result(&format!(" ✓ Done (took {})", format_duration(duration))); } } self.step_start = None; + Ok(()) } /// Report a sub-step within the current step @@ -200,23 +250,35 @@ impl ProgressReporter { /// /// * `description` - What is currently happening within this step /// + /// # Errors + /// + /// Returns `ProgressReporterError::UserOutputMutexPoisoned` if the mutex is poisoned. + /// /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); - /// let mut progress = ProgressReporter::new(output, 1); - /// - /// progress.start_step("Provisioning infrastructure"); - /// progress.sub_step("Creating virtual machine"); - /// progress.sub_step("Configuring network"); - /// progress.sub_step("Setting up storage"); - /// progress.complete_step(Some("Infrastructure ready")); + /// # fn main() -> Result<(), Box> { + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let mut progress = ProgressReporter::new(output.clone(), 1); + /// + /// progress.start_step("Provisioning infrastructure")?; + /// progress.sub_step("Creating virtual machine")?; + /// progress.sub_step("Configuring network")?; + /// progress.sub_step("Setting up storage")?; + /// progress.complete_step(Some("Infrastructure ready"))?; + /// # Ok(()) + /// # } /// ``` - pub fn sub_step(&mut self, description: &str) { - self.output.result(&format!(" → {description}")); + pub fn sub_step(&mut self, description: &str) -> Result<(), ProgressReporterError> { + self.output + .lock() + .map_err(|_| ProgressReporterError::UserOutputMutexPoisoned)? + .result(&format!(" → {description}")); + Ok(()) } /// Complete all steps and show summary @@ -228,25 +290,37 @@ impl ProgressReporter { /// /// * `summary` - Final success message describing what was accomplished /// + /// # Errors + /// + /// Returns `ProgressReporterError::UserOutputMutexPoisoned` if the mutex is poisoned. + /// /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); - /// let mut progress = ProgressReporter::new(output, 1); + /// # fn main() -> Result<(), Box> { + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let mut progress = ProgressReporter::new(output.clone(), 1); /// - /// progress.start_step("Creating environment"); - /// progress.complete_step(None); - /// progress.complete("Environment 'test-env' created successfully"); + /// progress.start_step("Creating environment")?; + /// progress.complete_step(None)?; + /// progress.complete("Environment 'test-env' created successfully")?; /// // Output: ✅ Environment 'test-env' created successfully + /// # Ok(()) + /// # } /// ``` - pub fn complete(&mut self, summary: &str) { - self.output.success(summary); + pub fn complete(&mut self, summary: &str) -> Result<(), ProgressReporterError> { + self.output + .lock() + .map_err(|_| ProgressReporterError::UserOutputMutexPoisoned)? + .success(summary); + Ok(()) } - /// Get a mutable reference to the underlying `UserOutput` + /// Get a reference to the shared `UserOutput` /// /// This allows using other output methods (like `error`, `warn`) /// while progress is being tracked. @@ -254,19 +328,20 @@ impl ProgressReporter { /// # Examples /// /// ```rust + /// use std::sync::{Arc, Mutex}; /// use torrust_tracker_deployer_lib::presentation::progress::ProgressReporter; /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// - /// let output = UserOutput::new(VerbosityLevel::Normal); - /// let mut progress = ProgressReporter::new(output, 1); + /// let output = Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal))); + /// let mut progress = ProgressReporter::new(output.clone(), 1); /// /// progress.start_step("Checking conditions"); - /// progress.output().warn("Some non-critical warning"); + /// progress.output().lock().unwrap().warn("Some non-critical warning"); /// progress.complete_step(None); /// ``` #[must_use] - pub fn output(&mut self) -> &mut UserOutput { - &mut self.output + pub fn output(&self) -> &Arc> { + &self.output } } @@ -316,6 +391,21 @@ mod tests { (output, stdout_buffer, stderr_buffer) } + /// Helper to create wrapped test `UserOutput` for `ProgressReporter` + /// + /// Returns: (`Arc>`, Arc to stdout buffer, Arc to stderr buffer) + #[allow(clippy::type_complexity)] + fn create_wrapped_test_output( + verbosity: VerbosityLevel, + ) -> ( + Arc>, + Arc>>, + Arc>>, + ) { + let (output, stdout, stderr) = create_test_user_output(verbosity); + (Arc::new(Mutex::new(output)), stdout, stderr) + } + /// A writer that shares a buffer through an Arc>> struct SharedWriter(Arc>>); @@ -331,7 +421,7 @@ mod tests { #[test] fn it_should_create_progress_reporter_with_total_steps() { - let output = UserOutput::new(VerbosityLevel::Normal); + let (output, _stdout, _stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let progress = ProgressReporter::new(output, 5); assert_eq!(progress.total_steps, 5); @@ -341,10 +431,12 @@ mod tests { #[test] fn it_should_start_step_and_increment_counter() { - let (output, _stdout, stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, _stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 3); - progress.start_step("Loading configuration"); + progress + .start_step("Loading configuration") + .expect("Failed to start step"); assert_eq!(progress.current_step, 1); assert!(progress.step_start.is_some()); @@ -355,16 +447,22 @@ mod tests { #[test] fn it_should_track_multiple_steps() { - let (output, _stdout, stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, _stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 3); - progress.start_step("Step 1"); + progress + .start_step("Step 1") + .expect("Failed to start step 1"); assert_eq!(progress.current_step, 1); - progress.start_step("Step 2"); + progress + .start_step("Step 2") + .expect("Failed to start step 2"); assert_eq!(progress.current_step, 2); - progress.start_step("Step 3"); + progress + .start_step("Step 3") + .expect("Failed to start step 3"); assert_eq!(progress.current_step, 3); let stderr_content = String::from_utf8(stderr.lock().unwrap().clone()).unwrap(); @@ -375,11 +473,15 @@ mod tests { #[test] fn it_should_complete_step_with_result_message() { - let (output, stdout, _stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, stdout, _stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 1); - progress.start_step("Loading data"); - progress.complete_step(Some("Data loaded successfully")); + progress + .start_step("Loading data") + .expect("Failed to start step"); + progress + .complete_step(Some("Data loaded successfully")) + .expect("Failed to complete step"); let stdout_content = String::from_utf8(stdout.lock().unwrap().clone()).unwrap(); assert!(stdout_content.contains("✓ Data loaded successfully")); @@ -389,11 +491,15 @@ mod tests { #[test] fn it_should_complete_step_without_result_message() { - let (output, stdout, _stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, stdout, _stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 1); - progress.start_step("Processing"); - progress.complete_step(None); + progress + .start_step("Processing") + .expect("Failed to start step"); + progress + .complete_step(None) + .expect("Failed to complete step"); let stdout_content = String::from_utf8(stdout.lock().unwrap().clone()).unwrap(); assert!(stdout_content.contains("✓ Done")); @@ -403,13 +509,21 @@ mod tests { #[test] fn it_should_report_sub_steps() { - let (output, stdout, _stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, stdout, _stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 1); - progress.start_step("Provisioning"); - progress.sub_step("Creating VM"); - progress.sub_step("Configuring network"); - progress.complete_step(None); + progress + .start_step("Provisioning") + .expect("Failed to start step"); + progress + .sub_step("Creating VM") + .expect("Failed to report sub-step"); + progress + .sub_step("Configuring network") + .expect("Failed to report sub-step"); + progress + .complete_step(None) + .expect("Failed to complete step"); let stdout_content = String::from_utf8(stdout.lock().unwrap().clone()).unwrap(); assert!(stdout_content.contains("→ Creating VM")); @@ -418,12 +532,18 @@ mod tests { #[test] fn it_should_display_completion_summary() { - let (output, _stdout, stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, _stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 1); - progress.start_step("Creating environment"); - progress.complete_step(None); - progress.complete("Environment created successfully"); + progress + .start_step("Creating environment") + .expect("Failed to start step"); + progress + .complete_step(None) + .expect("Failed to complete step"); + progress + .complete("Environment created successfully") + .expect("Failed to complete"); let stderr_content = String::from_utf8(stderr.lock().unwrap().clone()).unwrap(); assert!(stderr_content.contains("✅ Environment created successfully")); @@ -431,10 +551,14 @@ mod tests { #[test] fn it_should_provide_access_to_output() { - let (output, _stdout, stderr) = create_test_user_output(VerbosityLevel::Normal); - let mut progress = ProgressReporter::new(output, 1); + let (output, _stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Normal); + let progress = ProgressReporter::new(output, 1); - progress.output().warn("Test warning"); + progress + .output() + .lock() + .expect("UserOutput mutex poisoned") + .warn("Test warning"); let stderr_content = String::from_utf8(stderr.lock().unwrap().clone()).unwrap(); assert!(stderr_content.contains("⚠️ Test warning")); @@ -442,11 +566,13 @@ mod tests { #[test] fn it_should_respect_verbosity_levels() { - let (output, _stdout, stderr) = create_test_user_output(VerbosityLevel::Quiet); + let (output, _stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Quiet); let mut progress = ProgressReporter::new(output, 1); - progress.start_step("Step 1"); - progress.complete_step(Some("Done")); + progress.start_step("Step 1").expect("Failed to start step"); + progress + .complete_step(Some("Done")) + .expect("Failed to complete step"); // At Quiet level, progress messages should not appear let stderr_content = String::from_utf8(stderr.lock().unwrap().clone()).unwrap(); @@ -476,25 +602,43 @@ mod tests { #[test] fn it_should_handle_full_workflow() { - let (output, stdout, stderr) = create_test_user_output(VerbosityLevel::Normal); + let (output, stdout, stderr) = create_wrapped_test_output(VerbosityLevel::Normal); let mut progress = ProgressReporter::new(output, 3); // Step 1 - progress.start_step("Loading configuration"); - progress.complete_step(Some("Configuration loaded: test-env")); + progress + .start_step("Loading configuration") + .expect("Failed to start step 1"); + progress + .complete_step(Some("Configuration loaded: test-env")) + .expect("Failed to complete step 1"); // Step 2 with sub-steps - progress.start_step("Provisioning infrastructure"); - progress.sub_step("Creating virtual machine"); - progress.sub_step("Configuring network"); - progress.complete_step(Some("Instance created: test-instance")); + progress + .start_step("Provisioning infrastructure") + .expect("Failed to start step 2"); + progress + .sub_step("Creating virtual machine") + .expect("Failed to report sub-step"); + progress + .sub_step("Configuring network") + .expect("Failed to report sub-step"); + progress + .complete_step(Some("Instance created: test-instance")) + .expect("Failed to complete step 2"); // Step 3 - progress.start_step("Finalizing environment"); - progress.complete_step(None); + progress + .start_step("Finalizing environment") + .expect("Failed to start step 3"); + progress + .complete_step(None) + .expect("Failed to complete step 3"); // Complete - progress.complete("Environment 'test-env' created successfully"); + progress + .complete("Environment 'test-env' created successfully") + .expect("Failed to complete"); let stderr_content = String::from_utf8(stderr.lock().unwrap().clone()).unwrap(); assert!(stderr_content.contains("[1/3] Loading configuration..."));