Skip to content

Commit e2c961b

Browse files
committed
refactor: extract CommandExecutor from TestEnvironment with improved error handling
- Extract run_command method into new CommandExecutor type in src/command.rs - Replace println! with tracing::info for better logging practices - Add concrete CommandError enum with StartupFailed and ExecutionFailed variants - Improve error messages with detailed stdout/stderr information - Add comprehensive unit tests for command execution scenarios - Maintain backward compatibility in TestEnvironment wrapper - Export CommandExecutor through library's public API
1 parent b0aef09 commit e2c961b

File tree

3 files changed

+176
-33
lines changed

3 files changed

+176
-33
lines changed

src/bin/e2e_tests.rs

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ use std::time::{Duration, Instant};
88
use tempfile::TempDir;
99
use tokio::time::sleep;
1010

11+
// Import command execution system
12+
use torrust_tracker_deploy::command::CommandExecutor;
1113
// Import template system
1214
use torrust_tracker_deploy::template::file::File;
1315
use torrust_tracker_deploy::template::wrappers::ansible::inventory::{
@@ -41,6 +43,7 @@ struct TestEnvironment {
4143
verbose: bool,
4244
ssh_key_path: PathBuf,
4345
template_manager: TemplateManager,
46+
command_executor: CommandExecutor,
4447
#[allow(dead_code)] // Kept to maintain temp directory lifetime
4548
temp_dir: Option<tempfile::TempDir>,
4649
#[allow(dead_code)] // Used for cleanup but not directly accessed
@@ -55,6 +58,9 @@ impl TestEnvironment {
5558
// Create template manager
5659
let template_manager = TemplateManager::new(templates_dir);
5760

61+
// Create command executor with verbosity setting
62+
let command_executor = CommandExecutor::new(verbose);
63+
5864
// Clean templates directory to ensure we use fresh templates from embedded resources
5965
if verbose {
6066
println!("🧹 Cleaning templates directory to ensure fresh embedded templates...");
@@ -101,6 +107,7 @@ impl TestEnvironment {
101107
verbose,
102108
ssh_key_path: temp_ssh_key,
103109
template_manager,
110+
command_executor,
104111
temp_dir: Some(temp_dir),
105112
original_inventory: None,
106113
})
@@ -231,39 +238,9 @@ impl TestEnvironment {
231238
}
232239

233240
fn run_command(&self, cmd: &str, args: &[&str], working_dir: Option<&Path>) -> Result<String> {
234-
let mut command = Command::new(cmd);
235-
command.args(args);
236-
237-
if let Some(dir) = working_dir {
238-
command.current_dir(dir);
239-
}
240-
241-
if self.verbose {
242-
println!("🔧 Running: {} {}", cmd, args.join(" "));
243-
if let Some(dir) = working_dir {
244-
println!(" Working directory: {}", dir.display());
245-
}
246-
}
247-
248-
let output = command
249-
.stdout(Stdio::piped())
250-
.stderr(Stdio::piped())
251-
.output()
252-
.context(format!("Failed to execute: {} {}", cmd, args.join(" ")))?;
253-
254-
if !output.status.success() {
255-
let stderr = String::from_utf8_lossy(&output.stderr);
256-
let stdout = String::from_utf8_lossy(&output.stdout);
257-
return Err(anyhow!(
258-
"Command failed: {} {}\nStdout: {}\nStderr: {}",
259-
cmd,
260-
args.join(" "),
261-
stdout,
262-
stderr
263-
));
264-
}
265-
266-
Ok(String::from_utf8_lossy(&output.stdout).to_string())
241+
self.command_executor
242+
.run_command(cmd, args, working_dir)
243+
.map_err(anyhow::Error::from)
267244
}
268245

269246
fn provision_infrastructure(&self) -> Result<String> {

src/command.rs

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
use std::path::Path;
2+
use std::process::{Command, Stdio};
3+
use thiserror::Error;
4+
use tracing::info;
5+
6+
/// Errors that can occur during command execution
7+
#[derive(Error, Debug)]
8+
pub enum CommandError {
9+
/// The command could not be started (e.g., command not found, permission denied)
10+
#[error("Failed to start command '{command}': {source}")]
11+
StartupFailed {
12+
command: String,
13+
#[source]
14+
source: std::io::Error,
15+
},
16+
/// The command was started but exited with a non-zero status code
17+
#[error(
18+
"Command '{command}' failed with exit code {exit_code}\nStdout: {stdout}\nStderr: {stderr}"
19+
)]
20+
ExecutionFailed {
21+
command: String,
22+
exit_code: String,
23+
stdout: String,
24+
stderr: String,
25+
},
26+
}
27+
28+
/// A command executor that can run shell commands with optional verbosity
29+
pub struct CommandExecutor {
30+
verbose: bool,
31+
}
32+
33+
impl CommandExecutor {
34+
/// Creates a new `CommandExecutor`
35+
#[must_use]
36+
pub fn new(verbose: bool) -> Self {
37+
Self { verbose }
38+
}
39+
40+
/// Runs a command with the given arguments and optional working directory
41+
///
42+
/// # Arguments
43+
/// * `cmd` - The command to execute
44+
/// * `args` - Arguments to pass to the command
45+
/// * `working_dir` - Optional working directory to run the command in
46+
///
47+
/// # Returns
48+
/// * `Ok(String)` - The stdout output if the command succeeds
49+
/// * `Err(CommandError)` - A specific error describing what went wrong
50+
///
51+
/// # Errors
52+
/// This function will return an error if:
53+
/// * The command cannot be started (e.g., command not found) - `CommandError::StartupFailed`
54+
/// * The command execution fails with a non-zero exit code - `CommandError::ExecutionFailed`
55+
pub fn run_command(
56+
&self,
57+
cmd: &str,
58+
args: &[&str],
59+
working_dir: Option<&Path>,
60+
) -> Result<String, CommandError> {
61+
let mut command = Command::new(cmd);
62+
let command_display = format!("{} {}", cmd, args.join(" "));
63+
64+
command.args(args);
65+
66+
if let Some(dir) = working_dir {
67+
command.current_dir(dir);
68+
}
69+
70+
if self.verbose {
71+
info!("🔧 Running: {}", command_display);
72+
if let Some(dir) = working_dir {
73+
info!(" Working directory: {}", dir.display());
74+
}
75+
}
76+
77+
let output = command
78+
.stdout(Stdio::piped())
79+
.stderr(Stdio::piped())
80+
.output()
81+
.map_err(|source| CommandError::StartupFailed {
82+
command: command_display.clone(),
83+
source,
84+
})?;
85+
86+
if !output.status.success() {
87+
let stderr = String::from_utf8_lossy(&output.stderr).to_string();
88+
let stdout = String::from_utf8_lossy(&output.stdout).to_string();
89+
let exit_code = output
90+
.status
91+
.code()
92+
.map_or_else(|| "unknown".to_string(), |code| code.to_string());
93+
94+
return Err(CommandError::ExecutionFailed {
95+
command: command_display,
96+
exit_code,
97+
stdout,
98+
stderr,
99+
});
100+
}
101+
102+
Ok(String::from_utf8_lossy(&output.stdout).to_string())
103+
}
104+
}
105+
106+
#[cfg(test)]
107+
mod tests {
108+
use super::*;
109+
use std::env;
110+
111+
#[test]
112+
fn it_should_execute_simple_command_successfully() {
113+
let executor = CommandExecutor::new(false);
114+
let result = executor.run_command("echo", &["hello"], None);
115+
116+
assert!(result.is_ok());
117+
assert_eq!(result.unwrap().trim(), "hello");
118+
}
119+
120+
#[test]
121+
fn it_should_respect_working_directory() {
122+
let executor = CommandExecutor::new(false);
123+
let temp_dir = env::temp_dir();
124+
let result = executor.run_command("pwd", &[], Some(&temp_dir));
125+
126+
assert!(result.is_ok());
127+
// The output should contain the temp directory path
128+
assert!(result
129+
.unwrap()
130+
.contains(temp_dir.to_string_lossy().as_ref()));
131+
}
132+
133+
#[test]
134+
fn it_should_return_error_for_nonexistent_command() {
135+
let executor = CommandExecutor::new(false);
136+
let result = executor.run_command("nonexistent_command_xyz123", &[], None);
137+
138+
assert!(result.is_err());
139+
}
140+
141+
#[test]
142+
fn it_should_return_error_for_failing_command() {
143+
let executor = CommandExecutor::new(false);
144+
let result = executor.run_command("false", &[], None);
145+
146+
assert!(result.is_err());
147+
let error_msg = result.unwrap_err().to_string();
148+
assert!(error_msg.contains("failed with exit code"));
149+
}
150+
151+
#[test]
152+
fn it_should_use_verbose_logging_when_enabled() {
153+
// This test verifies that verbose mode uses tracing instead of println
154+
// We can't easily test the tracing output in unit tests without a subscriber
155+
// but we can verify the executor runs correctly in verbose mode
156+
let executor = CommandExecutor::new(true);
157+
let result = executor.run_command("echo", &["verbose_test"], None);
158+
159+
assert!(result.is_ok());
160+
assert_eq!(result.unwrap().trim(), "verbose_test");
161+
}
162+
}

src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111
//! - `ansible` - Wrappers for templates/ansible/ files
1212
//! - `tofu` - Wrappers for templates/tofu/ files
1313
//!
14+
//! ## Command Execution
15+
//! - `command` - Command execution utilities with optional verbosity
16+
//!
1417
//! Linting functionality has been moved to its own package: packages/linting
1518
19+
pub mod command;
1620
pub mod template;

0 commit comments

Comments
 (0)