feat: Improve package management and testing
Some checks failed
Rhai Tests / Run Rhai Tests (push) Has been cancelled
Rhai Tests / Run Rhai Tests (pull_request) Has been cancelled

- Improve platform detection logic for more robust package management.
- Enhance error handling and reporting in package commands.
- Refactor code for better readability and maintainability.
- Add comprehensive tests to cover package management functionality.
- Improve test coverage for various scenarios and edge cases.
This commit is contained in:
Mahmoud Emad 2025-05-09 09:54:32 +03:00
parent f002445c9e
commit 22f87b320e
4 changed files with 300 additions and 232 deletions

View File

@ -1,5 +1,5 @@
use std::process::Command;
use crate::process::CommandResult;
use std::process::Command;
/// Error type for package management operations
#[derive(Debug)]
@ -60,7 +60,7 @@ impl Platform {
}
}
/// Thread-local storage for debug flag
// Thread-local storage for debug flag
thread_local! {
static DEBUG: std::cell::RefCell<bool> = std::cell::RefCell::new(false);
}
@ -74,9 +74,7 @@ pub fn set_thread_local_debug(debug: bool) {
/// Get the debug flag for the current thread
pub fn thread_local_debug() -> bool {
DEBUG.with(|cell| {
*cell.borrow()
})
DEBUG.with(|cell| *cell.borrow())
}
/// Execute a package management command and return the result
@ -91,9 +89,7 @@ pub fn execute_package_command(args: &[&str], debug: bool) -> Result<CommandResu
println!("Executing command: {}", args.join(" "));
}
let output = Command::new(args[0])
.args(&args[1..])
.output();
let output = Command::new(args[0]).args(&args[1..]).output();
// Restore the previous debug flag
set_thread_local_debug(previous_debug);
@ -132,12 +128,19 @@ pub fn execute_package_command(args: &[&str], debug: bool) -> Result<CommandResu
} else {
// If command failed and debug is false, output stderr
if !debug {
println!("Command failed with code {}: {}", result.code, result.stderr.trim());
println!(
"Command failed with code {}: {}",
result.code,
result.stderr.trim()
);
}
Err(PackageError::CommandFailed(format!("Command failed with code {}: {}",
result.code, result.stderr.trim())))
Err(PackageError::CommandFailed(format!(
"Command failed with code {}: {}",
result.code,
result.stderr.trim()
)))
}
},
}
Err(e) => {
// Always output error information
println!("Command execution failed: {}", e);
@ -185,7 +188,10 @@ impl AptPackageManager {
impl PackageManager for AptPackageManager {
fn install(&self, package: &str) -> Result<CommandResult, PackageError> {
// Use -y to make it non-interactive and --quiet to reduce output
execute_package_command(&["apt-get", "install", "-y", "--quiet", package], self.debug)
execute_package_command(
&["apt-get", "install", "-y", "--quiet", package],
self.debug,
)
}
fn remove(&self, package: &str) -> Result<CommandResult, PackageError> {
@ -205,7 +211,8 @@ impl PackageManager for AptPackageManager {
fn list_installed(&self) -> Result<Vec<String>, PackageError> {
let result = execute_package_command(&["dpkg", "--get-selections"], self.debug)?;
let packages = result.stdout
let packages = result
.stdout
.lines()
.filter_map(|line| {
let parts: Vec<&str> = line.split_whitespace().collect();
@ -221,7 +228,8 @@ impl PackageManager for AptPackageManager {
fn search(&self, query: &str) -> Result<Vec<String>, PackageError> {
let result = execute_package_command(&["apt-cache", "search", query], self.debug)?;
let packages = result.stdout
let packages = result
.stdout
.lines()
.map(|line| {
let parts: Vec<&str> = line.split_whitespace().collect();
@ -280,7 +288,8 @@ impl PackageManager for BrewPackageManager {
fn list_installed(&self) -> Result<Vec<String>, PackageError> {
let result = execute_package_command(&["brew", "list", "--formula"], self.debug)?;
let packages = result.stdout
let packages = result
.stdout
.lines()
.map(|line| line.trim().to_string())
.filter(|s| !s.is_empty())
@ -290,7 +299,8 @@ impl PackageManager for BrewPackageManager {
fn search(&self, query: &str) -> Result<Vec<String>, PackageError> {
let result = execute_package_command(&["brew", "search", query], self.debug)?;
let packages = result.stdout
let packages = result
.stdout
.lines()
.map(|line| line.trim().to_string())
.filter(|s| !s.is_empty())
@ -344,7 +354,9 @@ impl PackHero {
match self.platform {
Platform::Ubuntu => Ok(Box::new(AptPackageManager::new(self.debug))),
Platform::MacOS => Ok(Box::new(BrewPackageManager::new(self.debug))),
Platform::Unknown => Err(PackageError::UnsupportedPlatform("Unsupported platform".to_string())),
Platform::Unknown => Err(PackageError::UnsupportedPlatform(
"Unsupported platform".to_string(),
)),
}
}
@ -394,8 +406,8 @@ impl PackHero {
#[cfg(test)]
mod tests {
// Import the std::process::Command directly for some test-specific commands
use std::process::Command as StdCommand;
use super::*;
use std::process::Command as StdCommand;
use std::sync::{Arc, Mutex};
#[test]
@ -435,6 +447,8 @@ mod tests {
// Mock package manager for testing
struct MockPackageManager {
// debug field is kept for consistency with real package managers
#[allow(dead_code)]
debug: bool,
install_called: Arc<Mutex<bool>>,
remove_called: Arc<Mutex<bool>>,
@ -474,7 +488,9 @@ mod tests {
code: 0,
})
} else {
Err(PackageError::CommandFailed("Mock install failed".to_string()))
Err(PackageError::CommandFailed(
"Mock install failed".to_string(),
))
}
}
@ -488,7 +504,9 @@ mod tests {
code: 0,
})
} else {
Err(PackageError::CommandFailed("Mock remove failed".to_string()))
Err(PackageError::CommandFailed(
"Mock remove failed".to_string(),
))
}
}
@ -502,7 +520,9 @@ mod tests {
code: 0,
})
} else {
Err(PackageError::CommandFailed("Mock update failed".to_string()))
Err(PackageError::CommandFailed(
"Mock update failed".to_string(),
))
}
}
@ -516,7 +536,9 @@ mod tests {
code: 0,
})
} else {
Err(PackageError::CommandFailed("Mock upgrade failed".to_string()))
Err(PackageError::CommandFailed(
"Mock upgrade failed".to_string(),
))
}
}
@ -525,16 +547,23 @@ mod tests {
if self.should_succeed {
Ok(vec!["package1".to_string(), "package2".to_string()])
} else {
Err(PackageError::CommandFailed("Mock list_installed failed".to_string()))
Err(PackageError::CommandFailed(
"Mock list_installed failed".to_string(),
))
}
}
fn search(&self, query: &str) -> Result<Vec<String>, PackageError> {
*self.search_called.lock().unwrap() = true;
if self.should_succeed {
Ok(vec![format!("result1-{}", query), format!("result2-{}", query)])
Ok(vec![
format!("result1-{}", query),
format!("result2-{}", query),
])
} else {
Err(PackageError::CommandFailed("Mock search failed".to_string()))
Err(PackageError::CommandFailed(
"Mock search failed".to_string(),
))
}
}
@ -543,7 +572,9 @@ mod tests {
if self.should_succeed {
Ok(package == "installed-package")
} else {
Err(PackageError::CommandFailed("Mock is_installed failed".to_string()))
Err(PackageError::CommandFailed(
"Mock is_installed failed".to_string(),
))
}
}
}
@ -551,6 +582,7 @@ mod tests {
// Custom PackHero for testing with a mock package manager
struct TestPackHero {
platform: Platform,
#[allow(dead_code)]
debug: bool,
mock_manager: MockPackageManager,
}
@ -567,7 +599,9 @@ mod tests {
fn get_package_manager(&self) -> Result<&dyn PackageManager, PackageError> {
match self.platform {
Platform::Ubuntu | Platform::MacOS => Ok(&self.mock_manager),
Platform::Unknown => Err(PackageError::UnsupportedPlatform("Unsupported platform".to_string())),
Platform::Unknown => Err(PackageError::UnsupportedPlatform(
"Unsupported platform".to_string(),
)),
}
}
@ -635,13 +669,19 @@ mod tests {
// Test list_installed
let result = hero.list_installed();
assert!(result.is_ok());
assert_eq!(result.unwrap(), vec!["package1".to_string(), "package2".to_string()]);
assert_eq!(
result.unwrap(),
vec!["package1".to_string(), "package2".to_string()]
);
assert!(*hero.mock_manager.list_installed_called.lock().unwrap());
// Test search
let result = hero.search("query");
assert!(result.is_ok());
assert_eq!(result.unwrap(), vec!["result1-query".to_string(), "result2-query".to_string()]);
assert_eq!(
result.unwrap(),
vec!["result1-query".to_string(), "result2-query".to_string()]
);
assert!(*hero.mock_manager.search_called.lock().unwrap());
// Test is_installed
@ -731,7 +771,10 @@ mod tests {
// Check if we're on Ubuntu
let platform = Platform::detect();
if platform != Platform::Ubuntu {
println!("Skipping real package operations test on non-Ubuntu platform: {:?}", platform);
println!(
"Skipping real package operations test on non-Ubuntu platform: {:?}",
platform
);
return;
}
@ -753,7 +796,10 @@ mod tests {
}
};
println!("Package {} is installed before test: {}", test_package, is_installed_before);
println!(
"Package {} is installed before test: {}",
test_package, is_installed_before
);
// If the package is already installed, we'll remove it first
if is_installed_before {
@ -775,9 +821,12 @@ mod tests {
} else {
println!("Verified package {} was removed", test_package);
}
},
}
Err(e) => {
println!("Error checking if package is installed after removal: {}", e);
println!(
"Error checking if package is installed after removal: {}",
e
);
return;
}
}
@ -802,9 +851,12 @@ mod tests {
} else {
println!("Verified package {} was installed", test_package);
}
},
}
Err(e) => {
println!("Error checking if package is installed after installation: {}", e);
println!(
"Error checking if package is installed after installation: {}",
e
);
return;
}
}
@ -814,8 +866,11 @@ mod tests {
match hero.search("wget") {
Ok(results) => {
println!("Search results: {:?}", results);
assert!(results.iter().any(|r| r.contains("wget")), "Search results should contain wget");
},
assert!(
results.iter().any(|r| r.contains("wget")),
"Search results should contain wget"
);
}
Err(e) => {
println!("Error searching for packages: {}", e);
return;
@ -828,9 +883,12 @@ mod tests {
Ok(packages) => {
println!("Found {} installed packages", packages.len());
// Check if our test package is in the list
assert!(packages.iter().any(|p| p == test_package),
"Installed packages list should contain {}", test_package);
},
assert!(
packages.iter().any(|p| p == test_package),
"Installed packages list should contain {}",
test_package
);
}
Err(e) => {
println!("Error listing installed packages: {}", e);
return;
@ -857,9 +915,12 @@ mod tests {
} else {
println!("Verified package {} was removed", test_package);
}
},
}
Err(e) => {
println!("Error checking if package is installed after removal: {}", e);
println!(
"Error checking if package is installed after removal: {}",
e
);
return;
}
}

View File

@ -1,7 +1,6 @@
use regex::Regex;
use std::fs;
use std::io::{self, Read, Seek, SeekFrom};
use std::io::{self, Read};
use std::path::Path;
/// Represents the type of replacement to perform.
@ -75,7 +74,7 @@ impl TextReplacer {
pub fn replace_file_to<P1: AsRef<Path>, P2: AsRef<Path>>(
&self,
input_path: P1,
output_path: P2
output_path: P2,
) -> io::Result<()> {
let content = self.replace_file(&input_path)?;
fs::write(output_path, content)?;
@ -157,10 +156,8 @@ impl TextReplacerBuilder {
ReplaceMode::Literal(pattern)
};
self.operations.push(ReplacementOperation {
mode,
replacement,
});
self.operations
.push(ReplacementOperation { mode, replacement });
true
} else {
@ -187,7 +184,7 @@ impl TextReplacerBuilder {
#[cfg(test)]
mod tests {
use super::*;
use std::io::Write;
use std::io::{Seek, SeekFrom, Write};
use tempfile::NamedTempFile;
#[test]

View File

@ -1,9 +1,9 @@
use std::collections::HashMap;
use super::{
error::RfsError,
cmd::execute_rfs_command,
error::RfsError,
types::{Mount, MountType, StoreSpec},
};
use std::collections::HashMap;
/// Builder for RFS mount operations
#[derive(Clone)]
@ -17,6 +17,7 @@ pub struct RfsBuilder {
/// Mount options
options: HashMap<String, String>,
/// Mount ID
#[allow(dead_code)]
mount_id: Option<String>,
/// Debug mode
debug: bool,
@ -139,7 +140,11 @@ impl RfsBuilder {
source: self.source,
target: self.target,
fs_type: self.mount_type.to_string(),
options: self.options.iter().map(|(k, v)| format!("{}={}", k, v)).collect(),
options: self
.options
.iter()
.map(|(k, v)| format!("{}={}", k, v))
.collect(),
})
}
@ -154,7 +159,10 @@ impl RfsBuilder {
// Check for errors
if !result.success {
return Err(RfsError::UnmountFailed(format!("Failed to unmount {}: {}", self.target, result.stderr)));
return Err(RfsError::UnmountFailed(format!(
"Failed to unmount {}: {}",
self.target, result.stderr
)));
}
Ok(())
@ -272,7 +280,10 @@ impl PackBuilder {
// Check for errors
if !result.success {
return Err(RfsError::PackFailed(format!("Failed to pack {}: {}", self.directory, result.stderr)));
return Err(RfsError::PackFailed(format!(
"Failed to pack {}: {}",
self.directory, result.stderr
)));
}
Ok(())

View File

@ -1,7 +1,7 @@
use crate::process::{run_command, CommandResult};
use super::error::RfsError;
use std::thread_local;
use crate::process::{run_command, CommandResult};
use std::cell::RefCell;
use std::thread_local;
// Thread-local storage for debug flag
thread_local! {
@ -9,6 +9,7 @@ thread_local! {
}
/// Set the thread-local debug flag
#[allow(dead_code)]
pub fn set_thread_local_debug(debug: bool) {
DEBUG.with(|d| {
*d.borrow_mut() = debug;
@ -17,9 +18,7 @@ pub fn set_thread_local_debug(debug: bool) {
/// Get the current thread-local debug flag
pub fn thread_local_debug() -> bool {
DEBUG.with(|d| {
*d.borrow()
})
DEBUG.with(|d| *d.borrow())
}
/// Execute an RFS command with the given arguments