diff --git a/masq_lib/src/shared_schema.rs b/masq_lib/src/shared_schema.rs index 7c060e6d2..894724540 100644 --- a/masq_lib/src/shared_schema.rs +++ b/masq_lib/src/shared_schema.rs @@ -250,7 +250,6 @@ pub fn data_directory_arg<'a>() -> Arg<'a, 'a> { .help(DATA_DIRECTORY_HELP) } -// TODO: Not an arg fn, move somewhere else pub fn official_chain_names() -> &'static [&'static str] { &[ POLYGON_MAINNET_FULL_IDENTIFIER, diff --git a/multinode_integration_tests/tests/min_hops_test.rs b/multinode_integration_tests/tests/min_hops_tests.rs similarity index 85% rename from multinode_integration_tests/tests/min_hops_test.rs rename to multinode_integration_tests/tests/min_hops_tests.rs index 037303434..af634444e 100644 --- a/multinode_integration_tests/tests/min_hops_test.rs +++ b/multinode_integration_tests/tests/min_hops_tests.rs @@ -12,15 +12,15 @@ use std::thread; use std::time::Duration; #[test] -fn http_end_to_end_routing_test_with_different_min_hops() { +fn data_can_be_routed_using_different_min_hops() { // This test fails sometimes due to a timeout: "Couldn't read chunk: Kind(TimedOut)" // You may fix it by increasing the timeout for the client. - assert_http_end_to_end_routing_test(Hops::OneHop); - assert_http_end_to_end_routing_test(Hops::TwoHops); - assert_http_end_to_end_routing_test(Hops::SixHops); + assert_http_end_to_end_routing(Hops::OneHop); + assert_http_end_to_end_routing(Hops::TwoHops); + assert_http_end_to_end_routing(Hops::SixHops); } -fn assert_http_end_to_end_routing_test(min_hops: Hops) { +fn assert_http_end_to_end_routing(min_hops: Hops) { let mut cluster = MASQNodeCluster::start().unwrap(); let config = NodeStartupConfigBuilder::standard() .min_hops(min_hops) @@ -69,15 +69,14 @@ fn min_hops_can_be_changed_during_runtime() { .build(); let first_node = cluster.start_real_node(first_node_config); let ui_client = first_node.make_ui(ui_port); - let mut prev_node_reference = first_node.node_reference(); for _ in 0..initial_min_hops as u8 { - let new_node_config = NodeStartupConfigBuilder::standard() - .neighbor(prev_node_reference) - .chain(cluster.chain) - .build(); - let new_node = cluster.start_real_node(new_node_config); - prev_node_reference = new_node.node_reference(); + cluster.start_real_node( + NodeStartupConfigBuilder::standard() + .neighbor(first_node.node_reference()) + .chain(cluster.chain) + .build(), + ); } thread::sleep(Duration::from_millis(1000)); diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index b16d6185a..0c07cce12 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -154,7 +154,16 @@ impl Handler for Neighborhood { } ConfigurationChange::UpdateMinHops(new_min_hops) => { self.set_min_hops_and_patch_size(new_min_hops); - // TODO: Should we make the stage transition for OverallConnectionStatus from RouteFound to ConnectedToNeighbor before we search for a new route + let node_to_ui_recipient = self + .node_to_ui_recipient_opt + .as_ref() + .expect("UI gateway is dead"); + self.overall_connection_status + .update_ocs_stage_and_send_message_to_ui( + OverallConnectionStage::ConnectedToNeighbor, + node_to_ui_recipient, + &self.logger, + ); self.search_for_a_new_route(); } } @@ -3059,14 +3068,15 @@ mod tests { fn can_update_min_hops_with_configuration_change_msg() { init_test_logging(); let test_name = "can_update_min_hops_with_configuration_change_msg"; + let new_min_hops = Hops::FourHops; let system = System::new(test_name); + let (ui_gateway, _, ui_gateway_recording) = make_recorder(); let mut subject = make_standard_subject(); subject.min_hops = Hops::TwoHops; subject.logger = Logger::new(test_name); subject.overall_connection_status.stage = OverallConnectionStage::RouteFound; - let new_min_hops = Hops::FourHops; let subject_addr = subject.start(); - let peer_actors = peer_actors_builder().build(); + let peer_actors = peer_actors_builder().ui_gateway(ui_gateway).build(); subject_addr.try_send(BindMessage { peer_actors }).unwrap(); subject_addr @@ -3077,19 +3087,39 @@ mod tests { subject_addr .try_send(AssertionsMessage { - assertions: Box::new(move |actor: &mut Neighborhood| { + assertions: Box::new(move |neighborhood: &mut Neighborhood| { let expected_db_patch_size = Neighborhood::calculate_db_patch_size(new_min_hops); - assert_eq!(actor.min_hops, new_min_hops); - assert_eq!(actor.db_patch_size, expected_db_patch_size); + assert_eq!(neighborhood.min_hops, new_min_hops); + assert_eq!(neighborhood.db_patch_size, expected_db_patch_size); + assert_eq!( + neighborhood.overall_connection_status.stage, + OverallConnectionStage::ConnectedToNeighbor + ); }), }) .unwrap(); System::current().stop(); system.run(); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: {test_name}: Searching for a {new_min_hops}-hop route..." - )); + let recording = ui_gateway_recording.lock().unwrap(); + let message: &NodeToUiMessage = recording.get_record(0); + assert_eq!( + message, + &NodeToUiMessage { + target: MessageTarget::AllClients, + body: UiConnectionChangeBroadcast { + stage: UiConnectionStage::ConnectedToNeighbor + } + .tmb(0), + } + ); + TestLogHandler::new().assert_logs_contain_in_order(vec![ + &format!( + "DEBUG: {test_name}: The stage of OverallConnectionStatus has been changed \ + from RouteFound to ConnectedToNeighbor. A message to the UI was also sent." + ), + &format!("DEBUG: {test_name}: Searching for a 4-hop route..."), + ]); } #[test] @@ -4000,9 +4030,12 @@ mod tests { let ui_recording = ui_gateway_arc.lock().unwrap(); assert_eq!(ui_recording.len(), 0); assert_eq!(subject.overall_connection_status.can_make_routes(), false); - TestLogHandler::new().exists_log_containing(&format!( - "DEBUG: {}: The connectivity check still can't find a good route.", - test_name + let tlh = TestLogHandler::new(); + tlh.exists_log_containing(&format!( + "DEBUG: {test_name}: The connectivity check still can't find a good route.", + )); + tlh.exists_no_log_containing(&format!( + "DEBUG: {test_name}: The connectivity check has found a 5-hop route." )); } diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 04358f076..18e5b2a13 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -266,9 +266,8 @@ impl OverallConnectionStatus { node_to_ui_recipient: &Recipient, logger: &Logger, ) { - // TODO: Modify this fn when you're implementing the regressing transitions let prev_stage = self.stage; - if new_stage as usize > prev_stage as usize { + if new_stage != prev_stage { self.stage = new_stage; OverallConnectionStatus::send_message_to_ui(self.stage.into(), node_to_ui_recipient); debug!( @@ -909,9 +908,9 @@ mod tests { } #[test] - fn doesn_t_send_message_to_the_ui_in_case_stage_hasn_t_updated() { + fn does_not_send_message_to_the_ui_in_case_the_stage_has_not_updated() { init_test_logging(); - let test_name = "doesn_t_send_message_to_the_ui_in_case_stage_hasn_t_updated"; + let test_name = "does_not_send_message_to_the_ui_in_case_the_stage_has_not_updated"; let initial_stage = OverallConnectionStage::ConnectedToNeighbor; let new_stage = initial_stage; @@ -928,21 +927,29 @@ mod tests { } #[test] - fn doesn_t_send_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor( - ) { + fn sends_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor() { init_test_logging(); - let test_name = "doesn_t_send_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor"; + let test_name = "sends_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor"; let initial_stage = OverallConnectionStage::RouteFound; let new_stage = OverallConnectionStage::ConnectedToNeighbor; let (stage, message_opt) = assert_stage_and_node_to_ui_message(initial_stage, new_stage, test_name); - assert_eq!(stage, initial_stage); - assert_eq!(message_opt, None); + assert_eq!(stage, new_stage); + assert_eq!( + message_opt, + Some(NodeToUiMessage { + target: MessageTarget::AllClients, + body: UiConnectionChangeBroadcast { + stage: new_stage.into() + } + .tmb(0) + }) + ); TestLogHandler::new().exists_log_containing(&format!( - "TRACE: {}: There was an attempt to update the stage of OverallConnectionStatus \ - from {:?} to {:?}. The request has been discarded.", + "DEBUG: {}: The stage of OverallConnectionStatus has been changed \ + from {:?} to {:?}. A message to the UI was also sent.", test_name, initial_stage, new_stage )); } diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index 1790daae2..7ec349aca 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -97,6 +97,7 @@ recorder_message_handler!(AddReturnRouteMessage); recorder_message_handler!(AddRouteMessage); recorder_message_handler!(AddStreamMsg); recorder_message_handler!(BindMessage); +recorder_message_handler!(ConfigurationChangeMessage); recorder_message_handler!(CrashNotification); recorder_message_handler!(DaemonBindMessage); recorder_message_handler!(DispatcherNodeQueryMessage); @@ -127,7 +128,6 @@ recorder_message_handler!(ScanError); recorder_message_handler!(ConsumingWalletBalancesAndQualifiedPayables); recorder_message_handler!(SentPayables); recorder_message_handler!(RequestBalancesToPayPayables); -recorder_message_handler!(ConfigurationChangeMessage); recorder_message_handler!(StartMessage); recorder_message_handler!(StreamShutdownMsg); recorder_message_handler!(TransmitDataMsg);