From 0c786c0ee241f2c798a1dc347ca8c4f23cb825fc Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 25 Aug 2022 10:26:55 +1000 Subject: [PATCH] Add tests for duplicates --- .../src/validators/move_validators.rs | 123 +++++++++++++++--- 1 file changed, 107 insertions(+), 16 deletions(-) diff --git a/validator_manager/src/validators/move_validators.rs b/validator_manager/src/validators/move_validators.rs index f9576260dd..f12c22f0cd 100644 --- a/validator_manager/src/validators/move_validators.rs +++ b/validator_manager/src/validators/move_validators.rs @@ -393,7 +393,8 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> { }; // We might as well just ignore validators that already exist on the destination machine, - // there doesn't appear to be much harm just adding them again. + // there doesn't appear to be much harm just adding them again and removing them from the + // source VC is an improvement. let ignore_duplicates = true; loop { @@ -531,7 +532,9 @@ mod test { const DEST_VC_TOKEN_FILE_NAME: &str = "dest_vc_token.json"; struct TestBuilder { - import_builder: Option, + src_import_builder: Option, + dest_import_builder: Option, + duplicates: usize, dir: TempDir, } @@ -539,7 +542,9 @@ mod test { async fn new() -> Self { let dir = tempdir().unwrap(); Self { - import_builder: None, + src_import_builder: None, + dest_import_builder: None, + duplicates: 0, dir, } } @@ -549,7 +554,21 @@ mod test { .await .create_validators(count, first_index) .await; - self.import_builder = Some(builder); + self.src_import_builder = Some(builder); + self + } + + async fn with_dest_validators(mut self, count: u32, first_index: u32) -> Self { + let builder = ImportTestBuilder::new() + .await + .create_validators(count, first_index) + .await; + self.dest_import_builder = Some(builder); + self + } + + fn register_duplicates(mut self, num_duplicates: usize) -> Self { + self.duplicates = num_duplicates; self } @@ -557,7 +576,7 @@ mod test { where F: Fn(&[PublicKeyBytes]) -> Validators, { - let src_vc = if let Some(import_builder) = self.import_builder { + let src_vc = if let Some(import_builder) = self.src_import_builder { let import_test_result = import_builder.run_test().await; assert!(import_test_result.result.is_ok()); import_test_result.vc @@ -578,10 +597,21 @@ mod test { .collect(); let validators = gen_validators_enum(&src_vc_initial_pubkeys); - let dest_vc = ApiTester::new().await; + let dest_vc = if let Some(import_builder) = self.dest_import_builder { + let import_test_result = import_builder.run_test().await; + assert!(import_test_result.result.is_ok()); + import_test_result.vc + } else { + ApiTester::new().await + }; let dest_vc_token_path = self.dir.path().join(DEST_VC_TOKEN_FILE_NAME); fs::write(&dest_vc_token_path, &dest_vc.api_token).unwrap(); + let (dest_vc_client, dest_vc_initial_keystores) = + vc_http_client(dest_vc.url.clone(), &dest_vc_token_path) + .await + .unwrap(); + let move_config = MoveConfig { src_vc_url: src_vc.url.clone(), src_vc_token_path, @@ -595,17 +625,26 @@ mod test { let result = run(move_config).await; - let (_dest_vc_client, dest_vc_keystores) = - vc_http_client(dest_vc.url.clone(), &dest_vc_token_path) - .await - .unwrap(); let src_vc_final_keystores = src_vc_client.get_keystores().await.unwrap().data; + let dest_vc_final_keystores = dest_vc_client.get_keystores().await.unwrap().data; match validators { Validators::All => { - assert!(src_vc_final_keystores.is_empty()); + assert!( + src_vc_final_keystores.is_empty(), + "all keystores should be removed from source vc" + ); + assert_eq!( + dest_vc_final_keystores.len(), + dest_vc_initial_keystores.len() + src_vc_initial_keystores.len() + - self.duplicates, + "the correct count of keystores should have been moved to the dest" + ); for initial_keystore in &src_vc_initial_keystores { - assert!(dest_vc_keystores.contains(initial_keystore)) + assert!( + dest_vc_final_keystores.contains(initial_keystore), + "the source keystore should be present at the dest" + ) } } Validators::Some(pubkeys) => { @@ -614,16 +653,27 @@ mod test { src_vc_initial_keystores .len() .checked_sub(pubkeys.len()) - .unwrap() + .unwrap(), + "the correct count of validators should have been removed from the src" + ); + assert_eq!( + dest_vc_final_keystores.len(), + dest_vc_initial_keystores.len() + pubkeys.len() - self.duplicates, + "the correct count of keystores should have been moved to the dest" ); - assert_eq!(dest_vc_keystores.len(), pubkeys.len()); for pubkey in pubkeys { let initial_keystore = src_vc_initial_keystores .iter() .find(|k| k.validating_pubkey == pubkey) .unwrap(); - assert!(!src_vc_final_keystores.contains(initial_keystore)); - assert!(dest_vc_keystores.contains(initial_keystore)); + assert!( + !src_vc_final_keystores.contains(initial_keystore), + "the keystore should not be present at the source" + ); + assert!( + dest_vc_final_keystores.contains(initial_keystore), + "the keystore should be present at the dest" + ); } } } @@ -678,6 +728,47 @@ mod test { .assert_ok(); } + #[tokio::test] + async fn one_validator_to_non_empty_dest() { + TestBuilder::new() + .await + .with_src_validators(1, 0) + .await + .with_dest_validators(1, 10) + .await + .run_test(|_| Validators::All) + .await + .assert_ok(); + } + + #[tokio::test] + async fn two_validators_move_all_where_one_is_a_duplicate() { + TestBuilder::new() + .await + .with_src_validators(2, 0) + .await + .with_dest_validators(1, 1) + .await + .register_duplicates(1) + .run_test(|_| Validators::All) + .await + .assert_ok(); + } + + #[tokio::test] + async fn two_validators_move_one_where_one_is_a_duplicate() { + TestBuilder::new() + .await + .with_src_validators(2, 0) + .await + .with_dest_validators(2, 0) + .await + .register_duplicates(1) + .run_test(|pubkeys| Validators::Some(pubkeys[0..1].to_vec())) + .await + .assert_ok(); + } + #[tokio::test] async fn three_validators_move_all() { TestBuilder::new()