Skip to content

Comments

fix: render command now respects --working-dir global argument#366

Merged
josecelano merged 12 commits intomainfrom
365-fix-render-working-dir
Feb 18, 2026
Merged

fix: render command now respects --working-dir global argument#366
josecelano merged 12 commits intomainfrom
365-fix-render-working-dir

Conversation

@josecelano
Copy link
Member

@josecelano josecelano commented Feb 18, 2026

Summary

Fixes the bug where the render command ignores the --working-dir global argument and always uses std::env::current_dir() instead. During investigation and testing, discovered and fixed a broader test isolation issue where E2E tests were polluting the production data/ folder with logs.

Problem Overview

Initial Issue (Issue #365)

The render command wasn't respecting the --working-dir global argument, causing tests to create environment data in the project root instead of isolated temporary directories.

Discovered Issue

After fixing the --working-dir bug, tests still polluted data/logs/ because --log-dir and --working-dir are independent by design. Investigation revealed 44 E2E test ProcessRunner calls across 6 test files that needed explicit --log-dir configuration for complete isolation.

Changes

Part 1: Render Command --working-dir Fix

Presentation Layer

  • ExecutionContext (context.rs):

    • Added working_dir() accessor method to retrieve the working directory from global CLI arguments
    • Follows the same pattern as the existing output_format() method
  • Router (router.rs):

    • Updated render command dispatch to pass context.working_dir() to the controller
    • Ensures the global --working-dir argument is properly forwarded
  • Render Controller (render/handler.rs):

    • Updated execute() method signature to accept working_dir: &Path parameter
    • Removed hardcoded std::env::current_dir() call
    • Uses the passed working_dir parameter instead

Application Layer

  • Render Command Handler (render/handler.rs):
    • Changed from Environment::new() to Environment::create() to respect the working directory
    • Properly passes working_dir parameter to environment creation

Part 2: Complete E2E Test Isolation (6-Phase Refactor)

ProcessRunner Enhancement

  • ProcessRunner (process_runner.rs):
    • Added log_dir field to ProcessRunner
    • Added .log_dir() builder method
    • Enhanced all 14 command methods (run_create_command, run_destroy_command, etc.) to pass --log-dir when configured
    • Enables black-box tests to control both data location (via --working-dir) and log location (via --log-dir)

Test Files Updated (44 ProcessRunner Calls Fixed)

Phase 0 - validate_command.rs (Commit 1d576a5a):

  • Fixed 4 ProcessRunner calls
  • All 4 tests pass, data/ folder empty

Phase 1 - create_command.rs (Commit e452efe6):

  • Fixed 5 ProcessRunner calls
  • All 4 tests pass, data/ folder empty

Phase 2 - list_command.rs (Commit 53b87a8d):

  • Fixed 6 ProcessRunner calls
  • All 3 tests pass, data/ folder empty

Phase 3 - show_command.rs (Commit 5682ae05):

  • Fixed 7 ProcessRunner calls
  • All 4 tests pass, data/ folder empty

Phase 4 - destroy_command.rs (Commit 82946459):

  • Fixed 7 ProcessRunner calls
  • All 4 tests pass, data/ folder empty

Phase 5 - purge_command.rs (Commit 1c437d80):

  • Fixed 15 ProcessRunner calls (largest file)
  • All 5 tests pass, data/ folder empty

Documentation

  • Refactor Plan (e2e-test-isolation-log-dir.md):
    • Comprehensive 6-phase refactor plan with implementation checklists
    • Documents the problem, solution, and systematic approach
    • All phases marked complete with commit references

Impact

Immediate Benefits

  • Fixes test pollution: Tests no longer create files/directories in production data/ folder
  • Consistent behavior: Render command now respects --working-dir like other commands
  • Complete test isolation: Both environment data AND logs are isolated in temporary directories
  • Better testability: Tests can fully control where environment data and logs are stored
  • User control: Users can specify where render command reads environment data from

Architectural Improvements

  • Independent control: --working-dir and --log-dir are properly independent
  • Clean test hygiene: All E2E tests follow proper isolation patterns
  • ProcessRunner maturity: Black-box testing utility now supports complete isolation
  • Pattern established: Clear pattern for future E2E tests to follow

Testing

Comprehensive Verification

  • All tests: 408 passed; 0 failed; 95 ignored
  • Production data/ folder: Completely empty (0 files, 0 directories) after running entire test suite
  • Pre-commit checks: All pass (markdown, yaml, toml, cspell, clippy, rustfmt, shellcheck)
  • E2E infrastructure lifecycle tests: Pass
  • E2E deployment workflow tests: Pass

Test Isolation Pattern

// Before (incomplete isolation - logs still polluted data/)
let temp_workspace = TempWorkspace::new().expect("Failed");
let result = ProcessRunner::new()
    .working_dir(temp_workspace.path())  // ✅ Environment data isolated
    .run_create_command("./config.json") // ❌ Logs go to ./data/logs
    .expect("Failed to run command");

// After (complete isolation)
let temp_workspace = TempWorkspace::new().expect("Failed");
let result = ProcessRunner::new()
    .working_dir(temp_workspace.path())  // ✅ Environment data isolated
    .log_dir(temp_workspace.path().join("logs"))  // ✅ Logs isolated
    .run_create_command("./config.json")
    .expect("Failed to run command");

Related Issues

Closes #365

Statistics

Metric Value
ProcessRunner calls fixed 44
Test files modified 6 E2E command test files
Refactor phases completed 6/6 (100%)
Total commits 11
Tests passing 408
Tests failing 0
Data folder pollution 0 files, 0 directories
Lines added 763
Lines removed 65

Checklist

  • Code compiles without errors
  • All tests pass (408 passed)
  • Pre-commit checks pass (all linters)
  • Documentation is up to date (inline docs + refactor plan)
  • Follows conventional commit format (11 commits)
  • Branch follows naming convention (365-fix-render-working-dir)
  • Complete test isolation verified (data/ folder empty after full test run)

Additional Notes

Architecture Alignment

This fix aligns the render command with the project's architecture where all commands should respect global CLI arguments. The --working-dir argument is now properly threaded through the entire command execution pipeline: CLI → Router → Controller → Application Handler → Domain.

Test Isolation Philosophy

The comprehensive test isolation work follows the project's principle that tests should never pollute production directories. By ensuring both --working-dir and --log-dir are properly controlled, we achieve complete test isolation where temporary directories are created, used, and automatically cleaned up without leaving any artifacts in the project workspace.

Systematic Approach

The refactor followed a systematic phase-by-phase approach (documented in e2e-test-isolation-log-dir.md):

  1. Start with simplest files (4 calls)
  2. Progress to larger files (5-7 calls)
  3. End with most complex file (15 calls)
  4. Verify isolation after each phase
  5. Commit incrementally

This approach ensured the pattern was solid before applying it to more complex test files, and made the review process more manageable with clear, focused commits.

@josecelano josecelano self-assigned this Feb 18, 2026
- Add working_dir() accessor method to ExecutionContext
- Update router to pass working_dir from context to render controller
- Update render controller to accept working_dir parameter instead of using std::env::current_dir()
- Update application handler to use Environment::create() with working_dir
- Update render command test to pass temp workspace as working_dir

This fixes the bug where the render command ignored the --working-dir global
argument and always used the current directory, causing tests to pollute the
project's data/ folder.

Resolves #365
@josecelano josecelano force-pushed the 365-fix-render-working-dir branch from ddab8eb to a434dda Compare February 18, 2026 11:53
Adds automated check to ensure unit tests don't pollute the production
data/ folder. This verification:

- Runs after unit tests in the testing workflow
- Checks if data/ folder contains any files or subdirectories
- Fails the workflow with clear error message if pollution detected
- Only runs in CI (isolated environment) to avoid interference with
  legitimate local data folder usage

E2E test workflows are intentionally excluded as they test real
functionality that requires using the data folder.

Related to #365
@josecelano josecelano force-pushed the 365-fix-render-working-dir branch from 21a2beb to e3edd01 Compare February 18, 2026 12:52
@josecelano
Copy link
Member Author

ACK e3edd01

@josecelano josecelano merged commit c638400 into main Feb 18, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: render command ignores --working-dir global argument

1 participant