agave-validator: add args tests for wait-for-restart-window

validator/src/cli.rs

@@ -78,7 +78,7 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> {
         .subcommand(commands::set_identity::command(default_args))
         .subcommand(commands::set_log_filter::command(default_args))
         .subcommand(commands::staked_nodes_overrides::command(default_args))
-        .subcommand(commands::wait_for_restart_window::command(default_args))
+        .subcommand(commands::wait_for_restart_window::command())
         .subcommand(commands::set_public_address::command(default_args));
 
     commands::run::add_args(app, default_args)
@@ -436,10 +436,6 @@ pub struct DefaultArgs {
     pub exit_min_idle_time: String,
     pub exit_max_delinquent_stake: String,
 
-    // Wait subcommand
-    pub wait_for_restart_window_min_idle_time: String,
-    pub wait_for_restart_window_max_delinquent_stake: String,
-
     pub banking_trace_dir_byte_limit: String,
 
     pub wen_restart_path: String,
@@ -537,8 +533,6 @@ impl DefaultArgs {
             rpc_max_request_body_size: MAX_REQUEST_BODY_SIZE.to_string(),
             exit_min_idle_time: "10".to_string(),
             exit_max_delinquent_stake: "5".to_string(),
-            wait_for_restart_window_min_idle_time: "10".to_string(),
-            wait_for_restart_window_max_delinquent_stake: "5".to_string(),
             banking_trace_dir_byte_limit: BANKING_TRACE_DIR_DEFAULT_BYTE_LIMIT.to_string(),
             wen_restart_path: "wen_restart_progress.proto".to_string(),
             thread_args: DefaultThreadArgs::default(),

validator/src/commands/wait_for_restart_window/mod.rs

@@ -1,5 +1,8 @@
 use {
-    crate::{admin_rpc_service, cli::DefaultArgs, new_spinner_progress_bar, println_name_value},
+    crate::{
+        admin_rpc_service, commands::FromClapArgMatches, new_spinner_progress_bar,
+        println_name_value,
+    },
     clap::{value_t_or_exit, App, Arg, ArgMatches, SubCommand},
     console::style,
     solana_clap_utils::{
@@ -20,16 +23,42 @@ use {
     },
 };
 
-pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> {
-    SubCommand::with_name("wait-for-restart-window")
+const COMMAND: &str = "wait-for-restart-window";
+
+const DEFAULT_MIN_IDLE_TIME: &str = "10";
+const DEFAULT_MAX_DELINQUENT_STAKE: &str = "5";
+
+#[derive(Debug, PartialEq)]
+pub struct WaitForRestartWindowArgs {
+    pub min_idle_time: usize,
+    pub identity: Option<Pubkey>,
+    pub max_delinquent_stake: u8,
+    pub skip_new_snapshot_check: bool,
+    pub skip_health_check: bool,
+}
+
+impl FromClapArgMatches for WaitForRestartWindowArgs {
+    fn from_clap_arg_match(matches: &ArgMatches) -> Result<Self, String> {
+        Ok(WaitForRestartWindowArgs {
+            min_idle_time: value_t_or_exit!(matches, "min_idle_time", usize),
+            identity: pubkey_of(matches, "identity"),
+            max_delinquent_stake: value_t_or_exit!(matches, "max_delinquent_stake", u8),
+            skip_new_snapshot_check: matches.is_present("skip_new_snapshot_check"),
+            skip_health_check: matches.is_present("skip_health_check"),
+        })
+    }
+}
+
+pub(crate) fn command<'a>() -> App<'a, 'a> {
+    SubCommand::with_name(COMMAND)
         .about("Monitor the validator for a good time to restart")
         .arg(
             Arg::with_name("min_idle_time")
                 .long("min-idle-time")
                 .takes_value(true)
                 .validator(is_parsable::<usize>)
                 .value_name("MINUTES")
-                .default_value(&default_args.wait_for_restart_window_min_idle_time)
+                .default_value(DEFAULT_MIN_IDLE_TIME)
                 .help(
                     "Minimum time that the validator should not be leader before restarting",
                 ),
@@ -47,8 +76,8 @@ pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> {
                 .long("max-delinquent-stake")
                 .takes_value(true)
                 .validator(is_valid_percentage)
-                .default_value(&default_args.wait_for_restart_window_max_delinquent_stake)
                 .value_name("PERCENT")
+                .default_value(DEFAULT_MAX_DELINQUENT_STAKE)
                 .help("The maximum delinquent stake % permitted for a restart"),
         )
         .arg(
@@ -67,19 +96,15 @@ pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> {
 }
 
 pub fn execute(matches: &ArgMatches, ledger_path: &Path) -> Result<(), String> {
-    let min_idle_time = value_t_or_exit!(matches, "min_idle_time", usize);
-    let identity = pubkey_of(matches, "identity");
-    let max_delinquent_stake = value_t_or_exit!(matches, "max_delinquent_stake", u8);
-    let skip_new_snapshot_check = matches.is_present("skip_new_snapshot_check");
-    let skip_health_check = matches.is_present("skip_health_check");
+    let wait_for_restart_window_args = WaitForRestartWindowArgs::from_clap_arg_match(matches)?;
 
     wait_for_restart_window(
         ledger_path,
-        identity,
-        min_idle_time,
-        max_delinquent_stake,
-        skip_new_snapshot_check,
-        skip_health_check,
+        wait_for_restart_window_args.identity,
+        wait_for_restart_window_args.min_idle_time,
+        wait_for_restart_window_args.max_delinquent_stake,
+        wait_for_restart_window_args.skip_new_snapshot_check,
+        wait_for_restart_window_args.skip_health_check,
     )
     .map_err(|err| format!("failed to wait for restart window: {err}"))
 }
@@ -335,3 +360,99 @@ pub fn wait_for_restart_window(
     println!("{}", style("Ready to restart").green());
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use {super::*, crate::commands::tests::verify_args_struct_by_command, std::str::FromStr};
+
+    impl Default for WaitForRestartWindowArgs {
+        fn default() -> Self {
+            WaitForRestartWindowArgs {
+                min_idle_time: DEFAULT_MIN_IDLE_TIME
+                    .parse()
+                    .expect("invalid DEFAULT_MIN_IDLE_TIME"),
+                identity: None,
+                max_delinquent_stake: DEFAULT_MAX_DELINQUENT_STAKE
+                    .parse()
+                    .expect("invalid DEFAULT_MAX_DELINQUENT_STAKE"),
+                skip_new_snapshot_check: false,
+                skip_health_check: false,
+            }
+        }
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_default() {
+        verify_args_struct_by_command(
+            command(),
+            vec![COMMAND],
+            WaitForRestartWindowArgs::default(),
+        );
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_skip_new_snapshot_check() {
+        verify_args_struct_by_command(
+            command(),
+            vec![COMMAND, "--skip-new-snapshot-check"],
+            WaitForRestartWindowArgs {
+                skip_new_snapshot_check: true,
+                ..WaitForRestartWindowArgs::default()
+            },
+        );
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_skip_health_check() {
+        verify_args_struct_by_command(
+            command(),
+            vec![COMMAND, "--skip-health-check"],
+            WaitForRestartWindowArgs {
+                skip_health_check: true,
+                ..WaitForRestartWindowArgs::default()
+            },
+        );
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_min_idle_time() {
+        verify_args_struct_by_command(
+            command(),
+            vec![COMMAND, "--min-idle-time", "60"],
+            WaitForRestartWindowArgs {
+                min_idle_time: 60,
+                ..WaitForRestartWindowArgs::default()
+            },
+        );
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_identity() {
+        verify_args_struct_by_command(
+            command(),
+            vec![
+                COMMAND,
+                "--identity",
+                "ch1do11111111111111111111111111111111111111",
+            ],
+            WaitForRestartWindowArgs {
+                identity: Some(
+                    Pubkey::from_str("ch1do11111111111111111111111111111111111111").unwrap(),
+                ),
+                ..WaitForRestartWindowArgs::default()
+            },
+        );
+    }
+
+    #[test]
+    fn verify_args_struct_by_command_wait_for_restart_window_max_delinquent_stake() {
+        verify_args_struct_by_command(
+            command(),
+            vec![COMMAND, "--max-delinquent-stake", "10"],
+            WaitForRestartWindowArgs {
+                max_delinquent_stake: 10,
+                ..WaitForRestartWindowArgs::default()
+            },
+        );
+    }
+}