From 377f650808e25caa30070bbb6bf708bce5364527 Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 05:59:13 +0000 Subject: [PATCH 01/10] Delete port-link tests The comment mentions a time for deletion, and that time has come. These tests have been ignored in CI for at least a year and reference a CLI command that no longer exists. --- swadm/tests/port-link.rs | 254 --------------------------------------- 1 file changed, 254 deletions(-) delete mode 100644 swadm/tests/port-link.rs diff --git a/swadm/tests/port-link.rs b/swadm/tests/port-link.rs deleted file mode 100644 index a9b3bb80..00000000 --- a/swadm/tests/port-link.rs +++ /dev/null @@ -1,254 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/ -// -// Copyright 2026 Oxide Computer Company - -//! Small integration test to verify that getting / setting properties either -//! via the port-based or link-based `swadm` APIs work as expected. -//! -//! This is a one-off test, and should be deleted once the port-v-link -//! conversion is merged. Note that the Dendrite server needs to be in a fresh -//! state for most of these tests to be valid. - -use std::net::IpAddr; -use std::process::Command; - -// Path to `swadm` executable. -const SWADM: &str = env!("CARGO_BIN_EXE_swadm"); - -// The name of the link we're operating on, in the new and old naming schemes -// respectively. -const LINK: &str = "rear0/0"; -const PORT: &str = "1:0"; - -fn swadm() -> Command { - Command::new(SWADM) -} - -#[derive(Debug)] -struct PropertyValue<'a> { - name: &'a str, - value: &'a str, -} - -#[derive(Debug)] -struct SetTest<'a> { - port: PropertyValue<'a>, - link: PropertyValue<'a>, -} - -impl SetTest<'_> { - fn run(self) { - // Check that properties fetched through link and port are the same, to - // start. - let port_val = get_port_prop(self.port.name); - let link_val = get_link_prop(self.link.name); - assert_eq!( - port_val, link_val, - "Property '{}'/'{}' differs between port and link schemes", - self.port.name, self.link.name, - ); - - // Check that we set the property via port, and fetch it via link. - set_port_prop(self.port.name, self.port.value); - let link_val = get_link_prop(self.link.name); - assert_eq!(self.port.value, link_val.trim()); - - // Check that we set the property via link, and fetch it via port. - set_link_prop(self.link.name, self.link.value); - let port_val = get_port_prop(self.port.name); - assert_eq!(self.link.value, port_val.trim()); - } -} - -fn get_link_prop(name: &str) -> String { - let link_val = swadm() - .arg("link") - .arg("get-prop") - .arg(LINK) - .arg(name) - .output() - .unwrap() - .stdout; - String::from_utf8(link_val).unwrap() -} - -fn get_port_prop(name: &str) -> String { - let port_val = swadm() - .arg("port") - .arg("get") - .arg(PORT) - .arg(name) - .output() - .unwrap() - .stdout; - String::from_utf8(port_val).unwrap() -} - -fn set_link_prop(name: &str, value: &str) { - swadm() - .arg("link") - .arg("set-prop") - .arg(LINK) - .arg(name) - .arg(value) - .output() - .unwrap(); -} - -fn set_port_prop(name: &str, value: &str) { - swadm() - .arg("port") - .arg("set") - .arg(PORT) - .arg(name) - .arg(value) - .output() - .unwrap(); -} - -#[test] -#[ignore] -fn test_mac() { - let test = SetTest { - port: PropertyValue { name: "mac", value: "a8:40:25:ff:ff:01" }, - link: PropertyValue { name: "mac", value: "a8:40:25:ff:ff:02" }, - }; - test.run(); -} - -#[test] -#[ignore] -fn test_an() { - let test = SetTest { - port: PropertyValue { name: "an", value: "true" }, - link: PropertyValue { name: "an", value: "false" }, - }; - test.run(); -} - -#[test] -#[ignore] -fn test_kr() { - let test = SetTest { - port: PropertyValue { name: "kr", value: "true" }, - link: PropertyValue { name: "kr", value: "false" }, - }; - test.run(); -} - -#[test] -#[ignore] -fn test_enable() { - let test = SetTest { - port: PropertyValue { name: "ena", value: "true" }, - link: PropertyValue { name: "ena", value: "false" }, - }; - test.run(); -} - -// Test getting/setting IP addresses on a port/link works correctly. -// -// This is a bit different, since there are multiple IP addresses on each link. -// Also, the port-based swadm API doesn't support operating on addresses; that's -// only available through `swadm addr`. -#[test] -#[ignore] -fn test_ip_addresses() { - let added_port_addrs: &[IpAddr] = - &["192.168.1.1".parse().unwrap(), "fd00::1".parse().unwrap()]; - let added_link_addrs: &[IpAddr] = - &["192.168.1.2".parse().unwrap(), "fd00::2".parse().unwrap()]; - - // Check that both schemes have the same addresses. - let port_addrs = String::from_utf8( - swadm().arg("addr").arg("list").arg(PORT).output().unwrap().stdout, - ) - .unwrap(); - let link_addrs = String::from_utf8( - swadm() - .arg("link") - .arg("get-prop") - .arg(LINK) - .arg("ip") - .output() - .unwrap() - .stdout, - ) - .unwrap(); - assert_eq!(port_addrs, link_addrs); - - // Add the IP addresses via the port scheme. Verify we get them back, and - // that they're also listed in the link scheme. - for addr in added_port_addrs.iter() { - swadm() - .arg("addr") - .arg("add") - .arg(PORT) - .arg(addr.to_string()) - .output() - .unwrap(); - } - let port_addrs: Vec = String::from_utf8( - swadm().arg("addr").arg("list").arg(PORT).output().unwrap().stdout, - ) - .unwrap() - .lines() - .map(|line| line.parse().unwrap()) - .collect(); - let link_addrs: Vec = String::from_utf8( - swadm() - .arg("link") - .arg("get-prop") - .arg(LINK) - .arg("ip") - .output() - .unwrap() - .stdout, - ) - .unwrap() - .lines() - .map(|line| line.parse().unwrap()) - .collect(); - assert_eq!(port_addrs, link_addrs); - assert_eq!(port_addrs, added_port_addrs); - - // Add the IP addresses via the link scheme. Verify we get them back, and - // that they're also listed in the port scheme. - for addr in added_link_addrs.iter() { - swadm() - .arg("link") - .arg("set-prop") - .arg(LINK) - .arg("ip") - .arg(addr.to_string()) - .output() - .unwrap(); - } - let port_addrs: Vec = String::from_utf8( - swadm().arg("addr").arg("list").arg(PORT).output().unwrap().stdout, - ) - .unwrap() - .lines() - .map(|line| line.parse().unwrap()) - .collect(); - let link_addrs: Vec = String::from_utf8( - swadm() - .arg("link") - .arg("get-prop") - .arg(LINK) - .arg("ip") - .output() - .unwrap() - .stdout, - ) - .unwrap() - .lines() - .map(|line| line.parse().unwrap()) - .collect(); - assert_eq!(port_addrs, link_addrs); - let mut all_addrs = [added_port_addrs, added_link_addrs].concat(); - all_addrs.sort(); - assert_eq!(port_addrs, all_addrs); -} From eb6e8e2374d9b3d12e6e2a0d42ed41a866bb617a Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 09:10:51 +0000 Subject: [PATCH 02/10] Run all --ignored swadm tests in CI The tests CI previously evaded were deleted below. --- .github/buildomat/packet-test-common.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/buildomat/packet-test-common.sh b/.github/buildomat/packet-test-common.sh index 6df876f4..0f638f91 100755 --- a/.github/buildomat/packet-test-common.sh +++ b/.github/buildomat/packet-test-common.sh @@ -106,8 +106,6 @@ DENDRITE_TEST_HOST='[::1]' \ cargo test \ --no-fail-fast \ $SWADM_FEATURES \ - --test \ - counters \ -- \ --ignored From 18fd4825a651230a37acf2848da95d35d0cddd7e Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 22:29:12 +0000 Subject: [PATCH 03/10] Test utils for CLI validation Decisions: - Use `std::process::Command` and string parsing instead of dpd dropshot endpoints or a --parsable flag. The goal of swadm regression tests is stability on the string typed interface. - But keep this in rust because bash tests would quickly get out of hand. - Keeping utils in the tests dir hopefully atones for putting hideous regexes in swadm. --- Cargo.lock | 55 +++++--- Cargo.toml | 1 + swadm/Cargo.toml | 4 + swadm/tests/cmd/mod.rs | 273 ++++++++++++++++++++++++++++++++++++++++ swadm/tests/counters.rs | 2 + 5 files changed, 321 insertions(+), 14 deletions(-) create mode 100644 swadm/tests/cmd/mod.rs diff --git a/Cargo.lock b/Cargo.lock index e4f7141f..4257703d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -946,7 +946,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.0", ] [[package]] @@ -2136,7 +2136,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.0", ] [[package]] @@ -3067,7 +3067,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.5.10", + "socket2 0.6.4", "system-configuration", "tokio", "tower-layer", @@ -3497,7 +3497,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3565,7 +3565,7 @@ dependencies = [ "portable-atomic", "portable-atomic-util", "serde", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -5718,7 +5718,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "27c6023962132f4b30eb4c172c91ce92d933da334c59c23cddee82358ddafb0b" dependencies = [ "anyhow", - "itertools 0.12.1", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.118", @@ -5784,7 +5784,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls 0.23.40", - "socket2 0.5.10", + "socket2 0.6.4", "thiserror 2.0.18", "tokio", "tracing", @@ -5821,9 +5821,9 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.5.10", + "socket2 0.6.4", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -6189,7 +6189,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6202,7 +6202,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.61.0", ] [[package]] @@ -6706,6 +6706,31 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serial_test" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "699f4197115b8a7e7ff19c9a315a4bd6fffec26cc4626ef45ecaea389e081c6d" +dependencies = [ + "futures-executor", + "futures-util", + "log", + "once_cell", + "parking_lot", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e153fc76e1c6a068703d6d29c508a0b15c061c4b7e43da59cc097bc342673c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.118", +] + [[package]] name = "sha1" version = "0.10.6" @@ -7070,7 +7095,7 @@ version = "0.8.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c1c97747dbf44bb1ca44a561ece23508e99cb592e862f22222dcf42f51d1e451" dependencies = [ - "heck 0.4.1", + "heck 0.5.0", "proc-macro2", "quote", "syn 2.0.118", @@ -7282,8 +7307,10 @@ dependencies = [ "oxnet", "regex", "reqwest 0.13.2", + "serial_test", "slog", "tabwriter", + "thiserror 2.0.18", "tokio", ] @@ -7438,7 +7465,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.61.0", ] [[package]] @@ -8986,7 +9013,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9691a0d1..828770ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,6 +100,7 @@ scuffle = { version = "0.1.0", features = ["smf-by-instance"] } semver = "1.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +serial_test = "3.5.0" signal-hook = "0.4" signal-hook-tokio = { version = "0.4", features = [ "futures-v0_3" ] } slog = { version = "2.7", features = [ "release_max_level_debug", "max_level_trace" ] } diff --git a/swadm/Cargo.toml b/swadm/Cargo.toml index 2c0bc18b..946d0c1d 100644 --- a/swadm/Cargo.toml +++ b/swadm/Cargo.toml @@ -24,3 +24,7 @@ reqwest.workspace = true slog.workspace = true tabwriter.workspace = true tokio.workspace = true + +[dev-dependencies] +serial_test.workspace = true +thiserror.workspace = true diff --git a/swadm/tests/cmd/mod.rs b/swadm/tests/cmd/mod.rs new file mode 100644 index 00000000..36277c13 --- /dev/null +++ b/swadm/tests/cmd/mod.rs @@ -0,0 +1,273 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +//! # Cmd +//! +//! This module defines helpers for executing swadm CLI +//! commands and validating their output. +//! +//! They assume a test environment in which swadm can +//! reach dpd and control it without interference. + +use std::borrow::Cow; +use std::process::Command; + +use anyhow::Context; +use anyhow::bail; +use regex::Regex; + +const SWADM: &str = env!("CARGO_BIN_EXE_swadm"); + +// Integration test libraries don't support cargo doc tests. +// See test modules for examples. + +/// This macro simplifies working with the [`Pattern`] type +/// when validating the output of [`swadm`]. +/// +/// Regex consts from [`re`], literals, and numbers are generally +/// accepted. +#[macro_export] +macro_rules! pat { + [$($p:expr),*] => { + [$($crate::cmd::Pattern::from($p)),*] + }; +} + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("subprocess exited in error: {0:?}")] + Proc(Output), + + #[error("failed to spawn process: {0:?}")] + Exec(#[from] std::io::Error), + + #[error("failed to decode output as utf-8: {0:?}")] + Utf8(#[from] std::str::Utf8Error), +} + +/// Runs a `swadm` CLI command with the given args. +pub fn swadm(input: impl AsRef) -> Result { + let host = std::env::var("DENDRITE_TEST_HOST") + .map(Cow::from) + .unwrap_or_else(|_| "[::1]".into()); + let port = std::env::var("DENDRITE_TEST_PORT") + .map(Cow::from) + .unwrap_or_else(|_| "12224".into()); + + let mut args = vec!["--host", host.as_ref(), "--port", port.as_ref()]; + args.extend(input.as_ref().trim().split_ascii_whitespace()); + + let output = Command::new(self::SWADM).args(&args).output()?; + + if !output.status.success() { + return Err(self::Error::Proc(Output::Stderr( + std::str::from_utf8(&output.stderr)?.into(), + ))); + } + + Ok(Output::Stdout(std::str::from_utf8(&output.stdout)?.into())) +} + +/// Common regex patterns for searching swadm output. +pub mod re { + use super::Pattern; + + /// Anything up until the next match. + pub const ANY: Pattern = Pattern::regex(r".*?"); + + /// One or more consecutive non whitespace chars. + pub const WORD: Pattern = Pattern::regex(r"\S+"); + + /// A block of parentheses with characters inside. Doesn't + /// support nested parentheses. + pub const PARENS: Pattern = Pattern::regex(r"\([^)]*\)"); +} + +/// A wrapper type for regex inputs that can be +/// chained to validate swadm CLI output. +pub struct Pattern(Cow<'static, str>); + +impl Pattern { + /// Creates a regex pattern. + pub const fn regex(re: &'static str) -> Self { + Self(Cow::Borrowed(re)) + } + + /// Creates a text literal that will not engage regex semantics. + pub fn literal(lit: impl AsRef) -> Self { + Self(regex::escape(lit.as_ref()).into()) + } +} + +impl AsRef for Pattern { + fn as_ref(&self) -> &str { + self.0.as_ref() + } +} + +impl From<&str> for Pattern { + fn from(value: &str) -> Self { + Self::literal(value) + } +} + +impl From for Pattern { + fn from(value: String) -> Self { + Self::literal(value) + } +} + +// The number implementations cause allocations. But this is +// for tests, and it's convenient. + +impl From for Pattern { + fn from(value: i8) -> Self { + Self::literal(format!("{value}")) + } +} + +impl From for Pattern { + fn from(value: i32) -> Self { + Self::literal(format!("{value}")) + } +} + +impl From for Pattern { + fn from(value: usize) -> Self { + Self::literal(format!("{value}")) + } +} + +/// This contains the output of a [`swadm`] command +/// and can be used for parsing CLI results. +#[derive(Debug)] +pub enum Output { + Stdout(Box), + Stderr(Box), +} + +impl Output { + /// Searches for a single line in the output matching the + /// given pattern. + /// + /// Patterns allow anything between members and can be + /// easily constructed using the [`pat`] macro and [`Pattern`] + /// type. Or use your own regex strings if so inclined. + /// + /// This function may fail if the merged patterns form an + /// invalid regex. + /// + /// Given a valid regex, this succeeds IFF there is a + /// single matching line in the output. + pub fn expect_line( + &self, + pattern: impl IntoIterator>, + ) -> anyhow::Result<()> { + let reg = Self::make_regex(pattern)?; + + let count = self + .as_ref() + .lines() + .filter(|line| reg.is_match(line.trim())) + .count(); + + if count != 1 { + bail!( + " +Expected exactly one match, found {count:?}. + Regex: {reg:?} + Text: {}", + self.as_ref() + ); + } + + Ok(()) + } + + /// Merges a user-friendly input iterator of patterns into a + /// line matching regex. Returns err if the regex fails to compile. + fn make_regex( + pattern: impl IntoIterator>, + ) -> anyhow::Result { + let mut line_match = re::ANY.as_ref().to_string(); + for pat in pattern.into_iter() { + line_match.push_str(pat.as_ref()); + line_match.push_str(re::ANY.as_ref()); + } + + Regex::new(&line_match) + .with_context(|| format!("Regex failed to compile: {line_match:?}")) + } +} + +impl AsRef for Output { + fn as_ref(&self) -> &str { + let (Self::Stdout(txt) | Self::Stderr(txt)) = self; + txt + } +} + +impl TryFrom for Output { + type Error = anyhow::Error; + + fn try_from(err: self::Error) -> anyhow::Result { + if let Error::Proc(out) = err { + return Ok(out); + } + bail!("Cannot parse cmd error: {err:?}"); + } +} + +#[cfg(test)] +mod test { + use crate::cmd::{ + Output, + re::{ANY, PARENS, WORD}, + }; + + /// Validates output parsing against txeq output. + #[test] + fn output_txeq() -> anyhow::Result<()> { + // Tap values are nonsensical. We could proptest this, but idk + // if that's warranted complexity in a test for a test for a CLI tool. + + const TXEQ_STDOUT: &str = " + lane 0 lane 1 lane 2 lane 3 +pre2 0 (111) 1 ( 11) 2 ( 1) 3 ( 11) +pre1 -1 ( 11) -2 ( 11) -3 ( 11) -4 ( 11) +main 19 ( 11) 20 ( 11) 21 ( 11) 22 ( 11) +post1 -2 ( 1) -13 ( -2) -9 (-11) -22 (-123) +post2 -123 ( 11) 456 ( 11) 0 ( 11) 0 ( 11) +"; + + let out = Output::Stdout(TXEQ_STDOUT.into()); + + // Verify header + out.expect_line(pat!["lane 0", "lane 1", "lane 2", "lane 3"])?; + + // Post2 across all four lanes. + out.expect_line(pat![ + "post2", "-123", PARENS, "456", PARENS, "0", PARENS, "0", PARENS + ])?; + + // Lane 2 across all five parameters. + for (spot, value) in [ + ("pre2", "1"), + ("pre1", "-2"), + ("main", "20"), + ("post1", "-13"), + ("post2", "456"), + ] { + out.expect_line(pat![ + spot, WORD, PARENS, value, PARENS, WORD, PARENS, WORD, PARENS + ])?; + + out.expect_line(pat![spot, WORD, PARENS, value, ANY])?; + } + + Ok(()) + } +} diff --git a/swadm/tests/counters.rs b/swadm/tests/counters.rs index f01f7e15..80d4341a 100644 --- a/swadm/tests/counters.rs +++ b/swadm/tests/counters.rs @@ -6,6 +6,7 @@ //! Integration test for swadm P4 counter functionality. +use serial_test::serial; use std::process::Command; // Path to `swadm` executable. @@ -17,6 +18,7 @@ fn swadm() -> Command { #[test] #[ignore] +#[serial] fn test_p4_counter_list() { let output = swadm() .arg("--host") From 791c7f84b2479d4a286c9a9ae0fed2d63c38985c Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 22:30:14 +0000 Subject: [PATCH 04/10] swadm tx eq tests --- swadm/tests/tx_eq.rs | 93 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 swadm/tests/tx_eq.rs diff --git a/swadm/tests/tx_eq.rs b/swadm/tests/tx_eq.rs new file mode 100644 index 00000000..9467a2f0 --- /dev/null +++ b/swadm/tests/tx_eq.rs @@ -0,0 +1,93 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +//! This module tests CLI commands that CRUD +//! tx equalization settings on a link. + +mod cmd; + +use anyhow::Context; +use serial_test::serial; + +use crate::cmd::re::PARENS; + +/// Sets some but not all tx eq taps on a port. +/// Unspecified taps default to zero. +// +// Aside: I would prefer different semantics, but +// docs/scripts in other repos expect this behavior. +#[test] +#[serial] +#[ignore] +fn set_partial_taps() -> anyhow::Result<()> { + let port = "rear0"; + let link = "rear0/0"; + + self::create_100g_link(port, link).context("link setup failed")?; + + cmd::swadm(format!("link serdes set txeq {link} --main=-20"))?; + + let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; + for (label, val) in + [("pre2", 0), ("pre1", 0), ("main", -20), ("post1", 0), ("post2", 0)] + { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) +} + +/// Sets tx eq when all taps on a link are declared. +#[test] +#[serial] +#[ignore] +fn set_all_taps() -> anyhow::Result<()> { + let port = "rear0"; + let link = "rear0/0"; + + self::create_100g_link(port, link).context("link setup failed")?; + + cmd::swadm(format!( + "link serdes set tx-eq {link} --pre2=-1 --pre1 0 --main 10 --post1=5 --post2 2" + ))?; + + let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; + for (label, val) in + [("pre2", -1), ("pre1", 0), ("main", 10), ("post1", 5), ("post2", 2)] + { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) +} + +/// Creates the new link and runs a few validations on it. +fn create_100g_link(port: &str, link: &str) -> anyhow::Result<()> { + cmd::swadm("link ls")? + .expect_line(pat!["Port/Link", "Media"]) + .context("Is swadm usable right now?")?; + + let delete_cmd = format!("link del {link}"); + if let Err(e) = cmd::swadm(&delete_cmd) { + println!( + "Delete failed. This can occur when the link doesn't exist: {e:?}" + ); + } + + cmd::swadm(format!("link create {port} -s 100g --fec rs"))? + .expect_line(pat![format!("Created link {link}")])?; + + cmd::swadm(format!("link enable {link}"))?; + + cmd::swadm(format!("link get {link} -v"))? + .expect_line(pat!["Speed", "100G"])?; + + Ok(()) +} From 8637dd0ee8a63a025643b205e65cb00a3e780d6f Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 06:17:39 +0000 Subject: [PATCH 05/10] swadm link apply tests --- swadm/src/link.rs | 2 +- swadm/tests/link_apply.rs | 95 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 swadm/tests/link_apply.rs diff --git a/swadm/src/link.rs b/swadm/src/link.rs index 47e7e34a..66d9c610 100644 --- a/swadm/src/link.rs +++ b/swadm/src/link.rs @@ -436,7 +436,7 @@ pub enum Link { /// Whether the link is configured to autonegotiate with its peer during /// link training. /// - /// This is generally only true for backplane links, and defaults to + /// This is generally only true for backplane links and defaults to false. #[clap(long)] autoneg: bool, /// Whether the link is configured in KR mode, an electrical specification diff --git a/swadm/tests/link_apply.rs b/swadm/tests/link_apply.rs new file mode 100644 index 00000000..b1b781ea --- /dev/null +++ b/swadm/tests/link_apply.rs @@ -0,0 +1,95 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +mod cmd; + +use serial_test::serial; + +use crate::cmd::re::PARENS; + +const LINK: &str = "rear0/0"; + +/// Tests the `tx-eq` flag in link settings apply. Verifies that +/// every tap of every lane on the resulting 100g link has +/// the same value. +#[test] +#[serial] +#[ignore] +fn apply_tx_eq_all() -> anyhow::Result<()> { + let val = -1; + + cmd::swadm(format!( + "link apply + --link {LINK} + --tag test + --fec rs + --speed 100g + --lane 0 + --tx-eq={val}" + ))?; + + let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; + for label in ["pre2", "pre1", "main", "post1", "post2"] { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) +} + +/// Tests the individual tx eq flags in link settings apply. +/// If one or more taps is explicitly declared, the other taps +/// should default to zero. +#[test] +#[serial] +#[ignore] +fn apply_tx_eq_custom() -> anyhow::Result<()> { + cmd::swadm(format!( + "link apply + --link {LINK} + --tag test + --fec rs + --speed 100g + --lane 0 + --main=-22 + --post1 5" + ))?; + + let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; + for (label, val) in + [("pre2", 0), ("pre1", 0), ("main", -22), ("post1", 5), ("post2", 0)] + { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) +} + +/// Verifies that the `tx-eq` shorthand and explicit +/// tap flags are mutually exclusive. +#[test] +fn tx_eq_exclusive() -> anyhow::Result<()> { + let out: cmd::Output = cmd::swadm(format!( + "link apply + --link {LINK} + --tag test + --fec rs + --speed 100g + --lane 0 + --main=-22 + --post1 5 + --tx-eq 1" + )) + .expect_err("Flags are exclusive") + .try_into()?; + + out.expect_line(pat!["Usage"])?; + + Ok(()) +} From 2cb00a308f5dec94bff0254b60ca2edc927cee75 Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 06:18:16 +0000 Subject: [PATCH 06/10] Add a README to swadm --- swadm/README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 swadm/README.md diff --git a/swadm/README.md b/swadm/README.md new file mode 100644 index 00000000..df0ce963 --- /dev/null +++ b/swadm/README.md @@ -0,0 +1,17 @@ +# SW(itch) ADM(in) + +This is the management CLI for Oxide's rack switch. + +## Testing + +swadm is widely used across scripts and documentation. Changes +should generally be backward compatible. + +This is definitionally a string-typed interface, so regressions are +easily missed. The integration tests module has infra for testing +commands, and adding a test before making swadm changes might +help prevent drift. + +These tests are run in Linux CI but currently ignored in Illumos CI. +Illumos CI is blocked by tofino simulator support: https://github.com/oxidecomputer/tofino-sde/issues/21 + From f68da4b96929c6d6fcd45ad8368b56e33e7ffc39 Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 13:52:13 +0000 Subject: [PATCH 07/10] Migrate counters test to use new infra --- swadm/tests/counters.rs | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/swadm/tests/counters.rs b/swadm/tests/counters.rs index 80d4341a..6306e63e 100644 --- a/swadm/tests/counters.rs +++ b/swadm/tests/counters.rs @@ -6,35 +6,18 @@ //! Integration test for swadm P4 counter functionality. -use serial_test::serial; -use std::process::Command; - -// Path to `swadm` executable. -const SWADM: &str = env!("CARGO_BIN_EXE_swadm"); +mod cmd; -fn swadm() -> Command { - Command::new(SWADM) -} +use serial_test::serial; #[test] #[ignore] #[serial] -fn test_p4_counter_list() { - let output = swadm() - .arg("--host") - .arg("[::1]") - .arg("counters") - .arg("list") - .output() +fn counters_list() { + let output = cmd::swadm("counters list") .expect("Failed to execute swadm counters list"); - assert!( - output.status.success(), - "swadm counters list failed with stderr: {}", - String::from_utf8_lossy(&output.stderr) - ); - - let stdout = String::from_utf8_lossy(&output.stdout); + let stdout = output.as_ref(); // Verify output is not empty and contains expected counter information assert!(!stdout.is_empty(), "Counter list output should not be empty"); From ba60a4a06a6337d435ebf6a2f7850c041f3605f0 Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 14:32:32 +0000 Subject: [PATCH 08/10] Move swadm cli tests into a single binary Otherwise we build the same cmd library for every test, which is prone to erroneous dead code warnings. --- swadm/tests/{cmd/mod.rs => cli/cmd.rs} | 0 swadm/tests/{ => cli}/counters.rs | 4 ++-- swadm/tests/{ => cli}/link_apply.rs | 4 ++-- swadm/tests/cli/main.rs | 10 ++++++++++ swadm/tests/{ => cli}/tx_eq.rs | 4 ++-- 5 files changed, 16 insertions(+), 6 deletions(-) rename swadm/tests/{cmd/mod.rs => cli/cmd.rs} (100%) rename swadm/tests/{ => cli}/counters.rs (98%) rename swadm/tests/{ => cli}/link_apply.rs (98%) create mode 100644 swadm/tests/cli/main.rs rename swadm/tests/{ => cli}/tx_eq.rs (98%) diff --git a/swadm/tests/cmd/mod.rs b/swadm/tests/cli/cmd.rs similarity index 100% rename from swadm/tests/cmd/mod.rs rename to swadm/tests/cli/cmd.rs diff --git a/swadm/tests/counters.rs b/swadm/tests/cli/counters.rs similarity index 98% rename from swadm/tests/counters.rs rename to swadm/tests/cli/counters.rs index 6306e63e..c03a72e3 100644 --- a/swadm/tests/counters.rs +++ b/swadm/tests/cli/counters.rs @@ -6,10 +6,10 @@ //! Integration test for swadm P4 counter functionality. -mod cmd; - use serial_test::serial; +use crate::cmd; + #[test] #[ignore] #[serial] diff --git a/swadm/tests/link_apply.rs b/swadm/tests/cli/link_apply.rs similarity index 98% rename from swadm/tests/link_apply.rs rename to swadm/tests/cli/link_apply.rs index b1b781ea..86e2511b 100644 --- a/swadm/tests/link_apply.rs +++ b/swadm/tests/cli/link_apply.rs @@ -4,11 +4,11 @@ // // Copyright 2026 Oxide Computer Company -mod cmd; - use serial_test::serial; +use crate::cmd; use crate::cmd::re::PARENS; +use crate::pat; const LINK: &str = "rear0/0"; diff --git a/swadm/tests/cli/main.rs b/swadm/tests/cli/main.rs new file mode 100644 index 00000000..722a07a6 --- /dev/null +++ b/swadm/tests/cli/main.rs @@ -0,0 +1,10 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +mod cmd; +mod counters; +mod link_apply; +mod tx_eq; diff --git a/swadm/tests/tx_eq.rs b/swadm/tests/cli/tx_eq.rs similarity index 98% rename from swadm/tests/tx_eq.rs rename to swadm/tests/cli/tx_eq.rs index 9467a2f0..45cdb8ee 100644 --- a/swadm/tests/tx_eq.rs +++ b/swadm/tests/cli/tx_eq.rs @@ -7,12 +7,12 @@ //! This module tests CLI commands that CRUD //! tx equalization settings on a link. -mod cmd; - use anyhow::Context; use serial_test::serial; +use crate::cmd; use crate::cmd::re::PARENS; +use crate::pat; /// Sets some but not all tx eq taps on a port. /// Unspecified taps default to zero. From af50ab3554157782495490b3aeeaecbc782ffad3 Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 17:09:35 +0000 Subject: [PATCH 09/10] Move swadm tests below dpd-client tests in CI swadm tests now mutate switch state, so they shouldn't precede dpd-client. A cleaner design might put swadm tests in their own job, but that's overkill until the suite is more expansive. --- .github/buildomat/packet-test-common.sh | 30 +++++++++++++------------ 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/.github/buildomat/packet-test-common.sh b/.github/buildomat/packet-test-common.sh index 0f638f91..33f4b0fa 100755 --- a/.github/buildomat/packet-test-common.sh +++ b/.github/buildomat/packet-test-common.sh @@ -97,20 +97,6 @@ banner "Links" ./target/debug/swadm --host '[::1]' link ls || echo "failed to list links" -banner "swadm Checks" - -pushd swadm - -DENDRITE_TEST_HOST='[::1]' \ - DENDRITE_TEST_VERBOSITY=3 \ - cargo test \ - --no-fail-fast \ - $SWADM_FEATURES \ - -- \ - --ignored - -popd - banner "Packet Tests" set +o errexit @@ -130,3 +116,19 @@ DENDRITE_TEST_HOST='[::1]' \ -- \ --ignored \ --skip succeeds_when_table_fragmented + +popd + +banner "swadm checks" + +pushd swadm + +DENDRITE_TEST_HOST='[::1]' \ + DENDRITE_TEST_VERBOSITY=3 \ + cargo test \ + --no-fail-fast \ + $SWADM_FEATURES \ + -- \ + --ignored + +popd From 6e43a0c9170b075bd247a891c2afa259560ef34d Mon Sep 17 00:00:00 2001 From: Cory Zimmerman Date: Sat, 22 Aug 2026 18:56:26 +0000 Subject: [PATCH 10/10] Add repeat timer to swadm tests Some writes through DPD's reconciler propagate asynchronously, which makes write-then-read tests prone to race conditions (if unlikely). This adds a repeat timer in sensitive reads to avoid flakiness. --- swadm/tests/cli/cmd.rs | 36 +++++++++++++++++++++ swadm/tests/cli/link_apply.rs | 40 ++++++++++++++--------- swadm/tests/cli/tx_eq.rs | 61 +++++++++++++++++++++-------------- 3 files changed, 97 insertions(+), 40 deletions(-) diff --git a/swadm/tests/cli/cmd.rs b/swadm/tests/cli/cmd.rs index 36277c13..48ea366f 100644 --- a/swadm/tests/cli/cmd.rs +++ b/swadm/tests/cli/cmd.rs @@ -14,6 +14,8 @@ use std::borrow::Cow; use std::process::Command; +use std::time::Duration; +use std::time::Instant; use anyhow::Context; use anyhow::bail; @@ -71,6 +73,40 @@ pub fn swadm(input: impl AsRef) -> Result { Ok(Output::Stdout(std::str::from_utf8(&output.stdout)?.into())) } +/// Executes the closure a reasonable number of times with a +/// reasonable linear backoff until it returns `Ok` or times out. +/// +/// Returns the most recent result if timeout is reached. +/// +/// The closure is not interrupted if execution exceeds timeout. +/// +/// This is useful for the read part of write-then-read tests, +/// where a reconciler may need some time to converge. +pub fn retry(mut f: impl FnMut() -> Result) -> Result { + // Modify these or make them configurable if a tested command + // ever requires longer than `TIMEOUT` to converge. This just + // avoids requiring more args if nobody cares. + const TIMEOUT: Duration = Duration::from_secs(500); + const SLEEP: Duration = Duration::from_millis(100); + + let timeout = Instant::now() + TIMEOUT; + let mut status = f(); + + while Instant::now() < timeout { + if status.is_ok() { + break; + } + + // Thread sleep in any test is suspicious. The intention here + // is only to smooth out variance in convergence time. + std::thread::sleep(SLEEP); + + status = f(); + } + + status +} + /// Common regex patterns for searching swadm output. pub mod re { use super::Pattern; diff --git a/swadm/tests/cli/link_apply.rs b/swadm/tests/cli/link_apply.rs index 86e2511b..a724ad24 100644 --- a/swadm/tests/cli/link_apply.rs +++ b/swadm/tests/cli/link_apply.rs @@ -31,14 +31,16 @@ fn apply_tx_eq_all() -> anyhow::Result<()> { --tx-eq={val}" ))?; - let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; - for label in ["pre2", "pre1", "main", "post1", "post2"] { - tx_eq.expect_line(pat![ - label, val, PARENS, val, PARENS, val, PARENS, val, PARENS - ])?; - } + cmd::retry(|| { + let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; + for label in ["pre2", "pre1", "main", "post1", "post2"] { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } - Ok(()) + Ok(()) + }) } /// Tests the individual tx eq flags in link settings apply. @@ -59,16 +61,22 @@ fn apply_tx_eq_custom() -> anyhow::Result<()> { --post1 5" ))?; - let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; - for (label, val) in - [("pre2", 0), ("pre1", 0), ("main", -22), ("post1", 5), ("post2", 0)] - { - tx_eq.expect_line(pat![ - label, val, PARENS, val, PARENS, val, PARENS, val, PARENS - ])?; - } + cmd::retry(|| { + let tx_eq = cmd::swadm(format!("link serdes get txeq {LINK}"))?; + for (label, val) in [ + ("pre2", 0), + ("pre1", 0), + ("main", -22), + ("post1", 5), + ("post2", 0), + ] { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } - Ok(()) + Ok(()) + }) } /// Verifies that the `tx-eq` shorthand and explicit diff --git a/swadm/tests/cli/tx_eq.rs b/swadm/tests/cli/tx_eq.rs index 45cdb8ee..b328c705 100644 --- a/swadm/tests/cli/tx_eq.rs +++ b/swadm/tests/cli/tx_eq.rs @@ -30,16 +30,22 @@ fn set_partial_taps() -> anyhow::Result<()> { cmd::swadm(format!("link serdes set txeq {link} --main=-20"))?; - let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; - for (label, val) in - [("pre2", 0), ("pre1", 0), ("main", -20), ("post1", 0), ("post2", 0)] - { - tx_eq.expect_line(pat![ - label, val, PARENS, val, PARENS, val, PARENS, val, PARENS - ])?; - } - - Ok(()) + cmd::retry(|| { + let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; + for (label, val) in [ + ("pre2", 0), + ("pre1", 0), + ("main", -20), + ("post1", 0), + ("post2", 0), + ] { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) + }) } /// Sets tx eq when all taps on a link are declared. @@ -56,16 +62,22 @@ fn set_all_taps() -> anyhow::Result<()> { "link serdes set tx-eq {link} --pre2=-1 --pre1 0 --main 10 --post1=5 --post2 2" ))?; - let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; - for (label, val) in - [("pre2", -1), ("pre1", 0), ("main", 10), ("post1", 5), ("post2", 2)] - { - tx_eq.expect_line(pat![ - label, val, PARENS, val, PARENS, val, PARENS, val, PARENS - ])?; - } - - Ok(()) + cmd::retry(|| { + let tx_eq = cmd::swadm(format!("link serdes get txeq {link}"))?; + for (label, val) in [ + ("pre2", -1), + ("pre1", 0), + ("main", 10), + ("post1", 5), + ("post2", 2), + ] { + tx_eq.expect_line(pat![ + label, val, PARENS, val, PARENS, val, PARENS, val, PARENS + ])?; + } + + Ok(()) + }) } /// Creates the new link and runs a few validations on it. @@ -86,8 +98,9 @@ fn create_100g_link(port: &str, link: &str) -> anyhow::Result<()> { cmd::swadm(format!("link enable {link}"))?; - cmd::swadm(format!("link get {link} -v"))? - .expect_line(pat!["Speed", "100G"])?; - - Ok(()) + cmd::retry(|| { + cmd::swadm(format!("link get {link} -v"))? + .expect_line(pat!["Speed", "100G"])?; + Ok(()) + }) }