From 7ed821bbe145afeef6fb70a3a82df5f413048f50 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 22 Aug 2022 11:35:13 +1000 Subject: [PATCH] Progress with testing --- .../src/validators/create_validators.rs | 53 ++++---- .../src/validators/import_validators.rs | 123 +++++++++++++++++- 2 files changed, 148 insertions(+), 28 deletions(-) diff --git a/validator_manager/src/validators/create_validators.rs b/validator_manager/src/validators/create_validators.rs index 2a7b050d14..0c5869c4a8 100644 --- a/validator_manager/src/validators/create_validators.rs +++ b/validator_manager/src/validators/create_validators.rs @@ -181,20 +181,20 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { /// The CLI arguments are parsed into this struct before running the application. This step of /// indirection allows for testing the underlying logic without needing to parse CLI arguments. #[derive(Clone)] -struct CreateConfig { - output_path: PathBuf, - first_index: u32, - count: u32, - deposit_gwei: u64, - mnemonic_path: Option, - stdin_inputs: bool, - disable_deposits: bool, - specify_voting_keystore_password: bool, - eth1_withdrawal_address: Option
, - builder_proposals: bool, - fee_recipient: Option
, - gas_limit: Option, - bn_url: Option, +pub struct CreateConfig { + pub output_path: PathBuf, + pub first_index: u32, + pub count: u32, + pub deposit_gwei: u64, + pub mnemonic_path: Option, + pub stdin_inputs: bool, + pub disable_deposits: bool, + pub specify_voting_keystore_password: bool, + pub eth1_withdrawal_address: Option
, + pub builder_proposals: bool, + pub fee_recipient: Option
, + pub gas_limit: Option, + pub bn_url: Option, } impl CreateConfig { @@ -518,7 +518,7 @@ fn write_to_json_file, S: Serialize>(path: P, contents: &S) -> Re } #[cfg(test)] -mod tests { +pub mod tests { use super::*; use eth2_network_config::Eth2NetworkConfig; use regex::Regex; @@ -530,7 +530,7 @@ mod tests { const TEST_VECTOR_DEPOSIT_CLI_VERSION: &str = "2.3.0"; - struct TestBuilder { + pub struct TestBuilder { spec: ChainSpec, output_dir: TempDir, mnemonic_dir: TempDir, @@ -544,7 +544,7 @@ mod tests { } impl TestBuilder { - fn new(spec: ChainSpec) -> Self { + pub fn new(spec: ChainSpec) -> Self { let output_dir = tempdir().unwrap(); let mnemonic_dir = tempdir().unwrap(); let mnemonic_path = mnemonic_dir.path().join("mnemonic"); @@ -578,12 +578,12 @@ mod tests { } } - fn mutate_config(mut self, func: F) -> Self { + pub fn mutate_config(mut self, func: F) -> Self { func(&mut self.config); self } - async fn run_test(self) -> TestResult { + pub async fn run_test(self) -> TestResult { let Self { spec, output_dir, @@ -686,12 +686,21 @@ mod tests { } #[must_use] // Use the `assert_ok` or `assert_err` fns to "use" this value. - struct TestResult { - result: Result<(), String>, - output_dir: TempDir, + pub struct TestResult { + pub result: Result<(), String>, + pub output_dir: TempDir, } impl TestResult { + pub fn validators_file_path(&self) -> PathBuf { + self.output_dir.path().join(VALIDATORS_FILENAME) + } + + pub fn validators(&self) -> Vec { + let contents = fs::read_to_string(self.validators_file_path()).unwrap(); + serde_json::from_str(&contents).unwrap() + } + fn assert_ok(self) { assert_eq!(self.result, Ok(())) } diff --git a/validator_manager/src/validators/import_validators.rs b/validator_manager/src/validators/import_validators.rs index 4ee6b0247a..9f10189131 100644 --- a/validator_manager/src/validators/import_validators.rs +++ b/validator_manager/src/validators/import_validators.rs @@ -225,17 +225,20 @@ async fn run<'a>(config: ImportConfig) -> Result<(), String> { #[cfg(test)] mod test { use super::*; + use crate::validators::create_validators::tests::TestBuilder as CreateTestBuilder; use std::fs; use tempfile::{tempdir, TempDir}; use validator_client::http_api::test_utils::ApiTester; - const VALIDATORS_FILE_NAME: &str = "validators.json"; const VC_TOKEN_FILE_NAME: &str = "vc_token.json"; struct TestBuilder { - config: ImportConfig, + import_config: ImportConfig, vc: ApiTester, dir: TempDir, + /// Holds the temp directory owned by the `CreateTestBuilder` so it doesn't get cleaned-up + /// before we can read it. + create_dir: Option, } impl TestBuilder { @@ -246,20 +249,128 @@ mod test { fs::write(&vc_token_path, &vc.api_token).unwrap(); Self { - config: ImportConfig { - validators_file_path: dir.path().join(VALIDATORS_FILE_NAME), + import_config: ImportConfig { + // This field will be overwritten later on. + validators_file_path: dir.path().into(), vc_url: vc.url.clone(), vc_token_path, ignore_duplicates: false, }, vc, dir, + create_dir: None, } } + + async fn create_validators(mut self, count: u32, first_index: u32) -> Self { + let create_result = CreateTestBuilder::default() + .mutate_config(|config| { + config.count = count; + config.first_index = first_index; + }) + .run_test() + .await; + assert!( + create_result.result.is_ok(), + "precondition: validators are created" + ); + self.import_config.validators_file_path = create_result.validators_file_path(); + self.create_dir = Some(create_result.output_dir); + self + } + + async fn run_test(self) -> TestResult { + let result = run(self.import_config.clone()).await; + + if result.is_ok() { + let local_validators: Vec = { + let contents = + fs::read_to_string(&self.import_config.validators_file_path).unwrap(); + serde_json::from_str(&contents).unwrap() + }; + let list_keystores_response = self.vc.client.get_keystores().await.unwrap().data; + + assert_eq!( + local_validators.len(), + list_keystores_response.len(), + "vc should have exactly the number of validators imported" + ); + + for local_validator in &local_validators { + let local_keystore = &local_validator.voting_keystore.0; + let local_pubkey = local_keystore.public_key().unwrap().into(); + let remote_validator = list_keystores_response + .iter() + .find(|validator| validator.validating_pubkey == local_pubkey) + .expect("validator must exist on VC"); + assert_eq!(&remote_validator.derivation_path, &local_keystore.path()); + // It's not immediately clear why Lighthouse returns `None` rather than + // `Some(false)` here, I would expect the latter to be the most accurate. + // However, it doesn't seem like a big deal. + assert_eq!(remote_validator.readonly, None); + } + } + + TestResult { result } + } + } + + #[must_use] // Use the `assert_ok` or `assert_err` fns to "use" this value. + struct TestResult { + result: Result<(), String>, + } + + impl TestResult { + fn assert_ok(self) { + assert_eq!(self.result, Ok(())) + } + + fn assert_err(self) { + assert!(self.result.is_err()) + } } #[tokio::test] - async fn blah() { - TestBuilder::new().await; + async fn create_one_validator() { + TestBuilder::new() + .await + .create_validators(1, 0) + .await + .run_test() + .await + .assert_ok(); + } + + #[tokio::test] + async fn create_three_validators() { + TestBuilder::new() + .await + .create_validators(3, 0) + .await + .run_test() + .await + .assert_ok(); + } + + #[tokio::test] + async fn create_one_validator_with_offset() { + TestBuilder::new() + .await + .create_validators(1, 42) + .await + .run_test() + .await + .assert_ok(); + } + + #[tokio::test] + async fn create_three_validators_with_offset() { + TestBuilder::new() + .await + .create_validators(3, 1337) + .await + .run_test() + .await + .assert_ok(); } }