Streamline integration tests and add first test for read timeouts

.github/workflows/build.yaml

@@ -51,10 +51,6 @@ env:
   PKG_CONFIG_ALLOW_CROSS: 1
   # Deny warnings.
   RUSTFLAGS: -D warnings
-  # Dummy ports for hardware tests. Keep in sync with other workflows (in
-  # ci.yaml).
-  SERIALPORT_RS_TEST_PORT_1: DUMMY_PORT_1
-  SERIALPORT_RS_TEST_PORT_2: DUMMY_PORT_2
 
 jobs:
   build:

.github/workflows/ci.yaml

@@ -7,11 +7,6 @@ on:
   push:
   workflow_dispatch:
 
-env:
-  # Dummy ports for hardware tests. Keep in sync with workflows in build.yaml.
-  SERIALPORT_RS_TEST_PORT_1: DUMMY_PORT_1
-  SERIALPORT_RS_TEST_PORT_2: DUMMY_PORT_2
-
 jobs:
   # --------------------------------------------------------------------------
   # LINT

Cargo.toml

@@ -45,6 +45,9 @@ serde = { version = "1.0", features = ["derive"], optional = true }
 [dev-dependencies]
 assert_hex = "0.4.1"
 clap = { version = "3.1.6", features = ["derive"] }
+envconfig = "0.10.0"
+rstest = { version = "0.12.0", default-features = false }
+rustversion = "1.0.16"
 
 [features]
 default = ["libudev"]

examples/loopback.rs

@@ -193,6 +193,16 @@
     }
 }
 
+#[rustversion::before(1.63)]
+fn loopback_split<'a>(
+    _port: &mut Box<dyn SerialPort>,
+    _read_stats: &mut Stats<'a>,
+    _write_stats: &mut Stats<'a>,
+) {
+    unimplemented!("requires Rust 1.63 or later");
+}
+
+#[rustversion::since(1.63)]
 fn loopback_split<'a>(
     port: &mut Box<dyn SerialPort>,
     read_stats: &mut Stats<'a>,
