Merge #125: Centralize test infrastructure for UserOutput to eliminate duplication

f5dfeb8 fix: add clippy annotations and panics documentation to TestUserOutput (copilot-swe-agent[bot])
eb5088a refactor: update all remaining test files to use TestUserOutput (copilot-swe-agent[bot])
a3a9cb8 refactor: update factory, context, and destroy/handler tests to use TestUserOutput (copilot-swe-agent[bot])
160c403 refactor: update progress.rs to use TestUserOutput helper (copilot-swe-agent[bot])
895be5c feat: add TestUserOutput helper for simplified test infrastructure (copilot-swe-agent[bot])
807a682 Initial plan (copilot-swe-agent[bot])

Pull request description:

  Eight test files each maintained duplicate `create_test_user_output()` helpers and `SharedWriter` implementations for capturing `UserOutput` in tests. This created maintenance burden and verbose test code requiring manual buffer lock management.

  ## Changes

  - **Created centralized `test_support` module** in `src/presentation/user_output.rs`
    - `TestUserOutput`: Wrapper providing clean API for test output capture
    - `TestWriter`: Write trait implementation using `Arc<Mutex<Vec<u8>>>`
    - Helper methods: `wrapped()`, `into_wrapped()`, `stdout()`, `stderr()`, `clear()`

  - **Updated 9 test files** to use centralized infrastructure
    - `user_output.rs`, `progress.rs`, `factory.rs`, `context.rs`
    - `destroy/handler.rs`, `destroy/tests/integration.rs`
    - `create/subcommands/environment.rs`, `create/tests/{template,integration}.rs`

  - **Removed duplicate code**
    - 8 `create_test_user_output()` functions
    - 2 `SharedWriter` implementations

  ## Example

  **Before:**
  ```rust
  let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal);
  output.progress("Processing...");
  let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap();
  assert_eq!(stderr_content, "⏳ Processing...\n");
  ```

  **After:**
  ```rust
  let mut test_output = TestUserOutput::new(VerbosityLevel::Normal);
  test_output.output.progress("Processing...");
  assert_eq!(test_output.stderr(), "⏳ Processing...\n");
  ```

  For tests requiring `Arc<Mutex<UserOutput>>`:
  ```rust
  let output = TestUserOutput::wrapped(VerbosityLevel::Normal);
  ```

  <!-- START COPILOT CODING AGENT SUFFIX -->

  <details>

  <summary>Original prompt</summary>

  >
  > ----
  >
  > *This section details on the original issue you should resolve*
  >
  > <issue_title>Simplify Test Infrastructure for UserOutput</issue_title>
  > <issue_description>## Overview
  >
  > Simplify the test infrastructure for `UserOutput` by replacing complex `Arc<Mutex<Vec<u8>>>` and custom `SharedWriter` with a simpler, more maintainable `TestUserOutput` wrapper. This makes test code easier to understand and reduces cognitive load for contributors.
  >
  > ## Specification
  >
  > See detailed specification: [docs/issues/TBD-simplify-test-infrastructure.md](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/issues/TBD-simplify-test-infrastructure.md)
  >
  > (Link will be updated after file rename)
  >
  > ## 🏗️ Architecture Requirements
  >
  > **DDD Layer**: Presentation (Test Support Module)
  > **Module Path**: `src/presentation/user_output.rs` (test module)
  > **Pattern**: Test Helper Struct
  >
  > ### Module Structure Requirements
  >
  > - [ ] Keep test infrastructure in `#[cfg(test)]` module within `user_output.rs`
  > - [ ] Create `TestUserOutput` helper struct for test scenarios
  > - [ ] Maintain separation between production code and test helpers
  > - [ ] Follow module organization conventions (see [docs/contributing/module-organization.md](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/contributing/module-organization.md))
  >
  > ### Architectural Constraints
  >
  > - [ ] **Test-only code**: All changes confined to `#[cfg(test)]` modules
  > - [ ] **No breaking changes**: Production `UserOutput` API unchanged
  > - [ ] **Standard library preference**: Use `Vec<u8>` and standard types over complex wrappers
  > - [ ] **Simplicity over cleverness**: Favor readable code over minimal boilerplate
  >
  > ### Anti-Patterns to Avoid
  >
  > - ❌ **Over-engineering test infrastructure** - Don't add unnecessary abstractions
  > - ❌ **Leaking test code to production** - Keep test helpers in test modules only
  > - ❌ **Complex synchronization primitives** - Avoid `Arc<Mutex<>>` unless truly needed
  >
  > ## Implementation Plan
  >
  > ### Phase 1: Create New Test Infrastructure (1-2 hours)
  >
  > - [ ] Create `test_support` module in `#[cfg(test)]` section
  > - [ ] Implement `TestUserOutput` struct with accessor methods
  > - [ ] Implement `TestWriter` with `Write` trait
  > - [ ] Add documentation for test infrastructure
  >
  > ### Phase 2: Update Existing Tests (2-3 hours)
  >
  > - [ ] Identify all tests using `create_test_user_output()`
  > - [ ] Update tests to use `TestUserOutput::new()`
  > - [ ] Simplify test code with new cleaner API
  > - [ ] Verify all tests pass
  >
  > ### Phase 3: Remove Old Infrastructure (30 minutes)
  >
  > - [ ] Remove `SharedWriter` struct
  > - [ ] Remove `create_test_user_output()` function
  > - [ ] Run linter and fix warnings
  >
  > ### Phase 4: Documentation and Verification (30 minutes)
  >
  > - [ ] Add module documentation
  > - [ ] Run full test suite
  > - [ ] Run pre-commit checks: `./scripts/pre-commit.sh`
  >
  > ## Acceptance Criteria
  >
  > > **Note for Contributors**: These criteria define what the PR reviewer will check. Use this as your pre-review checklist before submitting the PR to minimize back-and-forth iterations.
  >
  > **Quality Checks**:
  >
  > - [ ] Pre-commit checks pass: `./scripts/pre-commit.sh`
  >
  > **Infrastructure Checks**:
  >
  > - [ ] `TestUserOutput` struct exists in test module
  > - [ ] All accessor methods implemented: `stdout()`, `stderr()`, `output()`, `clear()`
  > - [ ] Old `SharedWriter` and `create_test_user_output()` removed
  >
  > **Test Migration Checks**:
  >
  > - [ ] All tests updated to new infrastructure
  > - [ ] No remaining references to old helpers
  > - [ ] All tests pass
  > - [ ] Test code is simpler and more readable
  >
  > **Code Quality Checks**:
  >
  > - [ ] Test infrastructure well-documented
  > - [ ] Usage examples provided
  > - [ ] Standard library types used (`Rc<RefCell<>>` instead of `Arc<Mutex<>>`)
  >
  > ## Related
  >
  > - Parent: #102 (Epic: User Output Architecture Improvements)
  > - Roadmap: N/A (Code quality improvement)
  > - Specification: docs/issues/TBD-simplify-test-infrastructure.md</issue_description>
  >
  > ## Comments on the Issue (you are @copilot in this section)
  >
  > <comments>
  > </comments>
  >

  </details>

  - Fixes #123

  <!-- START COPILOT CODING AGENT TIPS -->
  ---

  💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

