test: collapse per-variant test clusters in ib-fabric, admin-cli, and libmlx (#2710)

Author: chet

crates/admin-cli/src/boot_override/tests.rs

@@ -46,48 +46,35 @@ fn verify_cmd_structure() {
 // including testing required arguments, as well as optional
 // flag-specific checking.
 
-// parse_get ensures the get subcommand routes to the Get variant and
-// parses its interface_id.
+// The get and clear subcommands each route to their own variant and parse the
+// shared interface_id. The closure yields the routed variant name paired with the
+// parsed interface_id, so one table covers both.
 #[test]
-fn parse_get() {
+fn parse_get_and_clear() {
     scenarios!(
         run = |argv| {
             Cmd::try_parse_from(argv.iter().copied())
                 .map(|cmd| match cmd {
-                    Cmd::Get(args) => args.inner.interface_id.to_string(),
-                    _ => panic!("expected Get variant"),
+                    Cmd::Get(args) => ("get", args.inner.interface_id.to_string()),
+                    Cmd::Clear(args) => ("clear", args.inner.interface_id.to_string()),
+                    _ => panic!("expected Get or Clear variant"),
                 })
                 .map_err(drop)
         };
-        "get parses interface_id" {
+        "get routes to Get and parses interface_id" {
             &[
                 "boot-override",
                 "get",
                 "550e8400-e29b-41d4-a716-446655440000",
-            ][..] => Yields("550e8400-e29b-41d4-a716-446655440000".to_string()),
+            ][..] => Yields(("get", "550e8400-e29b-41d4-a716-446655440000".to_string())),
         }
-    );
-}
 
-// parse_clear ensures the clear subcommand routes to the Clear variant and
-// parses its interface_id.
-#[test]
-fn parse_clear() {
-    scenarios!(
-        run = |argv| {
-            Cmd::try_parse_from(argv.iter().copied())
-                .map(|cmd| match cmd {
-                    Cmd::Clear(args) => args.inner.interface_id.to_string(),
-                    _ => panic!("expected Clear variant"),
-                })
-                .map_err(drop)
-        };
-        "clear parses interface_id" {
+        "clear routes to Clear and parses interface_id" {
             &[
                 "boot-override",
                 "clear",
                 "550e8400-e29b-41d4-a716-446655440000",
-            ][..] => Yields("550e8400-e29b-41d4-a716-446655440000".to_string()),
+            ][..] => Yields(("clear", "550e8400-e29b-41d4-a716-446655440000".to_string())),
         }
     );
 }

crates/admin-cli/src/cfg/dispatch.rs

@@ -59,8 +59,11 @@ mod tests {
         }
     }
 
-    // Verify the derive generates a valid Dispatch impl when
-    // all variants are "leaf" commands (i.e. the Run trait).
+    // Deriving `Dispatch` on these enums is itself the test: the derive only
+    // compiles if it generates a valid `Dispatch` impl. `AllRunCmd` covers the
+    // all-leaf case (every variant is a `Run` command); `MixedCmd` covers mixing
+    // a leaf command with a nested `#[dispatch]` group. If either impl failed to
+    // generate, this module would not compile.
     #[derive(Dispatch)]
     #[allow(dead_code)]
     enum AllRunCmd {
@@ -69,26 +72,11 @@ mod tests {
         CmdC(StubRunArgs),
     }
 
-    // Verify the derive generates a valid Dispatch impl when
-    // mixing "leaf" commands (the `Run` trait) with nested command
-    // groups (which we annotate inline with `#[dispatch]`).
     #[derive(Dispatch)]
     #[allow(dead_code)]
     enum MixedCmd {
         SimpleRunCommand(StubRunArgs),
         #[dispatch]
         NestedCommandGroup(StubNestedCmd),
     }
-
-    fn assert_dispatch<T: Dispatch>() {}
-
-    #[test]
-    fn all_run_variants_derive_dispatch() {
-        assert_dispatch::<AllRunCmd>();
-    }
-
-    #[test]
-    fn mixed_run_and_dispatch_variants_derive_dispatch() {
-        assert_dispatch::<MixedCmd>();
-    }
 }

crates/admin-cli/src/dpu/tests.rs

@@ -50,14 +50,6 @@ fn verify_cmd_structure() {
 // including testing required arguments, as well as optional
 // flag-specific checking.
 
-// parse_status ensures status routes to the Status variant
-// with no arguments.
-#[test]
-fn parse_status() {
-    let cmd = Cmd::try_parse_from(["dpu", "status"]).expect("should parse status");
-    assert!(matches!(cmd, Cmd::Status(_)));
-}
-
 // versions parses with and without --updates-only; the parsed flag mirrors
 // whether the switch was supplied.
 #[test]

crates/admin-cli/src/expected_machines/tests.rs

@@ -499,87 +499,106 @@ fn parse_add_without_dpu_mode() {
     }
 }
 
-// parse_add_with_dpu_mode_nic ensures `--dpu-mode nic-mode`
-// parses to the NicMode variant.
+// `--dpu-mode <value>` parses to the matching DpuMode variant on both `add`
+// (alongside the required credential/chassis args) and `patch` (where flipping
+// dpu_mode on a single host is the whole point). The closure pulls dpu_mode off
+// whichever variant parsed; each row pins the parsed `Some(variant)`.
 #[test]
-fn parse_add_with_dpu_mode_nic() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "add",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--bmc-username",
-        "admin",
-        "--bmc-password",
-        "secret",
-        "--chassis-serial-number",
-        "SN12345",
-        "--dpu-mode",
-        "nic-mode",
-    ])
-    .expect("should parse nic-mode");
+fn parse_dpu_mode_to_its_variant() {
+    scenarios!(
+        run = |argv| {
+            Cmd::try_parse_from(argv.iter().copied())
+                .map(|cmd| match cmd {
+                    Cmd::Add(args) => args.dpu_mode,
+                    Cmd::Patch(args) => args.dpu_mode,
+                    _ => panic!("expected Add or Patch variant"),
+                })
+                .map_err(drop)
+        };
+        "add --dpu-mode nic-mode" {
+            &[
+                "expected-machine",
+                "add",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--bmc-username",
+                "admin",
+                "--bmc-password",
+                "secret",
+                "--chassis-serial-number",
+                "SN12345",
+                "--dpu-mode",
+                "nic-mode",
+            ][..] => Yields(Some(rpc::forge::DpuMode::NicMode)),
+        }
 
-    match cmd {
-        Cmd::Add(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::NicMode)));
+        "add --dpu-mode no-dpu" {
+            &[
+                "expected-machine",
+                "add",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--bmc-username",
+                "admin",
+                "--bmc-password",
+                "secret",
+                "--chassis-serial-number",
+                "SN12345",
+                "--dpu-mode",
+                "no-dpu",
+            ][..] => Yields(Some(rpc::forge::DpuMode::NoDpu)),
         }
-        _ => panic!("expected Add variant"),
-    }
-}
 
