From fa2c486c83903efbb02a2c9c16ef3047c32e86c3 Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe Date: Wed, 24 May 2023 10:39:42 +0300 Subject: [PATCH 1/2] metrics: tests: Fix flaky runtime_can_publish_metrics When an re-org happens wait_for_blocks(2) would actually exit after the second import of blocks 1, so the conditions for the metric to exist won't be met hence the occasional test failure. More details in: https://github.com/paritytech/polkadot/issues/7267 Signed-off-by: Alexandru Gheorghe --- node/metrics/src/tests.rs | 4 ++-- node/test/service/src/lib.rs | 24 +++++++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/node/metrics/src/tests.rs b/node/metrics/src/tests.rs index 486a88b99312..f8f988e1f665 100644 --- a/node/metrics/src/tests.rs +++ b/node/metrics/src/tests.rs @@ -61,8 +61,8 @@ async fn runtime_can_publish_metrics() { // Start validator Bob. let _bob = run_validator_node(bob_config, None); - // Wait for Alice to author two blocks. - alice.wait_for_blocks(2).await; + // Wait for Alice to see two finalized blocks. + alice.wait_for_finalized_blocks(2).await; let metrics_uri = format!("http://localhost:{}/metrics", DEFAULT_PROMETHEUS_PORT); let metrics = scrape_prometheus_metrics(&metrics_uri).await; diff --git a/node/test/service/src/lib.rs b/node/test/service/src/lib.rs index e36c74071b1a..2c163b4e651f 100644 --- a/node/test/service/src/lib.rs +++ b/node/test/service/src/lib.rs @@ -21,7 +21,7 @@ pub mod chain_spec; pub use chain_spec::*; -use futures::future::Future; +use futures::{future::Future, stream::StreamExt}; use polkadot_node_primitives::{CollationGenerationConfig, CollatorFn}; use polkadot_node_subsystem::messages::{CollationGenerationMessage, CollatorProtocolMessage}; use polkadot_overseer::Handle; @@ -35,8 +35,9 @@ use polkadot_test_runtime::{ ParasCall, ParasSudoWrapperCall, Runtime, SignedExtra, SignedPayload, SudoCall, UncheckedExtrinsic, VERSION, }; + use sc_chain_spec::ChainSpec; -use sc_client_api::execution_extensions::ExecutionStrategies; +use sc_client_api::{execution_extensions::ExecutionStrategies, BlockchainEvents}; use sc_network::{ config::{NetworkConfiguration, TransportConfig}, multiaddr, NetworkStateInfo, @@ -54,6 +55,7 @@ use sp_keyring::Sr25519Keyring; use sp_runtime::{codec::Encode, generic, traits::IdentifyAccount, MultiSigner}; use sp_state_machine::BasicExternalities; use std::{ + collections::HashSet, net::{Ipv4Addr, SocketAddr}, path::PathBuf, sync::Arc, @@ -61,7 +63,6 @@ use std::{ use substrate_test_client::{ BlockchainEventsExt, RpcHandlersExt, RpcTransactionError, RpcTransactionOutput, }; - /// Declare an instance of the native executor named `PolkadotTestExecutorDispatch`. Include the wasm binary as the /// equivalent wasm code. pub struct PolkadotTestExecutorDispatch; @@ -335,6 +336,23 @@ impl PolkadotTestNode { self.client.wait_for_blocks(count) } + /// Wait for `count` blocks to be finalized and then exit. Similarly with `wait_for_blocks` this function will + /// not return if no block are ever finalized. + pub async fn wait_for_finalized_blocks(&self, count: usize) { + let mut import_notification_stream = self.client.finality_notification_stream(); + let mut blocks = HashSet::new(); + + Box::pin(async move { + while let Some(notification) = import_notification_stream.next().await { + blocks.insert(notification.hash); + if blocks.len() == count { + break + } + } + }) + .await; + } + /// Register the collator functionality in the overseer of this node. pub async fn register_collator( &mut self, From 6d42862d025ad166bc69c1d8ad75a8653ba7a348 Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe Date: Wed, 24 May 2023 10:39:42 +0300 Subject: [PATCH 2/2] metrics: tests: Cleanup un-needed box pin Signed-off-by: Alexandru Gheorghe --- node/test/service/src/lib.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/node/test/service/src/lib.rs b/node/test/service/src/lib.rs index 2c163b4e651f..3e7f66128886 100644 --- a/node/test/service/src/lib.rs +++ b/node/test/service/src/lib.rs @@ -342,15 +342,12 @@ impl PolkadotTestNode { let mut import_notification_stream = self.client.finality_notification_stream(); let mut blocks = HashSet::new(); - Box::pin(async move { - while let Some(notification) = import_notification_stream.next().await { - blocks.insert(notification.hash); - if blocks.len() == count { - break - } + while let Some(notification) = import_notification_stream.next().await { + blocks.insert(notification.hash); + if blocks.len() == count { + break } - }) - .await; + } } /// Register the collator functionality in the overseer of this node.