Update E2E full tests to use CreateCommandHandler and fix preflight cleanup

[Copilot]

Subissue 5/7 of #34 - The E2E full test suite was creating environments directly via Environment::new() instead of exercising the new CreateCommandHandler, missing test coverage of the command layer. Additionally, the E2E preflight cleanup was not removing stale environment data from previous test runs, causing test failures.

Changes

• Created run_create_command task module: New module in src/testing/e2e/tasks/run_create_command.rs that encapsulates environment creation using CreateCommandHandler, following the established pattern of other E2E tasks like run_configure_command
• Replaced direct environment creation: Main test flow in e2e_tests_full.rs now uses the task module instead of Environment::new(), exercising the complete command pattern workflow
• Improved error handling: Task module includes comprehensive error handling with CreateTaskError enum and help() method for detailed troubleshooting guidance
• Consistent patterns: Uses RepositoryFactory and Clock as parameters, and "data" as base directory, matching the pattern in TestContext.create_repository()
• Added data directory cleanup: New cleanup_data_environment() function in src/testing/e2e/tasks/preflight_cleanup.rs that removes data/{environment_name} directories from previous test runs, preventing "environment already exists" errors and ensuring proper test isolation

Module Structure

The new task follows the same structure as other E2E tasks:

• Located in src/testing/e2e/tasks/run_create_command.rs
• Exported from src/testing/e2e/tasks/mod.rs
• Documented as an infrastructure-agnostic task

Preflight Cleanup Enhancement

The preflight cleanup now includes data directory removal in addition to build and templates cleanup:

• Container preflight (src/testing/e2e/tasks/container/preflight_cleanup.rs): Calls cleanup_data_environment() before Docker cleanup
• VM preflight (src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs): Calls cleanup_data_environment() before OpenTofu/LXD cleanup

This ensures E2E tests always start with a clean slate, preventing failures from stale environment state left by interrupted previous test runs.

Example

// Before: Direct construction bypassed command layer
let environment = Environment::new(environment_name, ssh_credentials, ssh_port);

// After: Exercises CreateCommandHandler via task module with full validation and persistence
let repository_factory = RepositoryFactory::new(Duration::from_secs(30));
let clock: Arc<dyn Clock> = Arc::new(SystemClock);

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,
)?;

The E2E test suite now validates the command handler end-to-end through a properly structured task module, ensuring repository persistence and configuration validation work correctly in real deployment scenarios while maintaining consistency with the existing E2E testing architecture. The enhanced preflight cleanup guarantees reliable test execution by removing all artifacts from previous runs.

• Fixes [Subissue 5/7] Update E2E Full Tests to Use Create Command #39

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Subissue 5/7] Update E2E Full Tests to Use Create Command</issue_title>
<issue_description>Parent Epic: #34 - Implement Create Environment Command
Depends On: #36 - Application Layer Command
Estimated Time: 1-2 hours

Overview

Update the src/bin/e2e_tests_full.rs to use the new create command handler instead of direct environment creation. This ensures the full E2E test exercises the complete create command functionality as part of the comprehensive test suite.

Goals

• Add new function to create environment using CreateCommand handler
• Update e2e_tests_full.rs to use the new create command for environment creation
• Maintain existing test flow while exercising create command logic
• Ensure comprehensive test coverage of create command in full E2E context
• Preserve existing test reliability and performance

Implementation Summary

• Location: src/bin/e2e_tests_full.rs
• Approach: Add create_environment_via_command() function that uses CreateCommand handler
• Integration: Replace direct environment creation with command-based creation
• Testing: Not black-box like Subissue 4 - this uses the command handler directly

Key Changes

// Add function to create environment using CreateCommand
async fn create_environment_via_command(context: &TestContext) -> Result<Environment<Created>> {
    let create_command = CreateCommand::new(context.environment_repository.clone());
    
    let config = EnvironmentCreationConfig {
        environment_name: context.environment_name.clone(),
        ssh_credentials: context.ssh_credentials.clone(),
    };
    
    create_command.execute(config)
}

// Replace direct environment creation in main test function
let environment = create_environment_via_command(&context)
    .await
    .expect("Failed to create environment via command");

Acceptance Criteria

• E2E full tests use CreateCommand handler for environment creation
• All existing E2E test functionality remains intact
• Create command is properly exercised in comprehensive test suite
• Test execution time and reliability maintained
• Proper error handling and logging preserved

For detailed specification, see: docs/issues/epic-create-environment-command-subissue-5-update-e2e-full-tests.md</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [Subissue 5/7] Update E2E Full Tests to Use Create Command #39

---

💡 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 in the docs.

[josecelano]

@copilot It looks good but move the new function "create_environment_via_command" to src/testing/e2e/tasks where we have other tasks for E2E testing like:

• src/testing/e2e/tasks/run_configure_command.rs

Besides, you do not need to calculate the working directory. Take a look at the "run_configure_command.rs" module. You can use the TestContext to access test context information.

[Copilot]

@copilot It looks good but move the new function "create_environment_via_command" to src/testing/e2e/tasks where we have other tasks for E2E testing like:

• src/testing/e2e/tasks/run_configure_comma...

Done! Moved to src/testing/e2e/tasks/run_create_command.rs following the pattern from run_configure_command.rs.