-// parse_add_with_dpu_mode_no_dpu ensures `--dpu-mode no-dpu` parses
-// correctly (host with zero DPU hardware at all).
-#[test]
-fn parse_add_with_dpu_mode_no_dpu() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "add",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--bmc-username",
-        "admin",
-        "--bmc-password",
-        "secret",
-        "--chassis-serial-number",
-        "SN12345",
-        "--dpu-mode",
-        "no-dpu",
-    ])
-    .expect("should parse no-dpu");
+        "add --dpu-mode dpu-mode" {
+            &[
+                "expected-machine",
+                "add",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--bmc-username",
+                "admin",
+                "--bmc-password",
+                "secret",
+                "--chassis-serial-number",
+                "SN12345",
+                "--dpu-mode",
+                "dpu-mode",
+            ][..] => Yields(Some(rpc::forge::DpuMode::DpuMode)),
+        }
 
-    match cmd {
-        Cmd::Add(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::NoDpu)));
+        "patch --dpu-mode nic-mode" {
+            &[
+                "expected-machine",
+                "patch",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--dpu-mode",
+                "nic-mode",
+            ][..] => Yields(Some(rpc::forge::DpuMode::NicMode)),
         }
-        _ => panic!("expected Add variant"),
-    }
-}
 
-// parse_add_with_dpu_mode_dpu ensures `--dpu-mode dpu-mode` parses.
-#[test]
-fn parse_add_with_dpu_mode_dpu() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "add",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--bmc-username",
-        "admin",
-        "--bmc-password",
-        "secret",
-        "--chassis-serial-number",
-        "SN12345",
-        "--dpu-mode",
-        "dpu-mode",
-    ])
-    .expect("should parse dpu-mode");
+        "patch --dpu-mode no-dpu" {
+            &[
+                "expected-machine",
+                "patch",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--dpu-mode",
+                "no-dpu",
+            ][..] => Yields(Some(rpc::forge::DpuMode::NoDpu)),
+        }
 
