Check specific error codes in the tests

If possible, check specific error codes instead of `is_err()`.  This
makes the code more readable and catches bugs resulting in the wrong
error code.  Also, using the assert_*_err and assert_ok macros yields
error messages containing the expected and the actual value.

To be able to use these macros with the `get_password_safe` method, we
also have to implement `Debug` for `PasswordSafe` and `Device`.

7 files changed, 32 insertions, 51 deletions

diff --git a/TODO.md b/TODO.md
index 53de7e9..487f56d 100644
--- a/TODO.md
+++ b/TODO.md
@@ -9,7 +9,6 @@
 - Clear passwords from memory.
 - Find a nicer syntax for the `write_config` test.
 - Prevent construction of internal types.
-- More specific error checking in the tests.
 - Check integer conversions.
 - Consider implementing `Into<CommandError>` for `(Device, CommandError)`
 - Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware
diff --git a/src/device.rs b/src/device.rs
index 1cf9da9..16064c3 100644
--- a/src/device.rs
+++ b/src/device.rs
@@ -286,7 +286,7 @@ pub struct StorageStatus {
 ///
 /// This trait provides the commands that can be executed without authentication and that are
 /// present on all supported Nitrokey devices.
-pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp {
+pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
     /// Returns the model of the connected Nitrokey device.
     ///
     /// # Example
diff --git a/src/pws.rs b/src/pws.rs
index 47965d7..a21527c 100644
--- a/src/pws.rs
+++ b/src/pws.rs
@@ -52,6 +52,7 @@ pub const SLOT_COUNT: u8 = 16;
 /// [`get_password_safe`]: trait.GetPasswordSafe.html#method.get_password_safe
 /// [`lock`]: trait.Device.html#method.lock
 /// [`GetPasswordSafe`]: trait.GetPasswordSafe.html
+#[derive(Debug)]
 pub struct PasswordSafe<'a> {
     _device: &'a dyn Device,
 }
diff --git a/tests/device.rs b/tests/device.rs
index ee5dae1..c502945 100644
--- a/tests/device.rs
+++ b/tests/device.rs
@@ -5,8 +5,8 @@ use std::process::Command;
 use std::{thread, time};
 
 use nitrokey::{
-    Authenticate, CommandError, Config, ConfigureOtp, Device, Error, GenerateOtp, GetPasswordSafe,
-    LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
+    Authenticate, CommandError, CommunicationError, Config, ConfigureOtp, Device, Error,
+    GenerateOtp, GetPasswordSafe, LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
 };
 use nitrokey_test::test as test_device;
 
@@ -31,11 +31,11 @@ fn count_nitrokey_block_devices() -> usize {
 
 #[test_device]
 fn connect_no_device() {
-    assert!(nitrokey::connect().is_err());
-    assert!(nitrokey::connect_model(nitrokey::Model::Pro).is_err());
-    assert!(nitrokey::connect_model(nitrokey::Model::Storage).is_err());
-    assert!(nitrokey::Pro::connect().is_err());
-    assert!(nitrokey::Storage::connect().is_err());
+    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect());
+    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Pro));
+    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Storage));
+    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Pro::connect());
+    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Storage::connect());
 }
 
 #[test_device]
@@ -148,9 +148,7 @@ fn change_user_pin(device: DeviceWrapper) {
     let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
     let device = device.authenticate_user(USER_NEW_PASSWORD).unwrap_err().0;
 
-    assert!(device
-        .change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD));
 
     let device = device.authenticate_user(USER_PASSWORD).unwrap_err().0;
     let device = device
@@ -161,9 +159,7 @@ fn change_user_pin(device: DeviceWrapper) {
     let result = device.change_user_pin(USER_PASSWORD, USER_PASSWORD);
     assert_cmd_err!(CommandError::WrongPassword, result);
 
-    assert!(device
-        .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));
 
     let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
     assert!(device.authenticate_user(USER_NEW_PASSWORD).is_err());
@@ -174,9 +170,7 @@ fn change_admin_pin(device: DeviceWrapper) {
     let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
     let device = device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err().0;
 
-    assert!(device
-        .change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD));
 
     let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap_err().0;
     let device = device
@@ -189,9 +183,7 @@ fn change_admin_pin(device: DeviceWrapper) {
         device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD)
     );
 
-    assert!(device
-        .change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD));
 
     let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
     device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err();
