Standardize on mod test

Keep a consistent style of `mod test`, rather than a mix of that and `mod tests`. Enforce this new rule with a rudimentary check, added as `xtask style`.
oxidecomputer · May 7, 2024 · 6d4fe85 · 6d4fe85
1 parent 0d8efa1
commit 6d4fe85
Show file tree

Hide file tree

Showing 21 changed files with 168 additions and 70 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -21,6 +21,8 @@ jobs:
       run: cargo fmt -- --version
     - name: Check style
       run: cargo fmt -- --check
+    - name: Check misc. style
+      run: cargo xtask style
   check-clippy:
     runs-on: ubuntu-latest
     steps:

diff --git a/bin/propolis-cli/src/main.rs b/bin/propolis-cli/src/main.rs
@@ -360,57 +360,6 @@ async fn stdin_to_websockets_task(
     }
 }
 
-#[tokio::test]
-async fn test_stdin_to_websockets_task() {
-    use tokio::sync::mpsc::error::TryRecvError;
-
-    let (stdintx, stdinrx) = tokio::sync::mpsc::channel(16);
-    let (wstx, mut wsrx) = tokio::sync::mpsc::channel(16);
-
-    tokio::spawn(async move { stdin_to_websockets_task(stdinrx, wstx).await });
-
-    // send characters, receive characters
-    stdintx
-        .send("test post please ignore".chars().map(|c| c as u8).collect())
-        .await
-        .unwrap();
-    let actual = wsrx.recv().await.unwrap();
-    assert_eq!(String::from_utf8(actual).unwrap(), "test post please ignore");
-
-    // don't send ctrl-a
-    stdintx.send("\x01".chars().map(|c| c as u8).collect()).await.unwrap();
-    assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
-
-    // the "t" here is sent "raw" because of last ctrl-a but that doesn't change anything
-    stdintx.send("test".chars().map(|c| c as u8).collect()).await.unwrap();
-    let actual = wsrx.recv().await.unwrap();
-    assert_eq!(String::from_utf8(actual).unwrap(), "test");
-
-    // ctrl-a ctrl-c = only ctrl-c sent
-    stdintx.send("\x01\x03".chars().map(|c| c as u8).collect()).await.unwrap();
-    let actual = wsrx.recv().await.unwrap();
-    assert_eq!(String::from_utf8(actual).unwrap(), "\x03");
-
-    // same as above, across two messages
-    stdintx.send("\x01".chars().map(|c| c as u8).collect()).await.unwrap();
-    stdintx.send("\x03".chars().map(|c| c as u8).collect()).await.unwrap();
-    assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
-    let actual = wsrx.recv().await.unwrap();
-    assert_eq!(String::from_utf8(actual).unwrap(), "\x03");
-
-    // ctrl-a ctrl-a = only ctrl-a sent
-    stdintx.send("\x01\x01".chars().map(|c| c as u8).collect()).await.unwrap();
-    let actual = wsrx.recv().await.unwrap();
-    assert_eq!(String::from_utf8(actual).unwrap(), "\x01");
-
-    // ctrl-c on its own means exit
-    stdintx.send("\x03".chars().map(|c| c as u8).collect()).await.unwrap();
-    assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
-
-    // channel is closed
-    assert!(wsrx.recv().await.is_none());
-}
-
 async fn serial(
     addr: SocketAddr,
     byte_offset: Option<i64>,
@@ -747,3 +696,70 @@ impl Drop for RawTermiosGuard {
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::stdin_to_websockets_task;
+
+    #[tokio::test]
+    async fn test_stdin_to_websockets_task() {
+        use tokio::sync::mpsc::error::TryRecvError;
+
+        let (stdintx, stdinrx) = tokio::sync::mpsc::channel(16);
+        let (wstx, mut wsrx) = tokio::sync::mpsc::channel(16);
+
+        tokio::spawn(
+            async move { stdin_to_websockets_task(stdinrx, wstx).await },
+        );
+
+        // send characters, receive characters
+        stdintx
+            .send("test post please ignore".chars().map(|c| c as u8).collect())
+            .await
+            .unwrap();
+        let actual = wsrx.recv().await.unwrap();
+        assert_eq!(
+            String::from_utf8(actual).unwrap(),
+            "test post please ignore"
+        );
+
+        // don't send ctrl-a
+        stdintx.send("\x01".chars().map(|c| c as u8).collect()).await.unwrap();
+        assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
+
+        // the "t" here is sent "raw" because of last ctrl-a but that doesn't change anything
+        stdintx.send("test".chars().map(|c| c as u8).collect()).await.unwrap();
+        let actual = wsrx.recv().await.unwrap();
+        assert_eq!(String::from_utf8(actual).unwrap(), "test");
+
+        // ctrl-a ctrl-c = only ctrl-c sent
+        stdintx
+            .send("\x01\x03".chars().map(|c| c as u8).collect())
+            .await
+            .unwrap();
+        let actual = wsrx.recv().await.unwrap();
+        assert_eq!(String::from_utf8(actual).unwrap(), "\x03");
+
+        // same as above, across two messages
+        stdintx.send("\x01".chars().map(|c| c as u8).collect()).await.unwrap();
+        stdintx.send("\x03".chars().map(|c| c as u8).collect()).await.unwrap();
+        assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
+        let actual = wsrx.recv().await.unwrap();
+        assert_eq!(String::from_utf8(actual).unwrap(), "\x03");
+
+        // ctrl-a ctrl-a = only ctrl-a sent
+        stdintx
+            .send("\x01\x01".chars().map(|c| c as u8).collect())
+            .await
+            .unwrap();
+        let actual = wsrx.recv().await.unwrap();
+        assert_eq!(String::from_utf8(actual).unwrap(), "\x01");
+
+        // ctrl-c on its own means exit
+        stdintx.send("\x03".chars().map(|c| c as u8).collect()).await.unwrap();
+        assert_eq!(wsrx.try_recv(), Err(TryRecvError::Empty));
+
+        // channel is closed
+        assert!(wsrx.recv().await.is_none());
+    }
+}
diff --git a/bin/propolis-server/src/lib/migrate/protocol.rs b/bin/propolis-server/src/lib/migrate/protocol.rs
@@ -256,7 +256,7 @@ pub(super) fn select_protocol_from_offer(
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     // N.B. The test protocol lists are sorted by version to meet the

diff --git a/bin/propolis-server/src/lib/serial/history_buffer.rs b/bin/propolis-server/src/lib/serial/history_buffer.rs
@@ -218,7 +218,7 @@ impl HistoryBuffer {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
     use SerialHistoryOffset::*;
 

diff --git a/bin/propolis-server/src/lib/server.rs b/bin/propolis-server/src/lib/server.rs
@@ -1147,7 +1147,7 @@ fn not_created_error() -> HttpError {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     #[test]
     fn test_propolis_server_openapi() {
         let mut buf: Vec<u8> = vec![];

diff --git a/bin/propolis-server/src/lib/stats/virtual_machine.rs b/bin/propolis-server/src/lib/stats/virtual_machine.rs
@@ -395,7 +395,7 @@ fn produce_vcpu_usage<'a>(
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::kstat_instance_from_instance_id;
     use super::kstat_microstate_to_state_name;
     use super::produce_vcpu_usage;

diff --git a/bin/propolis-server/src/lib/vm/request_queue.rs b/bin/propolis-server/src/lib/vm/request_queue.rs
@@ -444,7 +444,7 @@ impl ExternalRequestQueue {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     use uuid::Uuid;

diff --git a/bin/propolis-server/src/lib/vm/state_driver.rs b/bin/propolis-server/src/lib/vm/state_driver.rs
@@ -567,7 +567,7 @@ where
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use anyhow::bail;
     use mockall::Sequence;
 

diff --git a/crates/propolis-server-config/src/lib.rs b/crates/propolis-server-config/src/lib.rs
@@ -143,7 +143,7 @@ pub fn parse<P: AsRef<Path>>(path: P) -> Result<Config, ParseError> {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     #[test]

diff --git a/lib/propolis-client/src/support.rs b/lib/propolis-client/src/support.rs
@@ -449,7 +449,7 @@ fn _assert_impls() {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::InstanceSerialConsoleControlMessage;
     use super::InstanceSerialConsoleHelper;
     use super::Role;

diff --git a/lib/propolis/src/chardev/pollers.rs b/lib/propolis/src/chardev/pollers.rs
@@ -555,7 +555,7 @@ impl Params {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
     use crate::chardev::*;
 

diff --git a/lib/propolis/src/common.rs b/lib/propolis/src/common.rs
@@ -388,7 +388,7 @@ pub const GB: usize = 1024 * 1024 * 1024;
 pub const TB: usize = 1024 * 1024 * 1024 * 1024;
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     #[test]

diff --git a/lib/propolis/src/firmware/smbios/table.rs b/lib/propolis/src/firmware/smbios/table.rs
@@ -416,7 +416,7 @@ pub mod type1 {
     }
 
     #[cfg(test)]
-    mod tests {
+    mod test {
         use super::*;
 
         enum_serde_roundtrip_tests! {
@@ -607,7 +607,7 @@ pub mod type4 {
     }
 
     #[cfg(test)]
-    mod tests {
+    mod test {
         use super::*;
 
         enum_serde_roundtrip_tests! {
@@ -759,7 +759,7 @@ pub mod type16 {
     }
 
     #[cfg(test)]
-    mod tests {
+    mod test {
         use super::*;
 
         enum_serde_roundtrip_tests! {

diff --git a/lib/propolis/src/hw/nvme/queue.rs b/lib/propolis/src/hw/nvme/queue.rs
@@ -877,7 +877,7 @@ pub(super) mod migrate {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use rand::Rng;
 
     use super::*;

diff --git a/lib/propolis/src/hw/pci/bar.rs b/lib/propolis/src/hw/pci/bar.rs
@@ -314,7 +314,7 @@ pub mod migrate {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     fn setup() -> Bars {

diff --git a/lib/propolis/src/hw/uart/uart16550.rs b/lib/propolis/src/hw/uart/uart16550.rs
@@ -635,7 +635,7 @@ impl UartReg {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
 
     mod bits {
         #![allow(unused)]

diff --git a/lib/propolis/src/util/aspace.rs b/lib/propolis/src/util/aspace.rs
@@ -280,7 +280,7 @@ impl<'a, T> Iterator for Range<'a, T> {
 }
 
 #[cfg(test)]
-mod tests {
+mod test {
     use super::*;
 
     #[test]

diff --git a/lib/propolis/src/vmm/mem.rs b/lib/propolis/src/vmm/mem.rs
@@ -1039,7 +1039,7 @@ impl<'a, T: Copy> Iterator for MemMany<'a, T> {
 }
 
 #[cfg(test)]
-pub mod tests {
+pub mod test {
     use super::*;
 
     const TEST_LEN: usize = 16 * 1024;

diff --git a/xtask/src/main.rs b/xtask/src/main.rs
@@ -10,6 +10,7 @@ mod task_fmt;
 mod task_license;
 mod task_phd;
 mod task_prepush;
+mod task_style;
 mod util;
 
 #[derive(Parser)]
@@ -43,6 +44,8 @@ enum Cmds {
         #[clap(subcommand)]
         cmd: task_phd::Cmd,
     },
+    /// Perform misc style checks
+    Style,
 }
 
 fn main() -> Result<()> {
@@ -64,5 +67,11 @@ fn main() -> Result<()> {
             println!("Pre-push checks pass");
             Ok(())
         }
+        Cmds::Style => {
+            task_style::cmd_style()?;
+
+            println!("Style checks pass");
+            Ok(())
+        }
     }
 }
diff --git a/xtask/src/task_prepush.rs b/xtask/src/task_prepush.rs
@@ -4,7 +4,7 @@
 
 use anyhow::{bail, Result};
 
-use crate::{task_clippy, task_fmt, task_license};
+use crate::{task_clippy, task_fmt, task_license, task_style};
 
 pub(crate) fn cmd_prepush() -> Result<()> {
     let mut errs = Vec::new();
@@ -17,6 +17,9 @@ pub(crate) fn cmd_prepush() -> Result<()> {
     if task_license::cmd_license().is_err() {
         errs.push("license");
     }
+    if task_style::cmd_style().is_err() {
+        errs.push("style");
+    }
 
     if !errs.is_empty() {
         bail!("Pre-push error(s) in: {}", errs.join(", "))