From bbad3fe501b4246904b4081f268a4d8d4b8c469d Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Thu, 10 Dec 2020 17:28:52 -0800 Subject: [PATCH] TestValidator now implements Drop, no need to close() it --- cli/tests/deploy.rs | 2 -- cli/tests/nonce.rs | 4 --- cli/tests/request_airdrop.rs | 2 -- cli/tests/stake.rs | 20 ------------ cli/tests/transfer.rs | 6 ---- cli/tests/vote.rs | 2 -- core/src/test_validator.rs | 37 ++++++++++++++++------ core/src/validator.rs | 14 ++++---- core/tests/client.rs | 1 - core/tests/rpc.rs | 3 -- local-cluster/src/local_cluster.rs | 4 +-- tokens/src/commands.rs | 16 ---------- tokens/tests/commands.rs | 2 -- validator/src/bin/solana-test-validator.rs | 1 + validator/src/main.rs | 2 +- 15 files changed, 38 insertions(+), 78 deletions(-) diff --git a/cli/tests/deploy.rs b/cli/tests/deploy.rs index e6923bcbe8..0f430e74b6 100644 --- a/cli/tests/deploy.rs +++ b/cli/tests/deploy.rs @@ -141,6 +141,4 @@ fn test_cli_deploy_program() { assert_eq!(account2.owner, bpf_loader::id()); assert_eq!(account2.executable, true); assert_eq!(account0.data, account2.data); - - test_validator.close(); } diff --git a/cli/tests/nonce.rs b/cli/tests/nonce.rs index b974e921b1..6143b5277b 100644 --- a/cli/tests/nonce.rs +++ b/cli/tests/nonce.rs @@ -210,8 +210,6 @@ fn full_battery_tests( check_recent_balance(1000, &rpc_client, &config_payer.signers[0].pubkey()); check_recent_balance(800, &rpc_client, &nonce_account); check_recent_balance(200, &rpc_client, &payee_pubkey); - - test_validator.close(); } #[test] @@ -333,6 +331,4 @@ fn test_create_account_with_seed() { check_recent_balance(31, &rpc_client, &offline_nonce_authority_signer.pubkey()); check_recent_balance(4000, &rpc_client, &online_nonce_creator_signer.pubkey()); check_recent_balance(10, &rpc_client, &to_address); - - test_validator.close(); } diff --git a/cli/tests/request_airdrop.rs b/cli/tests/request_airdrop.rs index 78211a967d..0e9a2d8b46 100644 --- a/cli/tests/request_airdrop.rs +++ b/cli/tests/request_airdrop.rs @@ -38,6 +38,4 @@ fn test_cli_request_airdrop() { .unwrap() .value; assert_eq!(balance, 50); - - test_validator.close(); } diff --git a/cli/tests/stake.rs b/cli/tests/stake.rs index de7529a18b..39b1624cf6 100644 --- a/cli/tests/stake.rs +++ b/cli/tests/stake.rs @@ -108,8 +108,6 @@ fn test_stake_delegation_force() { fee_payer: 0, }; process_command(&config).unwrap(); - - test_validator.close(); } #[test] @@ -189,8 +187,6 @@ fn test_seed_stake_delegation_and_deactivation() { fee_payer: 0, }; process_command(&config_validator).unwrap(); - - test_validator.close(); } #[test] @@ -266,8 +262,6 @@ fn test_stake_delegation_and_deactivation() { fee_payer: 0, }; process_command(&config_validator).unwrap(); - - test_validator.close(); } #[test] @@ -400,8 +394,6 @@ fn test_offline_stake_delegation_and_deactivation() { fee_payer: 0, }; process_command(&config_payer).unwrap(); - - test_validator.close(); } #[test] @@ -516,8 +508,6 @@ fn test_nonced_stake_delegation_and_deactivation() { fee_payer: 0, }; process_command(&config).unwrap(); - - test_validator.close(); } #[test] @@ -789,8 +779,6 @@ fn test_stake_authorize() { .unwrap() .blockhash; assert_ne!(nonce_hash, new_nonce_hash); - - test_validator.close(); } #[test] @@ -916,8 +904,6 @@ fn test_stake_authorize_with_fee_payer() { // `config_offline` however has paid 1 sig due to being both authority // and fee payer check_recent_balance(100_000 - SIG_FEE, &rpc_client, &offline_pubkey); - - test_validator.close(); } #[test] @@ -1061,8 +1047,6 @@ fn test_stake_split() { &rpc_client, &split_account.pubkey(), ); - - test_validator.close(); } #[test] @@ -1324,8 +1308,6 @@ fn test_stake_set_lockup() { ); assert_eq!(current_lockup.epoch, lockup.epoch.unwrap()); assert_eq!(current_lockup.custodian, offline_pubkey); - - test_validator.close(); } #[test] @@ -1538,6 +1520,4 @@ fn test_offline_nonced_create_stake_account_and_withdraw() { let seed_address = Pubkey::create_with_seed(&stake_pubkey, seed, &solana_stake_program::id()).unwrap(); check_recent_balance(50_000, &rpc_client, &seed_address); - - test_validator.close(); } diff --git a/cli/tests/transfer.rs b/cli/tests/transfer.rs index d5b621975b..40cbaf4c3c 100644 --- a/cli/tests/transfer.rs +++ b/cli/tests/transfer.rs @@ -237,8 +237,6 @@ fn test_transfer() { process_command(&config).unwrap(); check_recent_balance(28, &rpc_client, &offline_pubkey); check_recent_balance(40, &rpc_client, &recipient_pubkey); - - test_validator.close(); } #[test] @@ -358,8 +356,6 @@ fn test_transfer_multisession_signing() { check_recent_balance(1, &rpc_client, &offline_from_signer.pubkey()); check_recent_balance(1, &rpc_client, &offline_fee_payer_signer.pubkey()); check_recent_balance(42, &rpc_client, &to_pubkey); - - test_validator.close(); } #[test] @@ -405,6 +401,4 @@ fn test_transfer_all() { process_command(&config).unwrap(); check_recent_balance(0, &rpc_client, &sender_pubkey); check_recent_balance(49_999, &rpc_client, &recipient_pubkey); - - test_validator.close(); } diff --git a/cli/tests/vote.rs b/cli/tests/vote.rs index e7418724ea..c33d15ea1e 100644 --- a/cli/tests/vote.rs +++ b/cli/tests/vote.rs @@ -125,6 +125,4 @@ fn test_vote_authorize_and_withdraw() { withdraw_authority: 1, }; process_command(&config).unwrap(); - - test_validator.close(); } diff --git a/core/src/test_validator.rs b/core/src/test_validator.rs index 9bd99bc900..25032b3249 100644 --- a/core/src/test_validator.rs +++ b/core/src/test_validator.rs @@ -5,6 +5,7 @@ use { rpc::JsonRpcConfig, validator::{Validator, ValidatorConfig}, }, + solana_client::rpc_client::RpcClient, solana_ledger::{blockstore::create_new_ledger, create_new_tmp_ledger}, solana_runtime::{ bank_forks::{CompressionType, SnapshotConfig, SnapshotVersion}, @@ -12,6 +13,8 @@ use { hardened_unpack::MAX_GENESIS_ARCHIVE_UNPACKED_SIZE, }, solana_sdk::{ + clock::DEFAULT_MS_PER_SLOT, + commitment_config::CommitmentConfig, fee_calculator::FeeRateGovernor, native_token::sol_to_lamports, pubkey::Pubkey, @@ -23,6 +26,8 @@ use { net::SocketAddr, path::{Path, PathBuf}, sync::Arc, + thread::sleep, + time::Duration, }, }; @@ -34,17 +39,19 @@ pub struct TestValidatorGenesisConfig { #[derive(Default)] pub struct TestValidatorStartConfig { + pub preserve_ledger: bool, pub rpc_config: JsonRpcConfig, pub rpc_ports: Option<(u16, u16)>, // (JsonRpc, JsonRpcPubSub), None == random ports } pub struct TestValidator { ledger_path: PathBuf, + preserve_ledger: bool, rpc_pubsub_url: String, rpc_url: String, tpu: SocketAddr, gossip: SocketAddr, - validator: Validator, + validator: Option, vote_account_address: Pubkey, } @@ -223,7 +230,7 @@ impl TestValidator { ..ValidatorConfig::default() }; - let validator = Validator::new( + let validator = Some(Validator::new( node, &Arc::new(validator_identity_keypair), &ledger_path, @@ -231,7 +238,7 @@ impl TestValidator { vec![Arc::new(validator_vote_account)], None, &validator_config, - ); + )); // Needed to avoid panics in `solana-responder-gossip` in tests that create a number of // test validators concurrently... @@ -239,6 +246,7 @@ impl TestValidator { Ok(TestValidator { ledger_path: ledger_path.to_path_buf(), + preserve_ledger: false, rpc_pubsub_url, rpc_url, gossip, @@ -248,12 +256,6 @@ impl TestValidator { }) } - /// Stop the test validator and delete its ledger directory - pub fn close(self) { - self.validator.close().unwrap(); - remove_dir_all(&self.ledger_path).unwrap(); - } - /// Return the test validator's TPU address pub fn tpu(&self) -> &SocketAddr { &self.tpu @@ -279,3 +281,20 @@ impl TestValidator { self.vote_account_address } } + +impl Drop for TestValidator { + fn drop(&mut self) { + if let Some(validator) = self.validator.take() { + validator.close(); + } + if !self.preserve_ledger { + remove_dir_all(&self.ledger_path).unwrap_or_else(|err| { + panic!( + "Failed to remove ledger directory {}: {}", + self.ledger_path.display(), + err + ) + }); + } + } +} diff --git a/core/src/validator.rs b/core/src/validator.rs index 9877ca8b01..3d06d68353 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -65,7 +65,7 @@ use std::{ sync::atomic::{AtomicBool, Ordering}, sync::mpsc::Receiver, sync::{mpsc::channel, Arc, Mutex, RwLock}, - thread::{sleep, Result}, + thread::sleep, time::Duration, }; @@ -635,9 +635,9 @@ impl Validator { } } - pub fn close(mut self) -> Result<()> { + pub fn close(mut self) { self.exit(); - self.join() + self.join(); } fn print_node_info(node: &Node) { @@ -665,7 +665,7 @@ impl Validator { ); } - pub fn join(self) -> Result<()> { + pub fn join(self) { self.poh_service.join().expect("poh_service"); drop(self.poh_recorder); if let Some(RpcServices { @@ -718,8 +718,6 @@ impl Validator { .join() .expect("completed_data_sets_service"); self.ip_echo_server.shutdown_now(); - - Ok(()) } } @@ -1267,7 +1265,7 @@ mod tests { Some(&leader_node.info), &config, ); - validator.close().unwrap(); + validator.close(); remove_dir_all(validator_ledger_path).unwrap(); } @@ -1345,7 +1343,7 @@ mod tests { // While join is called sequentially, the above exit call notified all the // validators to exit from all their threads validators.into_iter().for_each(|validator| { - validator.join().unwrap(); + validator.join(); }); for path in ledger_paths { diff --git a/core/tests/client.rs b/core/tests/client.rs index 1487f9b665..33c7b88097 100644 --- a/core/tests/client.rs +++ b/core/tests/client.rs @@ -82,7 +82,6 @@ fn test_rpc_client() { client.get_balance(&alice.pubkey()).unwrap(), original_alice_balance - sol_to_lamports(20.0) ); - test_validator.close(); } #[test] diff --git a/core/tests/rpc.rs b/core/tests/rpc.rs index 6a2e4ef34f..cfb58953e2 100644 --- a/core/tests/rpc.rs +++ b/core/tests/rpc.rs @@ -109,7 +109,6 @@ fn test_rpc_send_tx() { ); let json: Value = post_rpc(req, &rpc_url); info!("{:?}", json["result"]["value"]); - test_validator.close(); } #[test] @@ -142,7 +141,6 @@ fn test_rpc_invalid_requests() { let the_value = &json["result"]["value"]; assert!(the_value.is_null()); - test_validator.close(); } #[test] @@ -317,5 +315,4 @@ fn test_rpc_subscriptions() { } rt.shutdown_now().wait().unwrap(); - test_validator.close(); } diff --git a/local-cluster/src/local_cluster.rs b/local-cluster/src/local_cluster.rs index 0d57ee7063..8389c9890b 100644 --- a/local-cluster/src/local_cluster.rs +++ b/local-cluster/src/local_cluster.rs @@ -285,7 +285,7 @@ impl LocalCluster { self.exit(); for (_, node) in self.validators.iter_mut() { if let Some(v) = node.validator.take() { - v.join().expect("Validator join failed"); + v.join(); } } } @@ -609,7 +609,7 @@ impl Cluster for LocalCluster { // Shut down the validator let mut validator = node.validator.take().expect("Validator must be running"); validator.exit(); - validator.join().unwrap(); + validator.join(); node } diff --git a/tokens/src/commands.rs b/tokens/src/commands.rs index d36098e6c7..67108a21e5 100644 --- a/tokens/src/commands.rs +++ b/tokens/src/commands.rs @@ -1063,8 +1063,6 @@ mod tests { let client = RpcClient::new_with_commitment(url, CommitmentConfig::recent()); test_process_distribute_tokens_with_client(&client, alice, None); - - test_validator.close(); } #[test] @@ -1075,8 +1073,6 @@ mod tests { let client = RpcClient::new_with_commitment(url, CommitmentConfig::recent()); test_process_distribute_tokens_with_client(&client, alice, Some(sol_to_lamports(1.5))); - - test_validator.close(); } #[test] @@ -1087,8 +1083,6 @@ mod tests { let client = RpcClient::new_with_commitment(url, CommitmentConfig::recent()); test_process_distribute_stake_with_client(&client, alice); - - test_validator.close(); } #[test] @@ -1477,8 +1471,6 @@ mod tests { } else { panic!("check_payer_balances should have errored"); } - - test_validator.close(); } #[test] @@ -1550,8 +1542,6 @@ mod tests { } else { panic!("check_payer_balances should have errored"); } - - test_validator.close(); } fn initialize_stake_account( @@ -1699,8 +1689,6 @@ mod tests { } else { panic!("check_payer_balances should have errored"); } - - test_validator.close(); } #[test] @@ -1779,8 +1767,6 @@ mod tests { } else { panic!("check_payer_balances should have errored"); } - - test_validator.close(); } #[test] @@ -2076,8 +2062,6 @@ mod tests { let read_db = db::open_db(&db_file, true).unwrap(); let transaction_info = db::read_transaction_infos(&read_db); assert_eq!(transaction_info.len(), 1); - - test_validator.close(); } #[test] diff --git a/tokens/tests/commands.rs b/tokens/tests/commands.rs index a8f2fb5de4..f0425497e9 100644 --- a/tokens/tests/commands.rs +++ b/tokens/tests/commands.rs @@ -12,6 +12,4 @@ fn test_process_distribute_with_rpc_client() { let client = RpcClient::new(test_validator.rpc_url()); test_process_distribute_tokens_with_client(&client, mint_keypair, None); - - test_validator.close(); } diff --git a/validator/src/bin/solana-test-validator.rs b/validator/src/bin/solana-test-validator.rs index b0295f549d..55d983dd52 100644 --- a/validator/src/bin/solana-test-validator.rs +++ b/validator/src/bin/solana-test-validator.rs @@ -206,6 +206,7 @@ fn main() { TestValidator::start( &ledger_path, TestValidatorStartConfig { + preserve_ledger: true, rpc_config: JsonRpcConfig { enable_validator_exit: true, enable_rpc_transaction_history: true, diff --git a/validator/src/main.rs b/validator/src/main.rs index a093d82729..26c65785f9 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1740,6 +1740,6 @@ pub fn main() { }); } info!("Validator initialized"); - validator.join().expect("validator exit"); + validator.join(); info!("Validator exiting.."); }