@@ -215,9 +207,7 @@ where
 #[test_device]
 fn unlock_user_pin(device: DeviceWrapper) {
     let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
-    assert!(device
-        .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
     assert_cmd_err!(
         CommandError::WrongPassword,
         device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
@@ -235,9 +225,7 @@ fn unlock_user_pin(device: DeviceWrapper) {
         CommandError::WrongPassword,
         device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
     );
-    assert!(device
-        .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
     let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
 
     // block user PIN
@@ -251,14 +239,10 @@ fn unlock_user_pin(device: DeviceWrapper) {
         CommandError::WrongPassword,
         device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
     );
-    assert!(device
-        .unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD));
 
     // reset user PIN
-    assert!(device
-        .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
-        .is_ok());
+    assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));
 }
 
 #[test_device]
diff --git a/tests/otp.rs b/tests/otp.rs
index 51a6539..96da371 100644
--- a/tests/otp.rs
+++ b/tests/otp.rs
@@ -93,7 +93,7 @@ fn hotp_pin(device: DeviceWrapper) {
     let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
     check_hotp_codes(&user, 0);
 
-    assert!(user.device().get_hotp_code(1).is_err());
+    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_hotp_code(1));
 }
 
 #[test_device]
@@ -156,7 +156,7 @@ fn configure_totp(admin: &ConfigureOtp, factor: u64) {
 }
 
 fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) {
-    for (i, &(base_time, code)) in TOTP_CODES.iter().enumerate() {
+    for (base_time, code) in TOTP_CODES {
         let time = base_time.checked_mul(factor).unwrap();
         let is_u64 = time > u32::max_value() as u64;
         if is_u64 != (timestamp_size == TotpTimestampSize::U64) {
@@ -164,14 +164,7 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes
         }
 
         assert_ok!((), device.set_time(time, true));
-        let result = device.get_totp_code(1);
-        assert!(result.is_ok());
-        let result_code = result.unwrap();
-        assert_eq!(
-            code, result_code,
-            "TOTP code {} should be {} but is {}",
-            i, code, result_code
-        );
+        assert_ok!(code.to_string(), device.get_totp_code(1));
     }
 }
 
@@ -221,7 +214,7 @@ fn totp_pin(device: DeviceWrapper) {
     let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
     check_totp_codes(&user, 1, TotpTimestampSize::U32);
 
-    assert!(user.device().get_totp_code(1).is_err());
+    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
 }
 
 #[test_device]
@@ -235,7 +228,7 @@ fn totp_pin_64(device: Pro) {
     let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
     check_totp_codes(&user, 1, TotpTimestampSize::U64);
 
-    assert!(user.device().get_totp_code(1).is_err());
+    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
 }
 
 #[test_device]
@@ -246,8 +239,7 @@ fn totp_slot_name(device: DeviceWrapper) {
 
     let device = admin.device();
     let result = device.get_totp_slot_name(1);
-    assert!(result.is_ok());
-    assert_eq!("test-totp", result.unwrap());
+    assert_ok!("test-totp", result);
     let result = device.get_totp_slot_name(16);
     assert_lib_err!(LibraryError::InvalidSlot, result);
 }
diff --git a/tests/pws.rs b/tests/pws.rs
index b89d7f6..7a97983 100644
--- a/tests/pws.rs
+++ b/tests/pws.rs
@@ -39,11 +39,9 @@ where
 
 #[test_device]
 fn enable(device: DeviceWrapper) {
-    assert!(device
-        .get_password_safe(&(USER_PASSWORD.to_owned() + "123"))
-        .is_err());
+    assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(&(USER_PASSWORD.to_owned() + "123")));
     assert!(device.get_password_safe(USER_PASSWORD).is_ok());
-    assert!(device.get_password_safe(ADMIN_PASSWORD).is_err());
+    assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(ADMIN_PASSWORD));
     assert!(device.get_password_safe(USER_PASSWORD).is_ok());
 }
 
diff --git a/tests/util/mod.rs b/tests/util/mod.rs
index b1d3ea3..4a00a66 100644
--- a/tests/util/mod.rs
+++ b/tests/util/mod.rs
@@ -69,6 +69,13 @@ macro_rules! assert_cmd_err {
 }
 
 #[macro_export]
+macro_rules! assert_cmu_err {
+    ($left:expr, $right:expr) => {
+        assert_err!(::nitrokey::Error::CommunicationError, $left, $right);
+    };
+}
+
+#[macro_export]
 macro_rules! assert_lib_err {
     ($left:expr, $right:expr) => {
         assert_err!(::nitrokey::Error::LibraryError, $left, $right);