@@ -213,12 +223,12 @@
     //                 └──────────────────────────────────┘
     // 2. Write Thread: Write -> Unpark Read -> Park ──────┐
     //                  └──────────────────────────────────┘
     std::thread::scope(|scope| {
         // Get handle for writing thread
         let wr_thread = std::thread::current();
 
         // Spawn a thread that reads data for n iterations
         let handle = scope.spawn(move || {
             for _ in 0..read_stats.iterations {
                 // Wait for the write to complete
                 std::thread::park();
@@ -253,7 +263,7 @@
             write_stats.stop();
 
             // Notify that the write completed
             handle.thread().unpark();
 
             // Wait for read to complete
             std::thread::park();

src/lib.rs

@@ -331,6 +331,8 @@ pub struct SerialPortBuilder {
 
 impl SerialPortBuilder {
     /// Set the path to the serial port
+    // TODO: Switch to `clone_into` when bumping our MSRV past 1.63 and remove this exemption.
+    #[allow(clippy::assigning_clones)]
     #[must_use]
     pub fn path<'a>(mut self, path: impl Into<std::borrow::Cow<'a, str>>) -> Self {
         self.path = path.into().as_ref().to_owned();

tests/config.rs

@@ -0,0 +1,25 @@
+// Configuration for integration tests. This crate is about interacting with real serial ports and
+// so some tests need actual hardware.
+
+use envconfig::Envconfig;
+use rstest::fixture;
+
+// Configuration for tests requiring acutual hardware.
+//
+// For conveniently pulling this configuration into a test case as a parameter, you might want to
+// use the test fixture [`hw_config`].
+#[derive(Clone, Debug, Envconfig, Eq, PartialEq)]
+pub struct HardwareConfig {
+    #[envconfig(from = "SERIALPORT_TEST_PORT_1")]
+    pub port_1: String,
+    #[envconfig(from = "SERIALPORT_TEST_PORT_2")]
+    pub port_2: String,
+}
+
+// Test fixture for conveniently pulling the actual hardware configuration into test cases.
+//
+// See [`fixture`](rstest::fixture) for details.
+#[fixture]
+pub fn hw_config() -> HardwareConfig {
+    HardwareConfig::init_from_env().unwrap()
+}

tests/test_serialport.rs

@@ -1,13 +1,12 @@
 //! Tests for the `SerialPort` trait.
-#![cfg(unix)]
+mod config;
 
+use config::{hw_config, HardwareConfig};
+use rstest::rstest;
 use serialport::*;
 use std::time::Duration;
 
-const PORT_1: &str = env!("SERIALPORT_RS_TEST_PORT_1");
-const PORT_2: &str = env!("SERIALPORT_RS_TEST_PORT_2");
-
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
 fn test_listing_ports() {
     let ports = serialport::available_ports().expect("No ports found!");
@@ -16,32 +15,34 @@ fn test_listing_ports() {
     }
 }
 
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
-fn test_opening_found_ports() {
+fn test_opening_found_ports(hw_config: HardwareConfig) {
     // There is no guarantee that we even might open the ports returned by `available_ports`. But
     // the ports we are using for testing shall be among them.
     let ports = serialport::available_ports().unwrap();
-    assert!(ports.iter().any(|info| info.port_name == PORT_1));
-    assert!(ports.iter().any(|info| info.port_name == PORT_2));
+    assert!(ports.iter().any(|info| info.port_name == hw_config.port_1));
+    assert!(ports.iter().any(|info| info.port_name == hw_config.port_2));
 }
 
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
-fn test_opening_port() {
-    serialport::new(PORT_1, 9600).open().unwrap();
+fn test_opening_port(hw_config: HardwareConfig) {
+    serialport::new(hw_config.port_1, 9600).open().unwrap();
 }
 
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
-fn test_opening_native_port() {
-    serialport::new(PORT_1, 9600).open_native().unwrap();
+fn test_opening_native_port(hw_config: HardwareConfig) {
+    serialport::new(hw_config.port_1, 9600)
+        .open_native()
+        .unwrap();
 }
 
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
-fn test_configuring_ports() {
-    serialport::new(PORT_1, 9600)
+fn test_configuring_ports(hw_config: HardwareConfig) {
+    serialport::new(hw_config.port_1, 9600)
         .data_bits(DataBits::Five)
         .flow_control(FlowControl::None)
         .parity(Parity::None)
@@ -51,17 +52,20 @@ fn test_configuring_ports() {
         .unwrap();
 }
 
-#[test]
+#[rstest]
 #[cfg_attr(feature = "ignore-hardware-tests", ignore)]
-fn test_duplicating_port_config() {
-    let port1_config = serialport::new(PORT_1, 9600)
+fn test_duplicating_port_config(hw_config: HardwareConfig) {
+    let port1_config = serialport::new(hw_config.port_1, 9600)
         .data_bits(DataBits::Five)
         .flow_control(FlowControl::None)
         .parity(Parity::None)
         .stop_bits(StopBits::One)
         .timeout(Duration::from_millis(1));
 
-    let port2_config = port1_config.clone().path(PORT_2).baud_rate(115_200);
+    let port2_config = port1_config
+        .clone()
+        .path(hw_config.port_2)
+        .baud_rate(115_200);
 
     let _port1 = port1_config.open().unwrap();
     let _port1 = port2_config.open().unwrap();

tests/test_timeout.rs

@@ -0,0 +1,98 @@
+mod config;
+
+use config::{hw_config, HardwareConfig};
+use rstest::rstest;
+use serialport::*;
+use std::io::Read;
+use std::time::{Duration, Instant};
+
+#[rstest]
+#[case(b"a")]
+#[case(b"0123456789")]
+#[case(b"0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~")]
+#[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+fn test_timeout_zero(hw_config: HardwareConfig, #[case] message: &[u8]) {
+    let timeout = Duration::ZERO;
+    let margin = Duration::from_millis(100);
+
+    let mut sender = serialport::new(hw_config.port_1, 115200).open().unwrap();
+    let mut receiver = serialport::new(hw_config.port_2, 115200)
+        .timeout(timeout)
+        .open()
+        .unwrap();
+    let mut buffer: [u8; 1024] = [0xff; 1024];
+
+    sender.clear(ClearBuffer::All).unwrap();
+    receiver.clear(ClearBuffer::All).unwrap();
+
+    sender.write_all(message).unwrap();
+    sender.flush().unwrap();
+    let flushed_at = Instant::now();
+
+    let expected_until = flushed_at + timeout + margin;
+    let mut timeouts = 0usize;
+
+    loop {
+        match receiver.read(&mut buffer) {
+            Ok(read) => {
+                assert!(read > 0);
+                println!(
+                    "read: {} bytes of {} after {} timeouts/{} ms",
+                    read,
+                    message.len(),
+                    timeouts,
+                    (Instant::now() - flushed_at).as_millis()
+                );
+                assert_eq!(message[..read], buffer[..read]);
+                break;
+            }
+            Err(e) => {
+                assert_eq!(e.kind(), std::io::ErrorKind::TimedOut);
+                timeouts += 1;
+            }
+        }
+
+        assert!(expected_until > Instant::now());
+    }
+}
+
+#[rstest]
+#[case(Duration::from_millis(10))]
+#[case(Duration::from_millis(100))]
+#[case(Duration::from_millis(1000))]
+#[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+fn test_timeout_greater_zero(hw_config: HardwareConfig, #[case] timeout: Duration) {
+    let margin = Duration::from_millis(100);
+
+    let mut sender = serialport::new(hw_config.port_1, 115200).open().unwrap();
+    let mut receiver = serialport::new(hw_config.port_2, 115200)
+        .timeout(timeout)
+        .open()
+        .unwrap();
+
+    let message =
+        b"0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
+    let mut buffer: [u8; 1024] = [0xff; 1024];
+
+    sender.clear(ClearBuffer::All).unwrap();
+    receiver.clear(ClearBuffer::All).unwrap();
+
+    sender.write_all(message).unwrap();
+    sender.flush().unwrap();
+
+    let flushed_at = Instant::now();
+
+    let read = receiver.read(&mut buffer).unwrap();
+    let read_at = Instant::now();
+
+    println!(
+        "read: {} bytes of {} after {} ms",
+        read,
+        message.len(),
+        (Instant::now() - flushed_at).as_millis()
+    );
+
+    assert!(read > 0);
+    assert!(flushed_at + timeout + margin > read_at);
+    assert_eq!(buffer[..read], message[..read]);
+}