From 2b1b3644dae22918deb92b1f8d684e1ff5be5a59 Mon Sep 17 00:00:00 2001 From: Erik Schilling Date: Fri, 7 Jul 2023 09:31:01 +0200 Subject: [PATCH] tree-wide: drop the Eq trait from Error I plan to add some wrapper errors around vhost errors. These end up nesting other errors all the way to std::error::Error, which has no Eq trait. The implementations were only used for comparisions in tests. While there is a assert_matches!() in nightly [1] it seems unlikely that further testing lib additions are getting standarized soon (or ever). One could use assert!(matches!()), but that would worsen the error messages for test failures. Hence, during review [2] we agreed on introducing the assert_matches crate. It got no dependencies and allows us to keep the good error messages while not needing to depend on nightly. Signed-off-by: Erik Schilling [1] https://doc.rust-lang.org/std/assert_matches/macro.assert_matches.html [2] https://github.com/rust-vmm/vhost-device/pull/388#discussion_r1257831748 --- Cargo.lock | 10 +++++++ crates/gpio/Cargo.toml | 1 + crates/gpio/src/backend.rs | 18 +++++++------ crates/i2c/Cargo.toml | 1 + crates/i2c/src/main.rs | 26 ++++++++++--------- crates/rng/Cargo.toml | 1 + crates/rng/src/main.rs | 20 +++++++------- crates/scsi/Cargo.toml | 1 + .../scsi/src/scsi/emulation/tests/generic.rs | 3 ++- 9 files changed, 51 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 720117f..0858e53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -77,6 +77,12 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6" +[[package]] +name = "assert_matches" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" + [[package]] name = "async-trait" version = "0.1.71" @@ -1225,6 +1231,7 @@ dependencies = [ name = "vhost-device-gpio" version = "0.1.0" dependencies = [ + "assert_matches", "clap", "env_logger", "libc", @@ -1243,6 +1250,7 @@ dependencies = [ name = "vhost-device-i2c" version = "0.1.0" dependencies = [ + "assert_matches", "clap", "env_logger", "libc", @@ -1260,6 +1268,7 @@ dependencies = [ name = "vhost-device-rng" version = "0.1.0" dependencies = [ + "assert_matches", "clap", "env_logger", "epoll", @@ -1280,6 +1289,7 @@ dependencies = [ name = "vhost-device-scsi" version = "0.1.0" dependencies = [ + "assert_matches", "clap", "env_logger", "epoll", diff --git a/crates/gpio/Cargo.toml b/crates/gpio/Cargo.toml index 8dd607b..9570030 100644 --- a/crates/gpio/Cargo.toml +++ b/crates/gpio/Cargo.toml @@ -31,5 +31,6 @@ vmm-sys-util = "0.11" libgpiod = { version = "0.1" } [dev-dependencies] +assert_matches = "1.5" virtio-queue = { version = "0.9", features = ["test-utils"] } vm-memory = { version = "0.12", features = ["backend-mmap", "backend-atomic"] } diff --git a/crates/gpio/src/backend.rs b/crates/gpio/src/backend.rs index aff6913..0d93af8 100644 --- a/crates/gpio/src/backend.rs +++ b/crates/gpio/src/backend.rs @@ -22,7 +22,7 @@ use crate::vhu_gpio::VhostUserGpioBackend; pub(crate) type Result = std::result::Result; -#[derive(Debug, Eq, PartialEq, ThisError)] +#[derive(Debug, ThisError)] /// Errors related to low level GPIO helpers pub(crate) enum Error { #[error("Invalid socket count: {0}")] @@ -196,6 +196,8 @@ pub(crate) fn gpio_init() { #[cfg(test)] mod tests { + use assert_matches::assert_matches; + use super::*; use crate::gpio::tests::DummyDevice; @@ -220,7 +222,7 @@ mod tests { config.push(5).unwrap(); config.push(6).unwrap(); - assert_eq!(config.push(5).unwrap_err(), Error::DeviceDuplicate(5)); + assert_matches!(config.push(5).unwrap_err(), Error::DeviceDuplicate(5)); } #[test] @@ -229,28 +231,28 @@ mod tests { // Invalid device number let cmd_args = get_cmd_args(socket_name, "1:4d:5", 3); - assert_eq!( + assert_matches!( GpioConfiguration::try_from(cmd_args).unwrap_err(), - Error::ParseFailure("4d".parse::().unwrap_err()) + Error::ParseFailure(e) if e == "4d".parse::().unwrap_err() ); // Zero socket count let cmd_args = get_cmd_args(socket_name, "1:4", 0); - assert_eq!( + assert_matches!( GpioConfiguration::try_from(cmd_args).unwrap_err(), Error::SocketCountInvalid(0) ); // Duplicate client address: 4 let cmd_args = get_cmd_args(socket_name, "1:4:5:6:4", 5); - assert_eq!( + assert_matches!( GpioConfiguration::try_from(cmd_args).unwrap_err(), Error::DeviceDuplicate(4) ); // Device count mismatch let cmd_args = get_cmd_args(socket_name, "1:4:5:6", 5); - assert_eq!( + assert_matches!( GpioConfiguration::try_from(cmd_args).unwrap_err(), Error::DeviceCountMismatch(5, 4) ); @@ -280,7 +282,7 @@ mod tests { let socket_name = "~/path/not/present/gpio"; let cmd_args = get_cmd_args(socket_name, "1:4:3:5", 4); - assert_eq!( + assert_matches!( start_backend::(cmd_args).unwrap_err(), Error::FailedJoiningThreads ); diff --git a/crates/i2c/Cargo.toml b/crates/i2c/Cargo.toml index bbf6487..d44992e 100644 --- a/crates/i2c/Cargo.toml +++ b/crates/i2c/Cargo.toml @@ -28,5 +28,6 @@ vm-memory = "0.12" vmm-sys-util = "0.11" [dev-dependencies] +assert_matches = "1.5" virtio-queue = { version = "0.9", features = ["test-utils"] } vm-memory = { version = "0.12", features = ["backend-mmap", "backend-atomic"] } diff --git a/crates/i2c/src/main.rs b/crates/i2c/src/main.rs index be90ecb..0fa537c 100644 --- a/crates/i2c/src/main.rs +++ b/crates/i2c/src/main.rs @@ -25,7 +25,7 @@ use vhu_i2c::VhostUserI2cBackend; type Result = std::result::Result; -#[derive(Debug, PartialEq, ThisError)] +#[derive(Debug, ThisError)] /// Errors related to low level i2c helpers pub(crate) enum Error { #[error("Invalid socket count: {0}")] @@ -245,6 +245,8 @@ fn main() { #[cfg(test)] mod tests { + use assert_matches::assert_matches; + use super::*; use crate::i2c::tests::DummyDevice; @@ -281,12 +283,12 @@ mod tests { config.push(5).unwrap(); config.push(6).unwrap(); - assert_eq!( + assert_matches!( config.push(invalid_addr).unwrap_err(), - Error::ClientAddressInvalid(invalid_addr) + Error::ClientAddressInvalid(a) if a == invalid_addr ); - assert_eq!( + assert_matches!( config.push(5).unwrap_err(), Error::ClientAddressDuplicate(5) ); @@ -298,21 +300,21 @@ mod tests { // Invalid client address let cmd_args = I2cArgs::from_args(socket_name, "1:4d", 5); - assert_eq!( + assert_matches!( I2cConfiguration::try_from(cmd_args).unwrap_err(), - Error::ParseFailure("4d".parse::().unwrap_err()) + Error::ParseFailure(e) if e == "4d".parse::().unwrap_err() ); // Zero socket count let cmd_args = I2cArgs::from_args(socket_name, "1:4", 0); - assert_eq!( + assert_matches!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::SocketCountInvalid(0) ); // Duplicate client address: 4 let cmd_args = I2cArgs::from_args(socket_name, "1:4,2:32:21,5:4:23", 5); - assert_eq!( + assert_matches!( I2cConfiguration::try_from(cmd_args).unwrap_err(), Error::ClientAddressDuplicate(4) ); @@ -355,7 +357,7 @@ mod tests { .push(DeviceConfig::new_with(2, vec![32, 21])) .unwrap(); - assert_eq!( + assert_matches!( config .push(DeviceConfig::new_with(5, vec![4, 23])) .unwrap_err(), @@ -372,11 +374,11 @@ mod tests { .push(DeviceConfig::new_with(5, vec![10, 23])) .unwrap(); - assert_eq!( + assert_matches!( config .push(DeviceConfig::new_with(1, vec![32, 21])) .unwrap_err(), - Error::AdapterDuplicate(1.to_string()) + Error::AdapterDuplicate(n) if n == "1" ); } @@ -386,7 +388,7 @@ mod tests { let socket_name = "~/path/not/present/i2c"; let cmd_args = I2cArgs::from_args(socket_name, "1:4,3:5", 5); - assert_eq!( + assert_matches!( start_backend::(cmd_args).unwrap_err(), Error::FailedJoiningThreads ); diff --git a/crates/rng/Cargo.toml b/crates/rng/Cargo.toml index dd71b18..e0a17e4 100644 --- a/crates/rng/Cargo.toml +++ b/crates/rng/Cargo.toml @@ -29,5 +29,6 @@ vm-memory = "0.12" vmm-sys-util = "0.11" [dev-dependencies] +assert_matches = "1.5" virtio-queue = { version = "0.9", features = ["test-utils"] } vm-memory = { version = "0.12", features = ["backend-mmap", "backend-atomic"] } diff --git a/crates/rng/src/main.rs b/crates/rng/src/main.rs index 30bcc96..f1c97fb 100644 --- a/crates/rng/src/main.rs +++ b/crates/rng/src/main.rs @@ -25,7 +25,7 @@ const VHU_RNG_MAX_PERIOD_MS: u128 = 65536; type Result = std::result::Result; -#[derive(Debug, Eq, PartialEq, ThisError)] +#[derive(Debug, ThisError)] /// Errors related to vhost-device-rng daemon. pub(crate) enum Error { #[error("RNG source file doesn't exists or can't be accessed")] @@ -173,9 +173,11 @@ fn main() { #[cfg(test)] mod tests { - use super::*; + use assert_matches::assert_matches; use tempfile::tempdir; + use super::*; + #[test] fn verify_cmd_line_arguments() { // All parameters have default values, except for the socket path. White spaces are @@ -194,22 +196,22 @@ mod tests { // All configuration elements should be what we expect them to be. Using // VuRngConfig::try_from() ensures that strings have been properly trimmed. assert_eq!( - VuRngConfig::try_from(default_args), - VuRngConfig::try_from(args.clone()) + VuRngConfig::try_from(default_args).unwrap(), + VuRngConfig::try_from(args.clone()).unwrap() ); // Setting a invalid period should trigger an InvalidPeriodInput error. let mut invalid_period_args = args.clone(); invalid_period_args.period = VHU_RNG_MAX_PERIOD_MS + 1; - assert_eq!( + assert_matches!( VuRngConfig::try_from(invalid_period_args), - Err(Error::InvalidPeriodInput(VHU_RNG_MAX_PERIOD_MS + 1)) + Err(Error::InvalidPeriodInput(p)) if p == VHU_RNG_MAX_PERIOD_MS + 1 ); // Setting the socket count to 0 should trigger an InvalidSocketCount error. let mut invalid_socket_count_args = args; invalid_socket_count_args.socket_count = 0; - assert_eq!( + assert_matches!( VuRngConfig::try_from(invalid_socket_count_args), Err(Error::InvalidSocketCount(0)) ); @@ -230,7 +232,7 @@ mod tests { }; // An invalid RNG source file should trigger an AccessRngSourceFile error. - assert_eq!( + assert_matches!( start_backend(config.clone()).unwrap_err(), Error::AccessRngSourceFile ); @@ -239,7 +241,7 @@ mod tests { // of the socket file. Since the latter is invalid the vhost_user::Listener will // throw an error, forcing the thread to exit and the call to handle.join() to fail. config.rng_source = random_path.to_str().unwrap().to_string(); - assert_eq!( + assert_matches!( start_backend(config).unwrap_err(), Error::FailedJoiningThreads ); diff --git a/crates/scsi/Cargo.toml b/crates/scsi/Cargo.toml index 472935a..2dbfca5 100644 --- a/crates/scsi/Cargo.toml +++ b/crates/scsi/Cargo.toml @@ -29,5 +29,6 @@ vm-memory = "0.12" vmm-sys-util = "0.11" [dev-dependencies] +assert_matches = "1.5" tempfile = "3.2.0" diff --git a/crates/scsi/src/scsi/emulation/tests/generic.rs b/crates/scsi/src/scsi/emulation/tests/generic.rs index 19f4bd4..0ae1728 100644 --- a/crates/scsi/src/scsi/emulation/tests/generic.rs +++ b/crates/scsi/src/scsi/emulation/tests/generic.rs @@ -2,6 +2,7 @@ //! Tests for stuff shared between commands. +use assert_matches::assert_matches; use std::io::ErrorKind; use super::{do_command_fail, test_image}; @@ -103,5 +104,5 @@ fn test_short_cdb() { }, ); - assert!(matches!(res.unwrap_err(), CmdError::CdbTooShort)); + assert_matches!(res.unwrap_err(), CmdError::CdbTooShort); }