From c0f248d805dc5406127e421fc7574178abb34242 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 08:58:24 +0000 Subject: [PATCH 1/6] Initial plan From 4455c9bd47606fe973ac84fbb1913cdbd82f78c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 09:12:36 +0000 Subject: [PATCH 2/6] feat: [#37] update e2e-tests-full to use CreateCommandHandler - Add create_environment_via_command() function that uses CreateCommandHandler - Replace direct Environment::new() with command-based creation - Update imports to include CreateCommandHandler and related types - Maintain existing test flow while exercising create command logic - Ensure proper validation and repository persistence through command pattern Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/bin/e2e_tests_full.rs | 110 +++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 14 deletions(-) diff --git a/src/bin/e2e_tests_full.rs b/src/bin/e2e_tests_full.rs index f6d3d8d3..b91a9cad 100644 --- a/src/bin/e2e_tests_full.rs +++ b/src/bin/e2e_tests_full.rs @@ -55,14 +55,18 @@ use anyhow::Result; use clap::Parser; +use std::sync::Arc; use std::time::Instant; use tracing::{error, info}; // Import E2E testing infrastructure -use torrust_tracker_deployer_lib::adapters::ssh::{SshCredentials, DEFAULT_SSH_PORT}; -use torrust_tracker_deployer_lib::domain::{Environment, EnvironmentName}; +use torrust_tracker_deployer_lib::adapters::ssh::DEFAULT_SSH_PORT; +use torrust_tracker_deployer_lib::application::command_handlers::create::CreateCommandHandler; +use torrust_tracker_deployer_lib::domain::config::SshCredentialsConfig; +use torrust_tracker_deployer_lib::domain::config::{EnvironmentCreationConfig, EnvironmentSection}; +use torrust_tracker_deployer_lib::domain::environment::Created; +use torrust_tracker_deployer_lib::domain::Environment; use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder}; -use torrust_tracker_deployer_lib::shared::Username; use torrust_tracker_deployer_lib::testing::e2e::context::{TestContext, TestContextType}; use torrust_tracker_deployer_lib::testing::e2e::tasks::{ run_configure_command::run_configure_command, @@ -90,6 +94,87 @@ struct Cli { log_format: LogFormat, } +/// Creates a new environment using the `CreateCommandHandler`. +/// +/// This function demonstrates the use of the create command handler in the E2E test context. +/// It creates the environment using the command pattern, ensuring proper validation and +/// repository persistence. +/// +/// # Arguments +/// +/// * `environment_name` - Name of the environment to create +/// * `ssh_private_key_path` - Path to SSH private key file +/// * `ssh_public_key_path` - Path to SSH public key file +/// * `ssh_username` - SSH username for the environment +/// * `ssh_port` - SSH port number +/// +/// # Returns +/// +/// Returns the created `Environment` on success. +/// +/// # Errors +/// +/// Returns an error if: +/// - Configuration is invalid +/// - Environment already exists +/// - Repository operations fail +fn create_environment_via_command( + environment_name: &str, + ssh_private_key_path: String, + ssh_public_key_path: String, + ssh_username: &str, + ssh_port: u16, +) -> Result> { + info!( + environment_name = environment_name, + "Creating environment via CreateCommandHandler" + ); + + // Get the project root directory to create repository in the correct location + let project_root = std::env::current_dir().expect("Failed to get current directory"); + + // Create repository factory with default timeout + let repository_factory = torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory::new( + std::time::Duration::from_secs(30) + ); + + // Create repository pointing to project root (standard location for data directory) + let repository = repository_factory.create(project_root); + + // Create clock service for command handler + let clock: Arc = + Arc::new(torrust_tracker_deployer_lib::shared::SystemClock); + + // Create the command handler + let create_command = CreateCommandHandler::new(repository, clock); + + // Build the configuration + let config = EnvironmentCreationConfig::new( + EnvironmentSection { + name: environment_name.to_string(), + }, + SshCredentialsConfig::new( + ssh_private_key_path, + ssh_public_key_path, + ssh_username.to_string(), + ssh_port, + ), + ); + + // Execute the command + let environment = create_command + .execute(config) + .map_err(|e| anyhow::anyhow!("Failed to create environment via command: {e}"))?; + + info!( + environment_name = environment.name().as_str(), + instance_name = environment.instance_name().as_str(), + "Environment created successfully via CreateCommandHandler" + ); + + Ok(environment) +} + /// Main entry point for E2E tests. /// /// Runs the full deployment workflow: provision infrastructure, configure services, @@ -127,22 +212,19 @@ pub async fn main() -> Result<()> { "Starting E2E tests" ); - // Create environment entity for e2e-full testing - let environment_name = EnvironmentName::new("e2e-full".to_string())?; - // Use absolute paths to project root for SSH keys to ensure they can be found by Ansible let project_root = std::env::current_dir().expect("Failed to get current directory"); let ssh_private_key_path = project_root.join("fixtures/testing_rsa"); let ssh_public_key_path = project_root.join("fixtures/testing_rsa.pub"); - let ssh_user = Username::new("torrust").expect("Valid hardcoded username"); - let ssh_credentials = SshCredentials::new( - ssh_private_key_path.clone(), - ssh_public_key_path.clone(), - ssh_user.clone(), - ); - let ssh_port = DEFAULT_SSH_PORT; - let environment = Environment::new(environment_name, ssh_credentials, ssh_port); + // Create environment via CreateCommandHandler + let environment = create_environment_via_command( + "e2e-full", + ssh_private_key_path.to_string_lossy().to_string(), + ssh_public_key_path.to_string_lossy().to_string(), + "torrust", + DEFAULT_SSH_PORT, + )?; let mut test_context = TestContext::from_environment(cli.keep, environment, TestContextType::VirtualMachine)? From e5f2fb0d781a5077364fc49e889410022c49568e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 09:17:27 +0000 Subject: [PATCH 3/6] refactor: [#37] improve error handling in create_environment_via_command - Replace expect() with anyhow::Context for better error messages - Extract repository timeout constant (30 seconds) - Use .context() instead of .map_err() to preserve error chain - Add descriptive context messages for error scenarios Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/bin/e2e_tests_full.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/bin/e2e_tests_full.rs b/src/bin/e2e_tests_full.rs index b91a9cad..41282f1d 100644 --- a/src/bin/e2e_tests_full.rs +++ b/src/bin/e2e_tests_full.rs @@ -53,7 +53,7 @@ //! The test suite supports different VM providers (LXD, Multipass) and includes //! comprehensive logging and error reporting. -use anyhow::Result; +use anyhow::{Context, Result}; use clap::Parser; use std::sync::Arc; use std::time::Instant; @@ -94,6 +94,9 @@ struct Cli { log_format: LogFormat, } +/// Default timeout for repository lock acquisition operations +const REPOSITORY_LOCK_TIMEOUT_SECS: u64 = 30; + /// Creates a new environment using the `CreateCommandHandler`. /// /// This function demonstrates the use of the create command handler in the E2E test context. @@ -131,11 +134,12 @@ fn create_environment_via_command( ); // Get the project root directory to create repository in the correct location - let project_root = std::env::current_dir().expect("Failed to get current directory"); + let project_root = std::env::current_dir() + .context("Failed to determine current directory for environment creation")?; // Create repository factory with default timeout let repository_factory = torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory::new( - std::time::Duration::from_secs(30) + std::time::Duration::from_secs(REPOSITORY_LOCK_TIMEOUT_SECS) ); // Create repository pointing to project root (standard location for data directory) @@ -164,7 +168,7 @@ fn create_environment_via_command( // Execute the command let environment = create_command .execute(config) - .map_err(|e| anyhow::anyhow!("Failed to create environment via command: {e}"))?; + .context("Failed to create environment via command")?; info!( environment_name = environment.name().as_str(), From ce59c6a8dc69275de5e93968ed76db2f5f890250 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 09:58:49 +0000 Subject: [PATCH 4/6] refactor: [#37] move create_environment_via_command to e2e tasks module - Create new module src/testing/e2e/tasks/run_create_command.rs - Move environment creation logic to dedicated task module - Accept RepositoryFactory and Clock as parameters instead of calculating directory - Use "data" as base directory following TestContext pattern - Remove local function from e2e_tests_full.rs - Update imports and call site to use new task module - Add comprehensive error handling with help() method - Follow same pattern as run_configure_command.rs Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/bin/e2e_tests_full.rs | 108 ++----------- src/testing/e2e/tasks/mod.rs | 2 + src/testing/e2e/tasks/run_create_command.rs | 168 ++++++++++++++++++++ 3 files changed, 184 insertions(+), 94 deletions(-) create mode 100644 src/testing/e2e/tasks/run_create_command.rs diff --git a/src/bin/e2e_tests_full.rs b/src/bin/e2e_tests_full.rs index 41282f1d..c9336ca8 100644 --- a/src/bin/e2e_tests_full.rs +++ b/src/bin/e2e_tests_full.rs @@ -53,23 +53,21 @@ //! The test suite supports different VM providers (LXD, Multipass) and includes //! comprehensive logging and error reporting. -use anyhow::{Context, Result}; +use anyhow::Result; use clap::Parser; use std::sync::Arc; -use std::time::Instant; +use std::time::{Duration, Instant}; use tracing::{error, info}; // Import E2E testing infrastructure use torrust_tracker_deployer_lib::adapters::ssh::DEFAULT_SSH_PORT; -use torrust_tracker_deployer_lib::application::command_handlers::create::CreateCommandHandler; -use torrust_tracker_deployer_lib::domain::config::SshCredentialsConfig; -use torrust_tracker_deployer_lib::domain::config::{EnvironmentCreationConfig, EnvironmentSection}; -use torrust_tracker_deployer_lib::domain::environment::Created; -use torrust_tracker_deployer_lib::domain::Environment; +use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory; use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder}; +use torrust_tracker_deployer_lib::shared::{Clock, SystemClock}; use torrust_tracker_deployer_lib::testing::e2e::context::{TestContext, TestContextType}; use torrust_tracker_deployer_lib::testing::e2e::tasks::{ run_configure_command::run_configure_command, + run_create_command::run_create_command, run_test_command::run_test_command, virtual_machine::{ preflight_cleanup::preflight_cleanup_previous_resources, @@ -94,91 +92,6 @@ struct Cli { log_format: LogFormat, } -/// Default timeout for repository lock acquisition operations -const REPOSITORY_LOCK_TIMEOUT_SECS: u64 = 30; - -/// Creates a new environment using the `CreateCommandHandler`. -/// -/// This function demonstrates the use of the create command handler in the E2E test context. -/// It creates the environment using the command pattern, ensuring proper validation and -/// repository persistence. -/// -/// # Arguments -/// -/// * `environment_name` - Name of the environment to create -/// * `ssh_private_key_path` - Path to SSH private key file -/// * `ssh_public_key_path` - Path to SSH public key file -/// * `ssh_username` - SSH username for the environment -/// * `ssh_port` - SSH port number -/// -/// # Returns -/// -/// Returns the created `Environment` on success. -/// -/// # Errors -/// -/// Returns an error if: -/// - Configuration is invalid -/// - Environment already exists -/// - Repository operations fail -fn create_environment_via_command( - environment_name: &str, - ssh_private_key_path: String, - ssh_public_key_path: String, - ssh_username: &str, - ssh_port: u16, -) -> Result> { - info!( - environment_name = environment_name, - "Creating environment via CreateCommandHandler" - ); - - // Get the project root directory to create repository in the correct location - let project_root = std::env::current_dir() - .context("Failed to determine current directory for environment creation")?; - - // Create repository factory with default timeout - let repository_factory = torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory::new( - std::time::Duration::from_secs(REPOSITORY_LOCK_TIMEOUT_SECS) - ); - - // Create repository pointing to project root (standard location for data directory) - let repository = repository_factory.create(project_root); - - // Create clock service for command handler - let clock: Arc = - Arc::new(torrust_tracker_deployer_lib::shared::SystemClock); - - // Create the command handler - let create_command = CreateCommandHandler::new(repository, clock); - - // Build the configuration - let config = EnvironmentCreationConfig::new( - EnvironmentSection { - name: environment_name.to_string(), - }, - SshCredentialsConfig::new( - ssh_private_key_path, - ssh_public_key_path, - ssh_username.to_string(), - ssh_port, - ), - ); - - // Execute the command - let environment = create_command - .execute(config) - .context("Failed to create environment via command")?; - - info!( - environment_name = environment.name().as_str(), - instance_name = environment.instance_name().as_str(), - "Environment created successfully via CreateCommandHandler" - ); - - Ok(environment) -} - /// Main entry point for E2E tests. /// /// Runs the full deployment workflow: provision infrastructure, configure services, @@ -221,14 +134,21 @@ pub async fn main() -> Result<()> { let ssh_private_key_path = project_root.join("fixtures/testing_rsa"); let ssh_public_key_path = project_root.join("fixtures/testing_rsa.pub"); + // Create repository factory and clock for environment creation + let repository_factory = RepositoryFactory::new(Duration::from_secs(30)); + let clock: Arc = Arc::new(SystemClock); + // Create environment via CreateCommandHandler - let environment = create_environment_via_command( + let environment = run_create_command( + &repository_factory, + clock, "e2e-full", ssh_private_key_path.to_string_lossy().to_string(), ssh_public_key_path.to_string_lossy().to_string(), "torrust", DEFAULT_SSH_PORT, - )?; + ) + .map_err(|e| anyhow::anyhow!("{e}"))?; let mut test_context = TestContext::from_environment(cli.keep, environment, TestContextType::VirtualMachine)? diff --git a/src/testing/e2e/tasks/mod.rs b/src/testing/e2e/tasks/mod.rs index fc75c862..fd69e00d 100644 --- a/src/testing/e2e/tasks/mod.rs +++ b/src/testing/e2e/tasks/mod.rs @@ -9,6 +9,7 @@ //! The tasks are organized by deployment target: //! //! ### Infrastructure-agnostic tasks (can be used with both containers and VMs): +//! - `run_create_command` - Environment creation using `CreateCommandHandler` //! - `run_configure_command` - Infrastructure configuration via Ansible and playbook execution //! - `run_configuration_validation` - Configuration validation and testing //! - `run_test_command` - Deployment validation and testing @@ -30,5 +31,6 @@ pub mod container; pub mod preflight_cleanup; pub mod run_configuration_validation; pub mod run_configure_command; +pub mod run_create_command; pub mod run_test_command; pub mod virtual_machine; diff --git a/src/testing/e2e/tasks/run_create_command.rs b/src/testing/e2e/tasks/run_create_command.rs new file mode 100644 index 00000000..7a547aba --- /dev/null +++ b/src/testing/e2e/tasks/run_create_command.rs @@ -0,0 +1,168 @@ +//! Environment creation task for E2E testing +//! +//! This module provides the E2E testing task for creating deployment environments +//! using the `CreateCommandHandler`. It orchestrates environment creation with proper +//! validation and repository persistence. +//! +//! ## Key Operations +//! +//! - Creates environments using the `CreateCommandHandler` +//! - Handles environment creation workflow for E2E tests +//! - Provides structured error handling and reporting +//! +//! ## Integration +//! +//! This is a generic task that works with the command handler pattern: +//! - Uses the `RepositoryFactory` to create repositories +//! - Creates `EnvironmentCreationConfig` from test parameters +//! - Integrates with the existing `CreateCommandHandler` workflow + +use std::sync::Arc; +use thiserror::Error; +use tracing::info; + +use crate::application::command_handlers::create::{ + CreateCommandHandler, CreateCommandHandlerError, +}; +use crate::domain::config::{EnvironmentCreationConfig, EnvironmentSection, SshCredentialsConfig}; +use crate::domain::environment::Created; +use crate::domain::Environment; +use crate::infrastructure::persistence::repository_factory::RepositoryFactory; +use crate::shared::Clock; + +/// Create a new environment using the `CreateCommandHandler` +/// +/// This function executes environment creation using the `CreateCommandHandler` for E2E tests. +/// It builds the configuration from parameters and creates the environment, +/// ensuring proper validation and persistence. +/// +/// # Arguments +/// +/// * `repository_factory` - Repository factory for creating environment repositories +/// * `clock` - Clock service for timestamp generation +/// * `environment_name` - Name of the environment to create +/// * `ssh_private_key_path` - Path to SSH private key file +/// * `ssh_public_key_path` - Path to SSH public key file +/// * `ssh_username` - SSH username for the environment +/// * `ssh_port` - SSH port number +/// +/// # Returns +/// +/// Returns the created `Environment` on success. +/// +/// # Errors +/// +/// Returns an error if: +/// - Configuration is invalid +/// - Environment already exists +/// - Repository operations fail +pub fn run_create_command( + repository_factory: &RepositoryFactory, + clock: Arc, + environment_name: &str, + ssh_private_key_path: String, + ssh_public_key_path: String, + ssh_username: &str, + ssh_port: u16, +) -> Result, CreateTaskError> { + info!( + environment_name = environment_name, + "Creating environment via CreateCommandHandler" + ); + + // Create repository using RepositoryFactory with "data" as base directory + let base_data_dir = std::path::PathBuf::from("data"); + let repository = repository_factory.create(base_data_dir); + + // Create the command handler + let create_command = CreateCommandHandler::new(repository, clock); + + // Build the configuration + let config = EnvironmentCreationConfig::new( + EnvironmentSection { + name: environment_name.to_string(), + }, + SshCredentialsConfig::new( + ssh_private_key_path, + ssh_public_key_path, + ssh_username.to_string(), + ssh_port, + ), + ); + + // Execute the command + let environment = create_command + .execute(config) + .map_err(|source| CreateTaskError::CreationFailed { source })?; + + info!( + environment_name = environment.name().as_str(), + instance_name = environment.instance_name().as_str(), + "Environment created successfully via CreateCommandHandler" + ); + + Ok(environment) +} + +/// Errors that can occur during the create task +#[derive(Debug, Error)] +pub enum CreateTaskError { + /// Environment creation command execution failed + #[error( + "Failed to create environment: {source} +Tip: Check that the environment name is valid and doesn't already exist" + )] + CreationFailed { + #[source] + source: CreateCommandHandlerError, + }, +} + +impl CreateTaskError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + /// + /// # Example + /// + /// ```rust + /// # use torrust_tracker_deployer_lib::testing::e2e::tasks::run_create_command::CreateTaskError; + /// # use torrust_tracker_deployer_lib::application::command_handlers::create::CreateCommandHandlerError; + /// let error = CreateTaskError::CreationFailed { + /// source: CreateCommandHandlerError::EnvironmentAlreadyExists { + /// name: "test-env".to_string(), + /// }, + /// }; + /// println!("{}", error.help()); + /// ``` + #[must_use] + pub fn help(&self) -> &'static str { + match self { + Self::CreationFailed { .. } => { + "Environment Creation Failed - Detailed Troubleshooting: + +1. Check environment name: + - Must be lowercase letters and numbers only + - Use dashes as word separators + - Cannot start or end with separators + - Cannot start with numbers + +2. Verify environment doesn't already exist: + - Check the data directory for existing environment + - Use 'destroy' command to remove existing environment if needed + +3. Check SSH key files: + - Verify SSH private key file exists and is readable + - Verify SSH public key file exists and is readable + - Ensure key file permissions are correct (600 for private key) + +4. Check repository access: + - Ensure data directory is writable + - Verify no file locks are preventing write access + +For more information, see the E2E testing documentation." + } + } + } +} From 0e7d5a07564246e9e1422c7f8afa4476f18150f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 10:48:55 +0000 Subject: [PATCH 5/6] fix: [#37] add data directory cleanup to E2E preflight - Add cleanup_data_environment() helper to preflight_cleanup.rs - Clean data/{environment_name} directory before E2E test runs - Call from both container and VM preflight cleanup functions - Prevents "environment already exists" errors from stale state - Ensures proper test isolation by starting with clean data directory - Follows same pattern as build and templates directory cleanup Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- .../e2e/tasks/container/preflight_cleanup.rs | 6 +- src/testing/e2e/tasks/preflight_cleanup.rs | 76 +++++++++++++++++++ .../virtual_machine/preflight_cleanup.rs | 6 +- 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/testing/e2e/tasks/container/preflight_cleanup.rs b/src/testing/e2e/tasks/container/preflight_cleanup.rs index 40251164..d3666387 100644 --- a/src/testing/e2e/tasks/container/preflight_cleanup.rs +++ b/src/testing/e2e/tasks/container/preflight_cleanup.rs @@ -8,7 +8,8 @@ use crate::shared::command::CommandExecutor; use crate::testing::e2e::context::TestContext; use crate::testing::e2e::tasks::preflight_cleanup::{ - cleanup_build_directory, cleanup_templates_directory, PreflightCleanupError, + cleanup_build_directory, cleanup_data_environment, cleanup_templates_directory, + PreflightCleanupError, }; use tracing::{info, warn}; @@ -44,6 +45,9 @@ pub fn preflight_cleanup_previous_resources( // Clean the templates directory to ensure fresh embedded template extraction for E2E tests cleanup_templates_directory(env)?; + // Clean the data directory to ensure fresh environment state for E2E tests + cleanup_data_environment(env)?; + // Clean up any hanging Docker containers from interrupted test runs cleanup_hanging_docker_containers(env); diff --git a/src/testing/e2e/tasks/preflight_cleanup.rs b/src/testing/e2e/tasks/preflight_cleanup.rs index ee3fd85f..3c589871 100644 --- a/src/testing/e2e/tasks/preflight_cleanup.rs +++ b/src/testing/e2e/tasks/preflight_cleanup.rs @@ -192,3 +192,79 @@ pub fn cleanup_templates_directory( } } } + +/// Cleans the data directory for the test environment to ensure fresh state for E2E tests +/// +/// This function removes the environment's data directory if it exists, ensuring that +/// E2E tests start with a clean state and don't encounter conflicts with stale +/// environment data from previous test runs. This prevents "environment already exists" +/// errors and ensures proper test isolation. +/// +/// # Safety +/// +/// This function is only intended for E2E test environments and should never +/// be called in production code paths. It's designed to provide test isolation +/// by ensuring fresh environment state for each test run. +/// +/// # Arguments +/// +/// * `test_context` - The test context containing the environment configuration +/// +/// # Returns +/// +/// Returns `Ok(())` if cleanup succeeds or if the data directory doesn't exist. +/// +/// # Errors +/// +/// Returns a `PreflightCleanupError::ResourceConflicts` error if the data directory +/// cannot be removed due to permission issues or file locks. +pub fn cleanup_data_environment(test_context: &TestContext) -> Result<(), PreflightCleanupError> { + use std::path::Path; + + // Construct the data directory path: data/{environment_name} + let data_dir = Path::new("data").join(test_context.environment.name().as_str()); + + if !data_dir.exists() { + info!( + operation = "data_directory_cleanup", + status = "clean", + path = %data_dir.display(), + "Data directory doesn't exist, skipping cleanup" + ); + return Ok(()); + } + + info!( + operation = "data_directory_cleanup", + path = %data_dir.display(), + "Cleaning data directory for previous test environment" + ); + + match std::fs::remove_dir_all(&data_dir) { + Ok(()) => { + info!( + operation = "data_directory_cleanup", + status = "success", + path = %data_dir.display(), + "Data directory cleaned successfully" + ); + Ok(()) + } + Err(e) => { + warn!( + operation = "data_directory_cleanup", + status = "failed", + path = %data_dir.display(), + error = %e, + "Failed to clean data directory" + ); + Err(PreflightCleanupError::ResourceConflicts { + details: format!( + "Failed to clean data directory '{}': {}", + data_dir.display(), + e + ), + }) + } + } +} diff --git a/src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs b/src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs index ae882e16..d77b1d5a 100644 --- a/src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs +++ b/src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs @@ -9,7 +9,8 @@ use crate::adapters::tofu; use crate::infrastructure::external_tools::tofu::OPENTOFU_SUBFOLDER; use crate::testing::e2e::context::TestContext; use crate::testing::e2e::tasks::preflight_cleanup::{ - cleanup_build_directory, cleanup_templates_directory, PreflightCleanupError, + cleanup_build_directory, cleanup_data_environment, cleanup_templates_directory, + PreflightCleanupError, }; use tracing::{info, warn}; @@ -44,6 +45,9 @@ pub fn preflight_cleanup_previous_resources( // Clean the templates directory to ensure fresh embedded template extraction for E2E tests cleanup_templates_directory(test_context)?; + // Clean the data directory to ensure fresh environment state for E2E tests + cleanup_data_environment(test_context)?; + // Clean any existing OpenTofu infrastructure from previous test runs cleanup_opentofu_infrastructure(test_context)?; From 0f3f76ae9656e0538af5569abf86b000ad8f496b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 26 Oct 2025 11:16:53 +0000 Subject: [PATCH 6/6] refactor: [#39] move cleanup_previous_test_data to preflight_cleanup module - Move cleanup_previous_test_data from e2e_tests_full.rs to preflight_cleanup.rs - Fix preflight cleanup ordering: clean data BEFORE CreateCommandHandler - Add TODO comment for future TestContext refactoring - Separate early data cleanup (no TestContext) from full preflight cleanup - Prevents 'environment already exists' errors by cleaning before repository check The current workaround requires two cleanup functions because TestContext needs an Environment, but we must clean data before creating the Environment. Future refactoring should separate Environment from TestContext for better design. --- src/bin/e2e_tests_full.rs | 11 ++- src/testing/e2e/tasks/preflight_cleanup.rs | 94 ++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/bin/e2e_tests_full.rs b/src/bin/e2e_tests_full.rs index c9336ca8..43185404 100644 --- a/src/bin/e2e_tests_full.rs +++ b/src/bin/e2e_tests_full.rs @@ -66,6 +66,7 @@ use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder use torrust_tracker_deployer_lib::shared::{Clock, SystemClock}; use torrust_tracker_deployer_lib::testing::e2e::context::{TestContext, TestContextType}; use torrust_tracker_deployer_lib::testing::e2e::tasks::{ + preflight_cleanup::cleanup_previous_test_data, run_configure_command::run_configure_command, run_create_command::run_create_command, run_test_command::run_test_command, @@ -111,6 +112,7 @@ struct Cli { /// /// May panic during the match statement if unexpected error combinations occur /// that are not handled by the current error handling logic. +#[allow(clippy::too_many_lines)] #[tokio::main] pub async fn main() -> Result<()> { let cli = Cli::parse(); @@ -134,6 +136,11 @@ pub async fn main() -> Result<()> { let ssh_private_key_path = project_root.join("fixtures/testing_rsa"); let ssh_public_key_path = project_root.join("fixtures/testing_rsa.pub"); + // Cleanup any artifacts from previous test runs BEFORE creating the environment + // This prevents "environment already exists" errors from stale state + // We do this before CreateCommandHandler because it checks if environment exists in repository + cleanup_previous_test_data("e2e-full").map_err(|e| anyhow::anyhow!("{e}"))?; + // Create repository factory and clock for environment creation let repository_factory = RepositoryFactory::new(Duration::from_secs(30)); let clock: Arc = Arc::new(SystemClock); @@ -154,8 +161,8 @@ pub async fn main() -> Result<()> { TestContext::from_environment(cli.keep, environment, TestContextType::VirtualMachine)? .init()?; - // Cleanup any artifacts from previous test runs that may have failed to clean up - // This ensures a clean slate before starting new tests + // Additional preflight cleanup for infrastructure (OpenTofu, LXD resources) + // This handles any lingering infrastructure from interrupted previous runs preflight_cleanup_previous_resources(&test_context)?; let test_start = Instant::now(); diff --git a/src/testing/e2e/tasks/preflight_cleanup.rs b/src/testing/e2e/tasks/preflight_cleanup.rs index 3c589871..0f4d8484 100644 --- a/src/testing/e2e/tasks/preflight_cleanup.rs +++ b/src/testing/e2e/tasks/preflight_cleanup.rs @@ -48,6 +48,100 @@ impl std::error::Error for PreflightCleanupError { } } +// TODO: Refactor TestContext to eliminate the need for this workaround function +// +// Current issue: TestContext requires an Environment, but we need to clean up data +// directories BEFORE creating the Environment (because CreateCommandHandler checks +// if the environment already exists in the repository). +// +// Proposed solutions: +// 1. Make Environment optional in TestContext (TestContext { environment: Option }) +// 2. Move Environment out of TestContext (preferred - better separation of concerns) +// +// The second option is better because: +// - TestContext should manage test infrastructure (services, config, temp directories) +// - Environment is a domain entity that represents deployment state +// - Separating them provides clearer responsibilities and easier testing +// +// After refactoring, we could eliminate this standalone function and have all cleanup +// go through a single preflight_cleanup_previous_resources() that doesn't require +// a fully initialized TestContext with an Environment. + +/// Cleans the data directory for a specific environment name before `TestContext` creation +/// +/// This helper function removes the `data/{environment_name}` directory to prevent +/// "environment already exists" errors when `CreateCommandHandler` checks the repository. +/// Unlike `cleanup_data_environment`, this function works without a `TestContext` and is +/// intended to be called early in the test setup before environment creation. +/// +/// # Safety +/// +/// This function is only intended for E2E test environments and should never +/// be called in production code paths. It's designed to provide test isolation +/// by ensuring environments from previous test runs don't interfere. +/// +/// # Arguments +/// +/// * `environment_name` - The name of the environment to clean up +/// +/// # Returns +/// +/// Returns `Ok(())` if cleanup succeeds or if the directory doesn't exist. +/// +/// # Errors +/// +/// Returns a `PreflightCleanupError::ResourceConflicts` error if the data directory +/// cannot be removed due to permission issues or file locks. +pub fn cleanup_previous_test_data(environment_name: &str) -> Result<(), PreflightCleanupError> { + use std::path::Path; + + let data_dir = Path::new("data").join(environment_name); + + if !data_dir.exists() { + info!( + operation = "preflight_data_cleanup", + status = "clean", + path = %data_dir.display(), + "No previous data directory found, skipping cleanup" + ); + return Ok(()); + } + + info!( + operation = "preflight_data_cleanup", + path = %data_dir.display(), + "Cleaning data directory from previous test run" + ); + + match std::fs::remove_dir_all(&data_dir) { + Ok(()) => { + info!( + operation = "preflight_data_cleanup", + status = "success", + path = %data_dir.display(), + "Data directory cleaned successfully" + ); + Ok(()) + } + Err(e) => { + warn!( + operation = "preflight_data_cleanup", + status = "failed", + path = %data_dir.display(), + error = %e, + "Failed to clean data directory" + ); + Err(PreflightCleanupError::ResourceConflicts { + details: format!( + "Failed to clean data directory '{}': {}", + data_dir.display(), + e + ), + }) + } + } +} + /// Cleans the build directory to ensure fresh template state for E2E tests /// /// This function removes the build directory if it exists, ensuring that