From be61ad7243fe988d0825dec56e85628b430e82c7 Mon Sep 17 00:00:00 2001 From: Dmitry Savintsev Date: Fri, 28 Oct 2022 14:07:52 +0000 Subject: [PATCH] Make the integration tests handle errors internally. Instead of returning anyhow>>Result<()> handle errors 'in-place' with unwrap or panic, for more informative and user-friendly error messages on test failures. Fixes #421. Signed-off-by: Dmitry Savintsev --- test/integration-test/src/main.rs | 4 +- test/integration-test/src/tests/elf.rs | 10 ++-- test/integration-test/src/tests/load.rs | 76 ++++++++++++------------ test/integration-test/src/tests/mod.rs | 2 +- test/integration-test/src/tests/smoke.rs | 14 ++--- 5 files changed, 51 insertions(+), 55 deletions(-) diff --git a/test/integration-test/src/main.rs b/test/integration-test/src/main.rs index 60744da5..1fd32ea4 100644 --- a/test/integration-test/src/main.rs +++ b/test/integration-test/src/main.rs @@ -30,9 +30,7 @@ enum Command { macro_rules! exec_test { ($test:expr) => {{ info!("Running {}", $test.name); - if let Err(e) = ($test.test_fn)() { - panic!("{}", e) - } + ($test.test_fn)(); }}; } diff --git a/test/integration-test/src/tests/elf.rs b/test/integration-test/src/tests/elf.rs index f139dfaf..9522672d 100644 --- a/test/integration-test/src/tests/elf.rs +++ b/test/integration-test/src/tests/elf.rs @@ -1,15 +1,14 @@ use super::{integration_test, IntegrationTest}; -use anyhow::bail; use aya::include_bytes_aligned; use object::{Object, ObjectSymbol}; #[integration_test] -fn test_maps() -> anyhow::Result<()> { +fn test_maps() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/map_test"); - let obj_file = object::File::parse(bytes)?; + let obj_file = object::File::parse(bytes).unwrap(); if obj_file.section_by_name("maps").is_none() { - bail!("No 'maps' ELF section"); + panic!("No 'maps' ELF section"); } let mut found = false; for sym in obj_file.symbols() { @@ -21,7 +20,6 @@ fn test_maps() -> anyhow::Result<()> { } } if !found { - bail!("No symbol 'BAR' in ELF file") + panic!("No symbol 'BAR' in ELF file") } - Ok(()) } diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index de01e6c2..b88867f9 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -16,28 +16,30 @@ const MAX_RETRIES: u32 = 100; const RETRY_DURATION_MS: u64 = 10; #[integration_test] -fn long_name() -> anyhow::Result<()> { +fn long_name() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/name_test"); - let mut bpf = Bpf::load(bytes)?; - let name_prog: &mut Xdp = bpf.program_mut("ihaveaverylongname").unwrap().try_into()?; + let mut bpf = Bpf::load(bytes).unwrap(); + let name_prog: &mut Xdp = bpf + .program_mut("ihaveaverylongname") + .unwrap() + .try_into() + .unwrap(); name_prog.load().unwrap(); - name_prog.attach("lo", XdpFlags::default())?; + name_prog.attach("lo", XdpFlags::default()).unwrap(); // We used to be able to assert with bpftool that the program name was short. // It seem though that it now uses the name from the ELF symbol table instead. // Therefore, as long as we were able to load the program, this is good enough. - - Ok(()) } #[integration_test] -fn multiple_btf_maps() -> anyhow::Result<()> { +fn multiple_btf_maps() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/multimap-btf.bpf.o"); - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); - let map_1: Array<_, u64> = bpf.take_map("map_1").unwrap().try_into()?; - let map_2: Array<_, u64> = bpf.take_map("map_2").unwrap().try_into()?; + let map_1: Array<_, u64> = bpf.take_map("map_1").unwrap().try_into().unwrap(); + let map_2: Array<_, u64> = bpf.take_map("map_2").unwrap().try_into().unwrap(); let prog: &mut TracePoint = bpf.program_mut("tracepoint").unwrap().try_into().unwrap(); prog.load().unwrap(); @@ -46,17 +48,19 @@ fn multiple_btf_maps() -> anyhow::Result<()> { thread::sleep(time::Duration::from_secs(3)); let key = 0; - let val_1 = map_1.get(&key, 0)?; - let val_2 = map_2.get(&key, 0)?; + let val_1 = map_1.get(&key, 0).unwrap(); + let val_2 = map_2.get(&key, 0).unwrap(); assert_eq!(val_1, 24); assert_eq!(val_2, 42); - - Ok(()) } fn is_loaded(name: &str) -> bool { - let output = Command::new("bpftool").args(["prog"]).output().unwrap(); + let output = Command::new("bpftool").args(["prog"]).output(); + let output = match output { + Err(e) => panic!("Failed to run 'bpftool prog': {e}"), + Ok(out) => out, + }; let stdout = String::from_utf8(output.stdout).unwrap(); stdout.contains(name) } @@ -77,9 +81,9 @@ macro_rules! assert_loaded { } #[integration_test] -fn unload() -> anyhow::Result<()> { +fn unload() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/test"); - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); let prog: &mut Xdp = bpf.program_mut("test_unload").unwrap().try_into().unwrap(); prog.load().unwrap(); let link = prog.attach("lo", XdpFlags::default()).unwrap(); @@ -99,50 +103,47 @@ fn unload() -> anyhow::Result<()> { prog.unload().unwrap(); assert_loaded!("test_unload", false); - Ok(()) } #[integration_test] -fn pin_link() -> anyhow::Result<()> { +fn pin_link() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/test"); - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); let prog: &mut Xdp = bpf.program_mut("test_unload").unwrap().try_into().unwrap(); prog.load().unwrap(); let link_id = prog.attach("lo", XdpFlags::default()).unwrap(); - let link = prog.take_link(link_id)?; + let link = prog.take_link(link_id).unwrap(); assert_loaded!("test_unload", true); - let fd_link: FdLink = link.try_into()?; - let pinned = fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo")?; + let fd_link: FdLink = link.try_into().unwrap(); + let pinned = fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo").unwrap(); // because of the pin, the program is still attached - prog.unload()?; + prog.unload().unwrap(); assert_loaded!("test_unload", true); // delete the pin, but the program is still attached - let new_link = pinned.unpin()?; + let new_link = pinned.unpin().unwrap(); assert_loaded!("test_unload", true); // finally when new_link is dropped we're detached drop(new_link); assert_loaded!("test_unload", false); - - Ok(()) } #[integration_test] -fn pin_lifecycle() -> anyhow::Result<()> { +fn pin_lifecycle() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/pass"); // 1. Load Program and Pin { - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); let prog: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap(); prog.load().unwrap(); let link_id = prog.attach("lo", XdpFlags::default()).unwrap(); - let link = prog.take_link(link_id)?; - let fd_link: FdLink = link.try_into()?; - fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo")?; + let link = prog.take_link(link_id).unwrap(); + let fd_link: FdLink = link.try_into().unwrap(); + fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo").unwrap(); } // should still be loaded since link was pinned @@ -150,17 +151,18 @@ fn pin_lifecycle() -> anyhow::Result<()> { // 2. Load a new version of the program, unpin link, and atomically replace old program { - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); let prog: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap(); prog.load().unwrap(); - let link = PinnedLink::from_pin("/sys/fs/bpf/aya-xdp-test-lo")?.unpin()?; - prog.attach_to_link(link.try_into()?)?; + let link = PinnedLink::from_pin("/sys/fs/bpf/aya-xdp-test-lo") + .unwrap() + .unpin() + .unwrap(); + prog.attach_to_link(link.try_into().unwrap()).unwrap(); assert_loaded!("pass", true); } // program should be unloaded assert_loaded!("pass", false); - - Ok(()) } diff --git a/test/integration-test/src/tests/mod.rs b/test/integration-test/src/tests/mod.rs index 2dd5524a..53a98abe 100644 --- a/test/integration-test/src/tests/mod.rs +++ b/test/integration-test/src/tests/mod.rs @@ -12,7 +12,7 @@ pub use integration_test_macros::integration_test; #[derive(Debug)] pub struct IntegrationTest { pub name: &'static str, - pub test_fn: fn() -> anyhow::Result<()>, + pub test_fn: fn(), } pub(crate) fn kernel_version() -> anyhow::Result<(u8, u8, u8)> { diff --git a/test/integration-test/src/tests/smoke.rs b/test/integration-test/src/tests/smoke.rs index 795f0856..67ee8f06 100644 --- a/test/integration-test/src/tests/smoke.rs +++ b/test/integration-test/src/tests/smoke.rs @@ -8,29 +8,28 @@ use log::info; use super::{integration_test, kernel_version, IntegrationTest}; #[integration_test] -fn xdp() -> anyhow::Result<()> { +fn xdp() { let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/pass"); - let mut bpf = Bpf::load(bytes)?; + let mut bpf = Bpf::load(bytes).unwrap(); let dispatcher: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap(); dispatcher.load().unwrap(); dispatcher.attach("lo", XdpFlags::default()).unwrap(); - Ok(()) } #[integration_test] -fn extension() -> anyhow::Result<()> { - let (major, minor, _) = kernel_version()?; +fn extension() { + let (major, minor, _) = kernel_version().unwrap(); if major < 5 || minor < 9 { info!( "skipping as {}.{} does not meet version requirement of 5.9", major, minor ); - return Ok(()); + return; } // TODO: Check kernel version == 5.9 or later let main_bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/main.bpf.o"); - let mut bpf = Bpf::load(main_bytes)?; + let mut bpf = Bpf::load(main_bytes).unwrap(); let pass: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap(); pass.load().unwrap(); pass.attach("lo", XdpFlags::default()).unwrap(); @@ -39,5 +38,4 @@ fn extension() -> anyhow::Result<()> { let mut bpf = BpfLoader::new().extension("drop").load(ext_bytes).unwrap(); let drop_: &mut Extension = bpf.program_mut("drop").unwrap().try_into().unwrap(); drop_.load(pass.fd().unwrap(), "xdp_pass").unwrap(); - Ok(()) }