-    match cmd {
-        Cmd::Add(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::DpuMode)));
+        "patch --dpu-mode dpu-mode" {
+            &[
+                "expected-machine",
+                "patch",
+                "--bmc-mac-address",
+                "1a:2b:3c:4d:5e:6f",
+                "--dpu-mode",
+                "dpu-mode",
+            ][..] => Yields(Some(rpc::forge::DpuMode::DpuMode)),
         }
-        _ => panic!("expected Add variant"),
-    }
+    );
 }
 
 // parse_add_rejects_invalid_dpu_mode ensures clap rejects values that
@@ -606,74 +625,6 @@ fn parse_add_rejects_invalid_dpu_mode() {
     );
 }
 
-// parse_patch_with_dpu_mode_nic ensures `patch ... --dpu-mode nic-mode`
-// parses to NicMode. Patch users flip dpu_mode on a single host without
-// rewriting the entire ExpectedMachine.
-#[test]
-fn parse_patch_with_dpu_mode_nic() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "patch",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--dpu-mode",
-        "nic-mode",
-    ])
-    .expect("should parse patch --dpu-mode nic-mode");
-
-    match cmd {
-        Cmd::Patch(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::NicMode)));
-        }
-        _ => panic!("expected Patch variant"),
-    }
-}
-
-// parse_patch_with_dpu_mode_no_dpu ensures `patch ... --dpu-mode no-dpu`
-// parses (host with no DPU hardware at all).
-#[test]
-fn parse_patch_with_dpu_mode_no_dpu() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "patch",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--dpu-mode",
-        "no-dpu",
-    ])
-    .expect("should parse patch --dpu-mode no-dpu");
-
-    match cmd {
-        Cmd::Patch(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::NoDpu)));
-        }
-        _ => panic!("expected Patch variant"),
-    }
-}
-
-// parse_patch_with_dpu_mode_dpu ensures `patch ... --dpu-mode dpu-mode`
-// parses; this is how operators revert a host back to the default after
-// previously setting NicMode/NoDpu.
-#[test]
-fn parse_patch_with_dpu_mode_dpu() {
-    let cmd = Cmd::try_parse_from([
-        "expected-machine",
-        "patch",
-        "--bmc-mac-address",
-        "1a:2b:3c:4d:5e:6f",
-        "--dpu-mode",
-        "dpu-mode",
-    ])
-    .expect("should parse patch --dpu-mode dpu-mode");
-
-    match cmd {
-        Cmd::Patch(args) => {
-            assert!(matches!(args.dpu_mode, Some(rpc::forge::DpuMode::DpuMode)));
-        }
-        _ => panic!("expected Patch variant"),
-    }
-}
-
 // validate_patch_with_dpu_mode_only ensures `patch --dpu-mode nic-mode`
 // alone (no other patchable fields) satisfies clap's ArgGroup and the
 // `Args::validate()` "at least one field" check. The whole point of this

crates/admin-cli/src/firmware/tests.rs

@@ -23,7 +23,7 @@
 // Command Structure - Baseline debug_assert() of the entire command.
 // Argument Parsing  - Ensure required/optional arg combinations parse correctly.
 
-use clap::{CommandFactory, Parser};
+use clap::CommandFactory;
 
 use super::*;
 
@@ -36,18 +36,3 @@ use super::*;
 fn verify_cmd_structure() {
     Cmd::command().debug_assert();
 }
-
-/////////////////////////////////////////////////////////////////////////////
-// Argument Parsing
-//
-// This section contains tests specific to argument parsing,
-// including testing required arguments, as well as optional
-// flag-specific checking.
-
-// parse_show ensures show parses with no arguments.
-#[test]
-fn parse_show() {
-    let cmd = Cmd::try_parse_from(["firmware", "show"]).expect("should parse show");
-
-    assert!(matches!(cmd, Cmd::Show(_)));
-}