From 06fa52ef5a8857fcbc9ca5cb3ed6bff692da8978 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Wed, 24 Aug 2022 17:54:33 +1000
Subject: [PATCH] Improve tests

---
 .../src/validators/import_validators.rs       |  9 +-----
 .../src/validators/move_validators.rs         | 32 ++++++++++++++-----
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/validator_manager/src/validators/import_validators.rs b/validator_manager/src/validators/import_validators.rs
index 90e5db87f3..c551e552a5 100644
--- a/validator_manager/src/validators/import_validators.rs
+++ b/validator_manager/src/validators/import_validators.rs
@@ -283,14 +283,7 @@ pub mod tests {
                         .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.
-                    //
-                    // See: https://github.com/sigp/lighthouse/pull/3490
-                    //
-                    // If that PR changes we'll need to change this line.
-                    assert_eq!(remote_validator.readonly, None);
+                    assert_eq!(remote_validator.readonly, Some(false));
                 }
             }
 
diff --git a/validator_manager/src/validators/move_validators.rs b/validator_manager/src/validators/move_validators.rs
index 11abfd6734..127dc68980 100644
--- a/validator_manager/src/validators/move_validators.rs
+++ b/validator_manager/src/validators/move_validators.rs
@@ -29,6 +29,8 @@ pub const GAS_LIMIT_FLAG: &str = "gas-limit";
 pub const FEE_RECIPIENT_FLAG: &str = "suggested-fee-recipient";
 pub const BUILDER_PROPOSALS_FLAG: &str = "builder-proposals";
 
+const NO_VALIDATORS_MSG: &str = "No validators present on source validator client";
+
 pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
     App::new(CMD)
         .about(
@@ -248,6 +250,10 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> {
     let (dest_http_client, _dest_keystores) =
         vc_http_client(dest_vc_url.clone(), &dest_vc_token_path).await?;
 
+    if src_keystores.is_empty() {
+        return Err(NO_VALIDATORS_MSG.to_string());
+    }
+
     let pubkeys_to_move = match validators {
         Validators::All => src_keystores.iter().map(|v| v.validating_pubkey).collect(),
         Validators::Some(request_pubkeys) => {
@@ -561,13 +567,14 @@ mod test {
         where
             F: Fn(&[PublicKeyBytes]) -> Validators,
         {
-            let import_test_result = self
-                .import_builder
-                .expect("test requires an import builder")
-                .run_test()
-                .await;
-            assert!(import_test_result.result.is_ok());
-            let src_vc = import_test_result.vc;
+            let src_vc = if let Some(import_builder) = self.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 src_vc_token_path = self.dir.path().join(SRC_VC_TOKEN_FILE_NAME);
             fs::write(&src_vc_token_path, &src_vc.api_token).unwrap();
             let (src_vc_client, src_vc_initial_keystores) =
@@ -599,7 +606,7 @@ mod test {
 
             let result = run(move_config).await;
 
-            let (dest_vc_client, dest_vc_keystores) =
+            let (_dest_vc_client, dest_vc_keystores) =
                 vc_http_client(dest_vc.url.clone(), &dest_vc_token_path)
                     .await
                     .unwrap();
@@ -634,6 +641,15 @@ mod test {
         }
     }
 
+    #[tokio::test]
+    async fn no_validators() {
+        TestBuilder::new()
+            .await
+            .run_test(|_| Validators::All)
+            .await
+            .assert_err_is(NO_VALIDATORS_MSG.to_string());
+    }
+
     #[tokio::test]
     async fn one_validator_move_all() {
         TestBuilder::new()