refactor: removed redundant code

Author: DenzelPenzel

substrate/zombienet/zombienet-sdk/tests/utils.rs

@@ -1,16 +1,30 @@
 // Copyright (C) Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: Apache-2.0
 
+use std::time::Duration;
+
+use zombienet_orchestrator::network::node::LogLineCountOptions;
 use zombienet_sdk::{LocalFileSystem, Network, NetworkConfig};
 
 pub const BEST_BLOCK_METRIC: &str = "block_height{status=\"best\"}";
 pub const FINALIZED_BLOCK_METRIC: &str = "substrate_block_height{status=\"finalized\"}";
 pub const BEEFY_BEST_BLOCK_METRIC: &str = "substrate_beefy_best_block";
 pub const DEFAULT_SUBSTRATE_IMAGE: &str = "docker.io/paritypr/substrate:latest";
+pub const NODE_ROLE_METRIC: &str = "node_roles";
+pub const PEER_COUNT_METRIC: &str = "substrate_sub_libp2p_peers_count";
 
 pub const DEFAULT_DB_SNAPSHOT_URL: &str =
 	"https://storage.googleapis.com/zombienet-db-snaps/substrate/0001-basic-warp-sync/chains-0bb3f0be2ce41b5615b224215bcc8363aa0416a6.tgz";
