Re: [Libguestfs] [libnbd PATCH v5 02/12] rust: Add a couple of integration tests

On Thu, Aug 03, 2023 at 03:36:06PM +0000, Tage Johansson wrote: > A couple of integration tests are added in rust/tests. They are mostly > ported from the OCaml tests. > --- Overall, this looked like a nice counterpart to the OCaml unit tests, and I was able to easily figure out how to amend my own tests in for the unit tests I added in my 64-bit extension work. Keeping similar unit test numbers across language bindings has been a big boon :-) Style question: > +++ b/rust/tests/test_250_opt_set_meta.rs > + > +/// A struct with information about set meta contexts. > +#[derive(Debug, Clone, PartialEq, Eq)] > +struct CtxInfo { > + /// Whether the meta context "base:allocation" is set. > + has_alloc: bool, > + /// The number of set meta contexts. > + count: u32, > +} Here, you use a trailing comma, > + > +fn set_meta_ctxs(nbd: &libnbd::Handle) -> libnbd::Result<CtxInfo> { > + let info = Arc::new(Mutex::new(CtxInfo { > + has_alloc: false, > + count: 0, > + })); > + let info_clone = info.clone(); > + let replies = nbd.opt_set_meta_context(move |ctx| { > + let mut info = info_clone.lock().unwrap(); > + info.count += 1; > + if ctx == CONTEXT_BASE_ALLOCATION { > + info.has_alloc = true; > + } > + 0 > + })?; > + let info = Arc::into_inner(info).unwrap().into_inner().unwrap(); > + assert_eq!(info.count, replies); > + Ok(info) > +} > + ... > + > + // nbdkit does not match wildcard for SET, even though it does for LIST > + nbd.clear_meta_contexts().unwrap(); > + nbd.add_meta_context("base:").unwrap(); > + assert_eq!( > + set_meta_ctxs(&nbd).unwrap(), > + CtxInfo { > + count: 0, > + has_alloc: false > + } whereas here, you did not. Does it make a difference? 'make check' still passes, and rustfmt doesn't complain, if I temporarily add a trailing comma here.

I also note that 'rustfmt --check rust/tests/*.rs' flags a few style suggestions. I had started work on automating gofmt for all checked-in *.go files; maybe I should revive that patch and extend it to also automate rustfmt on all checked-in *.rs files. Here's what rustfmt suggested to me (rustfmt 1.5.2-stable ( ) on Fedora 38): Diff in /home/eblake/libnbd/rust/tests/test_200_connect_command.rs at line 17: #![deny(warnings)] - #[test] fn test_connect_command() { let nbd = libnbd::Handle::new().unwrap(); Diff in /home/eblake/libnbd/rust/tests/test_200_connect_command.rs at line 24: - nbd.connect_command(&[ - "nbdkit", - "-s", - "--exit-with-parent", - "-v", - "null", - ]) - .unwrap(); + nbd.connect_command(&["nbdkit", "-s", "--exit-with-parent", "-v", "null"]) + .unwrap(); } Diff in /home/eblake/libnbd/rust/tests/test_300_get_size.rs at line 17: #![deny(warnings)] - #[test] fn test_get_size() { let nbd = libnbd::Handle::new().unwrap(); Diff in /home/eblake/libnbd/rust/tests/test_620_stats.rs at line 17: #![deny(warnings)] - #[test] fn test_stats() { let nbd = libnbd::Handle::new().unwrap(); Diff in /home/eblake/libnbd/rust/tests/test_620_stats.rs at line 31: // Connection performs handshaking, which increments stats. // The number of bytes/chunks here may grow over time as more features get // automatically negotiated, so merely check that they are non-zero. - nbd.connect_command(&[ - "nbdkit", - "-s", - "--exit-with-parent", - "-v", - "null", - ]) - .unwrap(); + nbd.connect_command(&["nbdkit", "-s", "--exit-with-parent", "-v", "null"]) + .unwrap(); let bs1 = nbd.stats_bytes_sent(); let cs1 = nbd.stats_chunks_sent();