From 6eb26259237d5518f6ec9205123ce5445f31e301 Mon Sep 17 00:00:00 2001 From: Leonardo Forchini Date: Fri, 25 Sep 2026 08:24:42 +0000 Subject: [PATCH] Replace single-example tests with property checks Introduce proptest and convert example tests to the property-testing framework. General behaviour is enforced and the tests are able to catch non-hardcoded cases. Signed-off-by: Leonardo Forchini --- Cargo.lock | 184 +++++++++++++++++++++++- Cargo.toml | 1 + src/backends/qemu/helpers.rs | 113 +++++++++++---- src/backends/qemu/libvirt.rs | 142 +++++++++++++------ src/config.rs | 124 +++++++++------- src/engines/threshold.rs | 266 +++++++++++------------------------ src/instance.rs | 67 ++++++--- src/rolling.rs | 83 +++++++++++ src/state.rs | 68 +++++---- 9 files changed, 692 insertions(+), 356 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c63c33a..33d556b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -131,6 +131,21 @@ version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -449,6 +464,12 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "foldhash" version = "0.2.0" @@ -522,6 +543,18 @@ dependencies = [ "slab", ] +[[package]] +name = "getrandom" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" +dependencies = [ + "cfg-if", + "libc", + "r-efi 5.3.0", + "wasip2", +] + [[package]] name = "getrandom" version = "0.4.3" @@ -530,7 +563,7 @@ checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" dependencies = [ "cfg-if", "libc", - "r-efi", + "r-efi 6.0.0", ] [[package]] @@ -646,6 +679,7 @@ dependencies = [ "inotify", "linkme", "procfs", + "proptest", "ratatui", "regex", "rstest", @@ -1033,6 +1067,15 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" +[[package]] +name = "ppv-lite86" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" +dependencies = [ + "zerocopy", +] + [[package]] name = "proc-macro-crate" version = "3.5.0" @@ -1072,6 +1115,31 @@ dependencies = [ "hex", ] +[[package]] +name = "proptest" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744" +dependencies = [ + "bit-set", + "bit-vec", + "bitflags 2.13.2", + "num-traits", + "rand 0.9.5", + "rand_chacha 0.9.0", + "rand_xorshift 0.4.0", + "regex-syntax", + "rusty-fork", + "tempfile", + "unarray", +] + +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quote" version = "1.0.47" @@ -1081,6 +1149,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" + [[package]] name = "r-efi" version = "6.0.0" @@ -1095,17 +1169,27 @@ checksum = "6d71dacdc3c88c1fde3885a3be3fbab9f35724e6ce99467f7d9c5026132184ca" dependencies = [ "autocfg 0.1.8", "libc", - "rand_chacha", + "rand_chacha 0.1.1", "rand_core 0.4.3", "rand_hc", "rand_isaac", "rand_jitter", "rand_os", "rand_pcg", - "rand_xorshift", + "rand_xorshift 0.1.1", "winapi", ] +[[package]] +name = "rand" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ef1d0d795eb7d84685bca4f72f3649f064e6641543d3a8c415898726a57b41" +dependencies = [ + "rand_chacha 0.9.0", + "rand_core 0.9.5", +] + [[package]] name = "rand_chacha" version = "0.1.1" @@ -1116,6 +1200,16 @@ dependencies = [ "rand_core 0.3.2", ] +[[package]] +name = "rand_chacha" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" +dependencies = [ + "ppv-lite86", + "rand_core 0.9.5", +] + [[package]] name = "rand_core" version = "0.3.2" @@ -1131,6 +1225,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e5937858e6fd18cd595d558f90bb5de3b72ae23f9e3763af0e805949b04ef60" +[[package]] +name = "rand_core" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c" +dependencies = [ + "getrandom 0.3.4", +] + [[package]] name = "rand_hc" version = "0.1.0" @@ -1193,6 +1296,15 @@ dependencies = [ "rand_core 0.3.2", ] +[[package]] +name = "rand_xorshift" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a" +dependencies = [ + "rand_core 0.9.5", +] + [[package]] name = "ratatui" version = "0.30.2" @@ -1369,6 +1481,18 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" +[[package]] +name = "rusty-fork" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc6bf79ff24e648f6da1f8d1f011e9cac26491b619e6b9280f2b47f1774e6ee2" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "ryu" version = "1.0.23" @@ -1516,7 +1640,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49d57902bb128e5e38b5218d3681215ae3e322d99f65d5420e9849730d2ea372" dependencies = [ "num", - "rand", + "rand 0.6.5", ] [[package]] @@ -1575,7 +1699,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom", + "getrandom 0.4.3", "once_cell", "rustix", "windows-sys", @@ -1805,6 +1929,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unicode-ident" version = "1.0.26" @@ -1878,12 +2008,30 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "wait-timeout" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11" +dependencies = [ + "libc", +] + [[package]] name = "wasi" version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" +[[package]] +name = "wasip2" +version = "1.0.4+wasi-0.2.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b67efb37e106e55ce722a510d6b5f9c17f083e5fc79afc2badeb12cc313d9487" +dependencies = [ + "wit-bindgen", +] + [[package]] name = "wasm-bindgen" version = "0.2.129" @@ -1975,6 +2123,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "wit-bindgen" +version = "0.57.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" + [[package]] name = "zbus" version = "5.19.0" @@ -2040,6 +2194,26 @@ dependencies = [ "serde", ] +[[package]] +name = "zerocopy" +version = "0.8.59" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6df92bf3d9227be3d53173901ddbffac2babc27ae50f397776ffd6dc33f800cb" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.8.59" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac4f328cf2f05d084e496c3e9c3f33ed0a183656a16e1fcec4d464d8373aec82" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "zmij" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index 75934da..87912df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,3 +85,4 @@ rstest = "0.26" tempfile = "3" test-log = { version = "0.2", features = ["trace"] } tokio = { version = "1", features = ["test-util"] } +proptest = "1" diff --git a/src/backends/qemu/helpers.rs b/src/backends/qemu/helpers.rs index 48e7ca6..34b5479 100644 --- a/src/backends/qemu/helpers.rs +++ b/src/backends/qemu/helpers.rs @@ -288,44 +288,107 @@ pub fn next_qemu_iothread_id(existing: &[String]) -> String { #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; - /// Test that vQ round-robin mapping spreads queues across IOThreads - /// as evenly as possible. + proptest! { + /// Test that vQ round-robin mapping spreads queues across IOThreads + /// as evenly as possible. + #[test] + fn round_robin_partitions_queues( + ids in prop::collection::vec("[a-z]{1,4}", 0..8), + vq_count in 0u16..128, + ) { + // We run a mapping on arbitrary IOThread ids and number of queues. + let mapping = round_robin_vq_mapping(&ids, vq_count); + if ids.is_empty() || vq_count == 0 { + prop_assert!(mapping.is_empty()); + return Ok(()); + } + + // Check every queue has been assigned once. + prop_assert_eq!(mapping.len(), ids.len()); + let mut seen = vec![false; usize::from(vq_count)]; + for (idx, entry) in mapping.iter().enumerate() { + prop_assert_eq!(&entry.iothread, &ids[idx]); + for &vq in &entry.vqs { + prop_assert_eq!(usize::from(vq) % ids.len(), idx); + prop_assert!(!seen[usize::from(vq)]); + seen[usize::from(vq)] = true; + } + } + prop_assert!(seen.into_iter().all(|present| present)); + + // Check queues are evenly distributed. + let sizes: Vec = mapping.iter().map(|entry| entry.vqs.len()).collect(); + let min = sizes.iter().copied().min().unwrap(); + let max = sizes.iter().copied().max().unwrap(); + prop_assert!(max - min <= 1); + } + + /// Ensure the next IOThread to have a queue assigned is the one with + /// the lowest id. + #[test] + fn next_id_is_the_lowest_missing_iot( + // IOThreads with assigned queues, these are excluded from the search. + present in prop::collection::btree_set(0u32..1024, 0..32), + ) { + let ids: Vec = present.iter().map(|n| format!("iot{n}")).collect(); + // Find the thread with the lowest id. + let expected = (0..1024).find(|n| !present.contains(n)).unwrap(); + prop_assert_eq!(next_qemu_iothread_id(&ids), format!("iot{expected}")); + } + } + + /// Test that managed `iotN` threads are kept in id order and other ids are + /// dropped. #[test] - fn round_robin_distributes_evenly() { - let ids = vec!["iot0".to_string(), "iot1".to_string(), "iot2".to_string()]; - let mapping = round_robin_vq_mapping(&ids, 7); - let counts: Vec = mapping.iter().map(|m| m.vqs.len()).collect(); - assert_eq!(counts, vec![3, 2, 2]); - assert_eq!(mapping[0].vqs, vec![0, 3, 6]); - assert_eq!(mapping[1].vqs, vec![1, 4]); - assert_eq!(mapping[2].vqs, vec![2, 5]); + fn topology_keeps_managed_threads_in_id_order() { + let body = r#" +qemu_iothread_info{id="iot1",tid="124"} 1 +qemu_iothread_info{id="dirtybitmap",tid="999"} 1 +qemu_iothread_info{id="iot0",tid="123"} 1 +"#; + let topo = QemuTopology::new(body); + assert_eq!(topo.iothreads, vec!["iot0", "iot1"]); + assert_eq!(topo.iothread_tids.get("iot0"), Some(&123)); + assert_eq!(topo.iothread_tids.get("iot1"), Some(&124)); + assert_eq!(topo.iothread_tids.len(), 2); } - /// Test that `next_qemu_iothread_id` fills the lowest missing - /// `iotN` id. + /// Test that `thread_id` and `path` are accepted in place of `tid` and + /// `device`. #[test] - fn next_id_fills_gaps_in_order() { - let ids = vec!["iot0".to_string(), "iot2".to_string()]; - assert_eq!(next_qemu_iothread_id(&ids), "iot1"); - let ids = vec!["iot0".to_string(), "iot1".to_string()]; - assert_eq!(next_qemu_iothread_id(&ids), "iot2"); + fn topology_accepts_label_aliases() { + let body = r#" +qemu_iothread_info{id="iot0",thread_id="321"} 1 +qemu_virtio_scsi_num_queues{path="/machine/peripheral/scsi0"} 4 +"#; + let topo = QemuTopology::new(body); + assert_eq!(topo.iothreads, vec!["iot0"]); + assert_eq!(topo.iothread_tids.get("iot0"), Some(&321)); + assert_eq!(topo.device_path, "/machine/peripheral/scsi0"); + assert_eq!(topo.vq_count, 4); } - /// Test that prometheus scrape text yields IOThread ids, TIDs, and - /// the virtio-scsi device path. + /// Test that the first device wins, noise lines are ignored, and a later + /// thread is kept. #[test] - fn topology_parses_prometheus_body() { - let body = r#"# HELP foo -qemu_iothread_info{id="iot0",tid="123"} 1 -qemu_iothread_info{id="iot1",tid="124"} 1 -qemu_iothread_info{id="dirtybitmap",tid="999"} 1 + fn topology_keeps_the_first_device_and_later_threads() { + let body = r#" +# HELP foo +not a metric +qemu_virtio_scsi_num_queues{device=""} 3 +qemu_iothread_info{id="iot7",tid="nope"} 1 qemu_virtio_scsi_num_queues{device="/machine/peripheral/scsi0"} 4 +qemu_iothread_info{id="iot0",tid="123"} 1 +qemu_virtio_scsi_num_queues{device="/machine/peripheral/scsi1"} 99 "#; let topo = QemuTopology::new(body); - assert_eq!(topo.iothreads, vec!["iot0", "iot1"]); + assert_eq!(topo.iothreads, vec!["iot0"]); assert_eq!(topo.iothread_tids.get("iot0"), Some(&123)); + assert_eq!(topo.iothread_tids.len(), 1); assert_eq!(topo.device_path, "/machine/peripheral/scsi0"); assert_eq!(topo.vq_count, 4); } diff --git a/src/backends/qemu/libvirt.rs b/src/backends/qemu/libvirt.rs index 4f054cf..a92b843 100644 --- a/src/backends/qemu/libvirt.rs +++ b/src/backends/qemu/libvirt.rs @@ -441,8 +441,30 @@ 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), + ]; + 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,63 +479,91 @@ mod tests { assert!(!xml_has_virtio_scsi(d)); } - /// Test that a QMP success envelope yields the return payload. + proptest! { + /// Test that details in parsing errors are preserved. + #[test] + fn 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:?}"), + } + } + + /// Test that return payloads are passed through the parser. + #[test] + fn 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); + } + } + + /// Test that an empty blockstats array yields no sample. #[test] - fn parse_envelope_extracts_return() { - let body = r#"{"return":{"foo":42}}"#; - let v = parse_qmp_envelope("query", body).unwrap(); - assert_eq!(v.get(), r#"{"foo":42}"#); + fn blockstats_empty_array_is_none() { + assert!(parse_blockstats("[]").unwrap().is_none()); } - /// Test that a QMP error envelope becomes a typed error with - /// class/desc. + /// Test that one device's counters are copied and omitted keys stay zero. #[test] - fn parse_envelope_surfaces_qmp_error() { - 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"); - } - other => panic!("unexpected err: {other:?}"), - } + fn blockstats_reads_one_device() { + let body = r#"[{"stats":{"rd_operations":3,"wr_bytes":8,"flush_operations":1}}]"#; + let perf = parse_blockstats(body).unwrap().unwrap(); + assert_eq!(perf.read_io_count, 3); + assert_eq!(perf.write_io_count, 0); + assert_eq!(perf.other_io_count, 1); + assert_eq!(perf.read_bytes_total, 0); + assert_eq!(perf.write_bytes_total, 8); } - /// Test that blockstats counters are summed across disks. + /// Test that device counters are added, with flush and unmap folded into + /// other I/O. #[test] - fn parse_blockstats_sums_across_devices() { + fn blockstats_sums_devices() { let body = r#"[ - {"stats": { - "rd_operations": 100, - "wr_operations": 200, - "flush_operations": 3, - "unmap_operations": 1, - "rd_bytes": 4096, - "wr_bytes": 8192 - }}, - {"stats": { - "rd_operations": 50, - "wr_operations": 25, - "flush_operations": 0, - "unmap_operations": 0, - "rd_bytes": 2048, - "wr_bytes": 1024 - }} + {"stats":{"rd_bytes":10,"wr_bytes":20,"rd_operations":1,"wr_operations":2,"flush_operations":3,"unmap_operations":4}}, + {"stats":{"rd_bytes":5,"wr_bytes":6,"rd_operations":7,"wr_operations":8,"flush_operations":9,"unmap_operations":10}} ]"#; - let p = parse_blockstats(body).unwrap().unwrap(); - assert_eq!(p.read_io_count, 150); - assert_eq!(p.write_io_count, 225); - assert_eq!(p.other_io_count, 4); - assert_eq!(p.read_bytes_total, 6144); - assert_eq!(p.write_bytes_total, 9216); - assert_eq!(p.total_io_count(), 150 + 225 + 4); + let perf = parse_blockstats(body).unwrap().unwrap(); + assert_eq!(perf.read_io_count, 8); + assert_eq!(perf.write_io_count, 10); + assert_eq!(perf.other_io_count, 26); + assert_eq!(perf.read_bytes_total, 15); + assert_eq!(perf.write_bytes_total, 26); } - /// Test that empty blockstats parses as unavailable/`None`. + /// Test that a counter sum past `u64::MAX` saturates. #[test] - fn parse_blockstats_empty_returns_unavailable() { - let p = parse_blockstats("[]").unwrap(); - assert!(p.is_none()); + fn blockstats_saturates_on_overflow() { + let max = u64::MAX; + let body = format!( + r#"[ + {{"stats":{{"rd_bytes":{max},"wr_bytes":{max},"rd_operations":{max},"wr_operations":{max},"flush_operations":{max},"unmap_operations":1}}}}, + {{"stats":{{"rd_bytes":1,"wr_bytes":1,"rd_operations":1,"wr_operations":1,"flush_operations":1,"unmap_operations":{max}}}}} + ]"# + ); + let perf = parse_blockstats(&body).unwrap().unwrap(); + assert_eq!(perf.read_io_count, max); + assert_eq!(perf.write_io_count, max); + assert_eq!(perf.other_io_count, max); + assert_eq!(perf.read_bytes_total, max); + assert_eq!(perf.write_bytes_total, max); } } diff --git a/src/config.rs b/src/config.rs index a78deaf..b7b426b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -186,8 +186,21 @@ pub fn dump_default_config() -> String { #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; + fn nearly_eq(left: f64, right: f64) -> bool { + (left - right).abs() <= 1e-6 * (1.0 + left.abs().max(right.abs())) + } + + 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}")), + ] + } + /// Test that default `Config` validates and uses the threshold /// engine with a 10s poll. #[test] @@ -198,56 +211,69 @@ mod tests { validate_config(&cfg).unwrap(); } - /// Test that `validate_config` rejects an empty engine name. - #[test] - fn validate_rejects_empty_engine() { - let mut cfg = Config::default(); - cfg.engine.clear(); - let err = validate_config(&cfg).unwrap_err().to_string(); - assert!(err.contains("engine must be non-empty")); - } - - /// Test that `validate_config` rejects non-positive - /// `scale_poll_secs`. - #[test] - fn validate_rejects_non_positive_poll() { - let mut cfg = Config { - scale_poll_secs: 0.0, - ..Default::default() - }; - let err = validate_config(&cfg).unwrap_err().to_string(); - assert!(err.contains("scale_poll_secs must be > 0")); - - cfg.scale_poll_secs = f64::NAN; - let err = validate_config(&cfg).unwrap_err().to_string(); - assert!(err.contains("scale_poll_secs must be > 0")); - } - - /// Test that minimal valid JSON loads into `Config` with the - /// expected engine, poll, and paths. - #[test] - fn load_config_round_trips_required_fields() { - 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.5, - "vm_state_path": "/var/lib/io-thread-controller/vm-state.json" - }"#, - ) - .unwrap(); + proptest! { + #[test] + fn load_config_round_trips_every_field( + engine in "[a-z0-9]{0,12}", + engine_config_dir in arb_path_string(), + backend_config_dir in arb_path_string(), + vm_state_path in arb_path_string(), + min_thread_count in any::(), + max_thread_count in any::(), + host_cpu_scale_up_ceiling in -1_000.0..1_000.0f64, + cooldown_secs in -1_000.0..1_000.0f64, + scale_poll_secs in -1_000.0..1_000.0f64, + enable_per_vm_status_line in any::(), + enable_aggregate_status_line in any::(), + print_status_header in any::(), + dry_run in any::(), + ) { + let cfg = Config { + engine, + engine_config_dir: Path::new(&engine_config_dir), + backend_config_dir: Path::new(&backend_config_dir), + vm_state_path: Path::new(&vm_state_path), + min_thread_count, + max_thread_count, + host_cpu_scale_up_ceiling, + cooldown_secs, + scale_poll_secs, + enable_per_vm_status_line, + enable_aggregate_status_line, + print_status_header, + dry_run, + }; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.json"); + std::fs::write(&path, serde_json::to_string(&cfg).unwrap()).unwrap(); + let loaded: Config = load_config(Path::new(path.to_str().unwrap())).unwrap(); - let cfg: Config = load_config(Path::new(path.to_str().unwrap())).unwrap(); - assert_eq!(cfg.engine, "threshold"); - assert!((cfg.scale_poll_secs - 7.5).abs() < f64::EPSILON); - assert_eq!( - cfg.engine_config_dir.as_os_str(), - std::ffi::OsStr::new("/etc/io-thread-controller/engines") - ); + prop_assert_eq!(loaded.engine, cfg.engine); + prop_assert_eq!( + loaded.engine_config_dir.as_os_str(), + cfg.engine_config_dir.as_os_str() + ); + prop_assert_eq!( + loaded.backend_config_dir.as_os_str(), + cfg.backend_config_dir.as_os_str() + ); + prop_assert_eq!(loaded.vm_state_path.as_os_str(), cfg.vm_state_path.as_os_str()); + prop_assert_eq!(loaded.min_thread_count, cfg.min_thread_count); + prop_assert_eq!(loaded.max_thread_count, cfg.max_thread_count); + prop_assert!(nearly_eq( + loaded.host_cpu_scale_up_ceiling, + cfg.host_cpu_scale_up_ceiling + )); + prop_assert!(nearly_eq(loaded.cooldown_secs, cfg.cooldown_secs)); + prop_assert!(nearly_eq(loaded.scale_poll_secs, cfg.scale_poll_secs)); + prop_assert_eq!(loaded.enable_per_vm_status_line, cfg.enable_per_vm_status_line); + prop_assert_eq!( + loaded.enable_aggregate_status_line, + cfg.enable_aggregate_status_line + ); + prop_assert_eq!(loaded.print_status_header, cfg.print_status_header); + prop_assert_eq!(loaded.dry_run, cfg.dry_run); + } } /// Test that loading JSON with an unknown field returns diff --git a/src/engines/threshold.rs b/src/engines/threshold.rs index 6aa2b70..6640bcf 100644 --- a/src/engines/threshold.rs +++ b/src/engines/threshold.rs @@ -485,17 +485,12 @@ impl ScalingEngine for ThresholdEngine { #[cfg(test)] mod tests { - use std::{ - io::{self, Write}, - sync::{Arc, Mutex}, - time::{Duration, Instant}, - }; + use std::{sync::Arc, time::Instant}; use async_trait::async_trait; - use rstest::{fixture, rstest}; - use test_log::test; + use proptest::prelude::*; - use super::{ThresholdConfig, ThresholdEngine, log_performance_revert}; + use super::{ThresholdConfig, ThresholdEngine}; use crate::{ backends::BackendClientError, engines::{AppliedOutcome, EngineTickContext, ScaleAction, ScalingEngine}, @@ -505,20 +500,6 @@ mod tests { util::Path, }; - #[derive(Clone)] - struct BufferWriter(Arc>>); - - impl Write for BufferWriter { - fn write(&mut self, buffer: &[u8]) -> io::Result { - self.0.lock().unwrap().extend_from_slice(buffer); - Ok(buffer.len()) - } - - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } - } - struct SnapshotClient; #[async_trait] @@ -539,7 +520,8 @@ mod tests { async fn close(&self) {} } - #[fixture] + /// Create a sample Instance. + #[rstest::fixture] async fn instance() -> Arc { let instance = Arc::new(Instance::new( "vm-1".to_string(), @@ -554,215 +536,127 @@ mod tests { instance } - impl Default for EngineTickContext { - fn default() -> Self { - Self { - now: Instant::now() + Duration::from_secs(1), - min_thread_count: 2, - max_thread_count: 8, - host_cpu_util: 0.0, - tick_index: 0, - } - } - } - - fn context() -> EngineTickContext { - EngineTickContext::default() - } - + /// Set instance metrics and performance counters. async fn set_observation( instance: &Arc, thread_count: u32, per_thread_util: f64, - iops: u64, + iops_rate: u64, + io_count: u64, ) { let mut status = instance.status.write().await; status.thread_count = thread_count; status.per_thread_util = per_thread_util; - status.perf.as_mut().unwrap().read_io_count = iops; + status.read_iops = iops_rate; + status.write_iops = 0; + status.other_iops = 0; + let perf = status.perf.as_mut().unwrap(); + perf.read_io_count = io_count; + perf.write_io_count = 0; + perf.other_io_count = 0; } - /// Test that performance-revert log line includes VM id and the - /// reverted action. - #[test] - fn performance_revert_log_contains_decision_inputs() { - let output = Arc::new(Mutex::new(Vec::new())); - let writer_output = Arc::clone(&output); - let subscriber = tracing_subscriber::fmt() - .without_time() - .with_ansi(false) - .with_target(false) - .with_max_level(tracing::Level::INFO) - .with_writer(move || BufferWriter(Arc::clone(&writer_output))) - .finish(); - let _guard = tracing::subscriber::set_default(subscriber); - - log_performance_revert("vm-1", "up", 6, 5, 155_000, 122_000, 162_750.0); - - let rendered = String::from_utf8(output.lock().unwrap().clone()).unwrap(); - assert!(rendered.contains("performance validation failed; reverting previous scale")); - assert!(rendered.contains("vm=vm-1")); - assert!(rendered.contains("reverted_action=up")); - assert!(rendered.contains("baseline_iops=155000")); - assert!(rendered.contains("observed_iops=122000")); - assert!(rendered.contains("required_iops=162750")); - assert!(rendered.contains("thr=6->5")); - } - - #[fixture] - fn engine() -> ThresholdEngine { + #[rstest::fixture] + fn engine(#[default(3)] polls: u32) -> ThresholdEngine { ThresholdEngine::new(ThresholdConfig { - scale_up_threshold: 0.8, - scale_up_min_gain: 0.05, - scale_validation_sample_polls: 0, - ..Default::default() + scale_up_threshold: 0.5, + scale_up_min_gain: 0.10, + scale_validation_sample_polls: polls, + ..ThresholdConfig::default() }) } - /// Test that an IOPS-rate drop after scale-up triggers revert. - #[rstest] - #[tokio::test] - async fn scale_up_revert_fires_on_iops_rate_drop( - engine: ThresholdEngine, - #[future] instance: Arc, - ) { - let instance = instance.await; - engine.on_instance_added(&instance).await; - engine - .on_applied( - &instance.id, - AppliedOutcome::Success { - action: ScaleAction::Up(6), - prev_thread_count: 5, - prev_io_count_total: 155_000, - }, - ) - .await; - set_observation(&instance, 6, 0.6, 122_000).await; - - assert_eq!( - engine.evaluate(&instance, &context()).await, - ScaleAction::Revert(5) - ); + #[rstest::fixture] + fn context() -> EngineTickContext { + EngineTickContext { + now: Instant::now(), + min_thread_count: 1, + max_thread_count: 5, + host_cpu_util: 0.0, + tick_index: 0, + } } - /// Test that flat post-scale IOPS/rate during validation triggers - /// revert. - #[rstest] - #[tokio::test] - async fn scale_up_flat_rate_reverts( - engine: ThresholdEngine, - #[future] instance: Arc, - ) { - let instance = instance.await; - engine.on_instance_added(&instance).await; + async fn record_scale_up(engine: &ThresholdEngine, instance: &Arc, io_count: u64) { + engine.on_instance_added(instance).await; engine .on_applied( &instance.id, AppliedOutcome::Success { action: ScaleAction::Up(4), prev_thread_count: 3, - prev_io_count_total: 100_000, + prev_io_count_total: io_count, }, ) .await; - set_observation(&instance, 4, 0.6, 101_000).await; + } - assert_eq!( - engine.evaluate(&instance, &context()).await, - ScaleAction::Revert(3) - ); + /// Check the two floats are within at least 0.0001% of each other. + fn nearly_eq(left: f64, right: f64) -> bool { + (left - right).abs() <= 1e-6 * (1.0 + left.abs().max(right.abs())) } - /// Test that percent fields serde as human percents on the wire and - /// fractions in memory. + /// Test that an unknown percent key is rejected. #[test] - fn percent_wire_format_round_trips() { - let config: ThresholdConfig = - serde_json::from_str(r#"{"scale_up_min_gain_percent":10}"#).unwrap(); - assert!((config.scale_up_min_gain - 0.10).abs() < f64::EPSILON); - - let serialized = serde_json::to_string(&config).unwrap(); - assert!(serialized.contains(r#""scale_up_min_gain_percent":10.0"#)); + fn percent_wire_format_rejects_unknown_field() { assert!( serde_json::from_str::(r#"{"scale_up_revert_drop_percent":10}"#) .is_err() ); } - // Test that right after a scale up operation the engine does not ask for - // another scale up during the validation period. - /// Test that while a prior scale-up is pending validation, further - /// scale-ups are suppressed. - #[rstest] - #[test(tokio::test)] - async fn no_scale_up_during_pending_validation( - engine: ThresholdEngine, - #[future] instance: Arc, - ) { - let instance = instance.await; - - // inform the engine of the scale up - engine.on_instance_added(&instance).await; - engine - .on_applied( - &instance.id, - AppliedOutcome::Success { - action: ScaleAction::Up(5), - prev_thread_count: 4, - prev_io_count_total: 149_000, - }, - ) - .await; - - set_observation(&instance, 5, 0.88, 160_000).await; - - for _ in 0..engine.cfg.scale_validation_sample_polls { - // there should be no scale up during the validation period - assert_eq!( - engine.evaluate(&instance, &context()).await, - ScaleAction::None - ); + proptest! { + /// Test that percentage serialization round trips. + #[test] + fn percent_fields_round_trip_on_the_wire( + threshold in 0.0..=100.0f64, + min_gain in 0.0..=100.0f64, + revert_drop in 0.0..=100.0f64, + ) { + let input = serde_json::json!({ + "scale_up_threshold_percent": threshold, + "scale_up_min_gain_percent": min_gain, + "scale_down_revert_drop_percent": revert_drop, + }); + let config: ThresholdConfig = serde_json::from_str(&input.to_string()).unwrap(); + prop_assert!(nearly_eq(config.scale_up_threshold, threshold / 100.0)); + prop_assert!(nearly_eq(config.scale_up_min_gain, min_gain / 100.0)); + prop_assert!(nearly_eq(config.scale_down_revert_drop, revert_drop / 100.0)); + + let serialized: serde_json::Value = + serde_json::from_str(&serde_json::to_string(&config).unwrap()).unwrap(); + prop_assert!(nearly_eq( + serialized["scale_up_threshold_percent"].as_f64().unwrap(), + threshold + )); + prop_assert!(nearly_eq( + serialized["scale_up_min_gain_percent"].as_f64().unwrap(), + min_gain + )); + prop_assert!(nearly_eq( + serialized["scale_down_revert_drop_percent"].as_f64().unwrap(), + revert_drop + )); } - - // after the validation period the engine is allowed to scale up - assert_eq!( - engine.evaluate(&instance, &context()).await, - ScaleAction::Up(6) - ); } - /// Test that a scale up is revert after the validation period if - /// performance doesn't increase much. - #[rstest] - #[test(tokio::test)] - async fn pending_validation_reverts_regressive_scale( + /// Test that a scale-up stays held for every poll in the validation window. + #[rstest::rstest] + #[tokio::test] + async fn scale_up_validation_holds_for_the_sample_window( engine: ThresholdEngine, #[future] instance: Arc, + context: EngineTickContext, ) { let instance = instance.await; - engine.on_instance_added(&instance).await; - engine - .on_applied( - &instance.id, - AppliedOutcome::Success { - action: ScaleAction::Up(5), - prev_thread_count: 4, - prev_io_count_total: 149_000, - }, - ) - .await; - set_observation(&instance, 5, 0.88, 140_000).await; + set_observation(&instance, 4, 0.95, 100_000, 10_000_000).await; + record_scale_up(&engine, &instance, 10_000_000).await; - for _ in 0..engine.cfg.scale_validation_sample_polls { + for _ in 0..3 { assert_eq!( - engine.evaluate(&instance, &context()).await, + engine.evaluate(&instance, &context).await, ScaleAction::None ); } - assert_eq!( - engine.evaluate(&instance, &context()).await, - ScaleAction::Revert(4) - ); } } diff --git a/src/instance.rs b/src/instance.rs index 8b87723..82fffe2 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -583,6 +583,7 @@ mod tests { use async_trait::async_trait; + use super::compute_per_worker_util; use super::*; use crate::backends::BackendClientError; @@ -662,8 +663,6 @@ mod tests { assert!(closed.load(Ordering::Relaxed)); } - use super::{TaskCpuSample, compute_per_worker_util}; - fn task(tid: i32, name: &str, cpu_ticks: u64) -> TaskCpuSample { TaskCpuSample { tid, @@ -672,28 +671,58 @@ mod tests { } } - /// Test that two workers sharing a name still get independent - /// CPU-delta util samples. + /// Test that utilisation is the tick delta over wall ticks, in + /// current-sample order. #[test] - fn duplicate_worker_names_keep_independent_deltas() { - let previous = vec![task(10, "worker", 100), task(11, "worker", 200)]; - let current = vec![task(11, "worker", 400), task(10, "worker", 200)]; + fn per_worker_util_is_tick_delta_over_wall_ticks() { + let previous = vec![task(10, "a", 100), task(11, "b", 0)]; + let current = vec![task(11, "b", 25), task(10, "a", 150)]; + assert_eq!( + compute_per_worker_util(&previous, ¤t, 100.0), + vec![("b".to_string(), 0.25), ("a".to_string(), 0.5)] + ); + } - let util = compute_per_worker_util(&previous, ¤t, 500.0); - assert_eq!(util.len(), 2); - assert!((util[0].1 - 0.4).abs() < f64::EPSILON); - assert!((util[1].1 - 0.2).abs() < f64::EPSILON); + /// Test that a renamed task, an unknown tid, and a backwards + /// counter are dropped. + #[test] + fn per_worker_util_drops_unmatched_tasks() { + let previous = vec![ + task(1, "old", 100), + task(2, "kept", 50), + task(3, "back", 200), + ]; + let current = vec![ + task(1, "new", 150), + task(9, "unknown", 10), + task(3, "back", 100), + task(2, "kept", 75), + ]; + assert_eq!( + compute_per_worker_util(&previous, ¤t, 100.0), + vec![("kept".to_string(), 0.25)] + ); } - /// Test that a newly appeared worker does not spike util from - /// lifetime counters. + /// Test that a short interval cannot report more than one occupied CPU. #[test] - fn new_worker_starts_without_a_lifetime_spike() { - let previous = vec![task(10, "worker0", 100)]; - let current = vec![task(10, "worker0", 200), task(11, "worker1", 900_000)]; + fn per_worker_util_clamps_at_one() { + let previous = vec![task(1, "wrk", 0)]; + let current = vec![task(1, "wrk", 250)]; + assert_eq!( + compute_per_worker_util(&previous, ¤t, 100.0), + vec![("wrk".to_string(), 1.0)] + ); + } - let util = compute_per_worker_util(&previous, ¤t, 500.0); - assert_eq!(util.len(), 1); - assert!((util[0].1 - 0.2).abs() < f64::EPSILON); + /// Test that a repeated previous tid uses the last sample. + #[test] + fn per_worker_util_uses_the_last_sample_for_a_repeated_tid() { + let previous = vec![task(1, "wrk", 0), task(1, "wrk", 40)]; + let current = vec![task(1, "wrk", 90)]; + assert_eq!( + compute_per_worker_util(&previous, ¤t, 100.0), + vec![("wrk".to_string(), 0.5)] + ); } } diff --git a/src/rolling.rs b/src/rolling.rs index 915281f..294e42c 100644 --- a/src/rolling.rs +++ b/src/rolling.rs @@ -233,6 +233,8 @@ where #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; /// Test that the first sample records a baseline and yields no @@ -270,4 +272,85 @@ mod tests { metrics.push_from_procfs_delta(t0 + Duration::from_secs(1), 10, 10); assert!(metrics.is_empty()); } + + fn expected_cpu_ns(cpu_tick_delta: u64) -> u64 { + (cpu_tick_delta as f64 * 1_000_000_000.0 / *TICKS_PER_SECOND) as u64 + } + + proptest! { + #[test] + fn first_sample_leaves_the_window_empty( + io_ops in any::(), + cpu_ticks in any::(), + ) { + let mut metrics = RollingMetrics::new(); + metrics.push_from_procfs_delta(Instant::now(), io_ops, cpu_ticks); + prop_assert!(metrics.is_empty()); + prop_assert!(metrics.iops_over(Duration::from_secs(60)).is_none()); + prop_assert!(metrics.cpu_us_per_io_over(Duration::from_secs(60)).is_none()); + } + + #[test] + fn backwards_counter_keeps_len_and_resets_baseline( + io0 in 1u64..1_000_000, + cpu0 in 1u64..1_000_000, + io_delta in 0u64..10_000, + cpu_delta in 0u64..10_000, + elapsed_ms in 1u64..5_000, + reset_kind in 0u8..2, + ) { + let mut metrics = RollingMetrics::new(); + let t0 = Instant::now(); + let io1 = io0 + io_delta; + let cpu1 = cpu0 + cpu_delta; + metrics.push_from_procfs_delta(t0, io0, cpu0); + metrics.push_from_procfs_delta(t0 + Duration::from_millis(elapsed_ms), io1, cpu1); + prop_assert_eq!(metrics.len(), 1); + + let t_reset = t0 + Duration::from_millis(elapsed_ms + 1); + let (base_io, base_cpu) = match reset_kind { + 0 => { + metrics.push_from_procfs_delta(t_reset, io1 - 1, cpu1); + (io1 - 1, cpu1) + } + 1 => { + metrics.push_from_procfs_delta(t_reset, io1, cpu1 - 1); + (io1, cpu1 - 1) + } + _ => unreachable!() + }; + prop_assert_eq!(metrics.len(), 1); + + let t_next = t_reset + Duration::from_millis(elapsed_ms); + metrics.push_from_procfs_delta(t_next, base_io + io_delta, base_cpu + cpu_delta); + let (observed_io, observed_cpu_ns) = metrics.last_delta().unwrap(); + prop_assert_eq!(observed_io, io_delta); + prop_assert_eq!(observed_cpu_ns, expected_cpu_ns(cpu_delta)); + } + + #[test] + fn monotonic_pair_matches_elapsed_rate_formula( + io0 in 0u64..1_000_000, + cpu0 in 0u64..1_000_000, + io_delta in 1u64..100_000, + cpu_delta in 0u64..100_000, + elapsed_ms in 1u64..60_000, + ) { + let mut metrics = RollingMetrics::new(); + let t0 = Instant::now(); + let elapsed = Duration::from_millis(elapsed_ms); + metrics.push_from_procfs_delta(t0, io0, cpu0); + metrics.push_from_procfs_delta(t0 + elapsed, io0 + io_delta, cpu0 + cpu_delta); + + let wall_ns = u64::try_from(elapsed.as_nanos()).unwrap(); + let expected_iops = (u128::from(io_delta) * 1_000_000_000 / u128::from(wall_ns)) as u64; + prop_assert_eq!(metrics.iops_over(Duration::from_secs(60)), Some(expected_iops)); + + let cpu_ns = expected_cpu_ns(cpu_delta); + prop_assert_eq!( + metrics.cpu_us_per_io_over(Duration::from_secs(60)), + Some(cpu_ns / io_delta / 1_000) + ); + } + } } diff --git a/src/state.rs b/src/state.rs index 29dfcab..a45471c 100644 --- a/src/state.rs +++ b/src/state.rs @@ -130,21 +130,10 @@ impl VmStateStore { #[cfg(test)] mod tests { use super::*; + use proptest::prelude::*; - /// Test that managed/unmanaged sets save and reload from a single - /// JSON file. - #[test] - fn ownership_round_trips_in_one_file() { - let dir = tempfile::tempdir().unwrap(); - let store = VmStateStore::new(Path::new(&dir.path().join("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(dir.path()).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. @@ -159,17 +148,44 @@ mod tests { ); } - /// Test that a VM listed as both managed and unmanaged is rejected - /// on load. - #[test] - fn overlapping_classifications_are_rejected() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("ownership.json"); - std::fs::write( - &path, - r#"{"managed_vms":["same"],"unmanaged_vms":["same"]}"#, - ) - .unwrap(); - assert!(VmStateStore::new(Path::new(&path)).load().is_err()); + proptest! { + #[test] + fn disjoint_ownership_round_trips_in_one_file( + managed in id_set(), + unmanaged in id_set(), + ) { + let unmanaged: BTreeSet<_> = unmanaged.difference(&managed).cloned().collect(); + let state = VmOwnership { + managed_vms: managed, + unmanaged_vms: unmanaged, + }; + let dir = tempfile::tempdir().unwrap(); + let store = VmStateStore::new(Path::new(&dir.path().join("ownership.json"))); + store.save(&state).unwrap(); + prop_assert_eq!(store.load().unwrap(), state); + prop_assert_eq!(std::fs::read_dir(dir.path()).unwrap().count(), 1); + } + + #[test] + fn 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 dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("ownership.json"); + let store = VmStateStore::new(Path::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()); + } } }