[i2c] update existing unit tests

Updated the existing unit tests to map to the new implemented
abstraction.

In the process of updating the tests, a few other worth mentioning
changes are implemented:
- when initializing a resource needed for the test, it is a best
practice to keep any updates that change the test result in the same
function. This was not followed as the requests vector was initialized
in the test, and was cleared in a function that was asserting results.
This is an unexpected behavior, and is now removed. The requests are
always initialized in the test before using them (and clear is no longer
used).
- `assert_results` is deleted because it was hard to read which results
where asserted. It is also not necessary to mock functions in that way
because now we can mock only the parts we need from the device
implementation.

The tests are incomplete though because we also need to check the error
returned. This should be implemented in a future commit to keep things
separated.

Signed-off-by: Andreea Florescu <fandree@amazon.com>

src/i2c/src/i2c.rs

@@ -546,67 +546,60 @@ impl<D: I2cDevice> I2cMap<D> {
 #[cfg(test)]
 pub mod tests {
     use super::*;
+    use std::convert::TryFrom;
 
-    pub struct I2cMockAdapter {
-        bus: u32,
-        smbus: bool,
-        result: Result<()>,
+    #[derive(Debug, Default)]
+    pub struct DummyDevice {
+        funcs_result: i32,
+        rdwr_result: i32,
+        smbus_result: i32,
+        slave_result: i32,
     }
 
-    /*
-    impl I2cAdapterTrait for I2cMockAdapter {
-        fn new(bus: u32) -> Result<I2cMockAdapter> {
-            Ok(I2cMockAdapter {
-                bus,
-                smbus: false,
-                result: Ok(()),
-            })
+    impl I2cDevice for DummyDevice {
+        fn open(_device_path: String) -> Result<Self>
+        where
+            Self: Sized,
+        {
+            Ok(DummyDevice::default())
         }
 
-        fn adapter_no(&self) -> u32 {
-            self.bus
+        fn funcs(&mut self, _func: u64) -> i32 {
+            self.funcs_result
         }
 
-        fn is_smbus(&self) -> bool {
-            self.smbus
+        fn rdwr(&self, _data: &I2cRdwrIoctlData) -> i32 {
+            self.rdwr_result
         }
 
-        fn set_device_addr(&self, _addr: usize) -> Result<()> {
-            Ok(())
+        fn smbus(&self, _data: &I2cSmbusIoctlData) -> i32 {
+            self.smbus_result
         }
 
-        fn transfer(&self, reqs: &mut [I2cReq]) -> Result<()> {
-            self.result
+        fn slave(&self, _addr: u64) -> i32 {
+            self.slave_result
         }
     }
 
-    fn assert_results(
-        i2c_map: &mut I2cMap<I2cMockAdapter>,
-        reqs: &mut Vec<I2cReq>,
-        before: bool,
-        after: bool,
-    ) {
-        i2c_map.adapters[0].result = Ok(());
-        assert_eq!(i2c_map.transfer(reqs).is_err(), before);
-        i2c_map.adapters[0].result = Err(Error::new(EINVAL));
-        assert_eq!(i2c_map.transfer(reqs).is_err(), after);
-
-        reqs.clear();
-    }
-
     #[test]
     fn test_i2c_map_duplicate_device4() {
-        assert!(I2cMap::<I2cMockAdapter>::new("1:4,2:32:21,5:4:23").is_err());
+        assert!(AdapterConfig::try_from("1:4,2:32:21,5:4:23").is_err());
+    }
+
+    #[test]
+    fn test_duplicated_adapter_no() {
+        assert!(AdapterConfig::try_from("1:4,1:32:21,5:10:23").is_err());
     }
 
     #[test]
     fn test_i2c_map() {
-        let i2c_map: I2cMap<I2cMockAdapter> = I2cMap::new("1:4,2:32:21,5:10:23").unwrap();
+        let adapter_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();
+        let i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
 
         assert_eq!(i2c_map.adapters.len(), 3);
-        assert_eq!(i2c_map.adapters[0].bus, 1);
-        assert_eq!(i2c_map.adapters[1].bus, 2);
-        assert_eq!(i2c_map.adapters[2].bus, 5);
+        assert_eq!(i2c_map.adapters[0].adapter_no, 1);
+        assert_eq!(i2c_map.adapters[1].adapter_no, 2);
+        assert_eq!(i2c_map.adapters[2].adapter_no, 5);
 
         assert_eq!(i2c_map.device_map[4], 0);
         assert_eq!(i2c_map.device_map[32], 1);
@@ -617,22 +610,26 @@ pub mod tests {
 
     #[test]
     fn test_i2c_transfer() {
-        let mut i2c_map: I2cMap<I2cMockAdapter> = I2cMap::new("1:3").unwrap();
+        let adapter_config = AdapterConfig::try_from("1:3").unwrap();
+        let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
+
         i2c_map.adapters[0].smbus = false;
 
         let mut reqs: Vec<I2cReq> = vec![I2cReq {
             addr: 0x3,
             flags: 0,
             len: 2,
-            buf: [7, 4].to_vec(),
+            buf: vec![7, 4],
         }];
 
-        assert_results(&mut i2c_map, &mut reqs, false, true);
+        i2c_map.transfer(&mut *reqs).unwrap();
     }
 
     #[test]
     fn test_smbus_transfer() {
-        let mut i2c_map: I2cMap<I2cMockAdapter> = I2cMap::new("1:3").unwrap();
+        let adapter_config = AdapterConfig::try_from("1:3").unwrap();
+        let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
+
         i2c_map.adapters[0].smbus = true;
 
         let mut reqs: Vec<I2cReq> = vec![I2cReq {
@@ -643,28 +640,30 @@ pub mod tests {
         }];
 
         // I2C_SMBUS_WRITE (I2C_SMBUS_BYTE_DATA) operation
-        assert_results(&mut i2c_map, &mut reqs, false, true);
+        i2c_map.transfer(&mut reqs).unwrap();
 
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) operation
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 0,
-            len: 1,
-            buf: [34].to_vec(),
-        });
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 1,
-            len: 2,
-            buf: [3, 4].to_vec(),
-        });
-
-        assert_results(&mut i2c_map, &mut reqs, false, true);
+        let mut reqs = vec![
+            I2cReq {
+                addr: 0x3,
+                flags: 0,
+                len: 1,
+                buf: [34].to_vec(),
+            },
+            I2cReq {
+                addr: 0x3,
+                flags: 1,
+                len: 2,
+                buf: [3, 4].to_vec(),
+            },
+        ];
+        i2c_map.transfer(&mut reqs).unwrap();
     }
 
     #[test]
     fn test_smbus_transfer_failure() {
-        let mut i2c_map: I2cMap<I2cMockAdapter> = I2cMap::new("1:3").unwrap();
+        let adapter_config = AdapterConfig::try_from("1:3").unwrap();
+        let mut i2c_map: I2cMap<DummyDevice> = I2cMap::new(&adapter_config).unwrap();
         i2c_map.adapters[0].smbus = true;
 
         let mut reqs: Vec<I2cReq> = vec![
@@ -684,57 +683,63 @@ pub mod tests {
         ];
 
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) failure operation
-        assert_results(&mut i2c_map, &mut reqs, true, true);
+        // TODO: check the actual error once we have an error type defined.
+        // TODO-continued: otherwise this test is unreliable because it might
+        // fail for another reason than the expected one.
+        assert!(i2c_map.transfer(&mut reqs).is_err());
 
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) failure operation
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 0,
-            len: 1,
-            buf: [34].to_vec(),
-        });
-        reqs.push(I2cReq {
-            addr: 0x3,
-            // Will cause failure
-            flags: 0,
-            len: 2,
-            buf: [3, 4].to_vec(),
-        });
-
-        assert_results(&mut i2c_map, &mut reqs, true, true);
+        let mut reqs = vec![
+            I2cReq {
+                addr: 0x3,
+                flags: 0,
+                len: 1,
+                buf: [34].to_vec(),
+            },
+            I2cReq {
+                addr: 0x3,
+                // Will cause failure
+                flags: 0,
+                len: 2,
+                buf: [3, 4].to_vec(),
+            },
+        ];
+        assert!(i2c_map.transfer(&mut reqs).is_err());
 
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) failure operation
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 0,
-            // Will cause failure
-            len: 2,
-            buf: [3, 4].to_vec(),
-        });
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 1,
-            len: 2,
-            buf: [3, 4].to_vec(),
-        });
-
-        assert_results(&mut i2c_map, &mut reqs, true, true);
+        let mut reqs = vec![
+            I2cReq {
+                addr: 0x3,
+                flags: 0,
+                // Will cause failure
+                len: 2,
+                buf: [3, 4].to_vec(),
+            },
+            I2cReq {
+                addr: 0x3,
+                flags: 1,
+                len: 2,
+                buf: [3, 4].to_vec(),
+            },
+        ];
+        assert!(i2c_map.transfer(&mut reqs).is_err());
 
         // I2C_SMBUS_READ (I2C_SMBUS_WORD_DATA) failure operation
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 0,
-            len: 1,
-            buf: [34].to_vec(),
-        });
-        reqs.push(I2cReq {
-            addr: 0x3,
-            flags: 1,
-            // Will cause failure
-            len: 3,
-            buf: [3, 4, 5].to_vec(),
-        });
-
-        assert_results(&mut i2c_map, &mut reqs, true, true);
-    }*/
+        let mut reqs = vec![
+            I2cReq {
+                addr: 0x3,
+                flags: 0,
+                len: 1,
+                buf: [34].to_vec(),
+            },
+            I2cReq {
+                addr: 0x3,
+                flags: 1,
+                // Will cause failure
+                len: 3,
+                buf: [3, 4, 5].to_vec(),
+            },
+        ];
+        assert!(i2c_map.transfer(&mut reqs).is_err());
+    }
 }

src/i2c/src/main.rs

@@ -162,7 +162,7 @@ fn main() -> Result<(), String> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use i2c::tests::I2cMockAdapter;
+    use i2c::tests::DummyDevice;
 
     fn get_cmd_args(name: &str, devices: &str, count: u32) -> ArgMatches {
         let yaml = load_yaml!("cli.yaml");
@@ -191,29 +191,28 @@ mod tests {
     ) -> bool {
         true
     }
-    /*
+
     #[test]
     fn test_backend_single() {
         let cmd_args = get_cmd_args("vi2c.sock_single", "1:4,2:32:21,5:5:23", 0);
-        assert!(start_backend::<I2cMockAdapter>(cmd_args, mock_start_daemon).is_ok());
+        assert!(start_backend::<DummyDevice>(cmd_args, mock_start_daemon).is_ok());
     }
 
     #[test]
     fn test_backend_multiple() {
         let cmd_args = get_cmd_args("vi2c.sock", "1:4,2:32:21,5:5:23", 5);
-        assert!(start_backend::<I2cMockAdapter>(cmd_args, mock_start_daemon).is_ok());
+        assert!(start_backend::<DummyDevice>(cmd_args, mock_start_daemon).is_ok());
     }
 
     #[test]
     fn test_backend_failure() {
         let cmd_args = get_cmd_args("vi2c.sock_failure", "1:4d", 5);
-        assert!(start_backend::<I2cMockAdapter>(cmd_args, mock_start_daemon).is_err());
+        assert!(start_backend::<DummyDevice>(cmd_args, mock_start_daemon).is_err());
     }
 
     #[test]
     fn test_backend_failure_duplicate_device4() {
         let cmd_args = get_cmd_args("vi2c.sock_duplicate", "1:4,2:32:21,5:4:23", 5);
-        assert!(start_backend::<I2cMockAdapter>(cmd_args, mock_start_daemon).is_err());
+        assert!(start_backend::<DummyDevice>(cmd_args, mock_start_daemon).is_err());
     }
-     */
 }

src/i2c/src/vhu_i2c.rs

@@ -305,12 +305,13 @@ impl<D: 'static + I2cDevice + Sync + Send> VhostUserBackendMut<VringRwLock, ()>
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::i2c::tests::I2cMockAdapter;
+    use crate::i2c::tests::DummyDevice;
+    use std::convert::TryFrom;
 
-    /*
     #[test]
     fn verify_backend() {
-        let i2c_map: I2cMap<I2cMockAdapter> = I2cMap::new("1:4,2:32:21,5:10:23").unwrap();
+        let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();
+        let i2c_map: I2cMap<DummyDevice> = I2cMap::new(&device_config).unwrap();
         let mut backend = VhostUserI2cBackend::new(Arc::new(i2c_map)).unwrap();
 
         assert_eq!(backend.num_queues(), NUM_QUEUES);
@@ -323,5 +324,5 @@ mod tests {
 
         backend.set_event_idx(true);
         assert!(backend.event_idx);
-    }*/
+    }
 }