Skip to content

Commit fb42f50

Browse files
roypatroypat
committed
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 <roypat@amazon.co.uk>
1 parent 037f13d commit fb42f50

File tree

4 files changed

+69
-197
lines changed

4 files changed

+69
-197
lines changed

‎src/mmap/mod.rs‎

Lines changed: 56 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ mod tests {
355355

356356
use crate::bitmap::tests::test_guest_memory_and_region;
357357
use crate::bitmap::AtomicBitmap;
358-
use crate::{Error, GuestAddressSpace, GuestMemory};
358+
use crate::{Error, GuestAddressSpace, GuestMemory, GuestMemoryError};
359359

360360
use std::io::Write;
361361
use std::mem;
@@ -452,129 +452,66 @@ mod tests {
452452
fn test_no_memory_region() {
453453
let regions_summary = [];
454454

455-
assert_eq!(
456-
format!(
457-
"{:?}",
458-
new_guest_memory_mmap(&regions_summary).err().unwrap()
459-
),
460-
format!("{:?}", Error::NoMemoryRegion)
461-
);
462-
463-
assert_eq!(
464-
format!(
465-
"{:?}",
466-
new_guest_memory_mmap_with_files(&regions_summary)
467-
.err()
468-
.unwrap()
469-
),
470-
format!("{:?}", Error::NoMemoryRegion)
471-
);
472-
473-
assert_eq!(
474-
format!(
475-
"{:?}",
476-
new_guest_memory_mmap_from_regions(&regions_summary)
477-
.err()
478-
.unwrap()
479-
),
480-
format!("{:?}", Error::NoMemoryRegion)
481-
);
482-
483-
assert_eq!(
484-
format!(
485-
"{:?}",
486-
new_guest_memory_mmap_from_arc_regions(&regions_summary)
487-
.err()
488-
.unwrap()
489-
),
490-
format!("{:?}", Error::NoMemoryRegion)
491-
);
455+
assert!(matches!(
456+
new_guest_memory_mmap(&regions_summary).unwrap_err(),
457+
Error::NoMemoryRegion
458+
));
459+
assert!(matches!(
460+
new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
461+
Error::NoMemoryRegion
462+
));
463+
assert!(matches!(
464+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
465+
Error::NoMemoryRegion
466+
));
467+
assert!(matches!(
468+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
469+
Error::NoMemoryRegion
470+
));
492471
}
493472

494473
#[test]
495474
fn test_overlapping_memory_regions() {
496475
let regions_summary = [(GuestAddress(0), 100_usize), (GuestAddress(99), 100_usize)];
497476

498-
assert_eq!(
499-
format!(
500-
"{:?}",
501-
new_guest_memory_mmap(&regions_summary).err().unwrap()
502-
),
503-
format!("{:?}", Error::MemoryRegionOverlap)
504-
);
505-
506-
assert_eq!(
507-
format!(
508-
"{:?}",
509-
new_guest_memory_mmap_with_files(&regions_summary)
510-
.err()
511-
.unwrap()
512-
),
513-
format!("{:?}", Error::MemoryRegionOverlap)
514-
);
515-
516-
assert_eq!(
517-
format!(
518-
"{:?}",
519-
new_guest_memory_mmap_from_regions(&regions_summary)
520-
.err()
521-
.unwrap()
522-
),
523-
format!("{:?}", Error::MemoryRegionOverlap)
524-
);
525-
526-
assert_eq!(
527-
format!(
528-
"{:?}",
529-
new_guest_memory_mmap_from_arc_regions(&regions_summary)
530-
.err()
531-
.unwrap()
532-
),
533-
format!("{:?}", Error::MemoryRegionOverlap)
534-
);
477+
assert!(matches!(
478+
new_guest_memory_mmap(&regions_summary).unwrap_err(),
479+
Error::MemoryRegionOverlap
480+
));
481+
assert!(matches!(
482+
new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
483+
Error::MemoryRegionOverlap
484+
));
485+
assert!(matches!(
486+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
487+
Error::MemoryRegionOverlap
488+
));
489+
assert!(matches!(
490+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
491+
Error::MemoryRegionOverlap
492+
));
535493
}
536494

537495
#[test]
538496
fn test_unsorted_memory_regions() {
539497
let regions_summary = [(GuestAddress(100), 100_usize), (GuestAddress(0), 100_usize)];
540498

541-
assert_eq!(
542-
format!(
543-
"{:?}",
544-
new_guest_memory_mmap(&regions_summary).err().unwrap()
545-
),
546-
format!("{:?}", Error::UnsortedMemoryRegions)
547-
);
548-
549-
assert_eq!(
550-
format!(
551-
"{:?}",
552-
new_guest_memory_mmap_with_files(&regions_summary)
553-
.err()
554-
.unwrap()
555-
),
556-
format!("{:?}", Error::UnsortedMemoryRegions)
557-
);
558-
559-
assert_eq!(
560-
format!(
561-
"{:?}",
562-
new_guest_memory_mmap_from_regions(&regions_summary)
563-
.err()
564-
.unwrap()
565-
),
566-
format!("{:?}", Error::UnsortedMemoryRegions)
567-
);
568-
569-
assert_eq!(
570-
format!(
571-
"{:?}",
572-
new_guest_memory_mmap_from_arc_regions(&regions_summary)
573-
.err()
574-
.unwrap()
575-
),
576-
format!("{:?}", Error::UnsortedMemoryRegions)
577-
);
499+
assert!(matches!(
500+
new_guest_memory_mmap(&regions_summary).unwrap_err(),
501+
Error::UnsortedMemoryRegions
502+
));
503+
assert!(matches!(
504+
new_guest_memory_mmap_with_files(&regions_summary).unwrap_err(),
505+
Error::UnsortedMemoryRegions
506+
));
507+
assert!(matches!(
508+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
509+
Error::UnsortedMemoryRegions
510+
));
511+
assert!(matches!(
512+
new_guest_memory_mmap_from_regions(&regions_summary).unwrap_err(),
513+
Error::UnsortedMemoryRegions
514+
));
578515
}
579516

580517
#[test]
@@ -799,18 +736,13 @@ mod tests {
799736
for gm in gm_list.iter() {
800737
let val1: u64 = 0xaa55_aa55_aa55_aa55;
801738
let val2: u64 = 0x55aa_55aa_55aa_55aa;
802-
assert_eq!(
803-
format!("{:?}", gm.write_obj(val1, bad_addr).err().unwrap()),
804-
format!("InvalidGuestAddress({:?})", bad_addr,)
805-
);
806-
assert_eq!(
807-
format!("{:?}", gm.write_obj(val1, bad_addr2).err().unwrap()),
808-
format!(
809-
"PartialBuffer {{ expected: {:?}, completed: {:?} }}",
810-
mem::size_of::<u64>(),
811-
max_addr.checked_offset_from(bad_addr2).unwrap()
812-
)
813-
);
739+
assert!(matches!(
740+
gm.write_obj(val1, bad_addr).unwrap_err(),
741+
GuestMemoryError::InvalidGuestAddress(addr) if addr == bad_addr
742+
));
743+
assert!(matches!(
744+
gm.write_obj(val1, bad_addr2).unwrap_err(),
745+
GuestMemoryError::PartialBuffer { expected, completed} if expected == size_of::<u64>() && completed == max_addr.checked_offset_from(bad_addr2).unwrap() as usize));
814746

815747
gm.write_obj(val1, GuestAddress(0x500)).unwrap();
816748
gm.write_obj(val2, GuestAddress(0x1000 + 32)).unwrap();

‎src/mmap/unix.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ mod tests {
561561
prot,
562562
flags | libc::MAP_FIXED,
563563
);
564-
assert_eq!(format!("{:?}", r.unwrap_err()), "MapFixed");
564+
assert!(matches!(r.unwrap_err(), Error::MapFixed));
565565

566566
// Let's resize the file.
567567
assert_eq!(unsafe { libc::ftruncate(a.as_raw_fd(), 1024 * 10) }, 0);
@@ -606,7 +606,7 @@ mod tests {
606606
let flags = libc::MAP_NORESERVE | libc::MAP_PRIVATE;
607607

608608
let r = unsafe { MmapRegion::build_raw((addr + 1) as *mut u8, size, prot, flags) };
609-
assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidPointer");
609+
assert!(matches!(r.unwrap_err(), Error::InvalidPointer));
610610

611611
let r = unsafe { MmapRegion::build_raw(addr as *mut u8, size, prot, flags).unwrap() };
612612

‎src/mmap/xen.rs‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,26 +1069,18 @@ mod tests {
10691069
range.mmap_flags = 16;
10701070

10711071
let r = MmapXen::new(&range);
1072-
assert_eq!(
1073-
format!("{:?}", r.unwrap_err()),
1074-
format!("MmapFlags({})", range.mmap_flags),
1075-
);
1072+
assert!(matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == range.mmap_flags));
10761073

10771074
range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::GRANT.bits();
10781075
let r = MmapXen::new(&range);
1079-
assert_eq!(
1080-
format!("{:?}", r.unwrap_err()),
1081-
format!("MmapFlags({:x})", MmapXenFlags::ALL.bits()),
1076+
assert!(
1077+
matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == MmapXenFlags::ALL.bits())
10821078
);
10831079

10841080
range.mmap_flags = MmapXenFlags::FOREIGN.bits() | MmapXenFlags::NO_ADVANCE_MAP.bits();
10851081
let r = MmapXen::new(&range);
1086-
assert_eq!(
1087-
format!("{:?}", r.unwrap_err()),
1088-
format!(
1089-
"MmapFlags({:x})",
1090-
MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits(),
1091-
),
1082+
assert!(
1083+
matches!(r.unwrap_err(), Error::MmapFlags(flags) if flags == MmapXenFlags::NO_ADVANCE_MAP.bits() | MmapXenFlags::FOREIGN.bits())
10921084
);
10931085
}
10941086

@@ -1124,17 +1116,17 @@ mod tests {
11241116
range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 0));
11251117
range.prot = None;
11261118
let r = MmapXenForeign::new(&range);
1127-
assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
1119+
assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
11281120

11291121
let mut range = MmapRange::initialized(true);
11301122
range.flags = None;
11311123
let r = MmapXenForeign::new(&range);
1132-
assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
1124+
assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
11331125

11341126
let mut range = MmapRange::initialized(true);
11351127
range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
11361128
let r = MmapXenForeign::new(&range);
1137-
assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
1129+
assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
11381130

11391131
let mut range = MmapRange::initialized(true);
11401132
range.size = 0;
@@ -1156,7 +1148,7 @@ mod tests {
11561148
let mut range = MmapRange::initialized(true);
11571149
range.prot = None;
11581150
let r = MmapXenGrant::new(&range, MmapXenFlags::empty());
1159-
assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
1151+
assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
11601152

11611153
let mut range = MmapRange::initialized(true);
11621154
range.prot = None;
@@ -1166,12 +1158,12 @@ mod tests {
11661158
let mut range = MmapRange::initialized(true);
11671159
range.flags = None;
11681160
let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP);
1169-
assert_eq!(format!("{:?}", r.unwrap_err()), "UnexpectedError");
1161+
assert!(matches!(r.unwrap_err(), Error::UnexpectedError));
11701162

11711163
let mut range = MmapRange::initialized(true);
11721164
range.file_offset = Some(FileOffset::new(TempFile::new().unwrap().into_file(), 1));
11731165
let r = MmapXenGrant::new(&range, MmapXenFlags::NO_ADVANCE_MAP);
1174-
assert_eq!(format!("{:?}", r.unwrap_err()), "InvalidOffsetLength");
1166+
assert!(matches!(r.unwrap_err(), Error::InvalidOffsetLength));
11751167

11761168
let mut range = MmapRange::initialized(true);
11771169
range.size = 0;

‎src/volatile_memory.rs‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,58 +1488,6 @@ mod tests {
14881488
slice.compute_end_offset(6, 0).unwrap_err();
14891489
}
14901490

1491-
#[test]
1492-
fn test_display_error() {
1493-
assert_eq!(
1494-
format!("{}", Error::OutOfBounds { addr: 0x10 }),
1495-
"address 0x10 is out of bounds"
1496-
);
1497-
1498-
assert_eq!(
1499-
format!(
1500-
"{}",
1501-
Error::Overflow {
1502-
base: 0x0,
1503-
offset: 0x10
1504-
}
1505-
),
1506-
"address 0x0 offset by 0x10 would overflow"
1507-
);
1508-
1509-
assert_eq!(
1510-
format!(
1511-
"{}",
1512-
Error::TooBig {
1513-
nelements: 100_000,
1514-
size: 1_000_000_000
1515-
}
1516-
),
1517-
"100000 elements of size 1000000000 would overflow a usize"
1518-
);
1519-
1520-
assert_eq!(
1521-
format!(
1522-
"{}",
1523-
Error::Misaligned {
1524-
addr: 0x4,
1525-
alignment: 8
1526-
}
1527-
),
1528-
"address 0x4 is not aligned to 8"
1529-
);
1530-
1531-
assert_eq!(
1532-
format!(
1533-
"{}",
1534-
Error::PartialBuffer {
1535-
expected: 100,
1536-
completed: 90
1537-
}
1538-
),
1539-
"only used 90 bytes in 100 long buffer"
1540-
);
1541-
}
1542-
15431491
#[test]
15441492
fn misaligned_ref() {
15451493
let mut a = [0u8; 3];

0 commit comments

Comments
 (0)