From 1a88efa1b3c4129234e59e9acd613d8f4ae85983 Mon Sep 17 00:00:00 2001 From: "akihito.nakano" Date: Fri, 24 Jan 2020 19:36:44 +0900 Subject: [PATCH 1/4] Move arguments parsing and initialize_logger() to out of `execute()` initialize_logger() sets the logger into global variable. That is an obstacle to write tests for `execute()` because it will be panicked when initialize_logger() called twice. --- cli/src/main.rs | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index fbf5b5a..3f3ca44 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -206,7 +206,14 @@ impl fmt::Display for Error { fn main() { panic_hook::set_abort(); - match execute(env::args()) { + let args = match parse_args(env::args()) { + Ok(args) => args, + Err(e) => e.exit() + }; + + initialize_logger(&args.flag_log); + + match execute(args) { Ok(_) => { println!("whisper-cli terminated"); process::exit(1); @@ -219,14 +226,15 @@ fn main() { } } -fn execute(command: I) -> Result<(), Error> where I: IntoIterator, S: AsRef { +fn parse_args(command: I) -> Result where I: IntoIterator, S: AsRef { + Docopt::new(USAGE) + .and_then(|d| d.argv(command).deserialize()) +} - // Parse arguments - let args: Args = Docopt::new(USAGE).and_then(|d| d.argv(command).deserialize())?; +fn execute(args: Args) -> Result<(), Error> { let pool_size = args.flag_whisper_pool_size * POOL_UNIT; let rpc_url = format!("{}:{}", args.flag_rpc_address, args.flag_rpc_port); - initialize_logger(args.flag_log); info!(target: "whisper-cli", "start"); // Filter manager that will dispatch `decryption tasks` @@ -290,15 +298,16 @@ fn execute(command: I) -> Result<(), Error> where I: IntoIterator, Ok(()) } -fn initialize_logger(log_level: String) { +fn initialize_logger(log_level: &String) { env_logger::Builder::from_env(env_logger::Env::default()) - .parse(&log_level) + .parse(log_level) .init(); } #[cfg(test)] mod tests { use super::execute; + use parse_args; #[test] fn invalid_argument() { @@ -307,7 +316,8 @@ mod tests { .map(Into::into) .collect::>(); - assert!(execute(command).is_err()); + + assert!(parse_args(command).is_err()); } #[test] @@ -318,7 +328,7 @@ mod tests { .map(Into::into) .collect::>(); - assert!(execute(command).is_err()); + assert!(execute(parse_args(command).unwrap()).is_err()); } #[test] @@ -328,7 +338,7 @@ mod tests { .map(Into::into) .collect::>(); - assert!(execute(command).is_err()); + assert!(execute(parse_args(command).unwrap()).is_err()); } #[test] @@ -346,7 +356,7 @@ mod tests { .map(Into::into) .collect::>(); - assert!(execute(command_pool_size_too_low).is_err()); - assert!(execute(command_pool_size_too_high).is_err()); + assert!(execute(parse_args(command_pool_size_too_low).unwrap()).is_err()); + assert!(execute(parse_args(command_pool_size_too_high).unwrap()).is_err()); } } From 03bcc23bfc0ae49b7e5510bac418670c84b4e4b2 Mon Sep 17 00:00:00 2001 From: "akihito.nakano" Date: Fri, 24 Jan 2020 19:39:30 +0900 Subject: [PATCH 2/4] parse_args() returns Error if whisper_pool_size is invalid as `usize` --- cli/src/main.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 3f3ca44..db135bb 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -321,42 +321,42 @@ mod tests { } #[test] - #[ignore] - fn privileged_port() { - let command = vec!["whisper", "--port=3"] + // The Whisper pool size is of type usize. Invalid Whisper pool sizes include + // values below 0 and either above 2 ** 64 - 1 on a 64-bit processor or + // above 2 ** 32 - 1 on a 32-bit processor. + fn invalid_whisper_pool_size() { + let command_pool_size_too_low = vec!["whisper", "--whisper-pool-size=-1"] .into_iter() .map(Into::into) .collect::>(); - assert!(execute(parse_args(command).unwrap()).is_err()); - } - - #[test] - fn invalid_ip_address() { - let command = vec!["whisper", "--address=x.x.x.x"] + let command_pool_size_too_high = vec!["whisper", "--whisper-pool-size=18446744073709552000"] .into_iter() .map(Into::into) .collect::>(); - assert!(execute(parse_args(command).unwrap()).is_err()); + assert!(parse_args(command_pool_size_too_low).is_err()); + assert!(parse_args(command_pool_size_too_high).is_err()); } #[test] - // The Whisper pool size is of type usize. Invalid Whisper pool sizes include - // values below 0 and either above 2 ** 64 - 1 on a 64-bit processor or - // above 2 ** 32 - 1 on a 32-bit processor. - fn invalid_whisper_pool_size() { - let command_pool_size_too_low = vec!["whisper", "--whisper-pool-size=-1"] + #[ignore] + fn privileged_port() { + let command = vec!["whisper", "--port=3"] .into_iter() .map(Into::into) .collect::>(); - let command_pool_size_too_high = vec!["whisper", "--whisper-pool-size=18446744073709552000"] + assert!(execute(parse_args(command).unwrap()).is_err()); + } + + #[test] + fn invalid_ip_address() { + let command = vec!["whisper", "--address=x.x.x.x"] .into_iter() .map(Into::into) .collect::>(); - assert!(execute(parse_args(command_pool_size_too_low).unwrap()).is_err()); - assert!(execute(parse_args(command_pool_size_too_high).unwrap()).is_err()); + assert!(execute(parse_args(command).unwrap()).is_err()); } } From 6b5508739280bd0f063cabbb1fdf346155918c53 Mon Sep 17 00:00:00 2001 From: "akihito.nakano" Date: Fri, 24 Jan 2020 19:44:38 +0900 Subject: [PATCH 3/4] Enable privileged_port test as `execute()` became possible to be called more than once ref https://github.com/paritytech/whisper/commit/1a88efa1b3c4129234e59e9acd613d8f4ae85983 --- cli/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index db135bb..f5416db 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -340,7 +340,6 @@ mod tests { } #[test] - #[ignore] fn privileged_port() { let command = vec!["whisper", "--port=3"] .into_iter() From 660bbd61bd162d3189ee4d60c91f1333f22b1c02 Mon Sep 17 00:00:00 2001 From: "akihito.nakano" Date: Fri, 24 Jan 2020 19:54:29 +0900 Subject: [PATCH 4/4] Remove unnecessary error handling --- cli/src/main.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index f5416db..c4dc6ad 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -138,7 +138,6 @@ impl RpcFactory { #[derive(Debug)] enum Error { - Docopt(docopt::Error), Io(io::Error), JsonRpc(jsonrpc_core::Error), Network(net::Error), @@ -159,12 +158,6 @@ impl From for Error { } } -impl From for Error { - fn from(err: docopt::Error) -> Self { - Error::Docopt(err) - } -} - impl From for Error { fn from(err: io::Error) -> Self { Error::Io(err) @@ -193,7 +186,6 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { match *self { Error::SockAddr(ref e) => write!(f, "{}", e), - Error::Docopt(ref e) => write!(f, "{}", e), Error::Io(ref e) => write!(f, "{}", e), Error::JsonRpc(ref e) => write!(f, "{:?}", e), Error::Network(ref e) => write!(f, "{}", e), @@ -218,7 +210,6 @@ fn main() { println!("whisper-cli terminated"); process::exit(1); }, - Err(Error::Docopt(ref e)) => e.exit(), Err(err) => { println!("{}", err); process::exit(1);