From c43b63b70ee32f9fa8e980d89eff5383931f5c39 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 03:49:13 +0000 Subject: Add assert_cmd_err and assert_ok macros to tests These macros allow easier comparisions using the new error type. This patch fixes all tests and updates nitrokey-test to 0.2.0 so that it integrates with the new error structure. Some tests may still fail until CommunicationError::NotConnected is actually returned. --- tests/pws.rs | 99 ++++++++++++++++++++++++++---------------------------------- 1 file changed, 42 insertions(+), 57 deletions(-) (limited to 'tests/pws.rs') diff --git a/tests/pws.rs b/tests/pws.rs index fbcc0c1..f12af48 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -3,16 +3,16 @@ mod util; use std::ffi::CStr; use libc::{c_int, c_void, free}; -use nitrokey::{CommandError, Device, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; +use nitrokey::{CommandError, Device, Error, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; use nitrokey_sys; use nitrokey_test::test as test_device; use crate::util::{ADMIN_PASSWORD, USER_PASSWORD}; -fn get_slot_name_direct(slot: u8) -> Result { +fn get_slot_name_direct(slot: u8) -> Result { let ptr = unsafe { nitrokey_sys::NK_get_password_safe_slot_name(slot) }; if ptr.is_null() { - return Err(CommandError::Undefined); + return Err(CommandError::Undefined.into()); } let s = unsafe { CStr::from_ptr(ptr).to_string_lossy().into_owned() }; unsafe { free(ptr as *mut c_void) }; @@ -21,7 +21,7 @@ fn get_slot_name_direct(slot: u8) -> Result { let error = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; match error { 0 => Ok(s), - other => Err(CommandError::from(other)), + other => Err(CommandError::from(other).into()), } } false => Ok(s), @@ -49,35 +49,35 @@ fn enable(device: DeviceWrapper) { fn drop(device: DeviceWrapper) { { let pws = get_pws(&device); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); assert_eq!("name", pws.get_slot_name(1).unwrap()); let result = get_slot_name_direct(1); - assert_eq!(Ok(String::from("name")), result); + assert_ok!(String::from("name"), result); } let result = get_slot_name_direct(1); - assert_eq!(Ok(String::from("name")), result); - assert_eq!(Ok(()), device.lock()); + assert_ok!(String::from("name"), result); + assert_ok!((), device.lock()); let result = get_slot_name_direct(1); - assert_eq!(Err(CommandError::NotAuthorized), result); + assert_cmd_err!(CommandError::NotAuthorized, result); } #[test_device] fn get_status(device: DeviceWrapper) { let pws = get_pws(&device); for i in 0..SLOT_COUNT { - assert_eq!(Ok(()), pws.erase_slot(i), "Could not erase slot {}", i); + assert_ok!((), pws.erase_slot(i)); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [false; SLOT_COUNT as usize]); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); let status = pws.get_slot_status().unwrap(); for i in 0..SLOT_COUNT { assert_eq!(i == 1, status[i as usize]); } for i in 0..SLOT_COUNT { - assert_eq!(Ok(()), pws.write_slot(i, "name", "login", "password")); + assert_ok!((), pws.write_slot(i, "name", "login", "password")); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [true; SLOT_COUNT as usize]); @@ -86,76 +86,61 @@ fn get_status(device: DeviceWrapper) { #[test_device] fn get_data(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); assert_eq!("name", pws.get_slot_name(1).unwrap()); assert_eq!("login", pws.get_slot_login(1).unwrap()); assert_eq!("password", pws.get_slot_password(1).unwrap()); - assert_eq!(Ok(()), pws.erase_slot(1)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(1)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_login(1)); - assert_eq!( - Err(CommandError::SlotNotProgrammed), - pws.get_slot_password(1) - ); + assert_ok!((), pws.erase_slot(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_login(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_password(1)); let name = "with å"; let login = "pär@test.com"; let password = "'i3lJc[09?I:,[u7dWz9"; - assert_eq!(Ok(()), pws.write_slot(1, name, login, password)); + assert_ok!((), pws.write_slot(1, name, login, password)); assert_eq!(name, pws.get_slot_name(1).unwrap()); assert_eq!(login, pws.get_slot_login(1).unwrap()); assert_eq!(password, pws.get_slot_password(1).unwrap()); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_name(SLOT_COUNT) - ); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_login(SLOT_COUNT) - ); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_password(SLOT_COUNT) - ); + assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_name(SLOT_COUNT)); + assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_login(SLOT_COUNT)); + assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_password(SLOT_COUNT)); } #[test_device] fn write(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!( - Err(CommandError::InvalidSlot), + assert_cmd_err!( + CommandError::InvalidSlot, pws.write_slot(SLOT_COUNT, "name", "login", "password") ); - assert_eq!(Ok(()), pws.write_slot(0, "", "login", "password")); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(0)); - assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); - assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - - assert_eq!(Ok(()), pws.write_slot(0, "name", "", "password")); - assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_login(0)); - assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - - assert_eq!(Ok(()), pws.write_slot(0, "name", "login", "")); - assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); - assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); - assert_eq!( - Err(CommandError::SlotNotProgrammed), - pws.get_slot_password(0) - ); + assert_ok!((), pws.write_slot(0, "", "login", "password")); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(0)); + assert_ok!(String::from("login"), pws.get_slot_login(0)); + assert_ok!(String::from("password"), pws.get_slot_password(0)); + + assert_ok!((), pws.write_slot(0, "name", "", "password")); + assert_ok!(String::from("name"), pws.get_slot_name(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_login(0)); + assert_ok!(String::from("password"), pws.get_slot_password(0)); + + assert_ok!((), pws.write_slot(0, "name", "login", "")); + assert_ok!(String::from("name"), pws.get_slot_name(0)); + assert_ok!(String::from("login"), pws.get_slot_login(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_password(0)); } #[test_device] fn erase(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!(Err(CommandError::InvalidSlot), pws.erase_slot(SLOT_COUNT)); + assert_cmd_err!(CommandError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); - assert_eq!(Ok(()), pws.write_slot(0, "name", "login", "password")); - assert_eq!(Ok(()), pws.erase_slot(0)); - assert_eq!(Ok(()), pws.erase_slot(0)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(0)); + assert_ok!((), pws.write_slot(0, "name", "login", "password")); + assert_ok!((), pws.erase_slot(0)); + assert_ok!((), pws.erase_slot(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(0)); } -- cgit v1.2.1 From 944e1fa0d51e547dde2a9368d2b8431b109f63c4 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 04:15:31 +0000 Subject: Move the CommandError::Unknown to Error An error code can not only indiciate a command error, but also a library or device communication error. Therefore, the variant for an unknown error code should be placed in the top-level Error enum instead of the CommandError enum. --- tests/pws.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/pws.rs') diff --git a/tests/pws.rs b/tests/pws.rs index f12af48..a4647bd 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -21,7 +21,7 @@ fn get_slot_name_direct(slot: u8) -> Result { let error = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; match error { 0 => Ok(s), - other => Err(CommandError::from(other).into()), + other => Err(Error::from(other)), } } false => Ok(s), -- cgit v1.2.1 From 5e258d26b55af6bed7c316b1c7ac12e20946702d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 12:47:52 +0000 Subject: Refactor library errors into LibraryError enum Previously, library errors were part of the CommandError enum. As command errors and library errors are two different error types, they should be split into two enums. --- tests/pws.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'tests/pws.rs') diff --git a/tests/pws.rs b/tests/pws.rs index a4647bd..51e6189 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -3,7 +3,9 @@ mod util; use std::ffi::CStr; use libc::{c_int, c_void, free}; -use nitrokey::{CommandError, Device, Error, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; +use nitrokey::{ + CommandError, Device, Error, GetPasswordSafe, LibraryError, PasswordSafe, SLOT_COUNT, +}; use nitrokey_sys; use nitrokey_test::test as test_device; @@ -104,17 +106,17 @@ fn get_data(device: DeviceWrapper) { assert_eq!(login, pws.get_slot_login(1).unwrap()); assert_eq!(password, pws.get_slot_password(1).unwrap()); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_name(SLOT_COUNT)); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_login(SLOT_COUNT)); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_password(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_name(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_login(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_password(SLOT_COUNT)); } #[test_device] fn write(device: DeviceWrapper) { let pws = get_pws(&device); - assert_cmd_err!( - CommandError::InvalidSlot, + assert_lib_err!( + LibraryError::InvalidSlot, pws.write_slot(SLOT_COUNT, "name", "login", "password") ); @@ -137,7 +139,7 @@ fn write(device: DeviceWrapper) { #[test_device] fn erase(device: DeviceWrapper) { let pws = get_pws(&device); - assert_cmd_err!(CommandError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); assert_ok!((), pws.write_slot(0, "name", "login", "password")); assert_ok!((), pws.erase_slot(0)); -- cgit v1.2.1 From c191e875492ff8aeab1b4493b87486cd265f0edc Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:38:28 +0000 Subject: Introduce the Error::UnexpectedError variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UnexpectedError variant is used when a libnitrokey function returns a value that violates the function’s contract, for example if a function returns a null pointer although it guarantees to never return null. Previously, we returned a CommandError::Unspecified in these cases. --- tests/pws.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/pws.rs') diff --git a/tests/pws.rs b/tests/pws.rs index 51e6189..b89d7f6 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -14,7 +14,7 @@ use crate::util::{ADMIN_PASSWORD, USER_PASSWORD}; fn get_slot_name_direct(slot: u8) -> Result { let ptr = unsafe { nitrokey_sys::NK_get_password_safe_slot_name(slot) }; if ptr.is_null() { - return Err(CommandError::Undefined.into()); + return Err(Error::UnexpectedError); } let s = unsafe { CStr::from_ptr(ptr).to_string_lossy().into_owned() }; unsafe { free(ptr as *mut c_void) }; -- cgit v1.2.1 From d87859975dc158919ecd5bf11a1111a2da5fcb30 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 14:21:44 +0000 Subject: 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`. --- tests/pws.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'tests/pws.rs') 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()); } -- cgit v1.2.1