The function accepts RepositoryFactory and Clock as parameters (rather than TestContext) since we need the Environment to create a TestContext, which would create a circular dependency. It now uses "data" as the base directory, matching the pattern in TestContext.create_repository().

Commit: ce59c6a

[josecelano]

Hi @copilot — quick code review / automation note for PR #47.

Summary

I ran a quick local analysis of the E2E preflight cleanup and found that the current preflight flow cleans the build/ and templates directories but does not remove E2E environment data under data/ from previous runs. This leaves stale environment directories (and related repository state) behind and can cause cargo run --bin e2e-tests-full to fail when a previous environment already exists.

Where I looked

• src/testing/e2e/tasks/preflight_cleanup.rs — provides cleanup_build_directory and cleanup_templates_directory but no data cleanup.
• src/testing/e2e/tasks/container/preflight_cleanup.rs — calls cleanup_build_directory and cleanup_templates_directory (and docker container cleanup) — no data cleanup.
• src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs — same pattern: build + templates + OpenTofu + LXD cleanup, but no explicit data/ cleanup.
• src/presentation/commands/destroy.rs — the production destroy command exists and uses the data repository to remove environments; there is no separate e2e-destroy-tests binary.

Why this is a problem

• E2E preflight should guarantee a clean start for each run. Currently we remove build/ and templates, but leftover data/<env> directories still cause collisions when the test tries to create or load an environment.
• When a previous run failed to destroy state, tests can fail early (repository load / state conflicts), and the intended safety of the preflight step is lost.

Suggested fix (high-level)

Add an explicit data/env cleanup step in the preflight flow. Two safe approaches:

A) Remove the test environment data directory(s) directly in preflight

• Add a cleanup_data_environment(test_context: &TestContext) -> Result<(), PreflightCleanupError> helper in src/testing/e2e/tasks/preflight_cleanup.rs.
• It should compute the env path e.g. let data_env_dir = Path::new("data").join(test_context.environment.instance_name().as_str()); (or use the proper path in Config if available), check existence, and call std::fs::remove_dir_all(&data_env_dir).
• Call this helper from both container and virtual-machine preflight implementations (immediately after cleanup_templates_directory / cleanup_build_directory and before infra cleanup).

B) (Safer / more semantically correct) Invoke the existing production destroy flow

• Programmatically call the destroy handler for the environment (the code path exists in src/presentation/commands/destroy.rs) so the repository and infra teardown logic is used, rather than directly deleting files.
• This avoids leaving behind provider-specific artifacts that a raw remove_dir_all might miss.

Concrete minimal code snippet (Option A) to add to src/testing/e2e/tasks/preflight_cleanup.rs:

// new helper
pub fn cleanup_data_environment(test_context: &TestContext) -> Result<(), PreflightCleanupError> {
    use std::path::Path;

    let data_dir = Path::new("data").join(test_context.environment.instance_name().as_str());

    if !data_dir.exists() {
        tracing::info!(operation = "data_directory_cleanup", status = "clean", path = %data_dir.display(), "Data directory doesn't exist, skipping cleanup");
        return Ok(());
    }

    tracing::info!(operation = "data_directory_cleanup", path = %data_dir.display(), "Cleaning data directory for previous test environment");

    std::fs::remove_dir_all(&data_dir).map_err(|e| PreflightCleanupError::ResourceConflicts {
        details: format!("Failed to clean data directory '{}': {}", data_dir.display(), e),
    })?;

    tracing::info!(operation = "data_directory_cleanup", status = "success", path = %data_dir.display(), "Data directory cleaned successfully");
    Ok(())
}

Then call cleanup_data_environment(test_context)?; from

• src/testing/e2e/tasks/container/preflight_cleanup.rs::preflight_cleanup_previous_resources
• src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs::preflight_cleanup_previous_resources

Placement: run data cleanup after templates/build cleanup and before any provider-specific destroy logic.

Notes about using the production destroy command

• There's no special e2e-destroy-tests binary — that's intentional. The production destroy command already exists and can be used to tear down environments. If you prefer the production path, call the destroy handler programmatically (or run the CLI from the test harness):

  cargo run -- <args> will invoke the main binary; for programmatic use within tests, call the destruction handler in presentation::commands::destroy.

Recommendation

• Add the cleanup_data_environment helper (Option A) as quick, safe remediation; or
• Prefer Option B if you want to consistently apply all production destroy semantics during preflight — but be careful to handle partial failures and keep preflight resilient.

If you want, I can open a small patch to add cleanup_data_environment and call it from both preflight entrypoints (container + virtual machine) so e2e-tests-full reliably starts from a clean state.

Thanks — tagging you so you can update the PR: @copilot

[Copilot]

Hi @copilot — quick code review / automation note for PR #47.

Summary...

Implemented! Added cleanup_data_environment() helper function to src/testing/e2e/tasks/preflight_cleanup.rs that removes the data/{environment_name} directory before E2E test runs.

The function is now called from both:

• src/testing/e2e/tasks/container/preflight_cleanup.rs
• src/testing/e2e/tasks/virtual_machine/preflight_cleanup.rs

This prevents "environment already exists" errors when previous test runs fail to clean up properly, ensuring reliable E2E test execution with proper isolation.

Commit: 0e7d5a0

[josecelano]

ACK 0f3f76a