diff --git a/Cargo.lock b/Cargo.lock index d601a8f3..567d1ffb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6003,6 +6003,7 @@ dependencies = [ "color-eyre 0.5.11", "futures", "hex", + "indexmap", "insta", "lazy_static", "once_cell", diff --git a/zebra-test/Cargo.toml b/zebra-test/Cargo.toml index 0fc7156c..6abb5682 100644 --- a/zebra-test/Cargo.toml +++ b/zebra-test/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" [dependencies] hex = "0.4.3" +indexmap = "1.8.1" lazy_static = "1.4.0" insta = "1.14.0" proptest = "0.10.1" diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index adae944d..be901933 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -19,11 +19,12 @@ use color_eyre::{ use regex::RegexSet; use tracing::instrument; +#[macro_use] +mod arguments; pub mod to_regex; -use to_regex::{CollectRegexSet, ToRegex}; - -use self::to_regex::ToRegexSet; +pub use self::arguments::Arguments; +use self::to_regex::{CollectRegexSet, ToRegex, ToRegexSet}; /// A super-trait for [`Iterator`] + [`Debug`]. pub trait IteratorDebug: Iterator + Debug {} @@ -123,18 +124,18 @@ where /// Spawn `cmd` with `args` as a child process in this test directory, /// potentially taking ownership of the tempdir for the duration of the /// child process. - fn spawn_child_with_command(self, cmd: &str, args: &[&str]) -> Result>; + fn spawn_child_with_command(self, cmd: &str, args: Arguments) -> Result>; } impl TestDirExt for T where Self: AsRef + Sized, { - fn spawn_child_with_command(self, cmd: &str, args: &[&str]) -> Result> { + fn spawn_child_with_command(self, cmd: &str, args: Arguments) -> Result> { let mut cmd = test_cmd(cmd, self.as_ref())?; Ok(cmd - .args(args) + .args(args.into_arguments()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn2(self) diff --git a/zebra-test/src/command/arguments.rs b/zebra-test/src/command/arguments.rs new file mode 100644 index 00000000..00331058 --- /dev/null +++ b/zebra-test/src/command/arguments.rs @@ -0,0 +1,132 @@ +use indexmap::IndexMap; + +#[cfg(test)] +mod tests; + +/// Helper type to keep track of arguments for spawning a process. +/// +/// Stores the arguments in order, but is aware of key-value pairs to make overriding parameters +/// more predictable. +/// +/// # Notes +/// +/// Repeated arguments are not supported. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct Arguments(IndexMap>); + +impl Arguments { + /// Creates a new empty list of arguments. + pub fn new() -> Self { + Arguments(IndexMap::new()) + } + + /// Sets a lone `argument`. + /// + /// If the argument is already in the list, nothing happens. + /// + /// If there's a key-value pair where the key is identical to the `argument`, the value of the + /// pair is removed and the key maintains its position. + pub fn set_argument(&mut self, argument: impl Into) { + self.0.insert(argument.into(), None); + } + + /// Sets a key-value pair. + /// + /// If there is already a pair in the list with the same `parameter` key, the existing pair + /// keeps its position but the value is updated. + /// + /// If there is a lone argument that is identical to the `parameter` key, the value is inserted + /// after it. + pub fn set_parameter(&mut self, parameter: impl Into, value: impl Into) { + self.0.insert(parameter.into(), Some(value.into())); + } + + /// Merges two argument lists. + /// + /// Existing pairs have their values updated if needed, and new pairs and arguments are + /// appended to the end of this list. + pub fn merge_with(&mut self, extra_arguments: Arguments) { + for (key, value) in extra_arguments.0 { + self.0.insert(key, value); + } + } + + /// Converts this [`Arguments`] instance into a list of strings that can be passed when + /// spawning a process. + pub fn into_arguments(self) -> impl Iterator { + self.0 + .into_iter() + .flat_map(|(key, value)| Some(key).into_iter().chain(value)) + } +} + +/// Helper macro to create a list of arguments in an [`Arguments`] instance. +/// +/// Accepts items separated by commas, where an item is either: +/// +/// - a lone argument obtained from an expression +/// - a key-value pair in the form `"key": value_expression` +/// +/// The items are added to a new [`Arguments`] instance in order. +/// +/// # Implementation details +/// +/// This macro is called recursively, and can run into a macro recursion limit if the list of items +/// is very long. +/// +/// The first step of the macro is to create a new scope with an `args` binding to a new empty +/// `Arguments` list. That binding is returned from the scope, so the macro's generated code is an +/// expression that creates the argument list. Once the scope is created, a `@started` tag is +/// prefixed to the recursive calls, which then individually add each item. +#[macro_export] +macro_rules! args { + // Either an empty macro call or the end of items in a list. + ( @started $args:ident $(,)* ) => {}; + + // Handling the last (or sole) item in a list, if it's a key-value pair. + ( + @started $args:ident + $parameter:tt : $argument:expr $(,)* + ) => { + $args.set_parameter($parameter, $argument); + }; + + // Handling the last (or sole) item in a list, if it's a lone argument. + ( + @started $args:ident + $argument:expr $(,)* + ) => { + $args.set_argument($argument); + }; + + // Handling the next item in a list, if it's a key-value pair argument. + ( + @started $args:ident + $parameter:tt : $argument:expr + , $( $remaining_arguments:tt )+ + ) => { + $args.set_parameter($parameter, $argument); + args!(@started $args $( $remaining_arguments )*) + }; + + // Handling the next item in a list, if it's a lone argument. + ( + @started $args:ident + $argument:expr + , $( $remaining_arguments:tt )* + ) => { + $args.set_argument($argument); + args!(@started $args $( $remaining_arguments )*) + }; + + // Start of the macro, create a scope to return an `args` binding, and populate it with a + // recursive call to this macro. A `@started` tag is used to indicate that the scope has been + // set up. + ( $( $arguments:tt )* ) => { + { + let mut args = $crate::command::Arguments::new(); + args!(@started args $( $arguments )*); + args + } + }; +} diff --git a/zebra-test/src/command/arguments/tests.rs b/zebra-test/src/command/arguments/tests.rs new file mode 100644 index 00000000..6f4058a9 --- /dev/null +++ b/zebra-test/src/command/arguments/tests.rs @@ -0,0 +1,252 @@ +use proptest::{ + collection::{hash_set, vec}, + prelude::*, +}; + +use super::Arguments; + +proptest! { + /// Test that the argument order is preserved when building an [`Arguments`] instance. + /// + /// Check that the [`Arguments::into_arguments`] method creates a list of strings in the same + /// order as if each argument was individually added to a list. + #[test] + fn argument_order_is_preserved(argument_list in Argument::list_strategy()) { + let arguments = collect_arguments(argument_list.clone()); + let argument_strings: Vec<_> = arguments.into_arguments().collect(); + + let expected_strings = expand_arguments(argument_list); + + assert_eq!(argument_strings, expected_strings); + } + + /// Test that arguments in an [`Arguments`] instance can be overriden. + /// + /// Generate a list of arguments to add and a list of overrides for those arguments. Also + /// generate a list of extra arguments. + /// + /// All arguments from the three lists are added to an [`Arguments`] instance. The generated + /// list of strings from the [`Arguments::into_arguments`] method is compared to a list of + /// `expected_strings`. + /// + /// To build the list of `expected_strings`, a new `overriden_list` is compiled from the three + /// original lists. The list is compiled by manually handling overrides in a compatible way that + /// keeps the argument order when overriding and adding new arguments to the end of the list. + #[test] + fn arguments_can_be_overriden( + (argument_list, override_list) in Argument::list_and_overrides_strategy(), + extra_arguments in Argument::list_strategy(), + ) { + let arguments_to_add: Vec<_> = argument_list + .into_iter() + .chain(override_list) + .chain(extra_arguments) + .collect(); + + let arguments = collect_arguments(arguments_to_add.clone()); + let argument_strings: Vec<_> = arguments.into_arguments().collect(); + + let overriden_list = handle_overrides(arguments_to_add); + let expected_strings = expand_arguments(overriden_list); + + assert_eq!(argument_strings, expected_strings); + } + + /// Test that arguments in an [`Arguments`] instance can be merged. + /// + /// Generate a list of arguments to add and a list of overrides for those arguments. Also + /// generate a list of extra arguments. + /// + /// The first list is added to a first [`Arguments`] instance, while the second and third lists + /// are added to a second [`Arguments`] instance. The second instance is then merged into the + /// first instance. The generated list of strings from the [`Arguments::into_arguments`] method + /// of that first instance is compared to a list of `expected_strings`, which is built exactly + /// like in the [`arguments_can_be_overriden`] test. + #[test] + fn arguments_can_be_merged( + (argument_list, override_list) in Argument::list_and_overrides_strategy(), + extra_arguments in Argument::list_strategy(), + ) { + let all_arguments: Vec<_> = argument_list + .clone() + .into_iter() + .chain(override_list.clone()) + .chain(extra_arguments.clone()) + .collect(); + + let arguments_for_second_instance = override_list.into_iter().chain(extra_arguments).collect(); + + let mut first_arguments = collect_arguments(argument_list); + let second_arguments = collect_arguments(arguments_for_second_instance); + + first_arguments.merge_with(second_arguments); + + let argument_strings: Vec<_> = first_arguments.into_arguments().collect(); + + let overriden_list = handle_overrides(all_arguments); + let expected_strings = expand_arguments(overriden_list); + + assert_eq!(argument_strings, expected_strings); + } +} + +/// Collects a list of [`Argument`] items into an [`Arguments`] instance. +fn collect_arguments(argument_list: Vec) -> Arguments { + let mut arguments = Arguments::new(); + + for argument in argument_list { + match argument { + Argument::LoneArgument(argument) => arguments.set_argument(argument), + Argument::KeyValuePair(key, value) => arguments.set_parameter(key, value), + } + } + + arguments +} + +/// Expands a list of [`Argument`] items into a list of [`String`]s. +/// +/// This list is the list of expected strings that is expected to be generated from an [`Arguments`] +/// instance built from the same list of [`Argument`] items. +fn expand_arguments(argument_list: Vec) -> Vec { + let mut expected_strings = Vec::new(); + + for argument in argument_list { + match argument { + Argument::LoneArgument(argument) => expected_strings.push(argument), + Argument::KeyValuePair(key, value) => expected_strings.extend([key, value]), + } + } + + expected_strings +} + +/// Processes a list of [`Argument`] items handling overrides, returning a new list of [`Argument`] +/// items without duplicate arguments. +/// +/// This follows the behavior of [`Arguments`] when handling overrides, so that the returned list +/// is equivalent to an [`Arguments`] instance built from the same input list. +fn handle_overrides(argument_list: Vec) -> Vec { + let mut overriden_list = Vec::new(); + + for override_argument in argument_list { + let search_term = match &override_argument { + Argument::LoneArgument(argument) => argument, + Argument::KeyValuePair(key, _value) => key, + }; + + let argument_to_override = + overriden_list + .iter_mut() + .find(|existing_argument| match existing_argument { + Argument::LoneArgument(argument) => argument == search_term, + Argument::KeyValuePair(key, _value) => key == search_term, + }); + + if let Some(argument_to_override) = argument_to_override { + *argument_to_override = override_argument; + } else { + overriden_list.push(override_argument); + } + } + + overriden_list +} + +/// A helper type to generate argument items. +#[derive(Clone, Debug)] +pub enum Argument { + /// A lone argument, like for example an individual item or a flag. + LoneArgument(String), + + /// A key value pair, usually a parameter to be configured. + KeyValuePair(String, String), +} + +impl Argument { + /// Generates a list of unique arbitrary [`Argument`] instances. + /// + /// # Implementation + /// + /// 1. Generate a list with less than ten random strings. Then proceed by selecting which strings + /// will become key value pairs, and generate a new random string for each value that needs to + /// be paired to an argument key. + pub fn list_strategy() -> impl Strategy> { + // Generate a list with less than ten unique random strings. + hash_set("\\PC+", 0..10) + .prop_map(|keys| keys.into_iter().collect::>()) + .prop_shuffle() + // Select some (or none) of the keys to become key-value pairs. + .prop_flat_map(|keys| { + let key_count = keys.len(); + + (Just(keys), vec(any::(), key_count)) + }) + // Generate random strings for the values to be paired with keys. + .prop_flat_map(|(keys, is_pair)| { + let value_count = is_pair.iter().filter(|&&is_pair| is_pair).count(); + + (Just(keys), Just(is_pair), vec("\\PC+", value_count)) + }) + // Pair the selected keys with values, and make the non-selected keys lone arguments. + .prop_map(|(keys, is_pair, mut values)| { + keys.into_iter() + .zip(is_pair) + .map(|(key, is_pair)| { + if is_pair { + Argument::KeyValuePair( + key, + values.pop().expect("missing value from generated list"), + ) + } else { + Argument::LoneArgument(key) + } + }) + .collect() + }) + } + + /// Generates a list of unique arbitrary [`Argument`] instances and a list of [`Argument`] + /// instances that override arguments from the first list. + pub fn list_and_overrides_strategy() -> impl Strategy, Vec)> { + // Generate a list of unique arbitrary arguments. + Self::list_strategy() + // Generate a list of arguments to override (referenced by indices) with new arbitrary + // random strings. + .prop_flat_map(|argument_list| { + let argument_list_count = argument_list.len(); + + let max_overrides = argument_list_count * 3; + let min_overrides = 1.min(max_overrides); + + let override_strategy = (0..argument_list_count, "\\PC*"); + + ( + Just(argument_list), + hash_set(override_strategy, min_overrides..=max_overrides), + ) + }) + // Transform the override references and random strings into [`Argument`] instances, + // with empty strings leading to the creation of lone arguments. + .prop_map(|(argument_list, override_seeds)| { + let override_list = override_seeds + .into_iter() + .map(|(index, new_value)| { + let key = match &argument_list[index] { + Argument::LoneArgument(argument) => argument, + Argument::KeyValuePair(key, _value) => key, + } + .clone(); + + if new_value.is_empty() { + Argument::LoneArgument(key) + } else { + Argument::KeyValuePair(key, new_value) + } + }) + .collect(); + + (argument_list, override_list) + }) + } +} diff --git a/zebra-test/tests/command.rs b/zebra-test/tests/command.rs index 9f3d4eee..b8d443b2 100644 --- a/zebra-test/tests/command.rs +++ b/zebra-test/tests/command.rs @@ -5,6 +5,7 @@ use regex::RegexSet; use tempfile::tempdir; use zebra_test::{ + args, command::{TestDirExt, NO_MATCHES_REGEX_ITER}, prelude::Stdio, }; @@ -59,7 +60,7 @@ fn kill_on_timeout_output_continuous_lines() -> Result<()> { // Without '-v', hexdump hides duplicate lines. But we want duplicate lines // in this test. let mut child = tempdir()? - .spawn_child_with_command(TEST_CMD, &["-v", "/dev/zero"])? + .spawn_child_with_command(TEST_CMD, args!["-v", "/dev/zero"])? .with_timeout(Duration::from_secs(2)); // We need to use expect_stdout_line_matches, because wait_with_output ignores timeouts. @@ -86,7 +87,7 @@ fn finish_before_timeout_output_single_line() -> Result<()> { } let mut child = tempdir()? - .spawn_child_with_command(TEST_CMD, &["zebra_test_output"])? + .spawn_child_with_command(TEST_CMD, args!["zebra_test_output"])? .with_timeout(Duration::from_secs(2)); // We need to use expect_stdout_line_matches, because wait_with_output ignores timeouts. @@ -115,7 +116,7 @@ fn kill_on_timeout_continuous_output_no_newlines() -> Result<()> { } let mut child = tempdir()? - .spawn_child_with_command(TEST_CMD, &["/dev/zero"])? + .spawn_child_with_command(TEST_CMD, args!["/dev/zero"])? .with_timeout(Duration::from_secs(2)); // We need to use expect_stdout_line_matches, because wait_with_output ignores timeouts. @@ -143,7 +144,7 @@ fn finish_before_timeout_short_output_no_newlines() -> Result<()> { } let mut child = tempdir()? - .spawn_child_with_command(TEST_CMD, &["zebra_test_output"])? + .spawn_child_with_command(TEST_CMD, args!["zebra_test_output"])? .with_timeout(Duration::from_secs(2)); // We need to use expect_stdout_line_matches, because wait_with_output ignores timeouts. @@ -171,7 +172,7 @@ fn kill_on_timeout_no_output() -> Result<()> { } let mut child = tempdir()? - .spawn_child_with_command(TEST_CMD, &["120"])? + .spawn_child_with_command(TEST_CMD, args!["120"])? .with_timeout(Duration::from_secs(2)); // We need to use expect_stdout_line_matches, because wait_with_output ignores timeouts. @@ -201,7 +202,7 @@ fn failure_regex_matches_stdout_failure_message() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(2)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -237,7 +238,7 @@ fn failure_regex_matches_stderr_failure_message() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["-c", "read -t 1 -p failure_message"]) + .spawn_child_with_command(TEST_CMD, args![ "-c": "read -t 1 -p failure_message" ]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -267,7 +268,7 @@ fn failure_regex_matches_stdout_failure_message_drop() { let _child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -296,7 +297,7 @@ fn failure_regex_matches_stdout_failure_message_kill() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -327,7 +328,7 @@ fn failure_regex_matches_stdout_failure_message_kill_on_error() { let child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -359,7 +360,7 @@ fn failure_regex_matches_stdout_failure_message_no_kill_on_error() { let child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -398,7 +399,7 @@ fn failure_regex_timeout_continuous_output() { // in this test. let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["-v", "/dev/zero"]) + .spawn_child_with_command(TEST_CMD, args!["-v", "/dev/zero"]) .unwrap() .with_timeout(Duration::from_secs(2)) .with_failure_regex_set("0", RegexSet::empty()); @@ -430,7 +431,7 @@ fn failure_regex_matches_stdout_failure_message_wait_for_output() { let child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(5)) .with_failure_regex_set("fail", RegexSet::empty()); @@ -461,7 +462,7 @@ fn failure_regex_iter_matches_stdout_failure_message() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message"]) .unwrap() .with_timeout(Duration::from_secs(2)) .with_failure_regex_iter( @@ -489,7 +490,7 @@ fn ignore_regex_ignores_stdout_failure_message() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message ignore_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message ignore_message"]) .unwrap() .with_timeout(Duration::from_secs(2)) .with_failure_regex_set("fail", "ignore"); @@ -511,7 +512,7 @@ fn ignore_regex_iter_ignores_stdout_failure_message() { let mut child = tempdir() .unwrap() - .spawn_child_with_command(TEST_CMD, &["failure_message ignore_message"]) + .spawn_child_with_command(TEST_CMD, args!["failure_message ignore_message"]) .unwrap() .with_timeout(Duration::from_secs(2)) .with_failure_regex_iter(["fail"].iter().cloned(), ["ignore"].iter().cloned()); diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index bbdc5faf..6721fd47 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -36,6 +36,7 @@ use zebra_network::constants::PORT_IN_USE_ERROR; use zebra_state::constants::LOCK_FILE_ERROR; use zebra_test::{ + args, command::{ContextFrom, NO_MATCHES_REGEX_ITER}, net::random_known_port, prelude::*, @@ -64,7 +65,7 @@ fn generate_no_args() -> Result<()> { let child = testdir()? .with_config(&mut default_test_config()?)? - .spawn_child(&["generate"])?; + .spawn_child(args!["generate"])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -83,17 +84,17 @@ fn generate_args() -> Result<()> { let testdir = &testdir; // unexpected free argument `argument` - let child = testdir.spawn_child(&["generate", "argument"])?; + let child = testdir.spawn_child(args!["generate", "argument"])?; let output = child.wait_with_output()?; output.assert_failure()?; // unrecognized option `-f` - let child = testdir.spawn_child(&["generate", "-f"])?; + let child = testdir.spawn_child(args!["generate", "-f"])?; let output = child.wait_with_output()?; output.assert_failure()?; // missing argument to option `-o` - let child = testdir.spawn_child(&["generate", "-o"])?; + let child = testdir.spawn_child(args!["generate", "-o"])?; let output = child.wait_with_output()?; output.assert_failure()?; @@ -102,7 +103,7 @@ fn generate_args() -> Result<()> { // Valid let child = - testdir.spawn_child(&["generate", "-o", generated_config_path.to_str().unwrap()])?; + testdir.spawn_child(args!["generate", "-o": generated_config_path.to_str().unwrap()])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -127,7 +128,7 @@ fn help_no_args() -> Result<()> { let testdir = testdir()?.with_config(&mut default_test_config()?)?; - let child = testdir.spawn_child(&["help"])?; + let child = testdir.spawn_child(args!["help"])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -153,12 +154,12 @@ fn help_args() -> Result<()> { let testdir = &testdir; // The subcommand "argument" wasn't recognized. - let child = testdir.spawn_child(&["help", "argument"])?; + let child = testdir.spawn_child(args!["help", "argument"])?; let output = child.wait_with_output()?; output.assert_failure()?; // option `-f` does not accept an argument - let child = testdir.spawn_child(&["help", "-f"])?; + let child = testdir.spawn_child(args!["help", "-f"])?; let output = child.wait_with_output()?; output.assert_failure()?; @@ -172,7 +173,7 @@ fn start_no_args() -> Result<()> { // start caches state, so run one of the start tests with persistent state let testdir = testdir()?.with_config(&mut persistent_test_config()?)?; - let mut child = testdir.spawn_child(&["-v", "start"])?; + let mut child = testdir.spawn_child(args!["-v", "start"])?; // Run the program and kill it after a few seconds std::thread::sleep(LAUNCH_DELAY); @@ -200,7 +201,7 @@ fn start_args() -> Result<()> { let testdir = testdir()?.with_config(&mut default_test_config()?)?; let testdir = &testdir; - let mut child = testdir.spawn_child(&["start"])?; + let mut child = testdir.spawn_child(args!["start"])?; // Run the program and kill it after a few seconds std::thread::sleep(LAUNCH_DELAY); child.kill()?; @@ -212,7 +213,7 @@ fn start_args() -> Result<()> { output.assert_failure()?; // unrecognized option `-f` - let child = testdir.spawn_child(&["start", "-f"])?; + let child = testdir.spawn_child(args!["start", "-f"])?; let output = child.wait_with_output()?; output.assert_failure()?; @@ -226,7 +227,7 @@ fn persistent_mode() -> Result<()> { let testdir = testdir()?.with_config(&mut persistent_test_config()?)?; let testdir = &testdir; - let mut child = testdir.spawn_child(&["-v", "start"])?; + let mut child = testdir.spawn_child(args!["-v", "start"])?; // Run the program and kill it after a few seconds std::thread::sleep(LAUNCH_DELAY); @@ -295,7 +296,7 @@ fn ephemeral(cache_dir_config: EphemeralConfig, cache_dir_check: EphemeralCheck) let mut child = run_dir .path() .with_config(&mut config)? - .spawn_child(&["start"])?; + .spawn_child(args!["start"])?; // Run the program and kill it after a few seconds std::thread::sleep(LAUNCH_DELAY); child.kill()?; @@ -370,7 +371,7 @@ fn app_no_args() -> Result<()> { let testdir = testdir()?.with_config(&mut default_test_config()?)?; - let child = testdir.spawn_child(&[])?; + let child = testdir.spawn_child(args![])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -385,7 +386,7 @@ fn version_no_args() -> Result<()> { let testdir = testdir()?.with_config(&mut default_test_config()?)?; - let child = testdir.spawn_child(&["version"])?; + let child = testdir.spawn_child(args!["version"])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -408,12 +409,12 @@ fn version_args() -> Result<()> { let testdir = &testdir; // unexpected free argument `argument` - let child = testdir.spawn_child(&["version", "argument"])?; + let child = testdir.spawn_child(args!["version", "argument"])?; let output = child.wait_with_output()?; output.assert_failure()?; // unrecognized option `-f` - let child = testdir.spawn_child(&["version", "-f"])?; + let child = testdir.spawn_child(args!["version", "-f"])?; let output = child.wait_with_output()?; output.assert_failure()?; @@ -441,7 +442,7 @@ fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> R // Generate configuration in temp dir path let child = - testdir.spawn_child(&["generate", "-o", generated_config_path.to_str().unwrap()])?; + testdir.spawn_child(args!["generate", "-o": generated_config_path.to_str().unwrap()])?; let output = child.wait_with_output()?; let output = output.assert_success()?; @@ -453,7 +454,7 @@ fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> R ); // Run command using temp dir and kill it after a few seconds - let mut child = testdir.spawn_child(&[command])?; + let mut child = testdir.spawn_child(args![command])?; std::thread::sleep(LAUNCH_DELAY); child.kill()?; @@ -792,7 +793,7 @@ async fn metrics_endpoint() -> Result<()> { config.metrics.endpoint_addr = Some(endpoint.parse().unwrap()); let dir = testdir()?.with_config(&mut config)?; - let child = dir.spawn_child(&["start"])?; + let child = dir.spawn_child(args!["start"])?; // Run `zebrad` for a few seconds before testing the endpoint // Since we're an async function, we have to use a sleep future, not thread sleep. @@ -848,7 +849,7 @@ async fn tracing_endpoint() -> Result<()> { config.tracing.endpoint_addr = Some(endpoint.parse().unwrap()); let dir = testdir()?.with_config(&mut config)?; - let child = dir.spawn_child(&["start"])?; + let child = dir.spawn_child(args!["start"])?; // Run `zebrad` for a few seconds before testing the endpoint // Since we're an async function, we have to use a sleep future, not thread sleep. @@ -941,7 +942,7 @@ async fn rpc_endpoint() -> Result<()> { let url = format!("http://{}", config.rpc.listen_addr.unwrap()); let dir = testdir()?.with_config(&mut config)?; - let mut child = dir.spawn_child(&["start"])?; + let mut child = dir.spawn_child(args!["start"])?; // Wait until port is open. child.expect_stdout_line_matches( @@ -1132,7 +1133,7 @@ fn lightwalletd_integration() -> Result<()> { let zdir = testdir()?.with_config(&mut config)?; let mut zebrad = zdir - .spawn_child(&["start"])? + .spawn_child(args!["start"])? .with_timeout(LAUNCH_DELAY) .with_failure_regex_iter( // TODO: replace with a function that returns the full list and correct return type @@ -1155,7 +1156,7 @@ fn lightwalletd_integration() -> Result<()> { let ldir = ldir.with_lightwalletd_config(config.rpc.listen_addr.unwrap())?; // Launch the lightwalletd process - let result = ldir.spawn_lightwalletd_child(&[]); + let result = ldir.spawn_lightwalletd_child(args![]); let (lightwalletd, zebrad) = zebrad.kill_on_error(result)?; let mut lightwalletd = lightwalletd .with_timeout(LIGHTWALLETD_DELAY) @@ -1417,7 +1418,7 @@ where U: ZebradTestDirExt, { // Start the first node - let mut node1 = first_dir.spawn_child(&["start"])?; + let mut node1 = first_dir.spawn_child(args!["start"])?; // Wait until node1 has used the conflicting resource. node1.expect_stdout_line_matches(first_stdout_regex)?; @@ -1426,7 +1427,7 @@ where std::thread::sleep(BETWEEN_NODES_DELAY); // Spawn the second node - let node2 = second_dir.spawn_child(&["start"]); + let node2 = second_dir.spawn_child(args!["start"]); let (node2, mut node1) = node1.kill_on_error(node2)?; // Wait a few seconds and kill first node. diff --git a/zebrad/tests/common/launch.rs b/zebrad/tests/common/launch.rs index 82eae13b..8b4973be 100644 --- a/zebrad/tests/common/launch.rs +++ b/zebrad/tests/common/launch.rs @@ -11,7 +11,10 @@ use color_eyre::eyre::Result; use zebrad::config::ZebradConfig; -use zebra_test::{command::TestDirExt, prelude::*}; +use zebra_test::{ + command::{Arguments, TestDirExt}, + prelude::*, +}; /// After we launch `zebrad`, wait this long for the command to start up, /// take the actions expected by the tests, and log the expected logs. @@ -44,7 +47,7 @@ where /// child process. /// /// If there is a config in the test directory, pass it to `zebrad`. - fn spawn_child(self, args: &[&str]) -> Result>; + fn spawn_child(self, args: Arguments) -> Result>; /// Create a config file and use it for all subsequently spawned `zebrad` processes. /// Returns an error if the config already exists. @@ -89,23 +92,24 @@ impl ZebradTestDirExt for T where Self: TestDirExt + AsRef + Sized, { - fn spawn_child(self, args: &[&str]) -> Result> { + fn spawn_child(self, extra_args: Arguments) -> Result> { let dir = self.as_ref(); let default_config_path = dir.join("zebrad.toml"); + let mut args = Arguments::new(); if default_config_path.exists() { - let mut extra_args: Vec<_> = vec![ + args.set_parameter( "-c", default_config_path .as_path() .to_str() .expect("Path is valid Unicode"), - ]; - extra_args.extend_from_slice(args); - self.spawn_child_with_command(env!("CARGO_BIN_EXE_zebrad"), &extra_args) - } else { - self.spawn_child_with_command(env!("CARGO_BIN_EXE_zebrad"), args) + ); } + + args.merge_with(extra_args); + + self.spawn_child_with_command(env!("CARGO_BIN_EXE_zebrad"), args) } fn with_config(self, config: &mut ZebradConfig) -> Result { diff --git a/zebrad/tests/common/lightwalletd.rs b/zebrad/tests/common/lightwalletd.rs index 2b6c3cc1..7ff2dcfb 100644 --- a/zebrad/tests/common/lightwalletd.rs +++ b/zebrad/tests/common/lightwalletd.rs @@ -8,7 +8,7 @@ use std::{env, net::SocketAddr, path::Path}; use zebra_test::{ - command::{TestChild, TestDirExt}, + command::{Arguments, TestChild, TestDirExt}, net::random_known_port, prelude::*, }; @@ -78,7 +78,7 @@ where /// # Panics /// /// If there is no lightwalletd config in the test directory. - fn spawn_lightwalletd_child(self, args: &[&str]) -> Result>; + fn spawn_lightwalletd_child(self, extra_args: Arguments) -> Result>; /// Create a config file and use it for all subsequently spawned `lightwalletd` processes. /// Returns an error if the config already exists. @@ -92,7 +92,7 @@ impl LightWalletdTestDirExt for T where Self: TestDirExt + AsRef + Sized, { - fn spawn_lightwalletd_child(self, extra_args: &[&str]) -> Result> { + fn spawn_lightwalletd_child(self, extra_args: Arguments) -> Result> { let dir = self.as_ref().to_owned(); let default_config_path = dir.join("lightwalletd-zcash.conf"); @@ -103,35 +103,37 @@ where // By default, launch a working test instance with logging, // and avoid port conflicts. - let mut args: Vec<_> = vec![ - // the fake zcashd conf we just wrote - "--zcash-conf-path", - default_config_path - .as_path() - .to_str() - .expect("Path is valid Unicode"), - // the lightwalletd cache directory - // - // TODO: create a sub-directory for lightwalletd - "--data-dir", - dir.to_str().expect("Path is valid Unicode"), - // log to standard output - // - // TODO: if lightwalletd needs to run on Windows, - // work out how to log to the terminal on all platforms - "--log-file", - "/dev/stdout", - // let the OS choose a random available wallet client port - "--grpc-bind-addr", - "127.0.0.1:0", - "--http-bind-addr", - "127.0.0.1:0", - // don't require a TLS certificate for the HTTP server - "--no-tls-very-insecure", - ]; - args.extend_from_slice(extra_args); + let mut args = Arguments::new(); - self.spawn_child_with_command("lightwalletd", &args) + // the fake zcashd conf we just wrote + let zcash_conf_path = default_config_path + .as_path() + .to_str() + .expect("Path is valid Unicode"); + args.set_parameter("--zcash-conf-path", zcash_conf_path); + + // the lightwalletd cache directory + // + // TODO: create a sub-directory for lightwalletd + args.set_parameter("--data-dir", dir.to_str().expect("Path is valid Unicode")); + + // log to standard output + // + // TODO: if lightwalletd needs to run on Windows, + // work out how to log to the terminal on all platforms + args.set_parameter("--log-file", "/dev/stdout"); + + // let the OS choose a random available wallet client port + args.set_parameter("--grpc-bind-addr", "127.0.0.1:0"); + args.set_parameter("--http-bind-addr", "127.0.0.1:0"); + + // don't require a TLS certificate for the HTTP server + args.set_argument("--no-tls-very-insecure"); + + // apply user provided arguments + args.merge_with(extra_args); + + self.spawn_child_with_command("lightwalletd", args) } fn with_lightwalletd_config(self, zebra_rpc_listener: SocketAddr) -> Result { diff --git a/zebrad/tests/common/sync.rs b/zebrad/tests/common/sync.rs index 12da493a..0dbabbff 100644 --- a/zebrad/tests/common/sync.rs +++ b/zebrad/tests/common/sync.rs @@ -13,7 +13,7 @@ use tempfile::TempDir; use zebra_chain::{block::Height, parameters::Network}; use zebrad::{components::sync, config::ZebradConfig}; -use zebra_test::prelude::*; +use zebra_test::{args, prelude::*}; use super::{ config::{persistent_test_config, testdir}, @@ -195,7 +195,7 @@ pub fn sync_until( testdir()?.with_config(&mut config)? }; - let mut child = tempdir.spawn_child(&["start"])?.with_timeout(timeout); + let mut child = tempdir.spawn_child(args!["start"])?.with_timeout(timeout); let network = format!("network: {},", network); @@ -327,7 +327,7 @@ pub fn create_cached_database_height( let dir = PathBuf::from("/zebrad-cache"); let mut child = dir .with_exact_config(&config)? - .spawn_child(&["start"])? + .spawn_child(args!["start"])? .with_timeout(timeout) .bypass_test_capture(true);