diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 9201a4b2..70f28374 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -145,3 +145,28 @@ jobs: - id: test name: Run Unit Tests run: cargo test --tests --benches --examples --workspace --all-targets --all-features + + - id: verify-test-isolation + name: Verify Test Isolation (data folder should be empty) + run: | + echo "Checking if data/ folder is polluted by tests..." + if [ -d "data" ]; then + FILE_COUNT=$(find data/ -mindepth 1 -type f | wc -l) + DIR_COUNT=$(find data/ -mindepth 1 -type d | wc -l) + echo "Files in data/: $FILE_COUNT" + echo "Subdirectories in data/: $DIR_COUNT" + + if [ $FILE_COUNT -gt 0 ] || [ $DIR_COUNT -gt 0 ]; then + echo "❌ ERROR: Tests polluted the production data/ folder!" + echo "Contents of data/ folder:" + ls -laR data/ + echo "" + echo "Tests must use temporary directories (TempWorkspace) with proper isolation." + echo "See docs/contributing/testing/ for best practices." + exit 1 + else + echo "✅ Test isolation verified: data/ folder is clean" + fi + else + echo "✅ Test isolation verified: data/ folder does not exist" + fi diff --git a/docs/refactors/active-refactorings.md b/docs/refactors/active-refactorings.md index 6fedc51e..1b412de2 100644 --- a/docs/refactors/active-refactorings.md +++ b/docs/refactors/active-refactorings.md @@ -1,6 +1,7 @@ # Active Refactoring Plans -| Document | Status | Issue | Target | Created | -| ----------------------------------------------------------------------------------------------------- | ----------- | ----- | ----------------------------------------- | ---------- | -| [Separate View Data from Views](plans/separate-view-data-from-views.md) | 📋 Planning | TBD | Organize presentation layer command views | 2026-02-16 | -| [Simplify Controller Command Handler Creation](plans/simplify-controller-command-handler-creation.md) | 📋 Planning | TBD | Remove unnecessary progress steps | 2025-12-17 | +| Document | Status | Issue | Target | Created | +| ----------------------------------------------------------------------------------------------------- | ----------- | ---------------------------------------------------------------------- | ----------------------------------------- | ---------- | +| [E2E Test Isolation - Complete Log Directory Support](plans/e2e-test-isolation-log-dir.md) | 📋 Planning | [#365](https://github.com/torrust/torrust-tracker-deployer/issues/365) | Add log_dir to all E2E tests | 2026-02-18 | +| [Separate View Data from Views](plans/separate-view-data-from-views.md) | 📋 Planning | TBD | Organize presentation layer command views | 2026-02-16 | +| [Simplify Controller Command Handler Creation](plans/simplify-controller-command-handler-creation.md) | 📋 Planning | TBD | Remove unnecessary progress steps | 2025-12-17 | diff --git a/docs/refactors/plans/e2e-test-isolation-log-dir.md b/docs/refactors/plans/e2e-test-isolation-log-dir.md new file mode 100644 index 00000000..7b7fc939 --- /dev/null +++ b/docs/refactors/plans/e2e-test-isolation-log-dir.md @@ -0,0 +1,508 @@ +# E2E Test Isolation - Complete Log Directory Support + +## 📋 Overview + +Add `.log_dir()` support to all E2E test ProcessRunner calls to achieve complete test isolation and prevent pollution of the production `data/` folder. Currently, tests use `.working_dir()` for environment data isolation but logs still go to the default `./data/logs/` location, violating the test isolation principle. + +**Target Files:** + +- `tests/e2e/create_command.rs` (5 ProcessRunner calls) +- `tests/e2e/destroy_command.rs` (7 ProcessRunner calls) +- `tests/e2e/list_command.rs` (6 ProcessRunner calls) +- `tests/e2e/purge_command.rs` (14 ProcessRunner calls) +- `tests/e2e/show_command.rs` (6 ProcessRunner calls) +- `tests/e2e/validate_command.rs` (4 ProcessRunner calls) + +**Already Fixed:** + +- ✅ `tests/e2e/render_command.rs` (12 ProcessRunner calls) - Fixed in commit d21754f6 +- ✅ `tests/validate_ai_training_examples.rs` (2 ProcessRunner calls) - Fixed in commit d21754f6 +- ✅ `src/testing/e2e/process_runner.rs` - All 14 command methods now support `--log-dir` argument + +**Scope:** + +- Add `.log_dir(temp_workspace.path().join("logs"))` to all ProcessRunner calls in remaining E2E test files +- Verify complete test isolation: `data/` folder must remain completely empty after running all tests +- Follow the pattern established in render_command.rs tests + +## 📊 Progress Tracking + +**Total Active Proposals**: 6 +**Total Postponed**: 0 +**Total Discarded**: 0 +**Completed**: 6 +**In Progress**: 0 +**Not Started**: 0 + +### Phase Summary + +- **Phase 0 - Validate Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit 1d576a5a +- **Phase 1 - Create Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit e452efe6 +- **Phase 2 - List Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit 53b87a8d +- **Phase 3 - Show Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit 5682ae05 +- **Phase 4 - Destroy Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit 82946459 +- **Phase 5 - Purge Command Tests (High Impact, Low Effort)**: ✅ 1/1 completed (100%) - Commit 1c437d80 + +### Discarded Proposals + +None + +### Postponed Proposals + +None + +## 🎯 Key Problems Identified + +### 1. Incomplete Test Isolation + +Tests currently use `TempWorkspace` and `.working_dir()` for environment data isolation, but logs still write to production `./data/logs/`: + +```rust +// Current pattern (incomplete isolation) +let temp_workspace = TempWorkspace::new().expect("Failed to create temp workspace"); +let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) // ✅ Environment data isolated + .run_create_command("./environment.json") // ❌ Logs go to ./data/logs + .expect("Failed to run create command"); +``` + +After running `cargo test`, the production `data/logs/` directory contains a 37KB+ log file, violating the principle that tests should not pollute production directories. + +### 2. Independent Global Arguments + +The `--log-dir` and `--working-dir` arguments are independent by design (documented in ADR and code). This means: + +- Setting `--working-dir` does **not** automatically redirect logs +- Tests must explicitly pass both arguments for complete isolation +- ProcessRunner now supports `.log_dir()` builder method (added in commit d21754f6) + +### 3. Inconsistent Test Hygiene + +Some tests (render_command.rs, validate_ai_training_examples.rs) now follow proper isolation, while others don't: + +- ✅ **Properly Isolated**: render_command.rs (12 calls), validate_ai_training_examples.rs (2 calls) +- ❌ **Still Polluting**: create_command.rs, destroy_command.rs, list_command.rs, purge_command.rs, show_command.rs, validate_command.rs + +This creates technical debt and makes it unclear which tests follow best practices. + +## 🚀 Refactoring Phases + +--- + +## Phase 0: Validate Command Tests (Highest Priority) + +**Rationale**: Start with the simplest file (4 calls) to establish momentum and verify the pattern works across different test scenarios. + +### Proposal #0: Add log_dir to validate_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High (Prevents production folder pollution) +**Effort**: 🔵 Low (4 ProcessRunner calls, simple pattern) +**Priority**: P0 +**Depends On**: None +**Target File**: `tests/e2e/validate_command.rs` + +#### Problem + +The validate command tests create 4 ProcessRunner instances without `.log_dir()`, causing logs to write to production `./data/logs/`: + +```rust +// Line 58 +let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) + .run_validate_command(&config_path) + .expect("Failed to run validate command"); +``` + +This pattern repeats 4 times in the file, polluting the production data folder. + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all ProcessRunner calls: + +```rust +let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) // ✅ Complete isolation + .run_validate_command(&config_path) + .expect("Failed to run validate command"); +``` + +#### Rationale + +- Follows the established pattern from render_command.rs (commit d21754f6) +- ProcessRunner.run_validate_command() already supports `--log-dir` argument +- Each test uses TempWorkspace, so logs go to temp directory and are auto-cleaned +- Simple mechanical transformation with clear benefits + +#### Benefits + +- ✅ Prevents pollution of production `data/logs/` directory +- ✅ Complete test isolation (both environment data and logs) +- ✅ Consistent with other properly isolated tests +- ✅ Tests auto-clean logs with TempWorkspace cleanup + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~58 +- [x] Add `.log_dir()` to ProcessRunner call at line ~97 +- [x] Add `.log_dir()` to ProcessRunner call at line ~146 +- [x] Add `.log_dir()` to ProcessRunner call at line ~201 +- [x] Run `cargo test --test e2e_integration validate_command` +- [x] Verify all tests pass (4 tests expected) +- [x] Clean data folder: `rm -rf data/*` +- [x] Run tests again and verify `data/` folder remains empty +- [x] Commit changes: `fix: [#365] add log_dir to validate_command tests for complete isolation` + +**Status**: ✅ Completed (Commit 1d576a5a) +**Result**: All 4 tests pass, data/ folder remains empty + +#### Testing Strategy + +Before changes: + +```bash +rm -rf data/* && cargo test --test e2e_integration validate_command && ls -la data/ +# Expected: data/logs/ directory created with log file +``` + +After changes: + +```bash +rm -rf data/* && cargo test --test e2e_integration validate_command && ls -la data/ +# Expected: data/ folder completely empty (only . and ..) +``` + +#### Results (if completed) + +- **Lines Removed**: 0 +- **Lines Added**: 4 (one `.log_dir()` call per ProcessRunner) +- **Net Change**: +4 lines +- **Tests**: [Status] +- **Linters**: [Status] + +--- + +## Phase 1: Create Command Tests + +**Rationale**: Second simplest file (5 calls), establishes pattern for command tests that create environments. + +### Proposal #1: Add log_dir to create_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High +**Effort**: 🔵 Low (5 ProcessRunner calls) +**Priority**: P1 +**Depends On**: Proposal #0 (validate pattern works) +**Target File**: `tests/e2e/create_command.rs` + +#### Problem + +5 ProcessRunner calls without `.log_dir()` at lines: ~69, ~104, ~131, ~163, ~175 + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all 5 ProcessRunner calls. + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~69 +- [x] Add `.log_dir()` to ProcessRunner call at line ~104 +- [x] Add `.log_dir()` to ProcessRunner call at line ~131 +- [x] Add `.log_dir()` to ProcessRunner call at line ~163 +- [x] Add `.log_dir()` to ProcessRunner call at line ~175 +- [x] Run `cargo test --test e2e_integration create_command` +- [x] Verify all tests pass +- [x] Clean data folder and verify it remains empty after tests +- [x] Commit changes: `fix: [#365] add log_dir to create_command tests for complete isolation` + +**Status**: ✅ Completed (Commit e452efe6) +**Result**: All 4 tests pass, data/ folder remains empty + +#### Testing Strategy + +Verify `data/` folder remains empty after running create_command tests. + +--- + +## Phase 2: List Command Tests + +**Rationale**: Build on established pattern with slightly more complex scenarios (6 calls). + +### Proposal #2: Add log_dir to list_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High +**Effort**: 🔵 Low (6 ProcessRunner calls) +**Priority**: P2 +**Depends On**: Proposal #1 +**Target File**: `tests/e2e/list_command.rs` + +#### Problem + +6 ProcessRunner calls without `.log_dir()` at lines: ~53, ~91, ~107, ~142, ~159, ~176 + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all 6 ProcessRunner calls. + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~53 +- [x] Add `.log_dir()` to ProcessRunner call at line ~91 +- [x] Add `.log_dir()` to ProcessRunner call at line ~107 +- [x] Add `.log_dir()` to ProcessRunner call at line ~142 +- [x] Add `.log_dir()` to ProcessRunner call at line ~159 +- [x] Add `.log_dir()` to ProcessRunner call at line ~176 +- [x] Run `cargo test --test e2e_integration list_command` +- [x] Verify all tests pass +- [x] Clean data folder and verify it remains empty after tests +- [x] Commit changes: `fix: [#365] add log_dir to list_command tests for complete isolation` + +**Status**: ✅ Completed (Commit 53b87a8d) +**Result**: All 3 tests pass, data/ folder remains empty + +#### Testing Strategy + +Verify `data/` folder remains empty after running list_command tests. + +--- + +## Phase 3: Show Command Tests + +**Rationale**: Continue pattern with show command tests (6 calls). + +### Proposal #3: Add log_dir to show_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High +**Effort**: 🔵 Low (6 ProcessRunner calls) +**Priority**: P3 +**Depends On**: Proposal #2 +**Target File**: `tests/e2e/show_command.rs` + +#### Problem + +6 ProcessRunner calls without `.log_dir()` at lines: ~52, ~90, ~106, ~142, ~154, ~191, ~203 + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all 6 ProcessRunner calls. + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~52 +- [x] Add `.log_dir()` to ProcessRunner call at line ~90 +- [x] Add `.log_dir()` to ProcessRunner call at line ~106 +- [x] Add `.log_dir()` to ProcessRunner call at line ~142 +- [x] Add `.log_dir()` to ProcessRunner call at line ~154 +- [x] Add `.log_dir()` to ProcessRunner call at line ~191 +- [x] Add `.log_dir()` to ProcessRunner call at line ~203 +- [x] Run `cargo test --test e2e_integration show_command` +- [x] Verify all tests pass +- [x] Clean data folder and verify it remains empty after tests +- [x] Commit changes: `fix: [#365] add log_dir to show_command tests for complete isolation` + +**Status**: ✅ Completed (Commit 5682ae05) +**Result**: All 4 tests pass, data/ folder remains empty + +#### Testing Strategy + +Verify `data/` folder remains empty after running show_command tests. + +--- + +## Phase 4: Destroy Command Tests + +**Rationale**: More complex scenarios with environment lifecycle (7 calls). + +### Proposal #4: Add log_dir to destroy_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High +**Effort**: 🔵 Low (7 ProcessRunner calls) +**Priority**: P4 +**Depends On**: Proposal #3 +**Target File**: `tests/e2e/destroy_command.rs` + +#### Problem + +7 ProcessRunner calls without `.log_dir()` at lines: ~67, ~83, ~117, ~133, ~161, ~194, ~211 + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all 7 ProcessRunner calls. + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~67 +- [x] Add `.log_dir()` to ProcessRunner call at line ~83 +- [x] Add `.log_dir()` to ProcessRunner call at line ~117 +- [x] Add `.log_dir()` to ProcessRunner call at line ~133 +- [x] Add `.log_dir()` to ProcessRunner call at line ~161 +- [x] Add `.log_dir()` to ProcessRunner call at line ~194 +- [x] Add `.log_dir()` to ProcessRunner call at line ~211 +- [x] Run `cargo test --test e2e_integration destroy_command` +- [x] Verify all tests pass +- [x] Clean data folder and verify it remains empty after tests +- [x] Commit changes: `fix: [#365] add log_dir to destroy_command tests for complete isolation` + +**Status**: ✅ Completed (Commit 82946459) +**Result**: All 4 tests pass, data/ folder remains empty + +#### Testing Strategy + +Verify `data/` folder remains empty after running destroy_command tests. + +--- + +## Phase 5: Purge Command Tests (Final Phase) + +**Rationale**: Most complex file with most ProcessRunner calls (14 calls). Save for last to ensure pattern is solid. + +### Proposal #5: Add log_dir to purge_command.rs Tests + +**Status**: ⏳ Not Started +**Impact**: 🟢🟢🟢 High +**Effort**: 🔵🔵 Medium (14 ProcessRunner calls - largest file) +**Priority**: P5 +**Depends On**: Proposal #4 +**Target File**: `tests/e2e/purge_command.rs` + +#### Problem + +14 ProcessRunner calls without `.log_dir()` at lines: ~75, ~87, ~104, ~140, ~174, ~186, ~198, ~233, ~249, ~264, ~303, ~316, ~324, ~331, ~344 + +#### Proposed Solution + +Add `.log_dir(temp_workspace.path().join("logs"))` to all 14 ProcessRunner calls. + +#### Implementation Checklist + +- [x] Add `.log_dir()` to ProcessRunner call at line ~75 +- [x] Add `.log_dir()` to ProcessRunner call at line ~87 +- [x] Add `.log_dir()` to ProcessRunner call at line ~104 +- [x] Add `.log_dir()` to ProcessRunner call at line ~140 +- [x] Add `.log_dir()` to ProcessRunner call at line ~174 +- [x] Add `.log_dir()` to ProcessRunner call at line ~186 +- [x] Add `.log_dir()` to ProcessRunner call at line ~198 +- [x] Add `.log_dir()` to ProcessRunner call at line ~233 +- [x] Add `.log_dir()` to ProcessRunner call at line ~249 +- [x] Add `.log_dir()` to ProcessRunner call at line ~264 +- [x] Add `.log_dir()` to ProcessRunner call at line ~303 +- [x] Add `.log_dir()` to ProcessRunner call at line ~316 +- [x] Add `.log_dir()` to ProcessRunner call at line ~324 +- [x] Add `.log_dir()` to ProcessRunner call at line ~331 +- [x] Add `.log_dir()` to ProcessRunner call at line ~344 +- [x] Run `cargo test --test e2e_integration purge_command` +- [x] Verify all tests pass +- [x] Clean data folder and verify it remains empty after tests +- [x] Commit changes: `fix: [#365] add log_dir to purge_command tests for complete isolation` + +**Status**: ✅ Completed (Commit 1c437d80) +**Result**: All 5 tests pass, data/ folder remains empty +**Note**: Found 15 ProcessRunner calls (not 14 as estimated) + +#### Testing Strategy + +Verify `data/` folder remains empty after running purge_command tests. + +--- + +## 📈 Timeline + +- **Start Date**: 2026-02-18 +- **Target Completion**: 2026-02-18 (same day - low effort refactoring) +- **Actual Completion**: 2026-02-18 + +**Final Statistics**: + +- Total ProcessRunner calls fixed: 45 (4 + 5 + 6 + 7 + 7 + 15 + 1 additional) +- Total test files modified: 6 (validate, create, list, show, destroy, purge commands) +- Total commits: 6 (one per phase) +- All tests pass: 408 passed; 0 failed; 95 ignored +- Production data/ folder: Completely empty after all tests + +## 🔍 Review Process + +### Approval Criteria + +- [x] Technical feasibility validated (ProcessRunner already supports `--log-dir`) +- [x] Aligns with [Development Principles](../development-principles.md) (Testability, Observability) +- [x] Pattern proven in render_command.rs tests (commit d21754f6) +- [x] Implementation plan is clear and actionable + +### Completion Criteria + +- [ ] All 6 proposals implemented (44 ProcessRunner calls updated) +- [ ] Run `cargo test` and verify all tests pass +- [ ] Clean `data/` folder and run `cargo test` again +- [ ] Verify `data/` folder remains completely empty (only `.` and `..`) +- [ ] All linters passing (`cargo run --bin linter all`) +- [ ] Changes committed and pushed to branch 365-fix-render-working-dir +- [ ] PR #366 updated with final verification + +## 📚 Related Documentation + +- [Development Principles](../development-principles.md) - Testability principle +- [Testing Conventions](../contributing/testing/unit-testing/naming-conventions.md) +- [Issue #365](https://github.com/torrust/torrust-tracker-deployer/issues/365) - Original bug report +- [PR #366](https://github.com/torrust/torrust-tracker-deployer/pull/366) - Fix for render command +- [ADR: Environment Variable Independence](../decisions/environment-variable-prefix.md) - Explains why `--log-dir` and `--working-dir` are independent + +## 💡 Notes + +### Pattern to Follow + +All ProcessRunner calls should follow this pattern: + +```rust +let temp_workspace = TempWorkspace::new().expect("Failed to create temp workspace"); + +let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) // ✅ Isolate environment data + .log_dir(temp_workspace.path().join("logs")) // ✅ Isolate log files + .run_COMMAND_command(...) // Any command + .expect("Failed to run command"); +``` + +### Verification Command + +After implementing all phases, run this comprehensive verification: + +```bash +# Clean data folder +rm -rf data/* + +# Run ALL tests +cargo test + +# Verify data folder is empty +ls -la data/ +# Expected output: +# total 8 +# drwxrwxr-x 2 user user 4096 Feb 18 XX:XX . +# drwxrwxr-x 22 user user 4096 Feb 18 XX:XX .. +``` + +### Discovery Notes + +- Initial investigation found 56 total ProcessRunner calls across all E2E tests +- render_command.rs (12 calls) and validate_ai_training_examples.rs (2 calls) already fixed +- Remaining: 44 calls across 6 test files +- All ProcessRunner command methods now support `--log-dir` (implemented in commit d21754f6) +- Pattern proven to work - just needs mechanical application to remaining test files + +### Future Improvements + +After this refactoring: + +- Consider adding a ProcessRunner builder method that automatically sets both `.working_dir()` and `.log_dir()` from a single TempWorkspace +- Document the pattern in testing conventions guide +- Add a lint rule or test to catch future ProcessRunner usages without `.log_dir()` + +--- + +**Created**: 2026-02-18 +**Last Updated**: 2026-02-18 +**Status**: 📋 Planning diff --git a/project-words.txt b/project-words.txt index abd6547e..f42bb727 100644 --- a/project-words.txt +++ b/project-words.txt @@ -155,6 +155,7 @@ downcasting downloadedi dpkg drwxr +drwxrwxr dtolnay ehthumbs elif diff --git a/src/application/command_handlers/render/handler.rs b/src/application/command_handlers/render/handler.rs index 1c9ce901..18e608f3 100644 --- a/src/application/command_handlers/render/handler.rs +++ b/src/application/command_handlers/render/handler.rs @@ -225,7 +225,7 @@ impl RenderCommandHandler { config_path: &Path, ip_addr: IpAddr, output_dir: &Path, - _working_dir: &Path, + working_dir: &Path, ) -> Result { info!( config_file = %config_path.display(), @@ -260,13 +260,10 @@ impl RenderCommandHandler { // Create a temporary environment for template rendering (not persisted) let env_name = params.environment_name.clone(); let clock: Arc = Arc::new(SystemClock); - let created_env = Environment::::new( - params.environment_name, - params.provider_config, - params.ssh_credentials, - params.ssh_port, - clock.now(), - ); + let created_env = Environment::::create(params, working_dir, clock.now()) + .map_err(|e| RenderCommandHandlerError::DomainValidationFailed { + reason: e.to_string(), + })?; // Render all templates self.render_all_templates(&created_env, ip_addr, output_dir) diff --git a/src/presentation/controllers/render/handler.rs b/src/presentation/controllers/render/handler.rs index bc6e782c..049f10a4 100644 --- a/src/presentation/controllers/render/handler.rs +++ b/src/presentation/controllers/render/handler.rs @@ -101,6 +101,7 @@ impl RenderCommandController { /// * `ip` - Target instance IP address (required) /// * `output_dir` - Output directory for generated artifacts (required) /// * `force` - Whether to overwrite existing output directory + /// * `working_dir` - Working directory for environment data (from --working-dir global arg) /// /// # Returns /// @@ -123,6 +124,7 @@ impl RenderCommandController { ip: &str, output_dir: &Path, force: bool, + working_dir: &Path, ) -> Result<(), RenderCommandError> { // Step 1: Validate input self.progress @@ -169,12 +171,8 @@ impl RenderCommandController { self.progress .start_step(RenderStep::LoadConfiguration.description())?; - // Get working directory - let working_dir = std::env::current_dir().map_err(|e| { - RenderCommandError::WorkingDirectoryUnavailable { - reason: e.to_string(), - } - })?; + // working_dir is now passed as a parameter from the router + // which gets it from context.working_dir() (the --working-dir global argument) self.progress.complete_step(None)?; @@ -185,7 +183,7 @@ impl RenderCommandController { // Call application handler let result = self .handler - .execute(input_mode, ip, output_dir, force, &working_dir) + .execute(input_mode, ip, output_dir, force, working_dir) .await .map_err(RenderCommandError::from)?; diff --git a/src/presentation/dispatch/context.rs b/src/presentation/dispatch/context.rs index 1adfa6e3..ebfc9378 100644 --- a/src/presentation/dispatch/context.rs +++ b/src/presentation/dispatch/context.rs @@ -312,4 +312,44 @@ impl ExecutionContext { pub fn output_format(&self) -> OutputFormat { self.global_args.output_format } + + /// Get the working directory from global CLI arguments + /// + /// Returns the working directory path specified by the user (or default "."). + /// This is where environment data will be stored (data/ and build/ subdirectories). + /// + /// # Examples + /// + /// ```ignore + /// use torrust_tracker_deployer_lib::bootstrap::Container; + /// use torrust_tracker_deployer_lib::presentation::views::VerbosityLevel; + /// use torrust_tracker_deployer_lib::presentation::dispatch::ExecutionContext; + /// use torrust_tracker_deployer_lib::presentation::input::cli::args::GlobalArgs; + /// use torrust_tracker_deployer_lib::presentation::input::cli::OutputFormat; + /// use torrust_tracker_deployer_lib::bootstrap::logging::{LogFormat, LogOutput}; + /// use std::sync::Arc; + /// use std::path::PathBuf; + /// + /// # fn example() -> Result<(), Box> { + /// let container = Container::new(VerbosityLevel::Normal, &PathBuf::from(".")); + /// let global_args = GlobalArgs { + /// log_file_format: LogFormat::Compact, + /// log_stderr_format: LogFormat::Pretty, + /// log_output: LogOutput::FileOnly, + /// log_dir: PathBuf::from("./data/logs"), + /// working_dir: PathBuf::from("/tmp/test-workspace"), + /// output_format: OutputFormat::Text, + /// verbosity: 0, + /// }; + /// let context = ExecutionContext::new(Arc::new(container), global_args); + /// + /// let working_dir = context.working_dir(); + /// println!("Working directory: {}", working_dir.display()); + /// # Ok(()) + /// # } + /// ``` + #[must_use] + pub fn working_dir(&self) -> &std::path::Path { + &self.global_args.working_dir + } } diff --git a/src/presentation/dispatch/router.rs b/src/presentation/dispatch/router.rs index adade8d0..a0d587b4 100644 --- a/src/presentation/dispatch/router.rs +++ b/src/presentation/dispatch/router.rs @@ -197,6 +197,7 @@ pub async fn route_command( &instance_ip, output_dir.as_path(), force, + context.working_dir(), ) .await?; Ok(()) diff --git a/src/testing/e2e/process_runner.rs b/src/testing/e2e/process_runner.rs index 31e8e0ef..c70fcd85 100644 --- a/src/testing/e2e/process_runner.rs +++ b/src/testing/e2e/process_runner.rs @@ -13,13 +13,17 @@ use std::process::{Command, Output}; /// with different command-line arguments for black-box testing. pub struct ProcessRunner { working_dir: Option, + log_dir: Option, } impl ProcessRunner { /// Create a new process runner #[must_use] pub fn new() -> Self { - Self { working_dir: None } + Self { + working_dir: None, + log_dir: None, + } } /// Set the working directory for the test process (not the app working dir) @@ -32,6 +36,16 @@ impl ProcessRunner { self } + /// Set the log directory for the application + /// + /// This is passed as `--log-dir` to the application to control where + /// logs are written, enabling test isolation. + #[must_use] + pub fn log_dir>(mut self, dir: P) -> Self { + self.log_dir = Some(dir.as_ref().to_path_buf()); + self + } + /// Run the create command with the production binary /// /// This method runs `cargo run -- create environment --env-file ` with @@ -80,6 +94,12 @@ impl ProcessRunner { ]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute create command")?; Ok(ProcessResult::new(output)) @@ -115,6 +135,12 @@ impl ProcessRunner { cmd.args(["run", "--", "provision", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd .output() .context("Failed to execute provision command")?; @@ -152,6 +178,12 @@ impl ProcessRunner { cmd.args(["run", "--", "destroy", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute destroy command")?; Ok(ProcessResult::new(output)) @@ -216,6 +248,12 @@ impl ProcessRunner { cmd.args(args); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute register command")?; Ok(ProcessResult::new(output)) @@ -251,6 +289,12 @@ impl ProcessRunner { cmd.args(["run", "--", "configure", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd .output() .context("Failed to execute configure command")?; @@ -288,6 +332,12 @@ impl ProcessRunner { cmd.args(["run", "--", "test", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute test command")?; Ok(ProcessResult::new(output)) @@ -323,6 +373,12 @@ impl ProcessRunner { cmd.args(["run", "--", "release", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute release command")?; Ok(ProcessResult::new(output)) @@ -358,6 +414,12 @@ impl ProcessRunner { cmd.args(["run", "--", "run", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute run command")?; Ok(ProcessResult::new(output)) @@ -392,6 +454,12 @@ impl ProcessRunner { cmd.args(["run", "--", "list"]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute list command")?; Ok(ProcessResult::new(output)) @@ -427,6 +495,12 @@ impl ProcessRunner { cmd.args(["run", "--", "show", environment_name]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute show command")?; Ok(ProcessResult::new(output)) @@ -443,24 +517,23 @@ impl ProcessRunner { /// /// # Panics /// - /// Panics if the working directory path or config file path contains invalid UTF-8. + /// Panics if the working directory or log directory path contains invalid UTF-8. pub fn run_validate_command(&self, config_file: &str) -> Result { let mut cmd = Command::new("cargo"); + // Build base command + cmd.args(["run", "--", "validate", "-f", config_file]); + + // Add working-dir if specified if let Some(working_dir) = &self.working_dir { - // Build command with working directory - cmd.args([ - "run", - "--", - "validate", - "-f", - config_file, - "--working-dir", - working_dir.to_str().unwrap(), - ]); - } else { - // No working directory, use relative paths - cmd.args(["run", "--", "validate", "-f", config_file]); + cmd.arg("--working-dir"); + cmd.arg(working_dir); + } + + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); } let output = cmd.output().context("Failed to execute validate command")?; @@ -531,6 +604,12 @@ impl ProcessRunner { ]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd .output() .context("Failed to execute render command with env-name")?; @@ -541,7 +620,7 @@ impl ProcessRunner { /// Run the render command with config file input mode /// /// This method runs `cargo run -- render --env-file --instance-ip --output-dir ` - /// with optional working directory for the application itself via `--working-dir`. + /// with optional working directory and log directory for test isolation. /// /// # Errors /// @@ -549,7 +628,7 @@ impl ProcessRunner { /// /// # Panics /// - /// May panic if the working directory path is not valid UTF-8. + /// May panic if the working directory or log directory path is not valid UTF-8. pub fn run_render_command_with_config_file( &self, config_file: &str, @@ -558,34 +637,29 @@ impl ProcessRunner { ) -> Result { let mut cmd = Command::new("cargo"); + // Build base command + cmd.args([ + "run", + "--", + "render", + "--env-file", + config_file, + "--instance-ip", + instance_ip, + "--output-dir", + output_dir, + ]); + + // Add working-dir if specified if let Some(working_dir) = &self.working_dir { - // Build command with working directory - cmd.args([ - "run", - "--", - "render", - "--env-file", - config_file, - "--instance-ip", - instance_ip, - "--output-dir", - output_dir, - "--working-dir", - working_dir.to_str().unwrap(), - ]); - } else { - // No working directory, use relative paths - cmd.args([ - "run", - "--", - "render", - "--env-file", - config_file, - "--instance-ip", - instance_ip, - "--output-dir", - output_dir, - ]); + cmd.arg("--working-dir"); + cmd.arg(working_dir); + } + + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); } let output = cmd @@ -627,6 +701,12 @@ impl ProcessRunner { cmd.args(["run", "--", "purge", environment_name, "--force"]); } + // Add log-dir if specified + if let Some(log_dir) = &self.log_dir { + cmd.arg("--log-dir"); + cmd.arg(log_dir); + } + let output = cmd.output().context("Failed to execute purge command")?; Ok(ProcessResult::new(output)) diff --git a/tests/e2e/create_command.rs b/tests/e2e/create_command.rs index 3c478d04..3e1fd089 100644 --- a/tests/e2e/create_command.rs +++ b/tests/e2e/create_command.rs @@ -68,6 +68,7 @@ fn it_should_create_environment_from_config_file_black_box() { // Act: Run production application as external process let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -103,6 +104,7 @@ fn it_should_fail_gracefully_with_invalid_config() { // Run command and expect failure let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./invalid.json") .expect("Failed to run create command"); @@ -130,6 +132,7 @@ fn it_should_fail_when_config_file_not_found() { // Run command with non-existent config file let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./nonexistent.json") .expect("Failed to run create command"); @@ -162,6 +165,7 @@ fn it_should_fail_when_environment_already_exists() { // Create environment first time let result1 = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./config.json") .expect("Failed to run create command"); @@ -174,6 +178,7 @@ fn it_should_fail_when_environment_already_exists() { // Try to create same environment again let result2 = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./config.json") .expect("Failed to run create command"); diff --git a/tests/e2e/destroy_command.rs b/tests/e2e/destroy_command.rs index 0990e2b6..31c38ad6 100644 --- a/tests/e2e/destroy_command.rs +++ b/tests/e2e/destroy_command.rs @@ -66,6 +66,7 @@ fn it_should_destroy_environment_with_default_working_directory() { // Create environment in default location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -82,6 +83,7 @@ fn it_should_destroy_environment_with_default_working_directory() { // Act: Destroy environment using destroy command let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-destroy-default") .expect("Failed to run destroy command"); @@ -116,6 +118,7 @@ fn it_should_destroy_environment_with_custom_working_directory() { // Create environment in custom location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -132,6 +135,7 @@ fn it_should_destroy_environment_with_custom_working_directory() { // Act: Destroy environment using same working directory let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-destroy-custom") .expect("Failed to run destroy command"); @@ -160,6 +164,7 @@ fn it_should_fail_when_environment_not_found_in_working_directory() { // Act: Try to destroy non-existent environment let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("nonexistent-environment") .expect("Failed to run destroy command"); @@ -193,6 +198,7 @@ fn it_should_complete_full_lifecycle_with_custom_working_directory() { // Act: Create environment in custom location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -210,6 +216,7 @@ fn it_should_complete_full_lifecycle_with_custom_working_directory() { // Act: Destroy environment let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-lifecycle") .expect("Failed to run destroy command"); diff --git a/tests/e2e/list_command.rs b/tests/e2e/list_command.rs index 393985ca..c03c56b6 100644 --- a/tests/e2e/list_command.rs +++ b/tests/e2e/list_command.rs @@ -52,6 +52,7 @@ fn it_should_report_no_data_directory_when_workspace_is_empty() { // Act: Run list command on empty workspace let list_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_list_command() .expect("Failed to run list command"); @@ -90,6 +91,7 @@ fn it_should_list_created_environment() { // Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -106,6 +108,7 @@ fn it_should_list_created_environment() { // Act: Run list command let list_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_list_command() .expect("Failed to run list command"); @@ -141,6 +144,7 @@ fn it_should_list_multiple_environments() { let create_result1 = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./env1.json") .expect("Failed to run first create command"); @@ -158,6 +162,7 @@ fn it_should_list_multiple_environments() { let create_result2 = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./env2.json") .expect("Failed to run second create command"); @@ -175,6 +180,7 @@ fn it_should_list_multiple_environments() { // Act: Run list command let list_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_list_command() .expect("Failed to run list command"); diff --git a/tests/e2e/purge_command.rs b/tests/e2e/purge_command.rs index 847017fb..40a7f4cf 100644 --- a/tests/e2e/purge_command.rs +++ b/tests/e2e/purge_command.rs @@ -74,6 +74,7 @@ fn it_should_purge_destroyed_environment_successfully() { // Create environment in default location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -86,6 +87,7 @@ fn it_should_purge_destroyed_environment_successfully() { // Destroy environment first (normal workflow) let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-purge-destroyed") .expect("Failed to run destroy command"); @@ -103,6 +105,7 @@ fn it_should_purge_destroyed_environment_successfully() { // Act: Purge environment using purge command with --force let purge_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_purge_command("test-purge-destroyed") .expect("Failed to run purge command"); @@ -139,6 +142,7 @@ fn it_should_fail_when_purging_nonexistent_environment() { // Act: Try to purge non-existent environment let purge_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_purge_command("nonexistent-env") .expect("Failed to run purge command"); @@ -173,6 +177,7 @@ fn it_should_purge_with_custom_working_directory() { // Create environment in custom location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -185,6 +190,7 @@ fn it_should_purge_with_custom_working_directory() { // Destroy environment let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-purge-custom-dir") .expect("Failed to run destroy command"); @@ -197,6 +203,7 @@ fn it_should_purge_with_custom_working_directory() { // Act: Purge environment from custom working directory let purge_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_purge_command("test-purge-custom-dir") .expect("Failed to run purge command"); @@ -232,6 +239,7 @@ fn it_should_complete_full_lifecycle_from_create_to_purge() { // Step 1: Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -248,6 +256,7 @@ fn it_should_complete_full_lifecycle_from_create_to_purge() { // Step 2: Destroy environment let destroy_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-full-lifecycle-purge") .expect("Failed to run destroy command"); @@ -263,6 +272,7 @@ fn it_should_complete_full_lifecycle_from_create_to_purge() { // Step 3: Purge environment let purge_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_purge_command("test-full-lifecycle-purge") .expect("Failed to run purge command"); @@ -302,6 +312,7 @@ fn it_should_remove_only_specified_environment_data() { let create1_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./env1.json") .expect("Failed to run create command"); @@ -315,6 +326,7 @@ fn it_should_remove_only_specified_environment_data() { let create2_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./env2.json") .expect("Failed to run create command"); @@ -323,6 +335,7 @@ fn it_should_remove_only_specified_environment_data() { // Destroy both environments let destroy1_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-purge-env1") .expect("Failed to run destroy command"); @@ -330,6 +343,7 @@ fn it_should_remove_only_specified_environment_data() { let destroy2_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_destroy_command("test-purge-env2") .expect("Failed to run destroy command"); @@ -343,6 +357,7 @@ fn it_should_remove_only_specified_environment_data() { // Act: Purge ONLY the first environment let purge_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_purge_command("test-purge-env1") .expect("Failed to run purge command"); diff --git a/tests/e2e/render_command.rs b/tests/e2e/render_command.rs index 02640c08..b7a7cf3c 100644 --- a/tests/e2e/render_command.rs +++ b/tests/e2e/render_command.rs @@ -78,6 +78,7 @@ fn it_should_render_artifacts_using_env_name_successfully() { // Create environment in default location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -96,6 +97,7 @@ fn it_should_render_artifacts_using_env_name_successfully() { let output_dir = temp_workspace.path().join("render-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "test-render-env-name", "192.168.1.100", @@ -160,6 +162,7 @@ fn it_should_render_artifacts_using_config_file_successfully() { let output_dir = temp_workspace.path().join("render-config-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_config_file( &config_path, "192.168.1.101", @@ -217,6 +220,7 @@ fn it_should_fail_when_output_directory_already_exists() { let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -230,6 +234,7 @@ fn it_should_fail_when_output_directory_already_exists() { let output_dir = temp_workspace.path().join("render-idempotent-output"); let render1_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "test-render-output-dir-exists", "192.168.1.102", @@ -246,6 +251,7 @@ fn it_should_fail_when_output_directory_already_exists() { // Act: Render artifacts second time (should fail with OutputDirectoryExists error) let render2_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "test-render-output-dir-exists", "192.168.1.102", @@ -284,6 +290,7 @@ fn it_should_fail_when_environment_not_found() { let output_dir = temp_workspace.path().join("nonexistent-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "nonexistent-env", "192.168.1.103", @@ -317,6 +324,7 @@ fn it_should_fail_when_config_file_not_found() { let output_dir = temp_workspace.path().join("missing-config-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_config_file( "./nonexistent.json", "192.168.1.104", @@ -355,6 +363,7 @@ fn it_should_work_with_custom_working_directory() { // Create environment in custom location let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -368,6 +377,7 @@ fn it_should_work_with_custom_working_directory() { let output_dir = temp_workspace.path().join("custom-dir-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "test-render-custom-dir", "192.168.1.105", @@ -414,6 +424,7 @@ fn it_should_complete_full_lifecycle_from_create_to_render() { // Step 1: Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -432,6 +443,7 @@ fn it_should_complete_full_lifecycle_from_create_to_render() { let output_dir = temp_workspace.path().join("lifecycle-output"); let render_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_render_command_with_env_name( "test-full-lifecycle-render", "192.168.1.106", diff --git a/tests/e2e/show_command.rs b/tests/e2e/show_command.rs index a29b6616..d8b9265d 100644 --- a/tests/e2e/show_command.rs +++ b/tests/e2e/show_command.rs @@ -51,6 +51,7 @@ fn it_should_report_environment_not_found_when_environment_does_not_exist() { // Act: Run show command for a non-existing environment let show_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_show_command("non-existing-env") .expect("Failed to run show command"); @@ -89,6 +90,7 @@ fn it_should_show_created_environment_details() { // Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -105,6 +107,7 @@ fn it_should_show_created_environment_details() { // Act: Run show command let show_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_show_command("test-show-env") .expect("Failed to run show command"); @@ -141,6 +144,7 @@ fn it_should_show_environment_state() { // Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -153,6 +157,7 @@ fn it_should_show_environment_state() { // Act: Run show command let show_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_show_command("test-show-state") .expect("Failed to run show command"); @@ -190,6 +195,7 @@ fn it_should_show_provider_information() { // Create environment let create_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_create_command("./environment.json") .expect("Failed to run create command"); @@ -202,6 +208,7 @@ fn it_should_show_provider_information() { // Act: Run show command let show_result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_show_command("test-show-provider") .expect("Failed to run show command"); diff --git a/tests/e2e/validate_command.rs b/tests/e2e/validate_command.rs index b3a76940..e407a84b 100644 --- a/tests/e2e/validate_command.rs +++ b/tests/e2e/validate_command.rs @@ -57,6 +57,7 @@ fn it_should_report_file_not_found_when_configuration_file_does_not_exist() { // Act: Run validate command for non-existent file let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_validate_command(nonexistent_file.to_str().unwrap()) .expect("Failed to run validate command"); @@ -96,6 +97,7 @@ fn it_should_report_invalid_json_when_configuration_file_has_malformed_json() { // Act: Run validate command let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_validate_command(config_path.to_str().unwrap()) .expect("Failed to run validate command"); @@ -145,6 +147,7 @@ fn it_should_succeed_when_configuration_file_is_valid() { // Act: Run validate command let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_validate_command(config_path.to_str().unwrap()) .expect("Failed to run validate command"); @@ -200,6 +203,7 @@ fn it_should_validate_configuration_without_creating_deployment() { // Act: Run validate command let result = ProcessRunner::new() .working_dir(temp_workspace.path()) + .log_dir(temp_workspace.path().join("logs")) .run_validate_command(config_path.to_str().unwrap()) .expect("Failed to run validate command"); diff --git a/tests/validate_ai_training_examples.rs b/tests/validate_ai_training_examples.rs index a06c1d53..99eb2e93 100644 --- a/tests/validate_ai_training_examples.rs +++ b/tests/validate_ai_training_examples.rs @@ -103,7 +103,14 @@ fn collect_example_files(examples_dir: &PathBuf) -> Vec { fn validate_configuration(config_path: &Path) -> Result<(), String> { let config_path_str = path_to_str(config_path, "config path")?; + // Create a temporary workspace for this validation + let temp_workspace = + TempDir::new().map_err(|e| format!("Failed to create temp workspace: {e}"))?; + let log_dir = temp_workspace.path().join("logs"); + let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) + .log_dir(&log_dir) .run_validate_command(config_path_str) .map_err(|e| format!("Failed to run validate command: {e}"))?; @@ -208,7 +215,7 @@ fn render_configuration(config_path: &Path) -> Result<(), String> { let temp_config_file = prepare_test_config(config_path, &temp_workspace)?; let output_dir = temp_workspace.path().join("output"); - execute_render_command(&temp_config_file, &output_dir)?; + execute_render_command(&temp_config_file, &output_dir, &temp_workspace)?; verify_render_output_directories(&output_dir)?; Ok(()) @@ -263,11 +270,20 @@ fn replace_ssh_credentials_with_fixtures(config: &mut serde_json::Value) -> Resu } /// Execute the render command using `ProcessRunner` -fn execute_render_command(temp_config_file: &Path, output_dir: &Path) -> Result<(), String> { +fn execute_render_command( + temp_config_file: &Path, + output_dir: &Path, + temp_workspace: &TempDir, +) -> Result<(), String> { let temp_config_file_str = path_to_str(temp_config_file, "temp config file path")?; let output_dir_str = path_to_str(output_dir, "output directory path")?; + // Create log directory in temp workspace + let log_dir = temp_workspace.path().join("logs"); + let result = ProcessRunner::new() + .working_dir(temp_workspace.path()) + .log_dir(&log_dir) .run_render_command_with_config_file(temp_config_file_str, PLACEHOLDER_IP, output_dir_str) .map_err(|e| format!("Failed to run render command: {e}"))?;