From 422f060f177bdd35b5ba418525670ae90edc8499 Mon Sep 17 00:00:00 2001 From: Stanimal Date: Wed, 3 Jun 2020 19:01:43 +0200 Subject: [PATCH] Fix for flaky base node tests Base node and mempool integration tests construct their test networks using the most optimal ordering. This reduces/removes test flakiness. The databases files for nodes are no longer shared as this would cause tests to sporadically fail on MacOS. --- base_layer/core/tests/helpers/nodes.rs | 35 ++++++++--------- base_layer/core/tests/mempool.rs | 2 +- base_layer/core/tests/node_service.rs | 53 ++++++++++++-------------- base_layer/p2p/src/initialization.rs | 1 + 4 files changed, 42 insertions(+), 49 deletions(-) diff --git a/base_layer/core/tests/helpers/nodes.rs b/base_layer/core/tests/helpers/nodes.rs index 3e5b40befa..9a27b0a26a 100644 --- a/base_layer/core/tests/helpers/nodes.rs +++ b/base_layer/core/tests/helpers/nodes.rs @@ -22,7 +22,7 @@ use futures::Sink; use rand::{distributions::Alphanumeric, rngs::OsRng, Rng}; -use std::{error::Error, iter, sync::Arc, time::Duration}; +use std::{error::Error, iter, path::Path, sync::Arc, time::Duration}; use tari_comms::{ peer_manager::{NodeIdentity, PeerFeatures}, transports::MemoryTransport, @@ -275,14 +275,14 @@ pub fn create_network_with_2_base_nodes( } // Creates a network with two Base Nodes where each node in the network knows the other nodes in the network. -pub fn create_network_with_2_base_nodes_with_config( +pub fn create_network_with_2_base_nodes_with_config>( runtime: &mut Runtime, base_node_service_config: BaseNodeServiceConfig, mmr_cache_config: MmrCacheConfig, mempool_service_config: MempoolServiceConfig, liveness_service_config: LivenessConfig, consensus_manager: ConsensusManager, - data_path: &str, + data_path: P, ) -> (NodeInterfaces, NodeInterfaces, ConsensusManager) { let alice_node_identity = random_node_identity(); @@ -290,13 +290,12 @@ pub fn create_network_with_2_base_nodes_with_config( let network = Network::LocalNet; let (alice_node, consensus_manager) = BaseNodeBuilder::new(network) .with_node_identity(alice_node_identity.clone()) - .with_peers(vec![bob_node_identity.clone()]) .with_base_node_service_config(base_node_service_config) .with_mmr_cache_config(mmr_cache_config) .with_mempool_service_config(mempool_service_config) .with_liveness_service_config(liveness_service_config.clone()) .with_consensus_manager(consensus_manager) - .start(runtime, data_path); + .start(runtime, data_path.as_ref().join("alice").as_os_str().to_str().unwrap()); let (bob_node, consensus_manager) = BaseNodeBuilder::new(network) .with_node_identity(bob_node_identity) .with_peers(vec![alice_node_identity]) @@ -305,7 +304,7 @@ pub fn create_network_with_2_base_nodes_with_config( .with_mempool_service_config(mempool_service_config) .with_liveness_service_config(liveness_service_config.clone()) .with_consensus_manager(consensus_manager) - .start(runtime, data_path); + .start(runtime, data_path.as_ref().join("bob").as_os_str().to_str().unwrap()); wait_until_online(runtime, &[&alice_node, &bob_node]); @@ -333,14 +332,14 @@ pub fn create_network_with_3_base_nodes( } // Creates a network with three Base Nodes where each node in the network knows the other nodes in the network. -pub fn create_network_with_3_base_nodes_with_config( +pub fn create_network_with_3_base_nodes_with_config>( runtime: &mut Runtime, base_node_service_config: BaseNodeServiceConfig, mmr_cache_config: MmrCacheConfig, mempool_service_config: MempoolServiceConfig, liveness_service_config: LivenessConfig, consensus_manager: ConsensusManager, - data_path: &str, + data_path: P, ) -> (NodeInterfaces, NodeInterfaces, NodeInterfaces, ConsensusManager) { let alice_node_identity = random_node_identity(); @@ -354,34 +353,32 @@ pub fn create_network_with_3_base_nodes_with_config( bob_node_identity.node_id().short_str(), carol_node_identity.node_id().short_str() ); - - let (alice_node, consensus_manager) = BaseNodeBuilder::new(network) - .with_node_identity(alice_node_identity.clone()) - .with_peers(vec![bob_node_identity.clone(), carol_node_identity.clone()]) + let (carol_node, consensus_manager) = BaseNodeBuilder::new(network) + .with_node_identity(carol_node_identity.clone()) .with_base_node_service_config(base_node_service_config) .with_mmr_cache_config(mmr_cache_config) .with_mempool_service_config(mempool_service_config) .with_liveness_service_config(liveness_service_config.clone()) .with_consensus_manager(consensus_manager) - .start(runtime, data_path); + .start(runtime, data_path.as_ref().join("carol").as_os_str().to_str().unwrap()); let (bob_node, consensus_manager) = BaseNodeBuilder::new(network) .with_node_identity(bob_node_identity.clone()) - .with_peers(vec![carol_node_identity.clone(), alice_node_identity.clone()]) + .with_peers(vec![carol_node_identity.clone()]) .with_base_node_service_config(base_node_service_config) .with_mmr_cache_config(mmr_cache_config) .with_mempool_service_config(mempool_service_config) .with_liveness_service_config(liveness_service_config.clone()) .with_consensus_manager(consensus_manager) - .start(runtime, data_path); - let (carol_node, consensus_manager) = BaseNodeBuilder::new(network) - .with_node_identity(carol_node_identity.clone()) - .with_peers(vec![alice_node_identity.clone(), bob_node_identity.clone()]) + .start(runtime, data_path.as_ref().join("bob").as_os_str().to_str().unwrap()); + let (alice_node, consensus_manager) = BaseNodeBuilder::new(network) + .with_node_identity(alice_node_identity.clone()) + .with_peers(vec![bob_node_identity.clone(), carol_node_identity.clone()]) .with_base_node_service_config(base_node_service_config) .with_mmr_cache_config(mmr_cache_config) .with_mempool_service_config(mempool_service_config) .with_liveness_service_config(liveness_service_config.clone()) .with_consensus_manager(consensus_manager) - .start(runtime, data_path); + .start(runtime, data_path.as_ref().join("alice").as_os_str().to_str().unwrap()); wait_until_online(runtime, &[&alice_node, &bob_node, &carol_node]); diff --git a/base_layer/core/tests/mempool.rs b/base_layer/core/tests/mempool.rs index f98643a203..0c4124036d 100644 --- a/base_layer/core/tests/mempool.rs +++ b/base_layer/core/tests/mempool.rs @@ -492,7 +492,7 @@ fn request_response_get_stats() { MempoolServiceConfig::default(), LivenessConfig::default(), consensus_manager, - temp_dir.path().to_str().unwrap(), + temp_dir.path(), ); // Create a tx spending the genesis output. Then create 2 orphan txs diff --git a/base_layer/core/tests/node_service.rs b/base_layer/core/tests/node_service.rs index 1b0d95a5c8..f1125e317e 100644 --- a/base_layer/core/tests/node_service.rs +++ b/base_layer/core/tests/node_service.rs @@ -408,28 +408,23 @@ fn propagate_and_forward_valid_block() { .build(); let (mut alice_node, rules) = BaseNodeBuilder::new(network) .with_node_identity(alice_node_identity.clone()) - .with_peers(vec![bob_node_identity.clone()]) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); + .start(&mut runtime, temp_dir.path().join("alice").to_str().unwrap()); let (bob_node, rules) = BaseNodeBuilder::new(network) .with_node_identity(bob_node_identity.clone()) - .with_peers(vec![ - alice_node_identity, - carol_node_identity.clone(), - dan_node_identity.clone(), - ]) + .with_peers(vec![alice_node_identity]) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); + .start(&mut runtime, temp_dir.path().join("bob").to_str().unwrap()); let (carol_node, rules) = BaseNodeBuilder::new(network) .with_node_identity(carol_node_identity.clone()) - .with_peers(vec![bob_node_identity.clone(), dan_node_identity.clone()]) + .with_peers(vec![bob_node_identity.clone()]) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); + .start(&mut runtime, temp_dir.path().join("carol").to_str().unwrap()); let (dan_node, rules) = BaseNodeBuilder::new(network) .with_node_identity(dan_node_identity) - .with_peers(vec![bob_node_identity, carol_node_identity]) + .with_peers(vec![carol_node_identity, bob_node_identity]) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); + .start(&mut runtime, temp_dir.path().join("dan").to_str().unwrap()); wait_until_online(&mut runtime, &[&alice_node, &bob_node, &carol_node, &dan_node]); @@ -513,36 +508,36 @@ fn propagate_and_forward_invalid_block() { let stateless_block_validator = StatelessBlockValidator::new(rules.clone(), factories.clone()); let mock_validator = MockValidator::new(true); let mock_accum_difficulty_validator = MockAccumDifficultyValidator {}; - let (mut alice_node, rules) = BaseNodeBuilder::new(network) - .with_node_identity(alice_node_identity.clone()) - .with_peers(vec![bob_node_identity.clone(), carol_node_identity.clone()]) + + let (dan_node, rules) = BaseNodeBuilder::new(network) + .with_node_identity(dan_node_identity.clone()) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); - let (bob_node, rules) = BaseNodeBuilder::new(network) - .with_node_identity(bob_node_identity.clone()) - .with_peers(vec![alice_node_identity.clone(), dan_node_identity.clone()]) + .start(&mut runtime, temp_dir.path().join("dan").to_str().unwrap()); + let (carol_node, rules) = BaseNodeBuilder::new(network) + .with_node_identity(carol_node_identity.clone()) + .with_peers(vec![dan_node_identity.clone()]) .with_consensus_manager(rules) .with_validators( mock_validator.clone(), stateless_block_validator.clone(), mock_accum_difficulty_validator.clone(), ) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); - let (carol_node, rules) = BaseNodeBuilder::new(network) - .with_node_identity(carol_node_identity.clone()) - .with_peers(vec![alice_node_identity, dan_node_identity.clone()]) + .start(&mut runtime, temp_dir.path().join("carol").to_str().unwrap()); + let (bob_node, rules) = BaseNodeBuilder::new(network) + .with_node_identity(bob_node_identity.clone()) + .with_peers(vec![dan_node_identity.clone()]) .with_consensus_manager(rules) .with_validators( mock_validator.clone(), - stateless_block_validator, + stateless_block_validator.clone(), mock_accum_difficulty_validator.clone(), ) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); - let (dan_node, rules) = BaseNodeBuilder::new(network) - .with_node_identity(dan_node_identity) - .with_peers(vec![bob_node_identity, carol_node_identity]) + .start(&mut runtime, temp_dir.path().join("bob").to_str().unwrap()); + let (mut alice_node, rules) = BaseNodeBuilder::new(network) + .with_node_identity(alice_node_identity) + .with_peers(vec![bob_node_identity.clone(), carol_node_identity.clone()]) .with_consensus_manager(rules) - .start(&mut runtime, temp_dir.path().to_str().unwrap()); + .start(&mut runtime, temp_dir.path().join("alice").to_str().unwrap()); wait_until_online(&mut runtime, &[&alice_node, &bob_node, &carol_node, &dan_node]); diff --git a/base_layer/p2p/src/initialization.rs b/base_layer/p2p/src/initialization.rs index a383d66994..8e10017d51 100644 --- a/base_layer/p2p/src/initialization.rs +++ b/base_layer/p2p/src/initialization.rs @@ -125,6 +125,7 @@ where .take(8) .collect::() }; + std::fs::create_dir_all(data_path).unwrap(); let datastore = LMDBBuilder::new() .set_path(data_path) .set_environment_size(50)