Fix try-runtime follow-chain, try-runtime upgrade tuple tests, cli test utils (#13794)

* new test for try-runtime tuple stuff

* fix

* remove development comment

* formatting

* remove todo comment

* follow-chain working test

* refactor common cli testing utils

* fix comment

* revert Cargo.lock changes

* update Cargo.lock

* improve doc comment

* fix error typo

* update Cargo.lock

* feature gate try-runtime test

* build_substrate cli test util

* feature gate follow_chain tests

* move fn start_node to test-utils

* improve test pkg name

* use tokio Child and Command

* remove redundant import

* fix ci

* fix ci

* don't leave hanging processes

* improved child process cleanup

* use existing KillChildOnDrop

* remove redundant comment

* Update test-utils/cli/src/lib.rs

Co-authored-by: Koute <koute@users.noreply.github.com>

---------

Co-authored-by: kianenigma <kian@parity.io>
Co-authored-by: Koute <koute@users.noreply.github.com>

substrate/bin/node/cli/Cargo.toml

@@ -127,6 +127,7 @@ tokio-util = { version = "0.7.4", features = ["compat"] }
 wait-timeout = "0.2"
 substrate-rpc-client = { path = "../../../utils/frame/rpc/client" }
 pallet-timestamp = { version = "4.0.0-dev", path = "../../../frame/timestamp" }
+substrate-cli-test-utils = { path = "../../../test-utils/cli" }
 
 [build-dependencies]
 clap = { version = "4.0.9", optional = true }

substrate/bin/node/cli/tests/benchmark_block_works.rs

@@ -23,7 +23,7 @@ use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 /// `benchmark block` works for the dev runtime using the wasm executor.
 #[tokio::test]

substrate/bin/node/cli/tests/benchmark_pallet_works.rs

@@ -21,8 +21,6 @@
 use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 
-pub mod common;
-
 /// `benchmark pallet` works for the different combinations of `steps` and `repeat`.
 #[test]
 fn benchmark_pallet_works() {

substrate/bin/node/cli/tests/check_block_works.rs

@@ -22,7 +22,7 @@ use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[tokio::test]
 async fn check_block_works() {

substrate/bin/node/cli/tests/common.rs

@@ -1,168 +0,0 @@
-// This file is part of Substrate.
-
-// Copyright (C) Parity Technologies (UK) Ltd.
-// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
-
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-
-// This program is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License
-// along with this program. If not, see <https://www.gnu.org/licenses/>.
-
-#![cfg(unix)]
-
-use assert_cmd::cargo::cargo_bin;
-use nix::{
-	sys::signal::{kill, Signal, Signal::SIGINT},
-	unistd::Pid,
-};
-use node_primitives::{Hash, Header};
-use regex::Regex;
-use std::{
-	io::{BufRead, BufReader, Read},
-	ops::{Deref, DerefMut},
-	path::{Path, PathBuf},
-	process::{self, Child, Command},
-	time::Duration,
-};
-
-/// Run the given `future` and panic if the `timeout` is hit.
-pub async fn run_with_timeout(timeout: Duration, future: impl futures::Future<Output = ()>) {
-	tokio::time::timeout(timeout, future).await.expect("Hit timeout");
-}
-
-/// Wait for at least n blocks to be finalized from a specified node
-pub async fn wait_n_finalized_blocks(n: usize, url: &str) {
-	use substrate_rpc_client::{ws_client, ChainApi};
-
-	let mut built_blocks = std::collections::HashSet::new();
-	let mut interval = tokio::time::interval(Duration::from_secs(2));
-	let rpc = ws_client(url).await.unwrap();
-
-	loop {
-		if let Ok(block) = ChainApi::<(), Hash, Header, ()>::finalized_head(&rpc).await {
-			built_blocks.insert(block);
-			if built_blocks.len() > n {
-				break
-			}
-		};
-		interval.tick().await;
-	}
-}
-
-/// Run the node for a while (3 blocks)
-pub async fn run_node_for_a_while(base_path: &Path, args: &[&str]) {
-	run_with_timeout(Duration::from_secs(60 * 10), async move {
-		let mut cmd = Command::new(cargo_bin("substrate"))
-			.stdout(process::Stdio::piped())
-			.stderr(process::Stdio::piped())
-			.args(args)
-			.arg("-d")
-			.arg(base_path)
-			.spawn()
-			.unwrap();
-
-		let stderr = cmd.stderr.take().unwrap();
-
-		let mut child = KillChildOnDrop(cmd);
-
-		let ws_url = extract_info_from_output(stderr).0.ws_url;
-
-		// Let it produce some blocks.
-		wait_n_finalized_blocks(3, &ws_url).await;
-
-		child.assert_still_running();
-
-		// Stop the process
-		child.stop();
-	})
-	.await
-}
-
-pub struct KillChildOnDrop(pub Child);
-
-impl KillChildOnDrop {
-	/// Stop the child and wait until it is finished.
-	///
-	/// Asserts if the exit status isn't success.
-	pub fn stop(&mut self) {
-		self.stop_with_signal(SIGINT);
-	}
-
-	/// Same as [`Self::stop`] but takes the `signal` that is sent to stop the child.
-	pub fn stop_with_signal(&mut self, signal: Signal) {
-		kill(Pid::from_raw(self.id().try_into().unwrap()), signal).unwrap();
-		assert!(self.wait().unwrap().success());
-	}
-
-	/// Asserts that the child is still running.
-	pub fn assert_still_running(&mut self) {
-		assert!(self.try_wait().unwrap().is_none(), "the process should still be running");
-	}
-}
-
-impl Drop for KillChildOnDrop {
-	fn drop(&mut self) {
-		let _ = self.0.kill();
-	}
-}
-
-impl Deref for KillChildOnDrop {
-	type Target = Child;
-
-	fn deref(&self) -> &Self::Target {
-		&self.0
-	}
-}
-
-impl DerefMut for KillChildOnDrop {
-	fn deref_mut(&mut self) -> &mut Self::Target {
-		&mut self.0
-	}
-}
-
-/// Information extracted from a running node.
-pub struct NodeInfo {
-	pub ws_url: String,
-	pub db_path: PathBuf,
-}
-
-/// Extract [`NodeInfo`] from a running node by parsing its output.
-///
-/// Returns the [`NodeInfo`] and all the read data.
-pub fn extract_info_from_output(read: impl Read + Send) -> (NodeInfo, String) {
-	let mut data = String::new();
-
-	let ws_url = BufReader::new(read)
-		.lines()
-		.find_map(|line| {
-			let line = line.expect("failed to obtain next line while extracting node info");
-			data.push_str(&line);
-			data.push_str("\n");
-
-			// does the line contain our port (we expect this specific output from substrate).
-			let sock_addr = match line.split_once("Running JSON-RPC WS server: addr=") {
-				None => return None,
-				Some((_, after)) => after.split_once(",").unwrap().0,
-			};
-
-			Some(format!("ws://{}", sock_addr))
-		})
-		.unwrap_or_else(|| {
-			eprintln!("Observed node output:\n{}", data);
-			panic!("We should get a WebSocket address")
-		});
-
-	// Database path is printed before the ws url!
-	let re = Regex::new(r"Database: .+ at (\S+)").unwrap();
-	let db_path = PathBuf::from(re.captures(data.as_str()).unwrap().get(1).unwrap().as_str());
-
-	(NodeInfo { ws_url, db_path }, data)
-}

substrate/bin/node/cli/tests/export_import_flow.rs

@@ -23,7 +23,7 @@ use regex::Regex;
 use std::{fs, path::PathBuf, process::Command};
 use tempfile::{tempdir, TempDir};
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 fn contains_error(logged_output: &str) -> bool {
 	logged_output.contains("Error")

substrate/bin/node/cli/tests/inspect_works.rs

@@ -22,7 +22,7 @@ use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[tokio::test]
 async fn inspect_works() {

substrate/bin/node/cli/tests/purge_chain_works.rs

@@ -20,7 +20,7 @@ use assert_cmd::cargo::cargo_bin;
 use std::process::Command;
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[tokio::test]
 #[cfg(unix)]

substrate/bin/node/cli/tests/remember_state_pruning_works.rs

@@ -18,7 +18,7 @@
 
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[tokio::test]
 #[cfg(unix)]

substrate/bin/node/cli/tests/running_the_node_and_interrupt.rs

@@ -20,12 +20,12 @@
 use assert_cmd::cargo::cargo_bin;
 use nix::sys::signal::Signal::{self, SIGINT, SIGTERM};
 use std::{
-	process::{self, Child, Command},
+	process::{self, Command},
 	time::Duration,
 };
 use tempfile::tempdir;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[tokio::test]
 async fn running_the_node_works_and_can_be_interrupted() {
@@ -71,17 +71,8 @@ async fn running_the_node_works_and_can_be_interrupted() {
 #[tokio::test]
 async fn running_two_nodes_with_the_same_ws_port_should_work() {
 	common::run_with_timeout(Duration::from_secs(60 * 10), async move {
-		fn start_node() -> Child {
-			Command::new(cargo_bin("substrate"))
-				.stdout(process::Stdio::piped())
-				.stderr(process::Stdio::piped())
-				.args(&["--dev", "--tmp", "--ws-port=45789", "--no-hardware-benchmarks"])
-				.spawn()
-				.unwrap()
-		}
-
-		let mut first_node = common::KillChildOnDrop(start_node());
-		let mut second_node = common::KillChildOnDrop(start_node());
+		let mut first_node = common::KillChildOnDrop(common::start_node());
+		let mut second_node = common::KillChildOnDrop(common::start_node());
 
 		let stderr = first_node.stderr.take().unwrap();
 		let ws_url = common::extract_info_from_output(stderr).0.ws_url;

substrate/bin/node/cli/tests/telemetry.rs

@@ -21,7 +21,7 @@ use std::{process, time::Duration};
 
 use crate::common::KillChildOnDrop;
 
-pub mod common;
+use substrate_cli_test_utils as common;
 pub mod websocket_server;
 
 #[tokio::test]

substrate/bin/node/cli/tests/temp_base_path_works.rs

@@ -24,7 +24,7 @@ use std::{
 	time::Duration,
 };
 
-pub mod common;
+use substrate_cli_test_utils as common;
 
 #[allow(dead_code)]
 // Apparently `#[ignore]` doesn't actually work to disable this one.