GH-662: Write tests for usages of 'panic_on_migration()' (#267)

* GH-662: Established a single function to test panic_on_migration

* GH-662: Tested a frew more occurrences

* GH-662: Finished last test

* GH-662: Fixed all warning messages & formatting

* GH-622: Decreased last_remapped duration for maybe_remap_handles_remapping_error

* Revert "GH-622: Decreased last_remapped duration for maybe_remap_handles_remapping_error"

This reverts commit 5d1e009.

* GH-662: Amended Subjects name

Author: Syther007

node/src/accountant/mod.rs

@@ -1008,7 +1008,7 @@ mod tests {
 
     use crate::accountant::dao_utils::from_time_t;
     use crate::accountant::dao_utils::{to_time_t, CustomQuery};
-    use crate::accountant::payable_dao::{PayableAccount, PayableDaoError};
+    use crate::accountant::payable_dao::{PayableAccount, PayableDaoError, PayableDaoFactory};
     use crate::accountant::pending_payable_dao::PendingPayableDaoError;
     use crate::accountant::receivable_dao::ReceivableAccount;
     use crate::accountant::scanners::{BeginScanError, NullScanner, ScannerMock};
@@ -1043,10 +1043,12 @@ mod tests {
     use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock;
     use crate::test_utils::unshared_test_utils::system_killer_actor::SystemKillerActor;
     use crate::test_utils::unshared_test_utils::{
-        make_bc_with_defaults, prove_that_crash_request_handler_is_hooked_up, AssertionsMessage,
+        assert_on_initialization_with_panic_on_migration, make_bc_with_defaults,
+        prove_that_crash_request_handler_is_hooked_up, AssertionsMessage,
     };
     use crate::test_utils::{make_paying_wallet, make_wallet};
     use masq_lib::messages::TopRecordsOrdering::{Age, Balance};
+    use masq_lib::test_utils::utils::ensure_node_home_directory_exists;
     use masq_lib::ui_gateway::MessagePath::Conversation;
     use web3::types::{TransactionReceipt, H256};
 
@@ -4338,4 +4340,19 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn make_dao_factory_uses_panic_on_migration() {
+        let data_dir = ensure_node_home_directory_exists(
+            "accountant",
+            "make_dao_factory_uses_panic_on_migration",
+        );
+
+        let act = |data_dir: &Path| {
+            let factory = Accountant::dao_factory(data_dir);
+            factory.make();
+        };
+
+        assert_on_initialization_with_panic_on_migration(&data_dir, &act);
+    }
 }

node/src/actor_system_factory.rs

@@ -49,6 +49,7 @@ use masq_lib::logger::Logger;
 use masq_lib::ui_gateway::{NodeFromUiMessage, NodeToUiMessage};
 use masq_lib::utils::{exit_process, AutomapProtocol};
 use std::net::{IpAddr, Ipv4Addr};
+use std::path::Path;
 
 pub trait ActorSystemFactory {
     fn make_and_start_actors(
@@ -453,11 +454,7 @@ impl ActorFactory for ActorFactoryReal {
         let pending_payable_dao_factory = Box::new(Accountant::dao_factory(data_directory));
         let receivable_dao_factory = Box::new(Accountant::dao_factory(data_directory));
         let banned_dao_factory = Box::new(Accountant::dao_factory(data_directory));
-        banned_cache_loader.load(connection_or_panic(
-            db_initializer,
-            data_directory,
-            DbInitializationConfig::panic_on_migration(),
-        ));
+        Self::load_banned_cache(db_initializer, banned_cache_loader, data_directory);
         let arbiter = Arbiter::builder().stop_system_on_panic(true);
         let addr: Addr<Accountant> = arbiter.start(move |_| {
             Accountant::new(
@@ -543,6 +540,20 @@ impl ActorFactory for ActorFactoryReal {
     }
 }
 
+impl ActorFactoryReal {
+    fn load_banned_cache(
+        db_initializer: &dyn DbInitializer,
+        banned_cache_loader: &dyn BannedCacheLoader,
+        data_directory: &Path,
+    ) {
+        banned_cache_loader.load(connection_or_panic(
+            db_initializer,
+            data_directory,
+            DbInitializationConfig::panic_on_migration(),
+        ));
+    }
+}
+
 fn is_crashable(config: &BootstrapperConfig) -> bool {
     config.crash_point == CrashPoint::Message
 }
@@ -639,6 +650,7 @@ mod tests {
     };
     use crate::test_utils::recorder::{make_recorder, Recorder};
     use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp;
+    use crate::test_utils::unshared_test_utils::assert_on_initialization_with_panic_on_migration;
     use crate::test_utils::unshared_test_utils::system_killer_actor::SystemKillerActor;
     use crate::test_utils::{alias_cryptde, rate_pack};
     use crate::test_utils::{main_cryptde, make_cryptde_pair};
@@ -2081,6 +2093,24 @@ mod tests {
         assert_eq!(msg, &msg_of_irrelevant_choice);
     }
 
+    #[test]
+    fn load_banned_cache_implements_panic_on_migration() {
+        let data_dir = ensure_node_home_directory_exists(
+            "actor_system_factory",
+            "load_banned_cache_implements_panic_on_migration",
+        );
+
+        let act = |data_dir: &Path| {
+            ActorFactoryReal::load_banned_cache(
+                &DbInitializerReal::default(),
+                &BannedCacheLoaderMock::default(),
+                &data_dir,
+            );
+        };
+
+        assert_on_initialization_with_panic_on_migration(&data_dir, &act);
+    }
+
     fn public_key_for_dyn_cryptde_being_null(cryptde: &dyn CryptDE) -> &PublicKey {
         let null_cryptde = <&CryptDENull>::from(cryptde);
         null_cryptde.public_key()

node/src/blockchain/blockchain_bridge.rs

@@ -11,7 +11,7 @@ use crate::blockchain::blockchain_interface::{
     BlockchainInterfaceNonClandestine, BlockchainResult, BlockchainTxnInputs,
 };
 use crate::database::db_initializer::DbInitializationConfig;
-use crate::database::db_initializer::{DbInitializer, DATABASE_FILE};
+use crate::database::db_initializer::DbInitializer;
 use crate::db_config::config_dao::ConfigDaoReal;
 use crate::db_config::persistent_configuration::{
     PersistentConfiguration, PersistentConfigurationReal,
@@ -20,7 +20,7 @@ use crate::sub_lib::blockchain_bridge::{BlockchainBridgeSubs, RequestBalancesToP
 use crate::sub_lib::blockchain_bridge::{ConsumingWalletBalances, ReportAccountsPayable};
 use crate::sub_lib::peer_actors::BindMessage;
 use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage;
-use crate::sub_lib::utils::handle_ui_crash_request;
+use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request};
 use crate::sub_lib::wallet::Wallet;
 use actix::Actor;
 use actix::Context;
@@ -230,12 +230,7 @@ impl BlockchainBridge {
                     &data_directory,
                     DbInitializationConfig::panic_on_migration(),
                 )
-                .unwrap_or_else(|_| {
-                    panic!(
-                        "Failed to connect to database at {:?}",
-                        &data_directory.join(DATABASE_FILE)
-                    )
-                }),
+                .unwrap_or_else(|err| db_connection_launch_panic(err, &data_directory)),
         ));
         (
             blockchain_interface,
@@ -508,6 +503,7 @@ mod tests {
     use crate::blockchain::test_utils::BlockchainInterfaceMock;
     use crate::blockchain::tool_wrappers::SendTransactionToolsWrapperNull;
     use crate::database::db_initializer::test_utils::DbInitializerMock;
+    use crate::database::db_initializer::DbInitializerReal;
     use crate::db_config::persistent_configuration::PersistentConfigError;
     use crate::match_every_type_id;
     use crate::node_test_utils::check_timestamp;
@@ -517,7 +513,8 @@ mod tests {
     use crate::test_utils::recorder_stop_conditions::StopCondition;
     use crate::test_utils::recorder_stop_conditions::StopConditions;
     use crate::test_utils::unshared_test_utils::{
-        configure_default_persistent_config, prove_that_crash_request_handler_is_hooked_up, ZERO,
+        assert_on_initialization_with_panic_on_migration, configure_default_persistent_config,
+        prove_that_crash_request_handler_is_hooked_up, ZERO,
     };
     use crate::test_utils::{make_paying_wallet, make_wallet};
     use actix::System;
@@ -527,8 +524,10 @@ mod tests {
     use masq_lib::messages::ScanType;
     use masq_lib::test_utils::logging::init_test_logging;
     use masq_lib::test_utils::logging::TestLogHandler;
+    use masq_lib::test_utils::utils::ensure_node_home_directory_exists;
     use rustc_hex::FromHex;
     use std::any::TypeId;
+    use std::path::Path;
     use std::sync::{Arc, Mutex};
     use std::time::{Duration, SystemTime};
     use web3::types::{TransactionReceipt, H160, H256, U256};
@@ -1713,4 +1712,23 @@ mod tests {
 
         prove_that_crash_request_handler_is_hooked_up(subject, CRASH_KEY);
     }
+
+    #[test]
+    fn make_connections_implements_panic_on_migration() {
+        let data_dir = ensure_node_home_directory_exists(
+            "blockchain_bridge",
+            "make_connections_with_panic_on_migration",
+        );
+
+        let act = |data_dir: &Path| {
+            BlockchainBridge::make_connections(
+                Some("http://127.0.0.1".to_string()),
+                &DbInitializerReal::default(),
+                data_dir.to_path_buf(),
+                Chain::PolyMumbai,
+            );
+        };
+
+        assert_on_initialization_with_panic_on_migration(&data_dir, &act);
+    }
 }

node/src/bootstrapper.rs

@@ -21,6 +21,7 @@ use crate::node_configurator::node_configurator_standard::{
 use crate::node_configurator::{initialize_database, DirsWrapper, NodeConfigurator};
 use crate::privilege_drop::{IdWrapper, IdWrapperReal};
 use crate::server_initializer::LoggerInitializerWrapper;
+use crate::stream_handler_pool::StreamHandlerPoolSubs;
 use crate::sub_lib::accountant;
 use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals};
 use crate::sub_lib::blockchain_bridge::BlockchainBridgeConfig;
@@ -32,6 +33,7 @@ use crate::sub_lib::neighborhood::{NeighborhoodConfig, NeighborhoodMode};
 use crate::sub_lib::node_addr::NodeAddr;
 use crate::sub_lib::socket_server::ConfiguredByPrivilege;
 use crate::sub_lib::ui_gateway::UiGatewayConfig;
+use crate::sub_lib::utils::db_connection_launch_panic;
 use crate::sub_lib::wallet::Wallet;
 use futures::try_ready;
 use itertools::Itertools;
@@ -515,15 +517,7 @@ impl ConfiguredByPrivilege for Bootstrapper {
                 if node_addr.ip_addr() == Ipv4Addr::new(0, 0, 0, 0) => {} // node_addr still coming
             _ => Bootstrapper::report_local_descriptor(cryptdes.main, &self.config.node_descriptor), // here or not coming
         }
-        let stream_handler_pool_subs = self.actor_system_factory.make_and_start_actors(
-            self.config.clone(),
-            Box::new(ActorFactoryReal {}),
-            initialize_database(
-                &self.config.data_directory,
-                DbInitializationConfig::panic_on_migration(),
-            ),
-        );
-
+        let stream_handler_pool_subs = self.start_actors_and_return_shp_subs();
         self.listener_handlers
             .iter_mut()
             .for_each(|f| f.bind_subs(stream_handler_pool_subs.add_sub.clone()));
@@ -612,6 +606,17 @@ impl Bootstrapper {
         }
     }
 
+    fn start_actors_and_return_shp_subs(&self) -> StreamHandlerPoolSubs {
+        self.actor_system_factory.make_and_start_actors(
+            self.config.clone(),
+            Box::new(ActorFactoryReal {}),
+            initialize_database(
+                &self.config.data_directory,
+                DbInitializationConfig::panic_on_migration(),
+            ),
+        )
+    }
+
     pub fn report_local_descriptor(cryptde: &dyn CryptDE, descriptor: &NodeDescriptor) {
         let descriptor_msg = format!(
             "MASQ Node local descriptor: {}",
@@ -630,7 +635,9 @@ impl Bootstrapper {
                         &self.config.data_directory,
                         DbInitializationConfig::panic_on_migration(),
                     )
-                    .expect("Cannot initialize database");
+                    .unwrap_or_else(|err| {
+                        db_connection_launch_panic(err, &self.config.data_directory)
+                    });
                 let config_dao = ConfigDaoReal::new(conn);
                 let mut persistent_config = PersistentConfigurationReal::new(Box::new(config_dao));
                 let clandestine_port = self.establish_clandestine_port(&mut persistent_config);
@@ -728,7 +735,9 @@ mod tests {
     use crate::sub_lib::cryptde::PublicKey;
     use crate::sub_lib::cryptde::{CryptDE, PlainData};
     use crate::sub_lib::cryptde_null::CryptDENull;
-    use crate::sub_lib::neighborhood::{NeighborhoodConfig, NeighborhoodMode, NodeDescriptor};
+    use crate::sub_lib::neighborhood::{
+        NeighborhoodConfig, NeighborhoodMode, NodeDescriptor, DEFAULT_RATE_PACK,
+    };
     use crate::sub_lib::node_addr::NodeAddr;
     use crate::sub_lib::socket_server::ConfiguredByPrivilege;
     use crate::sub_lib::stream_connector::ConnectionInfo;
@@ -738,7 +747,9 @@ mod tests {
     use crate::test_utils::recorder::Recording;
     use crate::test_utils::tokio_wrapper_mocks::ReadHalfWrapperMock;
     use crate::test_utils::tokio_wrapper_mocks::WriteHalfWrapperMock;
-    use crate::test_utils::unshared_test_utils::make_simplified_multi_config;
+    use crate::test_utils::unshared_test_utils::{
+        assert_on_initialization_with_panic_on_migration, make_simplified_multi_config,
+    };
     use crate::test_utils::{assert_contains, rate_pack};
     use crate::test_utils::{main_cryptde, make_wallet};
     use actix::Recipient;
@@ -762,7 +773,7 @@ mod tests {
     use std::io::ErrorKind;
     use std::marker::Sync;
     use std::net::{IpAddr, SocketAddr};
-    use std::path::PathBuf;
+    use std::path::{Path, PathBuf};
     use std::str::FromStr;
     use std::sync::{Arc, Mutex};
     use std::thread;
@@ -1383,6 +1394,24 @@ mod tests {
         assert_eq!(config.blockchain_bridge_config.gas_price, 11);
     }
 
+    #[test]
+    fn initialize_as_unprivileged_implements_panic_on_migration_for_make_and_start_actors() {
+        let _lock = INITIALIZATION.lock();
+        let data_dir = ensure_node_home_directory_exists(
+            "bootstrapper",
+            "initialize_as_unprivileged_implements_panic_on_migration_for_make_and_start_actors",
+        );
+
+        let act = |data_dir: &Path| {
+            let mut config = BootstrapperConfig::new();
+            config.data_directory = data_dir.to_path_buf();
+            let subject = BootstrapperBuilder::new().config(config).build();
+            subject.start_actors_and_return_shp_subs();
+        };
+
+        assert_on_initialization_with_panic_on_migration(&data_dir, &act);
+    }
+
     #[test]
     fn initialize_with_clandestine_port_produces_expected_clandestine_discriminator_factories_vector(
     ) {
@@ -2022,6 +2051,26 @@ mod tests {
             .is_none());
     }
 
+    #[test]
+    fn set_up_clandestine_port_panics_on_migration() {
+        let data_dir = ensure_node_home_directory_exists(
+            "bootstrapper",
+            "set_up_clandestine_port_panics_on_migration",
+        );
+
+        let act = |data_dir: &Path| {
+            let mut config = BootstrapperConfig::new();
+            config.data_directory = data_dir.to_path_buf();
+            config.neighborhood_config = NeighborhoodConfig {
+                mode: NeighborhoodMode::Standard(NodeAddr::default(), vec![], DEFAULT_RATE_PACK),
+            };
+            let mut subject = BootstrapperBuilder::new().config(config).build();
+            subject.set_up_clandestine_port();
+        };
+
+        assert_on_initialization_with_panic_on_migration(&data_dir, &act);
+    }
+
     #[test]
     #[should_panic(
         expected = "Database is corrupt: error setting clandestine port: TransactionError"

node/src/daemon/setup_reporter.rs

@@ -470,7 +470,9 @@ impl SetupReporterReal {
             }
             Err(InitializationError::Nonexistent | InitializationError::SuppressedMigration) => {
                 // When the Daemon runs for the first time, the database will not yet have been
-                // created. If the database is old, it should not be used by the Daemon.
+                // created. If the database is old, it should not be used by the Daemon (see more
+                // information at ConfigDaoNull).
+
                 let parse_args_configuration = UnprivilegedParseArgsConfigurationDaoNull {};
                 let mut persistent_config =
                     PersistentConfigurationReal::new(Box::new(ConfigDaoNull::default()));

node/src/database/db_initializer.rs

@@ -5,6 +5,7 @@ use crate::database::db_migrations::db_migrator::{DbMigrator, DbMigratorReal};
 use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED;
 use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS};
 use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK;
+use crate::sub_lib::utils::db_connection_launch_panic;
 use masq_lib::blockchains::chains::Chain;
 use masq_lib::constants::{
     DEFAULT_GAS_PRICE, HIGHEST_RANDOM_CLANDESTINE_PORT, LOWEST_USABLE_INSECURE_PORT,
@@ -488,12 +489,7 @@ pub fn connection_or_panic(
 ) -> Box<dyn ConnectionWrapper> {
     db_initializer
         .initialize(path, init_config)
-        .unwrap_or_else(|_| {
-            panic!(
-                "Failed to connect to database at {:?}",
-                path.join(DATABASE_FILE)
-            )
-        })
+        .unwrap_or_else(|err| db_connection_launch_panic(err, path))
 }
 
 #[derive(Clone)]