On 8/16/2023 11:35 PM, Eric Blake wrote:
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.
No, it doesn't make a difference. I nearly always runs rustfmt on my
code but it doesn't seem that rustfmt cares about that comma.
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();
Automating rustfmt on all checked-in rust files sounds like a good idea.
It is strange that those file were not proprly formatted since I just
mentioned that I nearly always runs rustfmt on every edit. For the files
which are not upstream yet, (test_async_*), I will fix the formatting in
the next patch series. For the other files I will create an extra patch
to correct the formatting.
--
Best regards,
Tage
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
