diff --git a/crates/integration-tests/src/lib.rs b/crates/integration-tests/src/lib.rs index ee3a8db..f2fd930 100644 --- a/crates/integration-tests/src/lib.rs +++ b/crates/integration-tests/src/lib.rs @@ -66,8 +66,9 @@ pub static PARAMETERIZED_INTEGRATION_TESTS: [ParameterizedIntegrationTest]; /// /// ```ignore /// fn test_basic_functionality() -> Result<()> { -/// let output = run_bcvk(&["some", "args"])?; -/// output.assert_success("test"); +/// let sh = shell()?; +/// let bck = get_bck_command()?; +/// cmd!(sh, "{bck} some args").run()?; /// Ok(()) /// } /// integration_test!(test_basic_functionality); @@ -91,8 +92,9 @@ macro_rules! integration_test { /// /// ```ignore /// fn test_with_image(image: &str) -> Result<()> { -/// let output = run_bcvk(&["command", image])?; -/// output.assert_success("test"); +/// let sh = shell()?; +/// let bck = get_bck_command()?; +/// cmd!(sh, "{bck} command {image}").run()?; /// Ok(()) /// } /// parameterized_integration_test!(test_with_image); diff --git a/crates/integration-tests/src/main.rs b/crates/integration-tests/src/main.rs index 9c71461..c8c9b82 100644 --- a/crates/integration-tests/src/main.rs +++ b/crates/integration-tests/src/main.rs @@ -1,8 +1,9 @@ //! Integration tests for bcvk -use camino::Utf8Path; use std::process::Output; +use camino::Utf8Path; + use color_eyre::eyre::{eyre, Context}; use color_eyre::Result; use libtest_mimic::{Arguments, Trial}; @@ -26,6 +27,11 @@ mod tests { pub mod to_disk; } +/// Create a new xshell Shell for running commands +pub(crate) fn shell() -> Result { + Shell::new().map_err(|e| eyre!("Failed to create shell: {}", e)) +} + /// Get the path to the bcvk binary, checking BCVK_PATH env var first, then falling back to "bcvk" pub(crate) fn get_bck_command() -> Result { if let Some(path) = std::env::var("BCVK_PATH").ok() { @@ -97,16 +103,6 @@ impl CapturedOutput { } } - /// Assert that the command succeeded, printing debug info on failure - pub fn assert_success(&self, context: &str) { - assert!( - self.output.status.success(), - "{} failed: {}", - context, - self.stderr - ); - } - /// Get the exit code pub fn exit_code(&self) -> Option { self.output.status.code() @@ -118,47 +114,16 @@ impl CapturedOutput { } } -/// Run a command, capturing output -pub(crate) fn run_command(program: &str, args: &[&str]) -> std::io::Result { - let output = std::process::Command::new(program).args(args).output()?; - Ok(CapturedOutput::new(output)) -} - -/// Run the bcvk command, capturing output -pub(crate) fn run_bcvk(args: &[&str]) -> std::io::Result { - let bck = get_bck_command().expect("Failed to get bcvk command"); - run_command(&bck, args) -} - -/// Run the bcvk command with inherited stdout/stderr (no capture) -/// Use this when you just need to verify the command succeeded without checking output -pub(crate) fn run_bcvk_nocapture(args: &[&str]) -> std::io::Result<()> { - let bck = get_bck_command().expect("Failed to get bcvk command"); - let status = std::process::Command::new(&bck).args(args).status()?; - assert!( - status.success(), - "bcvk command failed with args: {:?}", - args - ); - Ok(()) -} - fn test_images_list() -> Result<()> { println!("Running test: bcvk images list --json"); - let sh = Shell::new()?; + let sh = shell()?; let bck = get_bck_command()?; // Run the bcvk images list command with JSON output - let output = cmd!(sh, "{bck} images list --json").output()?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(eyre!("Failed to run 'bcvk images list --json': {}", stderr)); - } + let stdout = cmd!(sh, "{bck} images list --json").read()?; // Parse the JSON output - let stdout = String::from_utf8(output.stdout)?; let images: Value = serde_json::from_str(&stdout).context("Failed to parse JSON output")?; // Verify the structure and content of the JSON diff --git a/crates/integration-tests/src/tests/libvirt_base_disks.rs b/crates/integration-tests/src/tests/libvirt_base_disks.rs index f8e65a0..f8a4e89 100644 --- a/crates/integration-tests/src/tests/libvirt_base_disks.rs +++ b/crates/integration-tests/src/tests/libvirt_base_disks.rs @@ -8,14 +8,16 @@ use color_eyre::Result; use integration_tests::integration_test; +use xshell::cmd; use regex::Regex; -use std::process::Command; -use crate::{get_bck_command, get_test_image, run_bcvk}; +use crate::{get_bck_command, get_test_image, shell}; /// Test that base disk is created and reused for multiple VMs fn test_base_disk_creation_and_reuse() -> Result<()> { + let sh = shell()?; + let bck = get_bck_command()?; let test_image = get_test_image(); // Generate unique names for test VMs @@ -36,95 +38,84 @@ fn test_base_disk_creation_and_reuse() -> Result<()> { // Create first VM - this should create a new base disk println!("Creating first VM (should create base disk)..."); - let vm1_output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &vm1_name, - "--filesystem", - "ext4", - &test_image, - ])?; - - println!("VM1 stdout: {}", vm1_output.stdout); - println!("VM1 stderr: {}", vm1_output.stderr); - - if !vm1_output.success() { + let vm1_output = cmd!( + sh, + "{bck} libvirt run --name {vm1_name} --filesystem ext4 {test_image}" + ) + .ignore_status() + .output()?; + + let vm1_stdout = String::from_utf8_lossy(&vm1_output.stdout); + let vm1_stderr = String::from_utf8_lossy(&vm1_output.stderr); + + println!("VM1 stdout: {}", vm1_stdout); + println!("VM1 stderr: {}", vm1_stderr); + + if !vm1_output.status.success() { cleanup_domain(&vm1_name); cleanup_domain(&vm2_name); - panic!("Failed to create first VM: {}", vm1_output.stderr); + panic!("Failed to create first VM: {}", vm1_stderr); } // Verify base disk was created assert!( - vm1_output.stdout.contains("Using base disk") || vm1_output.stdout.contains("base disk"), + vm1_stdout.contains("Using base disk") || vm1_stdout.contains("base disk"), "Should mention base disk creation" ); // Create second VM - this should reuse the base disk println!("Creating second VM (should reuse base disk)..."); - let vm2_output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &vm2_name, - "--filesystem", - "ext4", - &test_image, - ])?; - - println!("VM2 stdout: {}", vm2_output.stdout); - println!("VM2 stderr: {}", vm2_output.stderr); + let vm2_output = cmd!( + sh, + "{bck} libvirt run --name {vm2_name} --filesystem ext4 {test_image}" + ) + .ignore_status() + .output()?; + + let vm2_stdout = String::from_utf8_lossy(&vm2_output.stdout); + let vm2_stderr = String::from_utf8_lossy(&vm2_output.stderr); + + println!("VM2 stdout: {}", vm2_stdout); + println!("VM2 stderr: {}", vm2_stderr); // Cleanup before assertions cleanup_domain(&vm1_name); cleanup_domain(&vm2_name); - if !vm2_output.success() { - panic!("Failed to create second VM: {}", vm2_output.stderr); + if !vm2_output.status.success() { + panic!("Failed to create second VM: {}", vm2_stderr); } // Verify base disk was reused (should be faster and mention using existing) assert!( - vm2_output.stdout.contains("Using base disk") || vm2_output.stdout.contains("base disk"), + vm2_stdout.contains("Using base disk") || vm2_stdout.contains("base disk"), "Should mention using base disk" ); // Test base-disks list shows creation timestamp println!("Testing that base-disks list shows creation timestamp..."); + let sh = shell()?; let bck = get_bck_command()?; - let list_output = std::process::Command::new(&bck) - .args(["libvirt", "base-disks", "list"]) - .output() - .expect("Failed to run base-disks list"); - - let list_stdout = String::from_utf8_lossy(&list_output.stdout); - let list_stderr = String::from_utf8_lossy(&list_output.stderr); - - if list_output.status.success() { - println!("base-disks list output:\n{}", list_stdout); + let list_stdout = cmd!(sh, "{bck} libvirt base-disks list").read()?; + println!("base-disks list output:\n{}", list_stdout); - // Should have CREATED column in header - assert!( - list_stdout.contains("CREATED"), - "Should show CREATED column in header" - ); + // Should have CREATED column in header + assert!( + list_stdout.contains("CREATED"), + "Should show CREATED column in header" + ); - // Should show timestamp values (either a date or "unknown") - // Timestamp format is YYYY-MM-DD HH:MM - let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}|unknown").unwrap(); - let has_timestamp = re.is_match(&list_stdout); - assert!( - has_timestamp, - "Should show timestamp values in CREATED column" - ); + // Should show timestamp values (either a date or "unknown") + // Timestamp format is YYYY-MM-DD HH:MM + let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}|unknown").unwrap(); + let has_timestamp = re.is_match(&list_stdout); + assert!( + has_timestamp, + "Should show timestamp values in CREATED column" + ); - println!("✓ base-disks list shows creation timestamp"); - } else { - println!("base-disks list failed: {}", list_stderr); - panic!("Failed to run base-disks list: {}", list_stderr); - } + println!("✓ base-disks list shows creation timestamp"); println!("✓ Base disk creation and reuse test passed"); Ok(()) @@ -133,15 +124,14 @@ integration_test!(test_base_disk_creation_and_reuse); /// Test base-disks list command fn test_base_disks_list_command() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; println!("Testing base-disks list command"); - let output = Command::new(&bck) - .args(["libvirt", "base-disks", "list"]) - .output() - .expect("Failed to run base-disks list"); - + let output = cmd!(sh, "{bck} libvirt base-disks list") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -174,15 +164,14 @@ integration_test!(test_base_disks_list_command); /// Test base-disks prune command with dry-run fn test_base_disks_prune_dry_run() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; println!("Testing base-disks prune --dry-run command"); - let output = Command::new(&bck) - .args(["libvirt", "base-disks", "prune", "--dry-run"]) - .output() - .expect("Failed to run base-disks prune --dry-run"); - + let output = cmd!(sh, "{bck} libvirt base-disks prune --dry-run") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -211,6 +200,8 @@ integration_test!(test_base_disks_prune_dry_run); /// Test that VM disks reference base disks correctly fn test_vm_disk_references_base() -> Result<()> { + let sh = shell()?; + let bck = get_bck_command()?; let test_image = get_test_image(); let timestamp = std::time::SystemTime::now() @@ -224,34 +215,23 @@ fn test_vm_disk_references_base() -> Result<()> { cleanup_domain(&vm_name); // Create VM - let output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &vm_name, - "--filesystem", - "ext4", - &test_image, - ])?; - - if !output.success() { + let output = cmd!( + sh, + "{bck} libvirt run --name {vm_name} --filesystem ext4 {test_image}" + ) + .ignore_status() + .output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); cleanup_domain(&vm_name); - panic!("Failed to create VM: {}", output.stderr); + panic!("Failed to create VM: {}", stderr); } // Get VM disk path from domain XML - let dumpxml_output = Command::new("virsh") - .args(&["dumpxml", &vm_name]) - .output() - .expect("Failed to dump domain XML"); - - if !dumpxml_output.status.success() { - cleanup_domain(&vm_name); - panic!("Failed to get domain XML"); - } - - let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout); + let sh = shell()?; + let domain_xml = cmd!(sh, "virsh dumpxml {vm_name}").read()?; // Parse XML using bcvk's xml_utils to extract disk path let dom = bcvk::xml_utils::parse_xml_dom(&domain_xml).expect("Failed to parse domain XML"); @@ -286,26 +266,34 @@ integration_test!(test_vm_disk_references_base); fn cleanup_domain(domain_name: &str) { println!("Cleaning up domain: {}", domain_name); + let sh = match shell() { + Ok(sh) => sh, + Err(_) => return, + }; + // Stop domain if running - let _ = Command::new("virsh") - .args(&["destroy", domain_name]) - .output(); + let _ = cmd!(sh, "virsh destroy {domain_name}") + .ignore_status() + .quiet() + .run(); // Use bcvk libvirt rm for proper cleanup let bck = match get_bck_command() { Ok(cmd) => cmd, Err(_) => return, }; - let cleanup_output = Command::new(&bck) - .args(&["libvirt", "rm", domain_name, "--force", "--stop"]) - .output(); - if let Ok(output) = cleanup_output { - if output.status.success() { + match cmd!(sh, "{bck} libvirt rm {domain_name} --force --stop") + .ignore_status() + .output() + { + Ok(output) if output.status.success() => { println!("Successfully cleaned up domain: {}", domain_name); - } else { + } + Ok(output) => { let stderr = String::from_utf8_lossy(&output.stderr); println!("Cleanup warning (may be expected): {}", stderr); } + Err(_) => {} } } diff --git a/crates/integration-tests/src/tests/libvirt_port_forward.rs b/crates/integration-tests/src/tests/libvirt_port_forward.rs index 52ab285..787a848 100644 --- a/crates/integration-tests/src/tests/libvirt_port_forward.rs +++ b/crates/integration-tests/src/tests/libvirt_port_forward.rs @@ -7,33 +7,27 @@ use color_eyre::Result; use integration_tests::integration_test; +use xshell::cmd; -use std::process::Command; - -use crate::{get_bck_command, get_test_image, run_bcvk, LIBVIRT_INTEGRATION_TEST_LABEL}; +use crate::{get_bck_command, get_test_image, shell, LIBVIRT_INTEGRATION_TEST_LABEL}; /// Test port forwarding argument parsing fn test_libvirt_port_forward_parsing() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; // Test valid port forwarding formats - let valid_port_tests = vec![ - vec!["--port", "8080:80"], - vec!["--port", "80:80"], - vec!["--port", "3000:3000", "--port", "8080:80"], - vec!["-p", "9090:90"], + let valid_port_tests: Vec<&[&str]> = vec![ + &["--port", "8080:80"], + &["--port", "80:80"], + &["--port", "3000:3000", "--port", "8080:80"], + &["-p", "9090:90"], ]; for ports in valid_port_tests { - let mut args = vec!["libvirt", "run"]; - args.extend(ports.iter()); - args.push("--help"); // Just test parsing, don't actually run - - let output = Command::new(&bck) - .args(&args) - .output() - .expect("Failed to run libvirt run with port forwarding"); - + let output = cmd!(sh, "{bck} libvirt run {ports...} --help") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -59,6 +53,7 @@ integration_test!(test_libvirt_port_forward_parsing); /// Test port forwarding error handling for invalid formats fn test_libvirt_port_forward_invalid() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; let test_image = get_test_image(); @@ -72,30 +67,20 @@ fn test_libvirt_port_forward_invalid() -> Result<()> { ); // Test invalid port forwarding formats (should fail) - let invalid_port_tests = vec![ - (vec!["--port", "8080"], "missing guest port"), - (vec!["--port", "abc:80"], "invalid host port"), - (vec!["--port", "8080:xyz"], "invalid guest port"), - (vec!["--port", "70000:80"], "port out of range"), + let invalid_port_tests: Vec<(&[&str], &str)> = vec![ + (&["--port", "8080"], "missing guest port"), + (&["--port", "abc:80"], "invalid host port"), + (&["--port", "8080:xyz"], "invalid guest port"), + (&["--port", "70000:80"], "port out of range"), ]; for (ports, error_desc) in invalid_port_tests { - let mut args = vec![ - "libvirt", - "run", - "--name", - &domain_name, - "--transient", - "--filesystem", - "ext4", - ]; - args.extend(ports.iter()); - args.push(&test_image); - - let output = Command::new(&bck).args(&args).output().expect(&format!( - "Failed to run error case for port forwarding: {}", - error_desc - )); + let output = cmd!( + sh, + "{bck} libvirt run --name {domain_name} --transient --filesystem ext4 {ports...} {test_image}" + ) + .ignore_status() + .output()?; // Cleanup in case domain was partially created cleanup_domain(&domain_name); @@ -125,7 +110,10 @@ integration_test!(test_libvirt_port_forward_invalid); /// Test that port forwarding is correctly configured in domain XML fn test_libvirt_port_forward_xml() -> Result<()> { + let sh = shell()?; + let bck = get_bck_command()?; let test_image = get_test_image(); + let label = LIBVIRT_INTEGRATION_TEST_LABEL; // Generate unique domain name for this test let domain_name = format!( @@ -146,31 +134,24 @@ fn test_libvirt_port_forward_xml() -> Result<()> { // Create domain with port forwarding println!("Creating libvirt domain with port forwarding..."); - let create_output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--port", - "8080:80", - "--port", - "9090:8080", - "--filesystem", - "ext4", - &test_image, - ]) - .expect("Failed to run libvirt run with port forwarding"); - - println!("Create stdout: {}", create_output.stdout); - println!("Create stderr: {}", create_output.stderr); - - if !create_output.success() { + let create_output = cmd!( + sh, + "{bck} libvirt run --name {domain_name} --label {label} --port 8080:80 --port 9090:8080 --filesystem ext4 {test_image}" + ) + .ignore_status() + .output()?; + + let create_stdout = String::from_utf8_lossy(&create_output.stdout); + let create_stderr = String::from_utf8_lossy(&create_output.stderr); + + println!("Create stdout: {}", create_stdout); + println!("Create stderr: {}", create_stderr); + + if !create_output.status.success() { cleanup_domain(&domain_name); panic!( "Failed to create domain with port forwarding: {}", - create_output.stderr + create_stderr ); } @@ -178,26 +159,16 @@ fn test_libvirt_port_forward_xml() -> Result<()> { // Verify port forwarding in output assert!( - create_output.stdout.contains("Port forwarding:") - || create_output.stdout.contains("localhost:8080") - || create_output.stdout.contains("localhost:9090"), + create_stdout.contains("Port forwarding:") + || create_stdout.contains("localhost:8080") + || create_stdout.contains("localhost:9090"), "Output should mention port forwarding configuration" ); // Check domain XML for QEMU args with port forwarding println!("Checking domain XML for port forwarding configuration..."); - let dumpxml_output = Command::new("virsh") - .args(&["dumpxml", &domain_name]) - .output() - .expect("Failed to dump domain XML"); - - if !dumpxml_output.status.success() { - cleanup_domain(&domain_name); - let stderr = String::from_utf8_lossy(&dumpxml_output.stderr); - panic!("Failed to dump domain XML: {}", stderr); - } - - let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout); + let sh = shell()?; + let domain_xml = cmd!(sh, "virsh dumpxml {domain_name}").read()?; // Verify QEMU commandline arguments contain hostfwd assert!( @@ -233,10 +204,14 @@ integration_test!(test_libvirt_port_forward_xml); /// Test actual network connectivity through forwarded ports fn test_libvirt_port_forward_connectivity() -> Result<()> { + let sh = shell()?; + let bck = get_bck_command()?; let test_image = get_test_image(); + let label = LIBVIRT_INTEGRATION_TEST_LABEL; // Find an available port on the host let host_port = find_available_port()?; + let port_mapping = format!("{}:8080", host_port); // Generate unique domain name for this test let domain_name = format!( @@ -260,30 +235,24 @@ fn test_libvirt_port_forward_connectivity() -> Result<()> { "Creating libvirt domain with port forwarding {}:8080...", host_port ); - let create_output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--port", - &format!("{}:8080", host_port), - "--filesystem", - "ext4", - "--ssh-wait", - &test_image, - ]) - .expect("Failed to run libvirt run with port forwarding"); - - println!("Create stdout: {}", create_output.stdout); - println!("Create stderr: {}", create_output.stderr); - - if !create_output.success() { + let create_output = cmd!( + sh, + "{bck} libvirt run --name {domain_name} --label {label} --port {port_mapping} --filesystem ext4 --ssh-wait {test_image}" + ) + .ignore_status() + .output()?; + + let create_stdout = String::from_utf8_lossy(&create_output.stdout); + let create_stderr = String::from_utf8_lossy(&create_output.stderr); + + println!("Create stdout: {}", create_stdout); + println!("Create stderr: {}", create_stderr); + + if !create_output.status.success() { cleanup_domain(&domain_name); panic!( "Failed to create domain with port forwarding: {}", - create_output.stderr + create_stderr ); } @@ -297,22 +266,18 @@ fn test_libvirt_port_forward_connectivity() -> Result<()> { // Create a test file to serve println!("Creating test file in VM..."); - let create_file = run_bcvk(&[ - "libvirt", - "ssh", - "--timeout", - "10", - &domain_name, - "--", - "sh", - "-c", - "echo 'port-forward-test-success' > /tmp/test.txt", - ]) - .expect("Failed to create test file in VM"); - - if !create_file.success() { + let create_file_cmd = "echo 'port-forward-test-success' > /tmp/test.txt"; + let create_file_output = cmd!( + sh, + "{bck} libvirt ssh --timeout 10 {domain_name} -- sh -c {create_file_cmd}" + ) + .ignore_status() + .output()?; + + if !create_file_output.status.success() { + let stderr = String::from_utf8_lossy(&create_file_output.stderr); cleanup_domain(&domain_name); - panic!("Failed to create test file in VM: {}", create_file.stderr); + panic!("Failed to create test file in VM: {}", stderr); } println!("✓ Test file created successfully"); @@ -323,22 +288,19 @@ fn test_libvirt_port_forward_connectivity() -> Result<()> { // 3. Put the command in a subshell and background it // 4. Use 'exec' to replace the shell with the server, making it cleaner println!("Starting background HTTP server..."); - let start_server = run_bcvk(&[ - "libvirt", - "ssh", - "--timeout", - "10", - &domain_name, - "--", - "bash", - "-c", - "(cd /tmp && exec python3 -m http.server 8080 > /tmp/http.log 2>&1 < /dev/null &) && sleep 0.1", - ]) - .expect("Failed to start HTTP server in VM"); - - if !start_server.success() { + let start_server_cmd = + "(cd /tmp && exec python3 -m http.server 8080 > /tmp/http.log 2>&1 < /dev/null &) && sleep 0.1"; + let start_server_output = cmd!( + sh, + "{bck} libvirt ssh --timeout 10 {domain_name} -- bash -c {start_server_cmd}" + ) + .ignore_status() + .output()?; + + if !start_server_output.status.success() { + let stderr = String::from_utf8_lossy(&start_server_output.stderr); cleanup_domain(&domain_name); - panic!("Failed to start HTTP server in VM: {}", start_server.stderr); + panic!("Failed to start HTTP server in VM: {}", stderr); } println!("✓ HTTP server command executed"); @@ -348,21 +310,14 @@ fn test_libvirt_port_forward_connectivity() -> Result<()> { // Test connectivity from host to forwarded port using curl println!("Testing connectivity to forwarded port from host..."); + let sh = shell()?; let mut retry_count = 0; let max_retries = 5; let mut connection_success = false; + let url = format!("http://localhost:{}/test.txt", host_port); while retry_count < max_retries { - let curl_output = Command::new("curl") - .args(&[ - "-s", - "-m", - "5", // 5 second timeout - &format!("http://localhost:{}/test.txt", host_port), - ]) - .output(); - - match curl_output { + match cmd!(sh, "curl -s -m 5 {url}").ignore_status().output() { Ok(output) if output.status.success() => { let response = String::from_utf8_lossy(&output.stdout); println!("Received response: {}", response); @@ -417,27 +372,35 @@ integration_test!(test_libvirt_port_forward_connectivity); fn cleanup_domain(domain_name: &str) { println!("Cleaning up domain: {}", domain_name); + let sh = match shell() { + Ok(sh) => sh, + Err(_) => return, + }; + // Stop domain if running - let _ = Command::new("virsh") - .args(&["destroy", domain_name]) - .output(); + let _ = cmd!(sh, "virsh destroy {domain_name}") + .ignore_status() + .quiet() + .run(); // Use bcvk libvirt rm for proper cleanup let bck = match get_bck_command() { Ok(cmd) => cmd, Err(_) => return, }; - let cleanup_output = Command::new(&bck) - .args(&["libvirt", "rm", domain_name, "--force", "--stop"]) - .output(); - if let Ok(output) = cleanup_output { - if output.status.success() { + match cmd!(sh, "{bck} libvirt rm {domain_name} --force --stop") + .ignore_status() + .output() + { + Ok(output) if output.status.success() => { println!("Successfully cleaned up domain: {}", domain_name); - } else { + } + Ok(output) => { let stderr = String::from_utf8_lossy(&output.stderr); println!("Cleanup warning (may be expected): {}", stderr); } + Err(_) => {} } } diff --git a/crates/integration-tests/src/tests/libvirt_verb.rs b/crates/integration-tests/src/tests/libvirt_verb.rs index dd7d642..f7b418f 100644 --- a/crates/integration-tests/src/tests/libvirt_verb.rs +++ b/crates/integration-tests/src/tests/libvirt_verb.rs @@ -9,12 +9,9 @@ use color_eyre::Result; use integration_tests::integration_test; +use xshell::cmd; -use std::process::Command; - -use crate::{ - get_bck_command, get_test_image, run_bcvk, run_bcvk_nocapture, LIBVIRT_INTEGRATION_TEST_LABEL, -}; +use crate::{get_bck_command, get_test_image, shell, LIBVIRT_INTEGRATION_TEST_LABEL}; use bcvk::xml_utils::parse_xml_dom; /// Generate a random alphanumeric suffix for VM names to avoid collisions @@ -29,17 +26,14 @@ fn random_suffix() -> String { /// Test libvirt list functionality (lists domains) fn test_libvirt_list_functionality() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; - let output = Command::new(&bck) - .args(["libvirt", "list"]) - .output() - .expect("Failed to run libvirt list"); - - // May succeed or fail depending on libvirt availability, but should not crash + let output = cmd!(sh, "{bck} libvirt list").ignore_status().output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); + // May succeed or fail depending on libvirt availability, but should not crash if output.status.success() { println!("libvirt list succeeded: {}", stdout); // Should show domain listing format @@ -65,13 +59,12 @@ integration_test!(test_libvirt_list_functionality); /// Test libvirt list with JSON output fn test_libvirt_list_json_output() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; - let output = Command::new(&bck) - .args(["libvirt", "list", "--format", "json"]) - .output() - .expect("Failed to run libvirt list --format json"); - + let output = cmd!(sh, "{bck} libvirt list --format json") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -99,29 +92,24 @@ integration_test!(test_libvirt_list_json_output); /// Test domain resource configuration options fn test_libvirt_run_resource_options() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; // Test various resource configurations are accepted syntactically - let resource_tests = vec![ - vec!["--memory", "1G", "--cpus", "1"], - vec!["--memory", "4G", "--cpus", "4"], - vec!["--memory", "2048M", "--cpus", "2"], + let resource_tests: Vec<&[&str]> = vec![ + &["--memory", "1G", "--cpus", "1"], + &["--memory", "4G", "--cpus", "4"], + &["--memory", "2048M", "--cpus", "2"], ]; for resources in resource_tests { - let mut args = vec!["libvirt", "run"]; - args.extend(resources.iter()); - args.push("--help"); // Just test parsing, don't actually run - - let output = Command::new(&bck) - .args(&args) - .output() - .expect("Failed to run libvirt run with resources"); - - // Should show help and not fail on resource parsing + let output = cmd!(sh, "{bck} libvirt run {resources...} --help") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); + // Should show help and not fail on resource parsing if !output.status.success() { assert!( !stderr.contains("invalid") && !stderr.contains("parse"), @@ -144,24 +132,19 @@ integration_test!(test_libvirt_run_resource_options); /// Test domain networking configuration fn test_libvirt_run_networking() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; - let network_configs = vec![ - vec!["--network", "user"], - vec!["--network", "bridge"], - vec!["--network", "none"], + let network_configs: Vec<&[&str]> = vec![ + &["--network", "user"], + &["--network", "bridge"], + &["--network", "none"], ]; for network in network_configs { - let mut args = vec!["libvirt", "run"]; - args.extend(network.iter()); - args.push("--help"); // Just test parsing, don't actually run - - let output = Command::new(&bck) - .args(&args) - .output() - .expect("Failed to run libvirt run with network config"); - + let output = cmd!(sh, "{bck} libvirt run {network...} --help") + .ignore_status() + .output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -188,17 +171,16 @@ integration_test!(test_libvirt_run_networking); /// Test SSH integration with created domains (syntax only) fn test_libvirt_ssh_integration() -> Result<()> { + let sh = shell()?; let bck = get_bck_command()?; // Test that SSH command integration works syntactically - let output = Command::new(&bck) - .args(["libvirt", "ssh", "test-domain", "--", "echo", "hello"]) - .output() - .expect("Failed to run libvirt ssh command"); - - // Will likely fail since no domain exists, but should not crash + let output = cmd!(sh, "{bck} libvirt ssh test-domain -- echo hello") + .ignore_status() + .output()?; let stderr = String::from_utf8_lossy(&output.stderr); + // Will likely fail since no domain exists, but should not crash if !output.status.success() { // Should fail gracefully with domain-related error assert!( @@ -216,8 +198,10 @@ integration_test!(test_libvirt_ssh_integration); /// Comprehensive workflow test: creates a VM and tests multiple features /// This consolidates several smaller tests to reduce expensive disk image creation fn test_libvirt_comprehensive_workflow() -> Result<()> { - let test_image = get_test_image(); + let sh = shell()?; let bck = get_bck_command()?; + let test_image = get_test_image(); + let label = LIBVIRT_INTEGRATION_TEST_LABEL; // Generate unique domain name for this test let domain_name = format!("test-workflow-{}", random_suffix()); @@ -232,39 +216,29 @@ fn test_libvirt_comprehensive_workflow() -> Result<()> { // Create domain with multiple features: instancetype, labels, SSH println!("Creating libvirt domain with instancetype and labels..."); - let create_output = run_bcvk(&[ - "libvirt", - "run", - "--name", - &domain_name, - "--label", - LIBVIRT_INTEGRATION_TEST_LABEL, - "--label", - "test-workflow", - "--itype", - "u1.small", - "--filesystem", - "ext4", - &test_image, - ]) - .expect("Failed to run libvirt run"); - - println!("Create stdout: {}", create_output.stdout); - println!("Create stderr: {}", create_output.stderr); - - if !create_output.success() { + let create_output = cmd!( + sh, + "{bck} libvirt run --name {domain_name} --label {label} --label test-workflow --itype u1.small --filesystem ext4 {test_image}" + ) + .ignore_status() + .output()?; + + let create_stdout = String::from_utf8_lossy(&create_output.stdout); + let create_stderr = String::from_utf8_lossy(&create_output.stderr); + + println!("Create stdout: {}", create_stdout); + println!("Create stderr: {}", create_stderr); + + if !create_output.status.success() { cleanup_domain(&domain_name); - panic!("Failed to create domain: {}", create_output.stderr); + panic!("Failed to create domain: {}", create_stderr); } println!("Successfully created domain: {}", domain_name); // Test 1: Verify instancetype configuration (u1.small: 1 vcpu, 2048 MB) println!("Test 1: Verifying instancetype configuration..."); - let inspect_output = run_bcvk(&["libvirt", "inspect", "--format", "xml", &domain_name]) - .expect("Failed to run libvirt inspect"); - - let inspect_stdout = inspect_output.stdout; + let inspect_stdout = cmd!(sh, "{bck} libvirt inspect --format xml {domain_name}").read()?; let dom = parse_xml_dom(&inspect_stdout).expect("Failed to parse domain XML"); let vcpu_node = dom.find("vcpu").expect("vcpu element not found"); @@ -284,12 +258,8 @@ fn test_libvirt_comprehensive_workflow() -> Result<()> { // Test 2: Verify labels in domain XML println!("Test 2: Verifying label functionality..."); - let dumpxml_output = Command::new("virsh") - .args(&["dumpxml", &domain_name]) - .output() - .expect("Failed to dump domain XML"); - - let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout); + let sh = shell()?; + let domain_xml = cmd!(sh, "virsh dumpxml {domain_name}").read()?; assert!( domain_xml.contains("bootc:label") || domain_xml.contains("