Remove sleep from tests (#4639)

* Remove sleep and use polkadot test service

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* updates

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Fix other tests

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Run metrics tests separately

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* copy some substrate utilities

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* update runtime metric test

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Remove sleep from cli tests

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* cargo

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Polkadot companion for Substrate#10463 (#4519)

* Grandpa and Beefy protocol names include chain id

Signed-off-by: acatangiu <adrian@parity.io>

* chain_spec: include fork id

* use simplified protocol name

* fix after merge

* avoid using hash default, even for protocol names

* update lockfile for substrate

Co-authored-by: parity-processbot <>

* configuration: Update upgrade validation delay doc (#4662)

* typo

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* review feedback

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* cargo lock

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* use testnet profile

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Don't run with runtime-benchmark feature

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* conditional compile up one level

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Co-authored-by: Sergei Shulepov <sergei@parity.io>

Cargo.toml

@@ -21,6 +21,9 @@ parity-util-mem = { version = "*", default-features = false, features = ["jemall
 assert_cmd = "2.0.2"
 nix = "0.23.1"
 tempfile = "3.2.0"
+tokio = "1.15.0"
+remote-externalities = { git = "https://github.com/paritytech/substrate", branch = "master" }
+node-primitives = { git = "https://github.com/paritytech/substrate", branch = "master" }
 
 [workspace]
 members = [

cli/Cargo.toml

@@ -52,7 +52,7 @@ cli = [
 	"polkadot-node-core-pvf",
 	"polkadot-performance-test",
 ]
-runtime-benchmarks = [ "service/runtime-benchmarks" ]
+runtime-benchmarks = [ "service/runtime-benchmarks", "polkadot-node-metrics/runtime-benchmarks" ]
 trie-memory-tracker = [ "sp-trie/memory-tracker" ]
 full-node = [ "service/full-node" ]
 try-runtime = [ "service/try-runtime" ]

node/metrics/Cargo.toml

@@ -29,7 +29,14 @@ nix = "0.23.1"
 tempfile = "3.2.0"
 hyper = { version = "0.14.16", default-features = false, features = ["http1", "tcp"] }
 tokio = "1.13"
+polkadot-test-service = { path = "../test/service", features=["runtime-metrics"]}
+substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }
+sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" }
+sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
+sc-client-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
+prometheus-parse = {version = "0.2.2"}
 
 [features]
 default = []
 runtime-metrics = []
+runtime-benchmarks = []

node/metrics/src/lib.rs

@@ -81,5 +81,5 @@ pub mod metrics {
 	}
 }
 
-#[cfg(test)]
+#[cfg(all(feature = "runtime-metrics", not(feature = "runtime-benchmarks"), test))]
 mod tests;

node/metrics/src/tests.rs

@@ -13,58 +13,95 @@
 
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
-#![cfg(feature = "runtime-metrics")]
-use assert_cmd::cargo::cargo_bin;
-use std::{convert::TryInto, process::Command, thread, time::Duration};
-use tempfile::tempdir;
-
-#[test]
-#[cfg(unix)]
-fn runtime_can_publish_metrics() {
-	use hyper::{Client, Uri};
-	use nix::{
-		sys::signal::{kill, Signal::SIGINT},
-		unistd::Pid,
+
+//! Polkadot runtime metrics integration test.
+
+use hyper::{Client, Uri};
+use polkadot_test_service::{node_config, run_validator_node, test_prometheus_config};
+use primitives::v1::metric_definitions::PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED;
+use sc_client_api::{execution_extensions::ExecutionStrategies, ExecutionStrategy};
+use sp_keyring::AccountKeyring::*;
+use std::{collections::HashMap, convert::TryFrom};
+
+const DEFAULT_PROMETHEUS_PORT: u16 = 9616;
+
+#[substrate_test_utils::test]
+async fn runtime_can_publish_metrics() {
+	let mut alice_config =
+		node_config(|| {}, tokio::runtime::Handle::current(), Alice, Vec::new(), true);
+
+	// Enable Prometheus metrics for Alice.
+	alice_config.prometheus_config = Some(test_prometheus_config(DEFAULT_PROMETHEUS_PORT));
+
+	let mut builder = sc_cli::LoggerBuilder::new("");
+
+	// Enable profiling with `wasm_tracing` target.
+	builder.with_profiling(Default::default(), String::from("wasm_tracing=trace"));
+
+	// Setup the runtime metrics provider.
+	crate::logger_hook()(&mut builder, &alice_config);
+
+	// Override default native strategy, runtime metrics are available only in the wasm runtime.
+	alice_config.execution_strategies = ExecutionStrategies {
+		syncing: ExecutionStrategy::AlwaysWasm,
+		importing: ExecutionStrategy::AlwaysWasm,
+		block_construction: ExecutionStrategy::AlwaysWasm,
+		offchain_worker: ExecutionStrategy::AlwaysWasm,
+		other: ExecutionStrategy::AlwaysWasm,
 	};
-	use std::convert::TryFrom;
 
-	const RUNTIME_METRIC_NAME: &str = "polkadot_parachain_inherent_data_bitfields_processed";
-	const DEFAULT_PROMETHEUS_PORT: u16 = 9615;
+	builder.init().expect("Failed to set up the logger");
+
+	// Start validator Alice.
+	let alice = run_validator_node(alice_config, None);
+
+	let bob_config =
+		node_config(|| {}, tokio::runtime::Handle::current(), Bob, vec![alice.addr.clone()], true);
+
+	// Start validator Bob.
+	let _bob = run_validator_node(bob_config, None);
+
+	// Wait for Alice to author two blocks.
+	alice.wait_for_blocks(2).await;
+
 	let metrics_uri = format!("http://localhost:{}/metrics", DEFAULT_PROMETHEUS_PORT);
+	let metrics = scrape_prometheus_metrics(&metrics_uri).await;
+
+	// There should be at least 1 bitfield processed by now.
+	assert!(
+		*metrics
+			.get(&PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED.name.to_owned())
+			.unwrap() > 1
+	);
+}
+
+async fn scrape_prometheus_metrics(metrics_uri: &str) -> HashMap<String, u64> {
+	let res = Client::new()
+		.get(Uri::try_from(metrics_uri).expect("bad URI"))
+		.await
+		.expect("GET request failed");
+
+	// Retrieve the `HTTP` response body.
+	let body = String::from_utf8(
+		hyper::body::to_bytes(res).await.expect("can't get body as bytes").to_vec(),
+	)
+	.expect("body is not an UTF8 string");
 
-	// Start the node with tracing enabled and forced wasm runtime execution.
-	let cmd = Command::new(cargo_bin("polkadot"))
-		// Runtime metrics require this trace target.
-		.args(&["--tracing-targets", "wasm_tracing=trace"])
-		.args(&["--execution", "wasm"])
-		.args(&["--dev", "-d"])
-		.arg(tempdir().expect("failed to create temp dir.").path())
-		.spawn()
-		.expect("failed to start the node process");
-
-	// Enough time to author one block.
-	thread::sleep(Duration::from_secs(30));
-
-	let runtime = tokio::runtime::Runtime::new().expect("failed to create tokio runtime");
-
-	runtime.block_on(async {
-		let client = Client::new();
-
-		let res = client
-			.get(Uri::try_from(&metrics_uri).expect("bad URI"))
-			.await
-			.expect("get request failed");
-
-		let body = String::from_utf8(
-			hyper::body::to_bytes(res).await.expect("can't get body as bytes").to_vec(),
-		)
-		.expect("body is not an UTF8 string");
-
-		// Time to die.
-		kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT)
-			.expect("failed to kill the node process");
-
-		// If the node has authored at least 1 block this should pass.
-		assert!(body.contains(&RUNTIME_METRIC_NAME));
-	});
+	let lines: Vec<_> = body.lines().map(|s| Ok(s.to_owned())).collect();
+	prometheus_parse::Scrape::parse(lines.into_iter())
+		.expect("Scraper failed to parse Prometheus metrics")
+		.samples
+		.into_iter()
+		.map(|sample| {
+			(
+				sample.metric.to_owned(),
+				match sample.value {
+					prometheus_parse::Value::Counter(value) => value as u64,
+					prometheus_parse::Value::Gauge(value) => value as u64,
+					prometheus_parse::Value::Untyped(value) => value as u64,
+					_ => unreachable!("unexpected metric type"),
+				},
+			)
+		})
+		.collect()
 }

node/test/service/Cargo.toml

@@ -62,3 +62,6 @@ pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "m
 serde_json = "1.0.74"
 substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }
 tokio = { version = "1.15", features = ["macros"] }
+
+[features]
+runtime-metrics=["polkadot-test-runtime/runtime-metrics"]

node/test/service/src/lib.rs

@@ -28,7 +28,9 @@ use polkadot_overseer::Handle;
 use polkadot_primitives::v1::{Balance, CollatorPair, HeadData, Id as ParaId, ValidationCode};
 use polkadot_runtime_common::BlockHashCount;
 use polkadot_runtime_parachains::paras::ParaGenesisArgs;
-use polkadot_service::{ClientHandle, Error, ExecuteWithClient, FullClient, IsCollator, NewFull};
+use polkadot_service::{
+	ClientHandle, Error, ExecuteWithClient, FullClient, IsCollator, NewFull, PrometheusConfig,
+};
 use polkadot_test_runtime::{
 	ParasSudoWrapperCall, Runtime, SignedExtra, SignedPayload, SudoCall, UncheckedExtrinsic,
 	VERSION,
@@ -48,7 +50,11 @@ use sp_blockchain::HeaderBackend;
 use sp_keyring::Sr25519Keyring;
 use sp_runtime::{codec::Encode, generic, traits::IdentifyAccount, MultiSigner};
 use sp_state_machine::BasicExternalities;
-use std::{path::PathBuf, sync::Arc};
+use std::{
+	net::{Ipv4Addr, SocketAddr},
+	path::PathBuf,
+	sync::Arc,
+};
 use substrate_test_client::{
 	BlockchainEventsExt, RpcHandlersExt, RpcTransactionError, RpcTransactionOutput,
 };
@@ -102,6 +108,14 @@ impl ClientHandle for TestClient {
 	}
 }
 
+/// Returns a prometheus config usable for testing.
+pub fn test_prometheus_config(port: u16) -> PrometheusConfig {
+	PrometheusConfig::new_with_default_registry(
+		SocketAddr::new(Ipv4Addr::LOCALHOST.into(), port),
+		"test-chain".to_string(),
+	)
+}
+
 /// Create a Polkadot `Configuration`.
 ///
 /// By default an in-memory socket will be used, therefore you need to provide boot
@@ -160,7 +174,7 @@ pub fn node_config(
 		keep_blocks: KeepBlocks::All,
 		transaction_storage: TransactionStorageMode::BlockBody,
 		chain_spec: Box::new(spec),
-		wasm_method: WasmExecutionMethod::Interpreted,
+		wasm_method: WasmExecutionMethod::Compiled,
 		wasm_runtime_overrides: Default::default(),
 		// NOTE: we enforce the use of the native runtime to make the errors more debuggable
 		execution_strategies: ExecutionStrategies {
@@ -195,21 +209,11 @@ pub fn node_config(
 	}
 }
 
-/// Run a test validator node that uses the test runtime.
-///
-/// The node will be using an in-memory socket, therefore you need to provide boot nodes if you
-/// want it to be connected to other nodes.
-///
-/// The `storage_update_func` function will be executed in an externalities provided environment
-/// and can be used to make adjustments to the runtime genesis storage.
+/// Run a test validator node that uses the test runtime and specified `config`.
 pub fn run_validator_node(
-	tokio_handle: tokio::runtime::Handle,
-	key: Sr25519Keyring,
-	storage_update_func: impl Fn(),
-	boot_nodes: Vec<MultiaddrWithPeerId>,
+	config: Configuration,
 	worker_program_path: Option<PathBuf>,
 ) -> PolkadotTestNode {
-	let config = node_config(storage_update_func, tokio_handle, key, boot_nodes, true);
 	let multiaddr = config.network.listen_addresses[0].clone();
 	let NewFull { task_manager, client, network, rpc_handlers, overseer_handle, .. } =
 		new_full(config, IsCollator::No, worker_program_path)

node/test/service/tests/build-blocks.rs

@@ -23,21 +23,23 @@ async fn ensure_test_service_build_blocks() {
 	let mut builder = sc_cli::LoggerBuilder::new("");
 	builder.with_colors(false);
 	builder.init().expect("Sets up logger");
-
-	let mut alice = run_validator_node(
+	let alice_config = node_config(
+		|| {},
 		tokio::runtime::Handle::current(),
 		Sr25519Keyring::Alice,
-		|| {},
 		Vec::new(),
-		None,
+		true,
 	);
-	let mut bob = run_validator_node(
+	let mut alice = run_validator_node(alice_config, None);
+
+	let bob_config = node_config(
+		|| {},
 		tokio::runtime::Handle::current(),
 		Sr25519Keyring::Bob,
-		|| {},
 		vec![alice.addr.clone()],
-		None,
+		true,
 	);
+	let mut bob = run_validator_node(bob_config, None);
 
 	{
 		let t1 = future::join(alice.wait_for_blocks(3), bob.wait_for_blocks(3)).fuse();

node/test/service/tests/call-function.rs

@@ -19,8 +19,10 @@ use sp_keyring::Sr25519Keyring::{Alice, Bob, Charlie};
 
 #[substrate_test_utils::test]
 async fn call_function_actually_work() {
-	let alice =
-		run_validator_node(tokio::runtime::Handle::current(), Alice, || {}, Vec::new(), None);
+	let alice_config =
+		node_config(|| {}, tokio::runtime::Handle::current(), Alice, Vec::new(), true);
+
+	let alice = run_validator_node(alice_config, None);
 
 	let function = polkadot_test_runtime::Call::Balances(pallet_balances::Call::transfer {
 		dest: Charlie.to_account_id().into(),