Skip to content

Commit

Permalink
misc: Eliminate use of assert!((...).is_ok())
Browse files Browse the repository at this point in the history 
Asserting on .is_ok()/.is_err() leads to hard to debug failures (as if
the test fails, it will only say "assertion failed: false". We replace
these with `.unwrap()`, which also prints the exact error variant that
was unexpectedly encountered (we can to this these days thanks to
efforts to implement Display and Debug for our error types). If the
assert!((...).is_ok()) was followed by an .unwrap() anyway, we just drop
the assert.

Inspired by and quoted from @roypat.

Signed-off-by: Ruoqing He <[email protected]>
  • Loading branch information
@RuoqingHe @rbradford
RuoqingHe authored and rbradford committed Oct 3, 2024
1 parent 83bcf2a commit 297236a
Show file tree
Hide file tree
Showing 25 changed files with 244 additions and 281 deletions.
8 changes: 4 additions & 4 deletions arch/src/x86_64/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,7 @@ mod tests {
None,
None,
);
assert!(config_err.is_err());
config_err.unwrap_err();

// Now assigning some memory that falls before the 32bit memory hole.
let arch_mem_regions = arch_memory_regions();
Expand Down Expand Up @@ -1686,13 +1686,13 @@ mod tests {
// Exercise the scenario where the field storing the length of the e820 entry table is
// is bigger than the allocated memory.
params.e820_entries = params.e820_table.len() as u8 + 1;
assert!(add_e820_entry(
add_e820_entry(
&mut params,
e820_table[0].addr,
e820_table[0].size,
e820_table[0].type_
e820_table[0].type_,
)
.is_err());
.unwrap_err();
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions arch/src/x86_64/mptable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ mod tests {
let mem = GuestMemoryMmap::from_ranges(&[(MPTABLE_START, compute_mp_size(num_cpus) - 1)])
.unwrap();

assert!(setup_mptable(MPTABLE_START, &mem, num_cpus, None).is_err());
setup_mptable(MPTABLE_START, &mem, num_cpus, None).unwrap_err();
}

#[test]
Expand Down Expand Up @@ -433,6 +433,6 @@ mod tests {
GuestMemoryMmap::from_ranges(&[(MPTABLE_START, compute_mp_size(cpus as u8))]).unwrap();

let result = setup_mptable(MPTABLE_START, &mem, cpus as u8, None);
assert!(result.is_err());
result.unwrap_err();
}
}
4 changes: 1 addition & 3 deletions block/src/qcow/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2051,8 +2051,7 @@ mod tests {
.expect("Failed to write header to shm.");
disk_file.rewind().unwrap();
// The maximum nesting depth is 0, which means backing file is not allowed.
let res = QcowFile::from_with_nesting_depth(disk_file, 0);
assert!(res.is_ok());
QcowFile::from_with_nesting_depth(disk_file, 0).unwrap();
}

#[test]
Expand All @@ -2068,7 +2067,6 @@ mod tests {
disk_file.rewind().unwrap();
// The maximum nesting depth is 0, which means backing file is not allowed.
let res = QcowFile::from_with_nesting_depth(disk_file, 0);
assert!(res.is_err());
assert!(matches!(res.unwrap_err(), Error::MaxNestingDepthExceeded));
}

Expand Down
4 changes: 2 additions & 2 deletions devices/src/legacy/serial.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ mod tests {

// write 1 to the interrupt event fd, so that read doesn't block in case the event fd
// counter doesn't change (for 0 it blocks)
assert!(intr_evt.write(1).is_ok());
intr_evt.write(1).unwrap();
serial.write(0, IER as u64, &[IER_RECV_BIT]);
serial.queue_input_bytes(b"abc").unwrap();

Expand Down Expand Up @@ -471,7 +471,7 @@ mod tests {

// write 1 to the interrupt event fd, so that read doesn't block in case the event fd
// counter doesn't change (for 0 it blocks)
assert!(intr_evt.write(1).is_ok());
intr_evt.write(1).unwrap();
serial.write(0, IER as u64, &[IER_THR_BIT]);
serial.write(0, DATA as u64, b"a");

Expand Down
2 changes: 1 addition & 1 deletion devices/src/legacy/uart_pl011.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ mod tests {

// write 1 to the interrupt event fd, so that read doesn't block in case the event fd
// counter doesn't change (for 0 it blocks)
assert!(intr_evt.write(1).is_ok());
intr_evt.write(1).unwrap();
pl011.queue_input_bytes(b"abc").unwrap();

assert_eq!(intr_evt.read().unwrap(), 2);
Expand Down
12 changes: 6 additions & 6 deletions hypervisor/src/arch/x86/emulator/instructions/cmp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x38, 0xc4]; // cmp ah,al
let mut vmm = MockVmm::new(ip, vec![(Register::RAX, rax)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(cpu_id).unwrap().flags() & FLAGS_MASK;
assert_eq!(0b1000100, rflags);
Expand All @@ -234,7 +234,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x83, 0xf8, 0x64]; // cmp eax,100
let mut vmm = MockVmm::new(ip, vec![(Register::RAX, rax)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(cpu_id).unwrap().flags() & FLAGS_MASK;
assert_eq!(0b100, rflags);
Expand All @@ -248,7 +248,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x83, 0xf8, 0xff]; // cmp eax,-1
let mut vmm = MockVmm::new(ip, vec![(Register::RAX, rax)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(cpu_id).unwrap().flags() & FLAGS_MASK;
assert_eq!(0b101, rflags);
Expand All @@ -263,7 +263,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x48, 0x39, 0xd8, 0x00, 0xc3]; // cmp rax,rbx + two bytes garbage
let mut vmm = MockVmm::new(ip, vec![(Register::RAX, rax), (Register::RBX, rbx)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(cpu_id).unwrap().flags() & FLAGS_MASK;
assert_eq!(0b100, rflags);
Expand All @@ -290,7 +290,7 @@ mod tests {
vec![(Register::RAX, rax), (Register::RBX, rbx)],
None,
);
assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(0).unwrap().flags() & FLAGS_MASK;
assert_eq!(d.2, rflags);
Expand Down Expand Up @@ -318,7 +318,7 @@ mod tests {
vec![(Register::RAX, rax), (Register::RBX, rbx)],
None,
);
assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

let rflags: u64 = vmm.cpu_state(0).unwrap().flags() & FLAGS_MASK;
assert_eq!(d.2, rflags);
Expand Down
38 changes: 19 additions & 19 deletions hypervisor/src/arch/x86/emulator/instructions/mov.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x48, 0x89, 0xd8];
let mut vmm = MockVmm::new(ip, vec![(Register::RBX, rbx)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rax: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -298,7 +298,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x48, 0xb8, 0x44, 0x33, 0x22, 0x11, 0x44, 0x33, 0x22, 0x11];
let mut vmm = MockVmm::new(ip, vec![], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rax: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -318,7 +318,7 @@ mod tests {
let memory: [u8; 8] = target_rax.to_le_bytes();
let insn = [0x48, 0x8b, 0x04, 0x00];
let mut vmm = MockVmm::new(ip, vec![(Register::RAX, rax)], Some((rax + rax, &memory)));
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

rax = vmm
.cpu_state(cpu_id)
Expand All @@ -336,7 +336,7 @@ mod tests {
let cpu_id = 0;
let insn = [0xb0, 0x11];
let mut vmm = MockVmm::new(ip, vec![], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let al = vmm
.cpu_state(cpu_id)
Expand All @@ -354,7 +354,7 @@ mod tests {
let cpu_id = 0;
let insn = [0xb8, 0x11, 0x00, 0x00, 0x00];
let mut vmm = MockVmm::new(ip, vec![], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let eax = vmm
.cpu_state(cpu_id)
Expand All @@ -372,7 +372,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x48, 0xc7, 0xc0, 0x44, 0x33, 0x22, 0x11];
let mut vmm = MockVmm::new(ip, vec![], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let rax: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -395,7 +395,7 @@ mod tests {
vec![(Register::RAX, rax), (Register::DH, dh.into())],
None,
);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let mut memory: [u8; 1] = [0; 1];
vmm.read_memory(rax, &mut memory).unwrap();
Expand All @@ -416,7 +416,7 @@ mod tests {
vec![(Register::RAX, rax), (Register::ESI, esi.into())],
None,
);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let mut memory: [u8; 4] = [0; 4];
vmm.read_memory(rax, &mut memory).unwrap();
Expand All @@ -438,7 +438,7 @@ mod tests {
vec![(Register::RAX, rax), (Register::EDI, edi.into())],
None,
);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let mut memory: [u8; 4] = [0; 4];
vmm.read_memory(rax + displacement, &mut memory).unwrap();
Expand All @@ -461,7 +461,7 @@ mod tests {
vec![(Register::RAX, rax)],
Some((rax + displacement, &memory)),
);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let new_eax = vmm
.cpu_state(cpu_id)
Expand All @@ -486,7 +486,7 @@ mod tests {
vec![(Register::RAX, rax)],
Some((rax + displacement, &memory)),
);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let new_al = vmm
.cpu_state(cpu_id)
Expand All @@ -511,7 +511,7 @@ mod tests {
0x48, 0x8b, 0x58, 0x10, // mov rbx, qword ptr [rax+10h]
];
let mut vmm = MockVmm::new(ip, vec![], Some((rax + displacement, &memory)));
assert!(vmm.emulate_insn(cpu_id, &insn, Some(2)).is_ok());
vmm.emulate_insn(cpu_id, &insn, Some(2)).unwrap();

let rbx: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -538,7 +538,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, vec![], Some((rax + displacement, &memory)));
// Only run the first instruction.
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

assert_eq!(ip + 7, vmm.cpu_state(cpu_id).unwrap().ip());

Expand Down Expand Up @@ -569,7 +569,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, vec![], Some((rax + displacement, &memory)));
// Run the 2 first instructions.
assert!(vmm.emulate_insn(cpu_id, &insn, Some(2)).is_ok());
vmm.emulate_insn(cpu_id, &insn, Some(2)).unwrap();

assert_eq!(ip + 7 + 4, vmm.cpu_state(cpu_id).unwrap().ip());

Expand Down Expand Up @@ -597,7 +597,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x0f, 0xb6, 0xc3];
let mut vmm = MockVmm::new(ip, vec![(Register::BX, bx as u64)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let eax: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -615,7 +615,7 @@ mod tests {
let cpu_id = 0;
let insn = [0x0f, 0xb6, 0xc7];
let mut vmm = MockVmm::new(ip, vec![(Register::BX, bx as u64)], None);
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let eax: u64 = vmm
.cpu_state(cpu_id)
Expand All @@ -635,7 +635,7 @@ mod tests {
let insn = [0x0f, 0xb7, 0x03];
let memory: [u8; 1] = value.to_le_bytes();
let mut vmm = MockVmm::new(ip, vec![(Register::RBX, rbx)], Some((rbx, &memory)));
assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let eax: u64 = vmm
.cpu_state(cpu_id)
Expand Down Expand Up @@ -680,7 +680,7 @@ mod tests {

let memory: [u8; 8] = mem_value.to_le_bytes();
let mut vmm = MockVmm::new(ip, vec![], Some((mem_addr, &memory)));
assert!(vmm.emulate_first_insn(cpu_id, &instruction_bytes).is_ok());
vmm.emulate_first_insn(cpu_id, &instruction_bytes).unwrap();

let ax: u64 = vmm.cpu_state(cpu_id).unwrap().read_reg(register).unwrap();

Expand Down Expand Up @@ -737,7 +737,7 @@ mod tests {
]);

let mut vmm = MockVmm::new(ip, vec![(Register::RAX, ax)], None);
assert!(vmm.emulate_first_insn(cpu_id, &instruction_bytes).is_ok());
vmm.emulate_first_insn(cpu_id, &instruction_bytes).unwrap();

match register {
Register::AX => {
Expand Down
14 changes: 7 additions & 7 deletions hypervisor/src/arch/x86/emulator/instructions/movs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0x10, &mut data).unwrap();
assert_eq!(0xaabbccdd12345678, <u64>::from_le_bytes(data));
Expand Down Expand Up @@ -159,7 +159,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0xc, &mut data).unwrap();
assert_eq!(0x12345678, <u32>::from_le_bytes(data));
Expand All @@ -184,7 +184,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0x8, &mut data).unwrap();
assert_eq!(0x12345678, <u32>::from_le_bytes(data));
Expand Down Expand Up @@ -212,7 +212,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0xc, &mut data).unwrap();
assert_eq!(0x5678, <u16>::from_le_bytes(data));
Expand Down Expand Up @@ -243,7 +243,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0x8, &mut data).unwrap();
assert_eq!(0x5678, <u16>::from_le_bytes(data));
Expand All @@ -269,7 +269,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0x8, &mut data).unwrap();
assert_eq!(0x78, data[0]);
Expand Down Expand Up @@ -299,7 +299,7 @@ mod tests {

let mut vmm = MockVmm::new(ip, regs, Some((0, &memory)));

assert!(vmm.emulate_first_insn(0, &insn).is_ok());
vmm.emulate_first_insn(0, &insn).unwrap();

vmm.read_memory(0x8, &mut data).unwrap();
assert_eq!(0x78, data[0]);
Expand Down
2 changes: 1 addition & 1 deletion hypervisor/src/arch/x86/emulator/instructions/or.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ mod tests {
Some((0, &memory)),
);

assert!(vmm.emulate_first_insn(cpu_id, &insn).is_ok());
vmm.emulate_first_insn(cpu_id, &insn).unwrap();

let mut out: [u8; 1] = [0; 1];

Expand Down
Loading

0 comments on commit 297236a

Please sign in to comment.