Less sleeps

bin/node/cli/Cargo.toml

@@ -124,6 +124,12 @@ regex = "1"
 platforms = "1.1"
 async-std = { version = "1.6.5", features = ["attributes"] }
 soketto = "0.4.2"
+jsonrpsee-ws-client = { version = "0.3.0", default-features = false, features = [
+	"tokio1",
+] }
+tokio = { version = "1.10", features = ["time"] }
+tokio-test = "0.4.2"
+wait-timeout = "0.2"
 
 [build-dependencies]
 structopt = { version = "0.3.8", optional = true }
@@ -135,7 +141,7 @@ try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../ut
 sc-cli = { version = "0.10.0-dev", path = "../../../client/cli", optional = true }
 
 [features]
-default = [ "cli" ]
+default = ["cli"]
 cli = [
 	"node-executor/wasmi-errno",
 	"node-inspect",

bin/node/cli/src/chain_spec.rs

@@ -265,7 +265,7 @@ pub fn testnet_genesis(
 		.map(|x| &x.0)
 		.chain(initial_nominators.iter())
 		.for_each(|x| {
-			if !endowed_accounts.contains(&x) {
+			if !endowed_accounts.contains(x) {
 				endowed_accounts.push(x.clone())
 			}
 		});

bin/node/cli/src/service.rs

@@ -86,7 +86,7 @@ pub fn new_partial(
 
 	let (client, backend, keystore_container, task_manager) =
 		sc_service::new_full_parts::<Block, RuntimeApi, _>(
-			&config,
+			config,
 			telemetry.as_ref().map(|(_, telemetry)| telemetry.handle()),
 			executor,
 		)?;
@@ -277,7 +277,7 @@ pub fn new_full_base(
 
 	let _rpc_handlers = sc_service::spawn_tasks(sc_service::SpawnTasksParams {
 		config,
-		backend: backend.clone(),
+		backend,
 		client: client.clone(),
 		keystore: keystore_container.sync_keystore(),
 		network: network.clone(),
@@ -507,7 +507,7 @@ pub fn new_light_base(
 		babe_block_import,
 		Some(Box::new(justification_import)),
 		client.clone(),
-		select_chain.clone(),
+		select_chain,
 		move |_, ()| async move {
 			let timestamp = sp_timestamp::InherentDataProvider::from_system_time();
 

bin/node/cli/tests/common.rs

@@ -30,42 +30,89 @@ use std::{
 	thread,
 	time::Duration,
 };
+use tokio::time::timeout;
+
+static LOCALHOST_WS: &str = "ws://127.0.0.1:9944/";
 
 /// Wait for the given `child` the given number of `secs`.
 ///
 /// Returns the `Some(exit status)` or `None` if the process did not finish in the given time.
-pub fn wait_for(child: &mut Child, secs: usize) -> Option<ExitStatus> {
-	for i in 0..secs {
-		match child.try_wait().unwrap() {
-			Some(status) => {
-				if i > 5 {
-					eprintln!("Child process took {} seconds to exit gracefully", i);
-				}
-				return Some(status)
-			},
-			None => thread::sleep(Duration::from_secs(1)),
+pub fn wait_for(child: &mut Child, secs: u64) -> Result<ExitStatus, ()> {
+	assert!(secs > 5);
+
+	let result =
+		wait_timeout::ChildExt::wait_timeout(child, Duration::from_secs(5)).map_err(|_| ())?;
+	if let Some(exit_status) = result {
+		Ok(exit_status)
+	} else {
+		eprintln!("Child process taking over 5 seconds to exit gracefully");
+		let result = wait_timeout::ChildExt::wait_timeout(child, Duration::from_secs(secs - 5))
+			.map_err(|_| ())?;
+		if let Some(exit_status) = result {
+			Ok(exit_status)
+		} else {
+			eprintln!("Took too long to exit (> {} seconds). Killing...", secs);
+			let _ = child.kill();
+			child.wait().unwrap();
+			Err(())
 		}
 	}
-	eprintln!("Took too long to exit (> {} seconds). Killing...", secs);
-	let _ = child.kill();
-	child.wait().unwrap();
+}
 
-	None
+pub async fn wait_n_blocks(n: usize, timeout_secs: u64) -> Result<(), tokio::time::error::Elapsed> {
+	timeout(Duration::from_secs(timeout_secs), wait_n_blocks_from(n, LOCALHOST_WS)).await
 }
 
-/// Run the node for a while (30 seconds)
+/// Wait for at least n blocks to be produced
+///
+/// Eg. to wait for 3 blocks or a timeout of 30 seconds:
+/// ```
+/// timeout(Duration::from_secs(30), wait_n_blocks("ws://127.0.0.1:9944/", 3)).await;
+/// ```
+pub async fn wait_n_blocks_from(n: usize, url: &str) {
+	let mut built_blocks = std::collections::HashSet::new();
+	let mut interval = tokio::time::interval(Duration::from_secs(2));
+
+	loop {
+		// We could call remote-externalities like this:
+		// = remote_externalities::rpc_api::get_finalized_head::<String,
+		// String>("ws://127.0.0.1:9944/".to_string()).await;
+		// but then we'd need to gen the types to get the BlockT type.
+		// https://github.com/paritytech/substrate-subxt/blob/aj-metadata-vnext/proc-macro/src/generate_types.rs
+
+		if let Ok(ws_client) = jsonrpsee_ws_client::WsClientBuilder::default().build(url).await {
+			let block_result: Result<String, _> =
+				<jsonrpsee_ws_client::WsClient as jsonrpsee_ws_client::types::traits::Client>::request(
+					&ws_client,
+					"chain_getFinalizedHead",
+					jsonrpsee_ws_client::types::v2::params::JsonRpcParams::NoParams,
+				)
+				.await;
+			if let Ok(block) = block_result {
+				built_blocks.insert(block);
+				if built_blocks.len() > n {
+					break
+				}
+			}
+		}
+		interval.tick().await;
+	}
+}
+
+/// Run the node for a while (3 blocks)
 pub fn run_node_for_a_while(base_path: &Path, args: &[&str]) {
 	let mut cmd = Command::new(cargo_bin("substrate"));
 
 	let mut cmd = cmd.args(args).arg("-d").arg(base_path).spawn().unwrap();
 
 	// Let it produce some blocks.
-	thread::sleep(Duration::from_secs(30));
+	tokio_test::block_on(wait_n_blocks(3, 30)).unwrap();
+
 	assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running");
 
 	// Stop the process
 	kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap();
-	assert!(wait_for(&mut cmd, 40).map(|x| x.success()).unwrap_or_default());
+	assert!(wait_for(&mut cmd, 40).map(|x| x.success()).unwrap());
 }
 
 /// Run the node asserting that it fails with an error

bin/node/cli/tests/running_the_node_and_interrupt.rs

@@ -17,7 +17,6 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 #![cfg(unix)]
-
 use assert_cmd::cargo::cargo_bin;
 use nix::{
 	sys::signal::{
@@ -31,36 +30,34 @@ use std::{
 	convert::TryInto,
 	ops::DerefMut,
 	process::{Child, Command},
-	thread,
-	time::Duration,
 };
 use tempfile::tempdir;
 
 pub mod common;
 
-#[test]
-fn running_the_node_works_and_can_be_interrupted() {
-	fn run_command_and_kill(signal: Signal) {
+#[tokio::test]
+async fn running_the_node_works_and_can_be_interrupted() {
+	async fn run_command_and_kill(signal: Signal) {
 		let base_path = tempdir().expect("could not create a temp dir");
 		let mut cmd = Command::new(cargo_bin("substrate"))
 			.args(&["--dev", "-d"])
 			.arg(base_path.path())
 			.spawn()
 			.unwrap();
 
-		thread::sleep(Duration::from_secs(20));
+		common::wait_n_blocks(3, 30).await.unwrap();
 		assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running");
 		kill(Pid::from_raw(cmd.id().try_into().unwrap()), signal).unwrap();
 		assert_eq!(
 			common::wait_for(&mut cmd, 30).map(|x| x.success()),
-			Some(true),
+			Ok(true),
 			"the process must exit gracefully after signal {}",
 			signal,
 		);
 	}
 
-	run_command_and_kill(SIGINT);
-	run_command_and_kill(SIGTERM);
+	run_command_and_kill(SIGINT).await;
+	run_command_and_kill(SIGTERM).await;
 }
 
 struct KillChildOnDrop(Child);
@@ -85,8 +82,8 @@ impl DerefMut for KillChildOnDrop {
 	}
 }
 
-#[test]
-fn running_two_nodes_with_the_same_ws_port_should_work() {
+#[tokio::test]
+async fn running_two_nodes_with_the_same_ws_port_should_work() {
 	fn start_node() -> Child {
 		Command::new(cargo_bin("substrate"))
 			.args(&["--dev", "--tmp", "--ws-port=45789"])
@@ -97,7 +94,7 @@ fn running_two_nodes_with_the_same_ws_port_should_work() {
 	let mut first_node = KillChildOnDrop(start_node());
 	let mut second_node = KillChildOnDrop(start_node());
 
-	thread::sleep(Duration::from_secs(30));
+	let _ = common::wait_n_blocks(3, 30).await;
 
 	assert!(first_node.try_wait().unwrap().is_none(), "The first node should still be running");
 	assert!(second_node.try_wait().unwrap().is_none(), "The second node should still be running");
@@ -107,12 +104,12 @@ fn running_two_nodes_with_the_same_ws_port_should_work() {
 
 	assert_eq!(
 		common::wait_for(&mut first_node, 30).map(|x| x.success()),
-		Some(true),
+		Ok(true),
 		"The first node must exit gracefully",
 	);
 	assert_eq!(
 		common::wait_for(&mut second_node, 30).map(|x| x.success()),
-		Some(true),
+		Ok(true),
 		"The second node must exit gracefully",
 	);
 }

bin/node/cli/tests/temp_base_path_works.rs

@@ -29,25 +29,24 @@ use std::{
 	io::Read,
 	path::PathBuf,
 	process::{Command, Stdio},
-	thread,
-	time::Duration,
 };
 
 pub mod common;
 
-#[test]
-fn temp_base_path_works() {
+#[tokio::test]
+async fn temp_base_path_works() {
+	// Test depends on log output so set RUST_LOG:
 	let mut cmd = Command::new(cargo_bin("substrate"));
-
 	let mut cmd = cmd
 		.args(&["--dev", "--tmp"])
+		.env("RUST_LOG", "info")
 		.stdout(Stdio::piped())
 		.stderr(Stdio::piped())
 		.spawn()
 		.unwrap();
 
 	// Let it produce some blocks.
-	thread::sleep(Duration::from_secs(30));
+	common::wait_n_blocks(3, 30).await.unwrap();
 	assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running");
 
 	// Stop the process
@@ -58,8 +57,7 @@ fn temp_base_path_works() {
 	let mut stderr = String::new();
 	cmd.stderr.unwrap().read_to_string(&mut stderr).unwrap();
 	let re = Regex::new(r"Database: .+ at (\S+)").unwrap();
-	let db_path =
-		PathBuf::from(re.captures(stderr.as_str()).unwrap().get(1).unwrap().as_str().to_string());
+	let db_path = PathBuf::from(re.captures(stderr.as_str()).unwrap().get(1).unwrap().as_str());
 
 	assert!(!db_path.exists());
 }

client/executor/src/native_executor.rs

@@ -192,7 +192,7 @@ impl WasmExecutor {
 
 	/// Perform a call into the given runtime.
 	///
-	/// The runtime is passed as a [`RuntimeBlob`]. The runtime will be isntantiated with the
+	/// The runtime is passed as a [`RuntimeBlob`]. The runtime will be instantiated with the
 	/// parameters this `WasmExecutor` was initialized with.
 	///
 	/// In case of problems with during creation of the runtime or instantation, a `Err` is
@@ -250,7 +250,7 @@ impl sp_core::traits::ReadRuntimeVersion for WasmExecutor {
 		}
 
 		// If the blob didn't have embedded runtime version section, we fallback to the legacy
-		// way of fetching the verison: i.e. instantiating the given instance and calling
+		// way of fetching the version: i.e. instantiating the given instance and calling
 		// `Core_version` on it.
 
 		self.uncached_call(