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();
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
