refactor: use matches! instead of to_string() for tests

Some tests that were explicitly testing for error conditions used
converted errors to strings to determine whether two errors are the same
(by saying they're only the same if their string representation was
identical). Replace this with more roboust assertions on `matches!`.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

src/mmap.rs

@@ -384,7 +384,7 @@ mod tests {
 
     use crate::bitmap::tests::test_guest_memory_and_region;
     use crate::bitmap::AtomicBitmap;
-    use crate::{Error, GuestAddressSpace, GuestMemory};
+    use crate::{Error, GuestAddressSpace, GuestMemory, GuestMemoryError};
 
     use std::io::Write;
     use std::mem;
@@ -481,129 +481,66 @@ mod tests {
     fn test_no_memory_region() {
         let regions_summary = [];
 
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap(&regions_summary).err().unwrap()
-            ),
-            format!("{:?}", Error::NoMemoryRegion)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_with_files(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::NoMemoryRegion)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::NoMemoryRegion)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_arc_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::NoMemoryRegion)
-        );
+        assert!(matches!(
+            new_guest_memory_mmap(&regions_summary).unwrap_err(),
+            Error::NoMemoryRegion
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
+            Error::NoMemoryRegion
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::NoMemoryRegion
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::NoMemoryRegion
+        ));
     }
 
     #[test]
     fn test_overlapping_memory_regions() {
         let regions_summary = [(GuestAddress(0), 100_usize), (GuestAddress(99), 100_usize)];
 
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap(&regions_summary).err().unwrap()
-            ),
-            format!("{:?}", Error::MemoryRegionOverlap)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_with_files(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::MemoryRegionOverlap)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::MemoryRegionOverlap)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_arc_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::MemoryRegionOverlap)
-        );
+        assert!(matches!(
+            new_guest_memory_mmap(&regions_summary).unwrap_err(),
+            Error::MemoryRegionOverlap
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
+            Error::MemoryRegionOverlap
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::MemoryRegionOverlap
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::MemoryRegionOverlap
+        ));
     }
 
     #[test]
     fn test_unsorted_memory_regions() {
         let regions_summary = [(GuestAddress(100), 100_usize), (GuestAddress(0), 100_usize)];
 
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap(&regions_summary).err().unwrap()
-            ),
-            format!("{:?}", Error::UnsortedMemoryRegions)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_with_files(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::UnsortedMemoryRegions)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::UnsortedMemoryRegions)
-        );
-
-        assert_eq!(
-            format!(
-                "{:?}",
-                new_guest_memory_mmap_from_arc_regions(&regions_summary)
-                    .err()
-                    .unwrap()
-            ),
-            format!("{:?}", Error::UnsortedMemoryRegions)
-        );
+        assert!(matches!(
+            new_guest_memory_mmap(&regions_summary).unwrap_err(),
+            Error::UnsortedMemoryRegions
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
+            Error::UnsortedMemoryRegions
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::UnsortedMemoryRegions
+        ));
+        assert!(matches!(
+            new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
+            Error::UnsortedMemoryRegions
+        ));
     }
 
     #[test]
