Skip to content
376 changes: 364 additions & 12 deletions .github/skills/write-unit-test/skill.md

Large diffs are not rendered by default.

19 changes: 17 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ Both production and test code must be:
- **Readable**: Clear intent that can be understood by other developers
- **Testable**: Designed to support comprehensive testing at all levels

**Beck's Four Rules of Simple Design:**

Follow Kent Beck's four rules of simple design (in priority order):

1. **Passes the tests**: The code must work as intended - testing is a first-class activity
2. **Reveals intention**: Code should be easy to understand, expressing purpose clearly
3. **No duplication**: Apply DRY (Don't Repeat Yourself) / Once and Only Once - eliminating duplication drives out good designs
4. **Fewest elements**: Remove anything that doesn't serve the prior three rules - avoid premature optimization for hypothetical future requirements

These rules feed off each other in refining code and apply to any language or paradigm. When in conflict, empathy for the reader wins over strict technical metrics.

Reference: [Beck Design Rules](https://martinfowler.com/bliki/BeckDesignRules.html)

These principles should guide all development decisions, code reviews, and feature implementations.

## 🔧 Essential Rules
Expand Down Expand Up @@ -216,7 +229,7 @@ The project provides Agent Skills in `.github/skills/` for specialized workflows
Available skills:

| Task | Skill to Load |
| ---------------------------- | --------------------------------------------------- | --- | --------------------- | --------------------------------------------- | --- | --------------------------- | -------------------------------------------------- |
| ---------------------------- | --------------------------------------------------- |
| Adding commands | `.github/skills/add-new-command/skill.md` |
| Cleaning up completed issues | `.github/skills/cleanup-completed-issues/skill.md` |
| Cleaning LXD environments | `.github/skills/clean-lxd-environments/skill.md` |
Expand All @@ -229,7 +242,9 @@ Available skills:
| Creating feature specs | `.github/skills/create-feature-spec/skill.md` |
| Creating issues | `.github/skills/create-issue/skill.md` |
| Creating new skills | `.github/skills/add-new-skill/skill.md` |
| Creating refactor plans | `.github/skills/create-refactor-plan/skill.md` | | Regenerating CLI docs | `.github/skills/regenerate-cli-docs/skill.md` | | Rendering tracker artifacts | `.github/skills/render-tracker-artifacts/skill.md` |
| Creating refactor plans | `.github/skills/create-refactor-plan/skill.md` |
| Regenerating CLI docs | `.github/skills/regenerate-cli-docs/skill.md` |
| Rendering tracker artifacts | `.github/skills/render-tracker-artifacts/skill.md` |
| Reviewing pull requests | `.github/skills/review-pr/skill.md` |
| Running linters | `.github/skills/run-linters/skill.md` |
| Running local E2E tests | `.github/skills/run-local-e2e-test/skill.md` |
Expand Down
5 changes: 5 additions & 0 deletions docs/contributing/pr-review-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Verify contributors followed these project conventions:
- [ ] **Tests follow naming conventions** (`it_should_...` pattern)
- [ ] **Tests are isolated** - use temporary resources, don't depend on external state
- [ ] **Tests are readable** - clear intent and easy to understand what's being tested
- [ ] **Test setup code is DRY** - no duplicate Arrange sections across tests
- [ ] **Test fixtures are decoupled** - helper functions use parameters, not hardcoded internal calls
- [ ] **Both production and test code** meet quality standards (clean, maintainable, sustainable)

### Documentation
Expand Down Expand Up @@ -80,6 +82,9 @@ Watch for these common issues that indicate quality problems:
- ❌ **Missing tests for new error paths** - Error handling should be tested
- ❌ **Tests that depend on external state** - Tests should be isolated
- ❌ **Test code that doesn't meet production quality standards** - Both should be clean
- ❌ **Duplicate test setup code** - Extract to helper functions (see PR [#373](https://github.com/torrust/torrust-tracker-deployer/pull/373))
- ❌ **Coupled helper functions** - Parameterize helpers instead of hardcoding dependencies
- ❌ **Field-by-field assertions on DTOs** - Add `PartialEq` and assert equality directly

### Known Issues vs. Real Problems

Expand Down
55 changes: 49 additions & 6 deletions src/presentation/controllers/configure/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::domain::environment::name::EnvironmentName;
use crate::domain::environment::repository::EnvironmentRepository;
use crate::domain::environment::state::Configured;
use crate::domain::environment::Environment;
use crate::presentation::input::cli::OutputFormat;
use crate::presentation::views::commands::configure::{ConfigureDetailsData, JsonView, TextView};
use crate::presentation::views::progress::ProgressReporter;
use crate::presentation::views::progress::VerboseProgressListener;
use crate::presentation::views::UserOutput;
Expand Down Expand Up @@ -101,11 +103,13 @@ impl ConfigureCommandController {
/// 2. Load and validate environment state
/// 3. Create command handler
/// 4. Configure infrastructure
/// 5. Complete with success message
/// 5. Display results (in specified format)
/// 6. Complete with success message
///
/// # Arguments
///
/// * `environment_name` - The name of the environment to configure
/// * `output_format` - The output format (Text or Json)
///
/// # Errors
///
Expand All @@ -123,6 +127,7 @@ impl ConfigureCommandController {
pub fn execute(
&mut self,
environment_name: &str,
output_format: OutputFormat,
) -> Result<Environment<Configured>, ConfigureSubcommandError> {
let env_name = self.validate_environment_name(environment_name)?;

Expand All @@ -132,6 +137,8 @@ impl ConfigureCommandController {

self.complete_workflow(environment_name)?;

self.display_configure_results(&configured, output_format)?;

Ok(configured)
}

Expand Down Expand Up @@ -224,6 +231,42 @@ impl ConfigureCommandController {
.complete(&format!("Environment '{name}' configured successfully"))?;
Ok(())
}

/// Display configure results in the specified format
///
/// Uses the Strategy Pattern to render configure details in either
/// human-readable text or machine-readable JSON format.
///
/// # Arguments
///
/// * `configured` - The configured environment to display
/// * `output_format` - The output format (Text or Json)
///
/// # Errors
///
/// Returns an error if:
/// - Progress reporting encounters a poisoned mutex
///
/// # Note
///
/// JSON serialization errors are handled inline by `JsonView::render()`,
/// which returns a fallback error JSON string. Therefore, this method
/// does not propagate serialization errors.
#[allow(clippy::result_large_err)]
fn display_configure_results(
&mut self,
configured: &Environment<Configured>,
output_format: OutputFormat,
) -> Result<(), ConfigureSubcommandError> {
self.progress.blank_line()?;
let details = ConfigureDetailsData::from(configured);
let output = match output_format {
OutputFormat::Text => TextView::render(&details),
OutputFormat::Json => JsonView::render(&details),
};
self.progress.result(&output)?;
Ok(())
}
}

#[cfg(test)]
Expand Down Expand Up @@ -269,7 +312,7 @@ mod tests {

// Test with invalid environment name (contains underscore)
let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("invalid_name");
.execute("invalid_name", OutputFormat::Text);

assert!(result.is_err());
match result.unwrap_err() {
Expand All @@ -288,8 +331,8 @@ mod tests {

let (user_output, repository, clock) = create_test_dependencies(&temp_dir);

let result =
ConfigureCommandController::new(repository, clock, user_output.clone()).execute("");
let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("", OutputFormat::Text);

assert!(result.is_err());
match result.unwrap_err() {
Expand All @@ -310,7 +353,7 @@ mod tests {

// Try to configure an environment that doesn't exist
let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("nonexistent-env");
.execute("nonexistent-env", OutputFormat::Text);

assert!(result.is_err());
// After refactoring, repository NotFound error is wrapped in ConfigureOperationFailed
Expand Down Expand Up @@ -339,7 +382,7 @@ mod tests {
// Valid environment name should pass validation, but will fail
// at configure operation since we don't have a real environment setup
let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("test-env");
.execute("test-env", OutputFormat::Text);

// Should fail at operation, not at name validation
if let Err(ConfigureSubcommandError::InvalidEnvironmentName { .. }) = result {
Expand Down
7 changes: 4 additions & 3 deletions src/presentation/controllers/configure/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod integration_tests {
use crate::presentation::controllers::configure;
use crate::presentation::controllers::configure::handler::ConfigureCommandController;
use crate::presentation::controllers::constants::DEFAULT_LOCK_TIMEOUT;
use crate::presentation::input::cli::OutputFormat;
use crate::presentation::views::testing::TestUserOutput;
use crate::presentation::views::{UserOutput, VerbosityLevel};
use crate::shared::clock::Clock;
Expand Down Expand Up @@ -47,7 +48,7 @@ mod integration_tests {
let (user_output, repository, clock) = create_test_dependencies(&temp_dir);

let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("invalid_name_with_underscore");
.execute("invalid_name_with_underscore", OutputFormat::Text);

assert!(result.is_err());
let error = result.unwrap_err();
Expand All @@ -65,7 +66,7 @@ mod integration_tests {
let (user_output, repository, clock) = create_test_dependencies(&temp_dir);

let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("bad_name");
.execute("bad_name", OutputFormat::Text);

assert!(result.is_err());
let error = result.unwrap_err();
Expand All @@ -82,7 +83,7 @@ mod integration_tests {
let (user_output, repository, clock) = create_test_dependencies(&temp_dir);

let result = ConfigureCommandController::new(repository, clock, user_output.clone())
.execute("nonexistent-environment");
.execute("nonexistent-environment", OutputFormat::Text);

assert!(result.is_err());
// Repository will return NotFound error, wrapped in ConfigureOperationFailed
Expand Down
3 changes: 2 additions & 1 deletion src/presentation/dispatch/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,11 @@ pub async fn route_command(
Ok(())
}
Commands::Configure { environment } => {
let output_format = context.output_format();
context
.container()
.create_configure_controller()
.execute(&environment)?;
.execute(&environment, output_format)?;
Ok(())
}
Commands::Test { environment } => {
Expand Down
52 changes: 52 additions & 0 deletions src/presentation/views/commands/configure/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//! Views for Configure Command
//!
//! This module contains view components for rendering configure command output.
//!
//! # Architecture
//!
//! This module follows the Strategy Pattern for rendering:
//! - `ConfigureDetailsData`: The data DTO passed to all views
//! - `TextView`: Renders human-readable text output
//! - `JsonView`: Renders machine-readable JSON output
//!
//! # Structure
//!
//! - `view_data/`: Data structures (DTOs) passed to views
//! - `configure_details.rs`: Main DTO with environment configure data
//! - `views/`: View rendering implementations
//! - `text_view.rs`: Human-readable text rendering
//! - `json_view.rs`: Machine-readable JSON rendering
//!
//! # SOLID Principles
//!
//! - **Single Responsibility**: Each view has one job - render in its format
//! - **Open/Closed**: Add new formats by creating new view files, not modifying existing ones
//! - **Strategy Pattern**: Different rendering strategies for the same data
//!
//! # Adding New Formats
//!
//! To add a new output format (e.g., XML, YAML, CSV):
//! 1. Create a new file in `views/`: `xml_view.rs`, `yaml_view.rs`, etc.
//! 2. Implement the view with `render(data: &ConfigureDetailsData) -> String`
//! 3. Export it from this module
//! 4. No need to modify existing views or the DTO

pub mod view_data {
pub mod configure_details;

// Re-export main types for convenience
pub use configure_details::ConfigureDetailsData;
}

pub mod views {
pub mod json_view;
pub mod text_view;

// Re-export views for convenience
pub use json_view::JsonView;
pub use text_view::TextView;
}

// Re-export at module root for convenience
pub use view_data::ConfigureDetailsData;
pub use views::{JsonView, TextView};
Loading
Loading