Skip to content

Commit 3572826

Browse files
committed
refactor: [#116] improve structured logging with consistent status field
- Add 'status' field to all detector log entries (installed/not installed) - Move error handling from main() into app::run() using tracing::error! - Change app::run() return type from Result<(), AppError> to ExitCode - Simplify main() binary to just call app::run() and exit - Update tests to verify exit codes only (no output with --log-level off) - Makes logging output easily parsable by automation tools
1 parent 7d9fefb commit 3572826

File tree

7 files changed

+53
-48
lines changed

7 files changed

+53
-48
lines changed

packages/dependency-installer/src/app.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,9 @@ impl From<ListError> for AppError {
101101

102102
/// Run the CLI application
103103
///
104-
/// # Errors
105-
///
106-
/// Returns an error if:
107-
/// - Dependencies are missing
108-
/// - Invalid tool name is provided
109-
/// - Internal error occurs during dependency checking
110-
pub fn run() -> Result<(), AppError> {
104+
/// Returns the appropriate exit code based on the operation result.
105+
/// Errors are logged using tracing and do not propagate to stderr.
106+
pub fn run() -> ExitCode {
111107
let cli = Cli::parse();
112108

113109
// Determine log level: verbose flag overrides log-level argument
@@ -122,12 +118,19 @@ pub fn run() -> Result<(), AppError> {
122118

123119
let manager = DependencyManager::new();
124120

125-
match cli.command {
121+
let result: Result<(), AppError> = match cli.command {
126122
Commands::Check { dependency } => {
127-
crate::handlers::check::handle_check(&manager, dependency)?;
123+
crate::handlers::check::handle_check(&manager, dependency).map_err(AppError::from)
128124
}
129-
Commands::List => crate::handlers::list::handle_list(&manager)?,
130-
}
125+
Commands::List => crate::handlers::list::handle_list(&manager).map_err(AppError::from),
126+
};
131127

132-
Ok(())
128+
match result {
129+
Ok(()) => ExitCode::Success,
130+
Err(e) => {
131+
// Log the error using tracing instead of eprintln
132+
tracing::error!(error = %e, "Command failed");
133+
e.to_exit_code()
134+
}
135+
}
133136
}

packages/dependency-installer/src/bin/dependency-installer.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,9 @@
1212
1313
use std::process;
1414

15-
use torrust_dependency_installer::app::{self, ExitCode};
15+
use torrust_dependency_installer::app;
1616

1717
fn main() {
18-
let exit_code = match app::run() {
19-
Ok(()) => ExitCode::Success,
20-
Err(e) => {
21-
eprintln!("Error: {e}");
22-
e.to_exit_code()
23-
}
24-
};
25-
18+
let exit_code = app::run();
2619
process::exit(exit_code.into());
2720
}

packages/dependency-installer/src/detector/ansible.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,17 @@ impl DependencyDetector for AnsibleDetector {
2222
})?;
2323

2424
if installed {
25-
info!(dependency = "ansible", "Ansible is installed");
25+
info!(
26+
dependency = "ansible",
27+
status = "installed",
28+
"Ansible is installed"
29+
);
2630
} else {
27-
info!(dependency = "ansible", "Ansible is not installed");
31+
info!(
32+
dependency = "ansible",
33+
status = "not installed",
34+
"Ansible is not installed"
35+
);
2836
}
2937

3038
Ok(installed)

packages/dependency-installer/src/detector/cargo_machete.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,15 @@ impl DependencyDetector for CargoMacheteDetector {
2626
})?;
2727

2828
if installed {
29-
info!(dependency = "cargo-machete", "cargo-machete is installed");
29+
info!(
30+
dependency = "cargo-machete",
31+
status = "installed",
32+
"cargo-machete is installed"
33+
);
3034
} else {
3135
info!(
3236
dependency = "cargo-machete",
37+
status = "not installed",
3338
"cargo-machete is not installed"
3439
);
3540
}

packages/dependency-installer/src/detector/lxd.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ impl DependencyDetector for LxdDetector {
2323
})?;
2424

2525
if installed {
26-
info!(dependency = "lxd", "LXD is installed");
26+
info!(dependency = "lxd", status = "installed", "LXD is installed");
2727
} else {
28-
info!(dependency = "lxd", "LXD is not installed");
28+
info!(
29+
dependency = "lxd",
30+
status = "not installed",
31+
"LXD is not installed"
32+
);
2933
}
3034

3135
Ok(installed)

packages/dependency-installer/src/detector/opentofu.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,17 @@ impl DependencyDetector for OpenTofuDetector {
2222
})?;
2323

2424
if installed {
25-
info!(dependency = "opentofu", "OpenTofu is installed");
25+
info!(
26+
dependency = "opentofu",
27+
status = "installed",
28+
"OpenTofu is installed"
29+
);
2630
} else {
27-
info!(dependency = "opentofu", "OpenTofu is not installed");
31+
info!(
32+
dependency = "opentofu",
33+
status = "not installed",
34+
"OpenTofu is not installed"
35+
);
2836
}
2937

3038
Ok(installed)

packages/dependency-installer/tests/docker_check_command.rs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,9 @@ async fn test_check_all_reports_missing_dependencies() {
1818
// Start Ubuntu container with the binary
1919
let container = UbuntuContainerBuilder::new(&binary_path).start().await;
2020

21-
// Run the check command with logging disabled for clean test output
22-
let output = container.exec(&["dependency-installer", "check", "--log-level", "off"]);
23-
24-
// Verify it reports missing dependencies in the error message
25-
assert!(
26-
output.contains("Missing 4 out of 4 required dependencies"),
27-
"Expected missing dependencies error message, got: {output}"
28-
);
29-
30-
// Verify exit code is non-zero (failure)
31-
let exit_code = container.exec_with_exit_code(&["dependency-installer", "check", "--log-level", "off"]);
21+
// Verify exit code is non-zero (failure) when dependencies are missing
22+
let exit_code =
23+
container.exec_with_exit_code(&["dependency-installer", "check", "--log-level", "off"]);
3224
assert_eq!(
3325
exit_code, 1,
3426
"check command should exit with 1 when dependencies missing"
@@ -42,15 +34,7 @@ async fn test_check_specific_dependency() {
4234

4335
let container = UbuntuContainerBuilder::new(&binary_path).start().await;
4436

45-
// Run check command for opentofu (which is not installed) with logging disabled
46-
let output = container.exec(&["dependency-installer", "check", "--dependency", "opentofu", "--log-level", "off"]);
47-
48-
// Verify it reports missing dependency in the error message
49-
assert!(
50-
output.contains("opentofu: not installed"),
51-
"Expected opentofu missing error message, got: {output}"
52-
);
53-
37+
// Verify exit code when checking missing specific dependency
5438
let exit_code = container.exec_with_exit_code(&[
5539
"dependency-installer",
5640
"check",

0 commit comments

Comments
 (0)