diff --git a/src/backends/qemu/libvirt.rs b/src/backends/qemu/libvirt.rs index 2c641ef..0f7b8bf 100644 --- a/src/backends/qemu/libvirt.rs +++ b/src/backends/qemu/libvirt.rs @@ -441,8 +441,32 @@ fn parse_qmp_envelope(cmd: &str, body: &str) -> Result, QemuError> #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; + /// Create an arbitrary parsed JSON value used to exercise parser failure + /// paths. + fn arb_json() -> impl Strategy { + let leaf = prop_oneof![ + Just(serde_json::Value::Null), + any::().prop_map(serde_json::Value::Bool), + any::().prop_map(serde_json::Value::from), + "[a-z0-9 ]{0,12}".prop_map(serde_json::Value::String), + ]; + // We set a depth of up to 2, a desired total size of 8 elements, with + // each collection having up to 4 elements. + leaf.prop_recursive(2, 8, 4, |inner| { + prop_oneof![ + prop::collection::vec(inner.clone(), 0..4).prop_map(serde_json::Value::Array), + prop::collection::hash_map("[a-z]{1,6}", inner, 0..3) + .prop_map(|map| { serde_json::Value::Object(map.into_iter().collect()) }), + ] + }) + // Option decodes a JSON null return as a missing payload. + .prop_filter("null return is absent", |value| !value.is_null()) + } + /// Test that domain XML detection accepts virtio-scsi model variants and /// rejects others. #[test] @@ -457,25 +481,39 @@ mod tests { assert!(!xml_has_virtio_scsi(d)); } - /// Test that parsing the qmp envelope does not modify the payload. - #[test] - fn test_libvirt_qmp_return_envelope_yields_the_payload_unchanged() { - let body = r#"{"return":{"foo":42}}"#; - let v = parse_qmp_envelope("query", body).unwrap(); - assert_eq!(v.get(), r#"{"foo":42}"#); - } - - /// Test that a QMP error envelope becomes a typed error with class/desc. - #[test] - fn test_libvirt_qmp_error_envelope_preserves_class_and_desc() { - let body = r#"{"error":{"class":"GenericError","desc":"boom"}}"#; - let err = parse_qmp_envelope("query", body).unwrap_err(); - match err { - QemuError::QmpError { class, desc, .. } => { - assert_eq!(class, "GenericError"); - assert_eq!(desc, "boom"); + proptest! { + /// Test that parsing errors contain details on the unexpected entry. + #[test] + fn test_libvirt_qmp_error_envelope_preserves_class_and_desc( + cmd in "[a-z-]{1,16}", + class in "[a-zA-Z0-9 .,_-]{0,32}", + desc in "[a-zA-Z0-9 .,_-]{0,32}", + ) { + let body = serde_json::json!({ + "error": {"class": class, "desc": desc} + }) + .to_string(); + match parse_qmp_envelope(&cmd, &body).unwrap_err() { + QemuError::QmpError { + cmd: got_cmd, + class: got_class, + desc: got_desc, + } => { + prop_assert_eq!(got_cmd, cmd); + prop_assert_eq!(got_class, class); + prop_assert_eq!(got_desc, desc); + } + other => panic!("unexpected err: {other:?}"), } - other => panic!("unexpected err: {other:?}"), + } + + /// Test that return payloads are passed through the parser. + #[test] + fn test_libvirt_qmp_return_envelope_yields_the_payload_unchanged(payload in arb_json()) { + let payload = serde_json::to_string(&payload).unwrap(); + let body = format!(r#"{{"return":{payload}}}"#); + let parsed = parse_qmp_envelope("query", &body).unwrap(); + prop_assert_eq!(parsed.get(), payload); } } diff --git a/src/config.rs b/src/config.rs index d75e00e..875bd96 100644 --- a/src/config.rs +++ b/src/config.rs @@ -32,7 +32,7 @@ pub enum ConfigError { } /// Tunable parameters for the controller. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(deny_unknown_fields)] pub struct Config { /// Which registered scaling engine drives decisions. @@ -363,8 +363,25 @@ pub fn dump_default_config() -> String { #[cfg(test)] mod tests { + use proptest::prelude::*; + use rstest::rstest; + use super::*; + use crate::test::{MockDir, mockdir::mockdir}; + + /// Generate a string which resembles a relative unix path. + fn arb_path_string() -> impl Strategy { + prop_oneof![ + "[a-z0-9]{1,8}(/[a-z0-9]{1,8}){0,2}", + "[a-z0-9]{1,8}(/[a-z0-9]{1,8}){0,2}".prop_map(|tail| format!("/{tail}")), + ] + } + + fn reasonable_f64() -> impl Strategy { + (-1000..=1000).prop_map(|i| i as f64 / 100.0) + } + /// Test that the default `Config` validates. #[test] fn test_config_default_is_valid() { @@ -468,39 +485,56 @@ mod tests { assert!(err.to_string().contains("expected u64"), "{err}"); } - /// Test that minimal valid JSON loads into `Config` with the - /// expected engine, poll, and paths. - #[test] - fn test_config_load_round_trips_every_field() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.json"); - std::fs::write( - &path, - r#"{ - "engine": "threshold", - "engine_config_dir": "/etc/io-thread-controller/engines", - "backend_config_dir": "/etc/io-thread-controller/backends", - "scale_poll_secs": 7, - "vm_state_path": "/var/lib/io-thread-controller/vm-state.json" - }"#, - ) - .unwrap(); - - let cfg: Config = load_or_default(PrefixedPath::new(path.to_str().unwrap())).unwrap(); - assert_eq!(cfg.engine, "threshold"); - assert_eq!(cfg.scale_poll_secs, 7); - assert_eq!( - cfg.engine_config_dir.as_os_str(), - std::ffi::OsStr::new("/etc/io-thread-controller/engines") - ); + macro_rules! test_round_trips { + ($i:ident in $v:expr) => { + mod $i { + use super::*; + + proptest! { + #[test] + fn test_config_load_round_trips_field( + value in $v + ) { + let _md = mockdir(); + let cfg = Config { + $i: value.into(), + ..Default::default() + }; + let path = PrefixedPath::new("/config.json"); + std::fs::write(&path, serde_json::to_string(&cfg).unwrap()).unwrap(); + let loaded: Config = load_or_default(path).unwrap(); + prop_assert_eq!(loaded, cfg); + } + } + } + }; } + // Test that each field in Config can survive a save-load round trip. + test_round_trips!(engine in any::()); + test_round_trips!(engine_config_dir in arb_path_string()); + test_round_trips!(backend_config_dir in arb_path_string()); + test_round_trips!(vm_state_path in arb_path_string()); + test_round_trips!(min_thread_count in any::()); + test_round_trips!(max_thread_count in any::()); + test_round_trips!(host_cpu_scale_up_ceiling in reasonable_f64()); + test_round_trips!(cooldown_secs in any::()); + test_round_trips!(scale_poll_secs in any::()); + test_round_trips!(round_up_durations in any::()); + test_round_trips!(enable_per_vm_status_line in any::()); + test_round_trips!(enable_per_vm_main_log in any::()); + test_round_trips!(enable_aggregate_status_line in any::()); + test_round_trips!(print_status_header in any::()); + test_round_trips!(max_instances_adjusted_per_poll in any::()); + test_round_trips!(experimental_psi_monitoring in any::()); + test_round_trips!(refresh_cgroup_on_each_read in any::()); + test_round_trips!(dry_run in any::()); + /// Test that loading JSON with an unknown field returns /// `ConfigError::SerdeJson`. - #[test] - fn test_config_load_rejects_unknown_fields() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.json"); + #[rstest] + fn test_config_load_rejects_unknown_fields(#[from(mockdir)] _mockdir: MockDir) { + let path = PrefixedPath::new("/config"); std::fs::write( &path, r#"{ @@ -513,7 +547,7 @@ mod tests { ) .unwrap(); - let err = load_or_default::(PrefixedPath::new(path.to_str().unwrap())).unwrap_err(); + let err = load_or_default::(&path).unwrap_err(); assert!(matches!(err, ConfigError::SerdeJson(_))); } /// Test that pretty-printed default JSON includes engine, poll @@ -526,21 +560,6 @@ mod tests { assert!(dumped.contains(r#""/etc/io-thread-controller.d/engines""#)); } - /// Test that serde defaults for min/max threads, host CPU ceiling, - /// and cooldown match `Config::default()`. - #[test] - fn test_controller_policy_defaults_round_trip() { - let cfg: Config = serde_json::from_str(r#"{"engine": "foo", "engine_config_dir": "/path/to/engines", "backend_config_dir": "/path/to/backends", "scale_poll_secs": 42, "vm_state_path": "/path/to/vm-state.json"}"#).unwrap(); - assert_eq!(cfg.min_thread_count, 1); - assert_eq!(cfg.max_thread_count, 8); - assert!((cfg.host_cpu_scale_up_ceiling - 0.9).abs() < f64::EPSILON); - assert_eq!(cfg.cooldown_secs, 30); - assert!(!cfg.round_up_durations); - - let serialized = serde_json::to_string(&cfg).unwrap(); - assert!(serialized.contains(r#""host_cpu_scale_up_ceiling_percent":90.0"#)); - } - /// Test that `refresh_cgroup_on_each_read` defaults to false in /// `Default` and empty JSON. #[test] diff --git a/src/engines/threshold.rs b/src/engines/threshold.rs index 08ede90..f9f67bf 100644 --- a/src/engines/threshold.rs +++ b/src/engines/threshold.rs @@ -926,6 +926,7 @@ mod tests { }; use async_trait::async_trait; + use proptest::prelude::*; use rstest::{fixture, rstest}; use test_log::test; @@ -1592,16 +1593,31 @@ mod tests { ); } - #[test] - fn test_validation_polls_increase_with_target_thread_count() { - let engine = new_engine(ThresholdConfig { - scale_validation_delay_secs: polls(2), - scale_validation_delay_per_thread_secs: polls(3), - ..Default::default() - }); - - assert_eq!(engine.validation_sample_polls(1), 5); - assert_eq!(engine.validation_sample_polls(4), 14); + proptest! { + /// Test that the amount of validation polls increases with the amount + /// of threads. + #[test] + fn test_prop_validation_polls_increase_with_target_thread_count( + target_a in 0u32..1_000, + target_b in 0u32..1_000, + ) { + prop_assume!(target_a != target_b); + + let engine = new_engine(ThresholdConfig { + scale_validation_delay_secs: polls(2), + scale_validation_delay_per_thread_secs: polls(3), + ..Default::default() + }); + + let polls_a = engine.validation_sample_polls(target_a); + let polls_b = engine.validation_sample_polls(target_b); + + if target_a > target_b { + prop_assert!(polls_a > polls_b); + } else { + prop_assert!(polls_a < polls_b); + } + } } /// Test that while a prior scale-up is pending validation, further diff --git a/src/path.rs b/src/path.rs index f5f2dae..b86cc4d 100644 --- a/src/path.rs +++ b/src/path.rs @@ -19,17 +19,24 @@ use std::{ use glob; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct PrefixedPath { path: std::path::PathBuf, } impl Serialize for PrefixedPath { + /// Strips the effective root so deserializing doesn't prefix twice. + /// Paths outside the effective root (e.g. created under another + /// `MockDir`) are serialized unchanged. fn serialize(&self, serializer: S) -> Result where S: Serializer, { - serializer.serialize_str(&self.path.to_string_lossy()) + let path = match self.path.strip_prefix(effective_root()) { + Ok(rel) if self.path.is_absolute() => std::path::Path::new("/").join(rel), + _ => self.path.clone(), + }; + serializer.serialize_str(&path.to_string_lossy()) } } diff --git a/src/rolling.rs b/src/rolling.rs index d57f96c..f5cce0c 100644 --- a/src/rolling.rs +++ b/src/rolling.rs @@ -235,6 +235,8 @@ impl RollingMetrics { #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; /// Test that the first sample records a baseline and yields no @@ -264,14 +266,26 @@ mod tests { ); } - /// Test that a counter reset drops prior rates and starts a new - /// baseline. - #[test] - fn test_rolling_counter_reset_replaces_baseline() { - let mut metrics = RollingMetrics::new(); - let t0 = Instant::now(); - metrics.push_from_procfs_delta(t0, 100, 100); - metrics.push_from_procfs_delta(t0 + Duration::from_secs(1), 10, 10); - assert!(metrics.is_empty()); + proptest! { + /// Test that if a counter decreases or resets, then the baseline is + /// also reset. + #[test] + fn test_rolling_counter_reset_replaces_baseline( + io0 in 1u64.., + io1 in 1u64.., + cpu0 in 1u64.., + cpu1 in 1u64.., + ) { + prop_assume!(io1 < io0 || cpu1 < cpu0); + + let mut metrics = RollingMetrics::new(); + let t0 = Instant::now(); + let t1 = t0 + Duration::from_secs(2); + + metrics.push_from_procfs_delta(t0, io0, cpu0); + metrics.push_from_procfs_delta(t1, io1, cpu1); + + prop_assert!(metrics.is_empty()); + } } } diff --git a/src/state.rs b/src/state.rs index b6b0916..2c81df5 100644 --- a/src/state.rs +++ b/src/state.rs @@ -129,24 +129,14 @@ impl VmStateStore { #[cfg(test)] mod tests { + use proptest::prelude::*; use rstest::rstest; use super::*; use crate::test::{MockDir, mockdir::mockdir}; - /// Test that managed/unmanaged sets save and reload from a single JSON - /// file. - #[rstest] - fn test_state_disjoint_ownership_round_trips_in_one_file(mockdir: MockDir) { - let store = VmStateStore::new(PrefixedPath::new("/ownership.json")); - let mut expected = VmOwnership::default(); - expected.record("managed", true).unwrap(); - expected.record("../unmanaged", false).unwrap(); - - store.save(&expected).unwrap(); - - assert_eq!(store.load().unwrap(), expected); - assert_eq!(std::fs::read_dir(mockdir.root()).unwrap().count(), 1); + fn id_set() -> impl Strategy> { + prop::collection::btree_set("[a-z0-9]{1,8}", 0..6) } /// Test that a missing ownership file loads as empty/default state. @@ -160,15 +150,52 @@ mod tests { ); } - /// Test that a VM listed as both managed and unmanaged is rejected on load. - #[rstest] - fn test_state_overlapping_ownership_is_rejected(#[from(mockdir)] _md: MockDir) { - let path = PrefixedPath::new("/ownership.json"); - std::fs::write( - &path, - r#"{"managed_vms":["same"],"unmanaged_vms":["same"]}"#, - ) - .unwrap(); - assert!(VmStateStore::new(&path).load().is_err()); + proptest! { + /// Test that ownership state round trips when there is no overlap + /// between managed and unmanaged VMs. + #[test] + fn test_state_disjoint_ownership_round_trips_in_one_file( + managed in id_set(), + unmanaged in id_set(), + ) { + let unmanaged: BTreeSet<_> = unmanaged.difference(&managed).cloned().collect(); + prop_assume!(!unmanaged.is_empty()); + let state = VmOwnership { + managed_vms: managed, + unmanaged_vms: unmanaged, + }; + let _mockdir = MockDir::new(); + let store = VmStateStore::new(PrefixedPath::new("/ownership.json")); + + store.save(&state).unwrap(); + + prop_assert_eq!(store.load().unwrap(), state); + prop_assert_eq!(std::fs::read_dir(PrefixedPath::new("/")).unwrap().count(), 1); + } + + /// Test that if there is an overlap between managed and unmanaged VMs, + /// the management state errors. + #[test] + fn test_state_overlapping_ownership_is_rejected( + shared in "[a-z0-9]{1,8}", + extra_managed in id_set(), + extra_unmanaged in id_set(), + ) { + let mut managed_vms = extra_managed; + let mut unmanaged_vms = extra_unmanaged; + managed_vms.insert(shared.clone()); + unmanaged_vms.insert(shared); + let state = VmOwnership { + managed_vms, + unmanaged_vms, + }; + let _mockdir = MockDir::new(); + let path = PrefixedPath::new("/ownership.json"); + let store = VmStateStore::new(&path); + + prop_assert!(store.save(&state).is_err()); + std::fs::write(&path, serde_json::to_vec(&state).unwrap()).unwrap(); + prop_assert!(store.load().is_err()); + } } } diff --git a/src/test/mockdir.rs b/src/test/mockdir.rs index 286ee3a..9ae7f5d 100644 --- a/src/test/mockdir.rs +++ b/src/test/mockdir.rs @@ -52,10 +52,6 @@ impl MockDir { _dir: dir, } } - - pub(crate) fn root(&self) -> &std::path::Path { - &self.path - } } /// Create a new `MockDir` as an `rstest` fixture