ACKs for top commit:
  josecelano:
    ACK f5dfeb8

Tree-SHA512: e21725010f45d282e65e365dde82235748270f161cc0d59a88bcd86c64d69e20bc799a6426d6166c71982f4014920570584be929ae7cab298a68572b78439dd4

Author: josecelano

src/presentation/commands/context.rs

@@ -292,23 +292,17 @@ mod tests {
     use super::*;
     use tempfile::TempDir;
 
+    use crate::presentation::user_output::test_support::TestUserOutput;
     use crate::presentation::user_output::VerbosityLevel;
 
-    /// Test helper to create a test user output
-    fn create_test_user_output() -> Arc<std::sync::Mutex<UserOutput>> {
-        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<Mutex<UserOutput>>`)
     /// The `TempDir` must be kept alive for the duration of the test.
     fn create_test_setup() -> (TempDir, PathBuf, Arc<std::sync::Mutex<UserOutput>>) {
         let temp_dir = TempDir::new().unwrap();
         let working_dir = temp_dir.path().to_path_buf();
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         (temp_dir, working_dir, user_output)
     }
 

src/presentation/commands/create/subcommands/environment.rs

@@ -234,15 +234,11 @@ fn display_creation_results(user_output: &Arc<Mutex<UserOutput>>, environment: &
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::presentation::user_output::test_support::TestUserOutput;
     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<Mutex<UserOutput>> {
-        Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal)))
-    }
-
     #[test]
     fn it_should_create_environment_from_valid_config() {
         let temp_dir = TempDir::new().unwrap();
@@ -268,7 +264,7 @@ mod tests {
         fs::write(&config_path, config_json).unwrap();
 
         let working_dir = temp_dir.path();
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         let result = handle_environment_creation(&config_path, working_dir, &user_output);
 
         assert!(
@@ -292,7 +288,7 @@ 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 user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         let result = handle_environment_creation(&config_path, working_dir, &user_output);
 
@@ -314,7 +310,7 @@ mod tests {
         fs::write(&config_path, r#"{"invalid json"#).unwrap();
 
         let working_dir = temp_dir.path();
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         let result = handle_environment_creation(&config_path, working_dir, &user_output);
 
         assert!(result.is_err());
@@ -350,7 +346,7 @@ mod tests {
         fs::write(&config_path, config_json).unwrap();
 
         let working_dir = temp_dir.path();
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         // Create environment first time
         let result1 = handle_environment_creation(&config_path, working_dir, &user_output);
@@ -394,7 +390,7 @@ mod tests {
         );
         fs::write(&config_path, config_json).unwrap();
 
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         let result = handle_environment_creation(&config_path, &custom_working_dir, &user_output);
 
         assert!(result.is_ok(), "Should create in custom working dir");
@@ -438,7 +434,7 @@ mod tests {
             );
             fs::write(&config_path, config_json).unwrap();
 
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let result = load_configuration(&user_output, &config_path);
 
             assert!(result.is_ok(), "Should load valid configuration");
@@ -451,7 +447,7 @@ mod tests {
             let temp_dir = TempDir::new().unwrap();
             let config_path = temp_dir.path().join("missing.json");
 
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let result = load_configuration(&user_output, &config_path);
 
             assert!(result.is_err());
@@ -469,7 +465,7 @@ mod tests {
             let config_path = temp_dir.path().join("invalid.json");
             fs::write(&config_path, r#"{"broken json"#).unwrap();
 
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let result = load_configuration(&user_output, &config_path);
 
             assert!(result.is_err());
@@ -507,7 +503,7 @@ mod tests {
             );
             fs::write(&config_path, config_json).unwrap();
 
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let loader = ConfigLoader;
             let config = loader.load_from_file(&config_path).unwrap();
 
@@ -543,7 +539,7 @@ mod tests {
             );
             fs::write(&config_path, config_json).unwrap();
 
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let loader = ConfigLoader;
             let config = loader.load_from_file(&config_path).unwrap();
 
@@ -597,7 +593,7 @@ mod tests {
             fs::write(&config_path, config_json).unwrap();
 
             // Create environment
-            let user_output = create_test_user_output();
+            let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
             let factory = CommandHandlerFactory::new();
             let ctx = factory.create_context(temp_dir.path().to_path_buf(), user_output.clone());
             let loader = ConfigLoader;

src/presentation/commands/create/tests/integration.rs

@@ -3,20 +3,14 @@
 //! 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<Mutex<UserOutput>> {
-    Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal)))
-}
+use crate::presentation::user_output::test_support::TestUserOutput;
+use crate::presentation::user_output::VerbosityLevel;
 
 /// Helper function to call the environment creation handler
 fn handle_environment_creation(
@@ -26,7 +20,7 @@ fn handle_environment_creation(
     let action = CreateAction::Environment {
         env_file: config_path.to_path_buf(),
     };
-    let user_output = create_test_user_output();
+    let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
     create::handle_create_command(action, working_dir, &user_output)
 }
 

src/presentation/commands/create/tests/template.rs

@@ -1,16 +1,10 @@
 //! 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<Mutex<UserOutput>> {
-    Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal)))
-}
+use crate::presentation::user_output::test_support::TestUserOutput;
+use crate::presentation::user_output::VerbosityLevel;
 
 #[test]
 fn it_should_generate_template_with_default_path() {
@@ -21,7 +15,7 @@ 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 user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
     let result = create::handle_create_command(action, test_context.working_dir(), &user_output);
 
@@ -65,7 +59,7 @@ 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 user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
     let result = create::handle_create_command(action, test_context.working_dir(), &user_output);
 
@@ -90,7 +84,7 @@ fn it_should_generate_valid_json_template() {
     let action = CreateAction::Template {
         output_path: Some(template_path.clone()),
     };
-    let user_output = create_test_user_output();
+    let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
     create::handle_create_command(action, test_context.working_dir(), &user_output).unwrap();
 
@@ -135,7 +129,7 @@ fn it_should_create_parent_directories() {
     let action = CreateAction::Template {
         output_path: Some(deep_path.clone()),
     };
-    let user_output = create_test_user_output();
+    let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
     let result = create::handle_create_command(action, test_context.working_dir(), &user_output);
 

src/presentation/commands/destroy/handler.rs

@@ -233,20 +233,16 @@ pub fn handle_destroy_command(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::presentation::user_output::test_support::TestUserOutput;
     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<Mutex<UserOutput>> {
-        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();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         // Test with invalid environment name (contains underscore)
         let result = handle_destroy_command("invalid_name", working_dir, &user_output);
@@ -264,7 +260,7 @@ 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 user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         let result = handle_destroy_command("", working_dir, &user_output);
 
@@ -281,7 +277,7 @@ 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();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         // Try to destroy an environment that doesn't exist
         let result = handle_destroy_command("nonexistent-env", working_dir, &user_output);
@@ -300,7 +296,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();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
         // Create a mock environment directory to test validation
         let env_dir = working_dir.join("test-env");

src/presentation/commands/destroy/tests/integration.rs

@@ -4,16 +4,11 @@
 //! 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 crate::presentation::user_output::{UserOutput, VerbosityLevel};
-
-/// Helper to create test `UserOutput`
-fn create_test_user_output() -> Arc<Mutex<UserOutput>> {
-    Arc::new(Mutex::new(UserOutput::new(VerbosityLevel::Normal)))
-}
+use crate::presentation::user_output::test_support::TestUserOutput;
+use crate::presentation::user_output::VerbosityLevel;
 
 #[test]
 fn it_should_reject_invalid_environment_names() {
@@ -27,7 +22,7 @@ fn it_should_reject_invalid_environment_names() {
     ];
 
     for name in invalid_names {
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         let result = handle_destroy_command(name, context.working_dir(), &user_output);
         assert!(
             result.is_err(),
@@ -44,7 +39,7 @@ 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 user_output = create_test_user_output();
+    let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
     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
@@ -64,7 +59,7 @@ fn it_should_accept_valid_environment_names() {
     ];
 
     for name in valid_names {
-        let user_output = create_test_user_output();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         let result = handle_destroy_command(name, context.working_dir(), &user_output);
 
         // Will fail at operation since environment doesn't exist,
@@ -77,7 +72,7 @@ fn it_should_accept_valid_environment_names() {
 
     // Test max length separately due to String allocation
     let max_length_name = "a".repeat(63);
-    let user_output = create_test_user_output();
+    let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
     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");
@@ -88,7 +83,7 @@ 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 user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
 
     let result = handle_destroy_command("nonexistent-env", context.working_dir(), &user_output);
 

src/presentation/commands/factory.rs

@@ -242,14 +242,10 @@ impl CommandHandlerFactory {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::presentation::user_output::test_support::TestUserOutput;
     use crate::presentation::user_output::VerbosityLevel;
     use tempfile::TempDir;
 
-    /// Test helper to create a test user output
-    fn create_test_user_output() -> Arc<Mutex<UserOutput>> {
-        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<Mutex<UserOutput>>`)
@@ -263,7 +259,7 @@ mod tests {
         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();
+        let user_output = TestUserOutput::wrapped(VerbosityLevel::Normal);
         (factory, temp_dir, working_dir, user_output)
     }
 
@@ -321,8 +317,12 @@ mod tests {
         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(), create_test_user_output());
-        let context2 = factory.create_context(working_dir, create_test_user_output());
+        let context1 = factory.create_context(
+            working_dir.clone(),
+            TestUserOutput::wrapped(VerbosityLevel::Normal),
+        );
+        let context2 =
+            factory.create_context(working_dir, TestUserOutput::wrapped(VerbosityLevel::Normal));
 
         // Both contexts should be functional
         let _ = context1.repository();