-pub const DEFAULT_CHAIN_SPEC: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/chain-spec.json");
+pub const DEFAULT_CHAIN_SPEC: &str =
+	"https://storage.googleapis.com/zombienet-db-snaps/substrate/chain-spec.json";
+
+pub const FULLNODE_ROLE_VALUE: f64 = 1.0;
+pub const VALIDATOR_ROLE_VALUE: f64 = 4.0;
+
+pub const INTEGRATION_IMAGE_ENV: &str = "ZOMBIENET_INTEGRATION_TEST_IMAGE";
+pub const DB_SNAPSHOT_ENV: &str = "DB_SNAPSHOT";
+pub const CHAIN_SPEC_ENV: &str = "WARP_CHAIN_SPEC_PATH";
+pub const DB_BLOCK_HEIGHT_ENV: &str = "DB_BLOCK_HEIGHT";
 
 pub async fn initialize_network(
 	config: NetworkConfig,
@@ -27,3 +41,46 @@ pub async fn initialize_network(
 
 	Ok(network)
 }
+
+pub fn ensure_env_default(var: &str, default: &str) {
+	if std::env::var(var).is_err() {
+		std::env::set_var(var, default);
+	}
+}
+
+pub fn ensure_env_defaults(defaults: &[(&str, &str)]) {
+	for &(var, default_value) in defaults {
+		ensure_env_default(var, default_value);
+	}
+}
+
+pub fn db_snapshot_height_override_from_env() -> Option<f64> {
+	std::env::var(DB_BLOCK_HEIGHT_ENV)
+		.ok()
+		.and_then(|value| value.parse::<f64>().ok())
+}
+
+pub async fn resolve_db_snapshot_height(
+	network: &Network<LocalFileSystem>,
+	node_name: &str,
+) -> anyhow::Result<f64> {
+	if let Some(override_height) = db_snapshot_height_override_from_env() {
+		return Ok(override_height);
+	}
+
+	let node = network.get_node(node_name)?;
+	let height = node.reports(BEST_BLOCK_METRIC).await?;
+	Ok(height)
+}
+
+pub fn log_line_at_least_once(timeout_secs: u64) -> LogLineCountOptions {
+	LogLineCountOptions::new(|count| count >= 1, Duration::from_secs(timeout_secs), false)
+}
+
+pub fn log_line_exactly_once(timeout_secs: u64) -> LogLineCountOptions {
+	LogLineCountOptions::new(|count| count == 1, Duration::from_secs(timeout_secs), false)
+}
+
+pub fn log_line_absent(timeout_secs: u64) -> LogLineCountOptions {
+	LogLineCountOptions::no_occurences_within_timeout(Duration::from_secs(timeout_secs))
+}

substrate/zombienet/zombienet-sdk/tests/zombie_ci/basic_warp_sync.rs

@@ -1,19 +1,18 @@
 // Copyright (C) Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::time::Duration;
-
 use crate::utils::{
-	initialize_network, BEST_BLOCK_METRIC, DEFAULT_CHAIN_SPEC, DEFAULT_DB_SNAPSHOT_URL,
+	ensure_env_defaults, initialize_network, log_line_absent, log_line_at_least_once,
+	resolve_db_snapshot_height, BEST_BLOCK_METRIC, CHAIN_SPEC_ENV, DB_SNAPSHOT_ENV,
+	DEFAULT_CHAIN_SPEC, DEFAULT_DB_SNAPSHOT_URL, DEFAULT_SUBSTRATE_IMAGE, FULLNODE_ROLE_VALUE,
+	INTEGRATION_IMAGE_ENV, NODE_ROLE_METRIC, PEER_COUNT_METRIC,
 };
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use env_logger::Env;
-use zombienet_orchestrator::network::node::LogLineCountOptions;
 use zombienet_sdk::{NetworkConfig, NetworkConfigBuilder, NetworkNode};
 
 const BEST_BLOCK_THRESHOLD: f64 = 1.0;
 const PEERS_THRESHOLD: f64 = 3.0;
-const VALIDATOR_ROLE_VALUE: f64 = 1.0;
 
 const NETWORK_READY_TIMEOUT_SECS: u64 = 180;
 const ROLE_TIMEOUT_SECS: u64 = 60;
@@ -22,35 +21,36 @@ const METRIC_TIMEOUT_SECS: u64 = 60;
 const LOG_TIMEOUT_LONG_SECS: u64 = 60;
 const LOG_TIMEOUT_SHORT_SECS: u64 = 10;
 const LOG_ERROR_TIMEOUT_SECS: u64 = 10;
-const NODE_ROLE_METRIC: &str = "node_roles";
-const PEER_COUNT_METRIC: &str = "substrate_sub_libp2p_peers_count";
-const NODE_NAMES: [&str; 4] = ["alice", "bob", "charlie", "dave"];
 const SNAPSHOT_NODES: [&str; 3] = ["alice", "bob", "charlie"];
-const INTEGRATION_IMAGE_ENV: &str = "ZOMBIENET_INTEGRATION_TEST_IMAGE";
-const DB_SNAPSHOT_ENV: &str = "DB_SNAPSHOT";
-const CHAIN_SPEC_ENV: &str = "WARP_CHAIN_SPEC_PATH";
-const DB_BLOCK_HEIGHT_ENV: &str = "DB_BLOCK_HEIGHT";
-const DEFAULT_SUBSTRATE_IMAGE: &str = "docker.io/paritypr/substrate:latest";
-
+const NODE_ROLE_EXPECTATIONS: [(&str, f64); 4] = [
+	("alice", FULLNODE_ROLE_VALUE),
+	("bob", FULLNODE_ROLE_VALUE),
+	("charlie", FULLNODE_ROLE_VALUE),
+	("dave", FULLNODE_ROLE_VALUE),
+];
 #[tokio::test(flavor = "multi_thread")]
 async fn basic_warp_sync() -> Result<()> {
 	let _ = env_logger::Builder::from_env(Env::default().default_filter_or("info")).try_init();
 
-	ensure_env_defaults();
+	ensure_env_defaults(&[
+		(INTEGRATION_IMAGE_ENV, DEFAULT_SUBSTRATE_IMAGE),
+		(DB_SNAPSHOT_ENV, DEFAULT_DB_SNAPSHOT_URL),
+		(CHAIN_SPEC_ENV, DEFAULT_CHAIN_SPEC),
+	]);
 
 	log::info!("Spawning network");
 	let config = build_network_config()?;
 	let network = initialize_network(config).await?;
 
 	network.wait_until_is_up(NETWORK_READY_TIMEOUT_SECS).await?;
 
-	for node_name in NODE_NAMES {
+	for &(node_name, expected_role) in NODE_ROLE_EXPECTATIONS.iter() {
 		let node = network.get_node(node_name)?;
-		assert_node_roles(node).await?;
+		assert_node_roles(expected_role, node).await?;
 		assert_peers_count(node).await?;
 	}
 
-	let db_snapshot_height = resolve_db_snapshot_height(&network).await?;
+	let db_snapshot_height = resolve_db_snapshot_height(&network, "alice").await?;
 
 	for node_name in SNAPSHOT_NODES {
 		network
@@ -85,36 +85,6 @@ async fn basic_warp_sync() -> Result<()> {
 	Ok(())
 }
 
-fn ensure_env_defaults() {
-	if std::env::var(INTEGRATION_IMAGE_ENV).is_err() {
-		std::env::set_var(INTEGRATION_IMAGE_ENV, DEFAULT_SUBSTRATE_IMAGE);
-	}
-	if std::env::var(DB_SNAPSHOT_ENV).is_err() {
-		std::env::set_var(DB_SNAPSHOT_ENV, DEFAULT_DB_SNAPSHOT_URL);
-	}
-	if std::env::var(CHAIN_SPEC_ENV).is_err() {
-		std::env::set_var(CHAIN_SPEC_ENV, DEFAULT_CHAIN_SPEC);
-	}
-}
-
-fn db_snapshot_height_override() -> Option<f64> {
-	std::env::var(DB_BLOCK_HEIGHT_ENV)
-		.ok()
-		.and_then(|value| value.parse::<f64>().ok())
-}
-
-async fn resolve_db_snapshot_height(
-	network: &zombienet_sdk::Network<zombienet_sdk::LocalFileSystem>,
-) -> Result<f64> {
-	if let Some(override_height) = db_snapshot_height_override() {
-		return Ok(override_height);
-	}
-
-	let alice = network.get_node("alice")?;
-	let height = alice.reports(BEST_BLOCK_METRIC).await?;
-	Ok(height)
-}
-
 fn build_network_config() -> Result<NetworkConfig> {
 	let integration_image = std::env::var(INTEGRATION_IMAGE_ENV)
 		.unwrap_or_else(|_| DEFAULT_SUBSTRATE_IMAGE.to_string());
@@ -159,13 +129,20 @@ fn build_network_config() -> Result<NetworkConfig> {
 		})
 }
 
-async fn assert_node_roles(node: &NetworkNode) -> Result<()> {
+async fn assert_node_roles(expected_role: f64, node: &NetworkNode) -> Result<()> {
+	let node_name = node.name();
+
 	node.wait_metric_with_timeout(
 		NODE_ROLE_METRIC,
-		|role| (role - VALIDATOR_ROLE_VALUE).abs() < f64::EPSILON,
+		|role| role == expected_role,
 		ROLE_TIMEOUT_SECS,
 	)
-	.await?;
+	.await
+	.with_context(|| {
+		format!(
+			"node {node_name} did not expose expected role {expected_role} on metric {NODE_ROLE_METRIC}"
+		)
+	})?;
 
 	Ok(())
 }
@@ -182,53 +159,41 @@ async fn assert_peers_count(node: &NetworkNode) -> Result<()> {
 }
 
 async fn wait_for_warp_logs(node: &NetworkNode) -> Result<()> {
-	let at_least_once = |timeout_secs| {
-		LogLineCountOptions::new(|count| count >= 1, Duration::from_secs(timeout_secs), false)
-	};
-
 	node.wait_log_line_count_with_timeout(
 		"Warp sync is complete",
 		false,
-		at_least_once(LOG_TIMEOUT_LONG_SECS),
+		log_line_at_least_once(LOG_TIMEOUT_LONG_SECS),
 	)
 	.await?;
 	node.wait_log_line_count_with_timeout(
 		r"Checking for displaced leaves after finalization\. leaves=\[0xc5e7b4cfd23932bb930e859865430a35f6741b4732d677822d492ca64cc8d059\]",
 		false,
-		at_least_once(LOG_TIMEOUT_SHORT_SECS),
+		log_line_at_least_once(LOG_TIMEOUT_SHORT_SECS),
 	)
 	.await?;
 	node.wait_log_line_count_with_timeout(
 		"State sync is complete",
 		false,
-		at_least_once(LOG_TIMEOUT_LONG_SECS),
+		log_line_at_least_once(LOG_TIMEOUT_LONG_SECS),
 	)
 	.await?;
 	node.wait_log_line_count_with_timeout(
 		"Block history download is complete",
 		false,
-		at_least_once(LOG_TIMEOUT_SHORT_SECS),
+		log_line_at_least_once(LOG_TIMEOUT_SHORT_SECS),
 	)
 	.await?;
 
 	Ok(())
 }
 
 async fn wait_for_absence_of_errors(node: &NetworkNode) -> Result<()> {
-	node.wait_log_line_count_with_timeout(
-		"error",
-		false,
-		LogLineCountOptions::no_occurences_within_timeout(Duration::from_secs(
-			LOG_ERROR_TIMEOUT_SECS,
-		)),
-	)
-	.await?;
+	node.wait_log_line_count_with_timeout("error", false, log_line_absent(LOG_ERROR_TIMEOUT_SECS))
+		.await?;
 	node.wait_log_line_count_with_timeout(
 		"verification failed",
 		false,
-		LogLineCountOptions::no_occurences_within_timeout(Duration::from_secs(
-			LOG_ERROR_TIMEOUT_SECS,
-		)),
+		log_line_absent(LOG_ERROR_TIMEOUT_SECS),
 	)
 	.await?;
 

substrate/zombienet/zombienet-sdk/tests/zombie_ci/block_building.rs

@@ -3,39 +3,38 @@
 
 use std::time::Duration;
 
-use crate::utils::{initialize_network, DEFAULT_CHAIN_SPEC, DEFAULT_SUBSTRATE_IMAGE};
+use crate::utils::{
+	ensure_env_defaults, initialize_network, log_line_absent, BEST_BLOCK_METRIC, CHAIN_SPEC_ENV,
+	DEFAULT_CHAIN_SPEC, DEFAULT_SUBSTRATE_IMAGE, INTEGRATION_IMAGE_ENV, NODE_ROLE_METRIC,
+	PEER_COUNT_METRIC,
+};
 use anyhow::{anyhow, Result};
 use subxt::{config::substrate::SubstrateConfig, dynamic::tx, OnlineClient};
 use subxt_signer::sr25519::dev;
-use zombienet_orchestrator::network::node::LogLineCountOptions;
 use zombienet_sdk::{NetworkConfig, NetworkConfigBuilder, NetworkNode};
 
 const NODE_NAMES: [&str; 2] = ["alice", "bob"];
 
-const NODE_ROLE_METRIC: &str = "node_roles";
-const PEER_COUNT_METRIC: &str = "substrate_sub_libp2p_peers_count";
-const BEST_BLOCK_METRIC: &str = "block_height{status=\"best\"}";
-
 const ROLE_VALIDATOR_VALUE: f64 = 4.0;
 const PEER_MIN_THRESHOLD: f64 = 1.0;
 const BLOCK_TARGET: f64 = 5.0;
 
 const NETWORK_READY_TIMEOUT_SECS: u64 = 60;
 const METRIC_TIMEOUT_SECS: u64 = 20;
 const LOG_TIMEOUT_SECS: u64 = 2;
-const SCRIPT_TIMEOUT_SECS: u64 = 30;
+const TRANSACTION_TIMEOUT_SECS: u64 = 30;
 
 const REMARK_PAYLOAD: &[u8] = b"block-building-test";
-const INTEGRATION_IMAGE_ENV: &str = "ZOMBIENET_INTEGRATION_TEST_IMAGE";
-const CHAIN_SPEC_ENV: &str = "WARP_CHAIN_SPEC_PATH";
-
 #[tokio::test(flavor = "multi_thread")]
 async fn block_building_test() -> Result<()> {
 	let _ = env_logger::try_init_from_env(
 		env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
 	);
 
-	ensure_env_defaults();
+	ensure_env_defaults(&[
+		(INTEGRATION_IMAGE_ENV, DEFAULT_SUBSTRATE_IMAGE),
+		(CHAIN_SPEC_ENV, DEFAULT_CHAIN_SPEC),
+	]);
 
 	log::info!("Spawning network");
 	let config = build_network_config()?;
@@ -56,15 +55,6 @@ async fn block_building_test() -> Result<()> {
 	Ok(())
 }
 
-fn ensure_env_defaults() {
-	if std::env::var(INTEGRATION_IMAGE_ENV).is_err() {
-		std::env::set_var(INTEGRATION_IMAGE_ENV, DEFAULT_SUBSTRATE_IMAGE);
-	}
-	if std::env::var(CHAIN_SPEC_ENV).is_err() {
-		std::env::set_var(CHAIN_SPEC_ENV, DEFAULT_CHAIN_SPEC);
-	}
-}
-
 fn build_network_config() -> Result<NetworkConfig> {
 	let integration_image = std::env::var(INTEGRATION_IMAGE_ENV)
 		.unwrap_or_else(|_| DEFAULT_SUBSTRATE_IMAGE.to_string());
@@ -101,7 +91,7 @@ async fn assert_node_health(node: &NetworkNode) -> Result<()> {
 
 	node.wait_metric_with_timeout(
 		NODE_ROLE_METRIC,
-		|role| (role - ROLE_VALIDATOR_VALUE).abs() < f64::EPSILON,
+		|role| role == ROLE_VALIDATOR_VALUE,
 		METRIC_TIMEOUT_SECS,
 	)
 	.await?;
@@ -120,12 +110,8 @@ async fn assert_node_health(node: &NetworkNode) -> Result<()> {
 	)
 	.await?;
 
-	node.wait_log_line_count_with_timeout(
-		"error",
-		false,
-		LogLineCountOptions::no_occurences_within_timeout(Duration::from_secs(LOG_TIMEOUT_SECS)),
-	)
-	.await?;
+	node.wait_log_line_count_with_timeout("error", false, log_line_absent(LOG_TIMEOUT_SECS))
+		.await?;
 
 	Ok(())
 }
@@ -137,7 +123,7 @@ async fn submit_transaction_and_wait_finalization(node: &NetworkNode) -> Result<
 	let remark_call =
 		tx("System", "remark", vec![subxt::dynamic::Value::from_bytes(REMARK_PAYLOAD)]);
 
-	tokio::time::timeout(Duration::from_secs(SCRIPT_TIMEOUT_SECS), async {
+	tokio::time::timeout(Duration::from_secs(TRANSACTION_TIMEOUT_SECS), async {
 		client
 			.tx()
 			.sign_and_submit_then_watch_default(&remark_call, &signer)