@@ -828,18 +765,13 @@ mod tests {
         for gm in gm_list.iter() {
             let val1: u64 = 0xaa55_aa55_aa55_aa55;
             let val2: u64 = 0x55aa_55aa_55aa_55aa;
-            assert_eq!(
-                format!("{:?}", gm.write_obj(val1, bad_addr).err().unwrap()),
-                format!("InvalidGuestAddress({:?})", bad_addr,)
-            );
-            assert_eq!(
-                format!("{:?}", gm.write_obj(val1, bad_addr2).err().unwrap()),
-                format!(
-                    "PartialBuffer {{ expected: {:?}, completed: {:?} }}",
-                    mem::size_of::<u64>(),
-                    max_addr.checked_offset_from(bad_addr2).unwrap()
-                )
-            );
+            assert!(matches!(
+                gm.write_obj(val1, bad_addr).unwrap_err(),
+                GuestMemoryError::InvalidGuestAddress(addr) if addr == bad_addr
+            ));
+            assert!(matches!(
+                gm.write_obj(val1, bad_addr2).unwrap_err(),
+                GuestMemoryError::PartialBuffer { expected, completed} if expected == size_of::<u64>() && completed == max_addr.checked_offset_from(bad_addr2).unwrap() as usize));
 
             gm.write_obj(val1, GuestAddress(0x500)).unwrap();
             gm.write_obj(val2, GuestAddress(0x1000 + 32)).unwrap();

src/mmap_unix.rs

@@ -558,7 +558,7 @@ mod tests {
             prot,
             flags,
         );
-        assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
+        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
 
         // Offset + size is greater than the size of the file (which is 0 at this point).
         let r = MmapRegion::build(
@@ -567,7 +567,7 @@ mod tests {
             prot,
             flags,
         );
-        assert_eq!(format!("{:?}", r.unwrap_err()), "MappingPastEof");
+        assert!(matches!(r.unwrap_err(), Error::MappingPastEof));
 
         // MAP_FIXED was specified among the flags.
         let r = MmapRegion::build(
@@ -576,7 +576,7 @@ mod tests {
             prot,
             flags | libc::MAP_FIXED,
         );
-        assert_eq!(format!("{:?}", r.unwrap_err()), "MapFixed");
+        assert!(matches!(r.unwrap_err(), Error::MapFixed));
 
         // Let's resize the file.
         assert_eq!(unsafe { libc::ftruncate(a.as_raw_fd(), 1024 * 10) }, 0);
@@ -621,7 +621,7 @@ mod tests {
         let flags = libc::MAP_NORESERVE | libc::MAP_PRIVATE;
 
         let r = unsafe { MmapRegion::build_raw((addr + 1) as *mut u8, size, prot, flags) };
-        assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidPointer");
+        assert!(matches!(r.unwrap_err(), Error::InvalidPointer));
 
         let r = unsafe { MmapRegion::build_raw(addr as *mut u8, size, prot, flags).unwrap() };
 

src/mmap_xen.rs

@@ -1077,26 +1077,18 @@ mod tests {
         range.mmap_flags = 16;
 
         let r = MmapXen::new(&range);
-        assert_eq!(
-            format!("{:?}", r.unwrap_err()),
-            format!("MmapFlags({})", range.mmap_flags),
-        );
+        assert!(matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == range.mmap_flags));
 
         range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::GRANT.bits();
         let r = MmapXen::new(&range);
-        assert_eq!(
-            format!("{:?}", r.unwrap_err()),
-            format!("MmapFlags({:x})", MmapXenFlags::ALL.bits()),
+        assert!(
+            matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == MmapXenFlags::ALL.bits())
         );
 
         range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::NO_ADVANCE_MAP.bits();
         let r = MmapXen::new(&range);
-        assert_eq!(
-            format!("{:?}", r.unwrap_err()),
-            format!(
-                "MmapFlags({:x})",
-                MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits(),
-            ),
+        assert!(
+            matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags ==  MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits())
         );
     }
 
@@ -1132,17 +1124,17 @@ mod tests {
         range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 0));
         range.prot = None;
         let r = MmapXenForeign::new(&range);
-        assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
+        assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
 
         let mut range = MmapRange::initialized(true);
         range.flags = None;
         let r = MmapXenForeign::new(&range);
-        assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
+        assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
 
         let mut range = MmapRange::initialized(true);
         range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
         let r = MmapXenForeign::new(&range);
-        assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
+        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
 
         let mut range = MmapRange::initialized(true);
         range.size = 0;
@@ -1164,7 +1156,7 @@ mod tests {
         let mut range = MmapRange::initialized(true);
         range.prot = None;
         let r = MmapXenGrant::new(&range, MmapXenFlags::empty());
-        assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
+        assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
 
         let mut range = MmapRange::initialized(true);
         range.prot = None;
@@ -1174,12 +1166,12 @@ mod tests {
         let mut range = MmapRange::initialized(true);
         range.flags = None;
         let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP);
-        assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
+        assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
 
         let mut range = MmapRange::initialized(true);
         range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
         let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP);
-        assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
+        assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
 
         let mut range = MmapRange::initialized(true);
         range.size = 0;

src/volatile_memory.rs

@@ -1488,58 +1488,6 @@ mod tests {
         slice.compute_end_offset(6, 0).unwrap_err();
     }
 
-    #[test]
-    fn test_display_error() {
-        assert_eq!(
-            format!("{}", Error::OutOfBounds { addr: 0x10 }),
-            "address 0x10 is out of bounds"
-        );
-
-        assert_eq!(
-            format!(
-                "{}",
-                Error::Overflow {
-                    base: 0x0,
-                    offset: 0x10
-                }
-            ),
-            "address 0x0 offset by 0x10 would overflow"
-        );
-
-        assert_eq!(
-            format!(
-                "{}",
-                Error::TooBig {
-                    nelements: 100_000,
-                    size: 1_000_000_000
-                }
-            ),
-            "100000 elements of size 1000000000 would overflow a usize"
-        );
-
-        assert_eq!(
-            format!(
-                "{}",
-                Error::Misaligned {
-                    addr: 0x4,
-                    alignment: 8
-                }
-            ),
-            "address 0x4 is not aligned to 8"
-        );
-
-        assert_eq!(
-            format!(
-                "{}",
-                Error::PartialBuffer {
-                    expected: 100,
-                    completed: 90
-                }
-            ),
-            "only used 90 bytes in 100 long buffer"
-        );
-    }
-
     #[test]
     fn misaligned_ref() {
         let mut a = [0u8; 3];