Fix try-runtime follow-chain, try-runtime upgrade tuple tests, cli test utils

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 }

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]

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() {

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() {

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")

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() {

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)]

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)]

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;

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]

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.

frame/support/src/lib.rs

@@ -371,16 +371,14 @@ macro_rules! parameter_types {
 
  /// Set the value of this parameter type in the storage.
  ///
- /// This needs to be executed in an externalities provided
- /// environment.
+ /// This needs to be executed in an externalities provided environment.
  pub fn set(value: &$type) {
  $crate::storage::unhashed::put(&Self::key(), value);
  }
 
  /// Returns the value of this parameter type.
  ///
- /// This needs to be executed in an externalities provided
- /// environment.
+ /// This needs to be executed in an externalities provided environment.
  #[allow(unused)]
  pub fn get() -> $type {
  $crate::storage::unhashed::get(&Self::key()).unwrap_or_else(|| $value)

frame/support/src/traits/hooks.rs

@@ -197,10 +197,10 @@ impl OnRuntimeUpgrade for Tuple {
  weight
  }
 
- #[cfg(feature = "try-runtime")]
  /// We are executing pre- and post-checks sequentially in order to be able to test several
  /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade
  /// hooks for tuples are a noop.
+ #[cfg(feature = "try-runtime")]
  fn try_on_runtime_upgrade(checks: bool) -> Result<Weight, &'static str> {
  let mut weight = Weight::zero();
  for_tuples!( #( weight = weight.saturating_add(Tuple::try_on_runtime_upgrade(checks)?); )* );
@@ -359,10 +359,64 @@ pub trait OnTimestampSet<Moment> {
 #[cfg(test)]
 mod tests {
  use super::*;
+ use sp_io::TestExternalities;
+
+ #[cfg(feature = "try-runtime")]
+ #[test]
+ fn on_runtime_upgrade_pre_post_executed_tuple() {
+ crate::parameter_types! {
+ pub static Pre: Vec<&'static str> = Default::default();
+ pub static Post: Vec<&'static str> = Default::default();
+ }
+
+ macro_rules! impl_test_type {
+ ($name:ident) => {
+ struct $name;
+ impl OnRuntimeUpgrade for $name {
+ fn on_runtime_upgrade() -> Weight {
+ Default::default()
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+ Pre::mutate(|s| s.push(stringify!($name)));
+ Ok(Vec::new())
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn post_upgrade(_: Vec<u8>) -> Result<(), &'static str> {
+ Post::mutate(|s| s.push(stringify!($name)));
+ Ok(())
+ }
+ }
+ };
+ }
+
+ impl_test_type!(Foo);
+ impl_test_type!(Bar);
+ impl_test_type!(Baz);
+
+ TestExternalities::default().execute_with(|| {
+ Foo::try_on_runtime_upgrade(true).unwrap();
+ assert_eq!(Pre::take(), vec!["Foo"]);
+ assert_eq!(Post::take(), vec!["Foo"]);
+
+ <(Foo, Bar, Baz)>::try_on_runtime_upgrade(true).unwrap();
+ assert_eq!(Pre::take(), vec!["Foo", "Bar", "Baz"]);
+ assert_eq!(Post::take(), vec!["Foo", "Bar", "Baz"]);
+
+ <((Foo, Bar), Baz)>::try_on_runtime_upgrade(true).unwrap();
+ assert_eq!(Pre::take(), vec!["Foo", "Bar", "Baz"]);
+ assert_eq!(Post::take(), vec!["Foo", "Bar", "Baz"]);
+
+ <(Foo, (Bar, Baz))>::try_on_runtime_upgrade(true).unwrap();
+ assert_eq!(Pre::take(), vec!["Foo", "Bar", "Baz"]);
+ assert_eq!(Post::take(), vec!["Foo", "Bar", "Baz"]);
+ });
+ }
 
  #[test]
  fn on_initialize_and_on_runtime_upgrade_weight_merge_works() {
- use sp_io::TestExternalities;
  struct Test;
 
  impl OnInitialize<u8> for Test {

test-utils/cli/Cargo.toml

@@ -0,0 +1,23 @@
+[package]
+name = "substrate-cli-test-utils"
+description = "CLI testing utilities"
+version = "0.1.0"
+authors = ["Parity Technologies <[email protected]>"]
+edition = "2021"
+license = "Apache-2.0"
+homepage = "https://substrate.io"
+repository = "https:/paritytech/substrate/"
+publish = false
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]
+
+[dependencies]
+substrate-rpc-client = { path = "../../utils/frame/rpc/client" }
+assert_cmd = "2.0.10"
+nix = "0.26.2"
+regex = "1.7.3"
+tempfile = "3.5.0"
+tokio = { version = "1.22.0", features = ["full"] }
+node-primitives = { path = "../../bin/node/primitives" }
+futures = "0.3.28"

bin/node/cli/tests/common.rs → test-utils/cli/src/lib.rs

@@ -26,12 +26,146 @@ use nix::{
 use node_primitives::{Hash, Header};
 use regex::Regex;
 use std::{
+ env,
  io::{BufRead, BufReader, Read},
  ops::{Deref, DerefMut},
  path::{Path, PathBuf},
  process::{self, Child, Command},
  time::Duration,
 };
+use tokio::io::{AsyncBufReadExt, AsyncRead};
+
+/// Starts a new Substrate node in development mode with a temporary chain.
+///
+/// This function creates a new Substrate node using the `substrate` binary.
+/// It configures the node to run in development mode (`--dev`) with a temporary chain (`--tmp`),
+/// sets the WebSocket port to 45789 (`--ws-port=45789`).
+///
+/// # Returns
+///
+/// A [`Child`] process representing the spawned Substrate node.
+///
+/// # Panics
+///
+/// This function will panic if the `substrate` binary is not found or if the node fails to start.
+///
+/// # Examples
+///
+/// ```ignore
+/// use my_crate::start_node;
+///
+/// let child = start_node();
+/// // Interact with the Substrate node using the WebSocket port 45789.
+/// // When done, the node will be killed when the `child` is dropped.
+/// ```
+///
+/// [`Child`]: std::process::Child
+pub 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()
+}
+
+/// Builds the Substrate project using the provided arguments.
+///
+/// This function reads the CARGO_MANIFEST_DIR environment variable to find the root workspace
+/// directory. It then runs the `cargo b` command in the root directory with the specified
+/// arguments.
+///
+/// This can be useful for building the Substrate binary with a desired set of features prior
+/// to using the binary in a CLI test.
+///
+/// # Arguments
+///
+/// * `args: &[&str]` - A slice of string references representing the arguments to pass to the
+/// `cargo b` command.
+///
+/// # Panics
+///
+/// This function will panic if:
+///
+/// * The CARGO_MANIFEST_DIR environment variable is not set.
+/// * The root workspace directory cannot be determined.
+/// * The 'cargo b' command fails to execute.
+/// * The 'cargo b' command returns a non-successful status.
+///
+/// # Examples
+///
+/// ```ignore
+/// build_substrate(&["--features=try-runtime"]);
+/// ```
+pub fn build_substrate(args: &[&str]) {
+ // Get the root workspace directory from the CARGO_MANIFEST_DIR environment variable
+ let manifest_dir = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
+ let root_dir = std::path::Path::new(&manifest_dir)
+ .parent()
+ .expect("Failed to find root workspace directory");
+ let output = Command::new("cargo")
+ .arg("b")
+ .args(args)
+ .current_dir(root_dir)
+ .output()
+ .expect(format!("Failed to execute 'cargo b' with args {:?}'", args).as_str());
+
+ if !output.status.success() {
+ panic!(
+ "Failed to execute 'cargo b' with args {:?}': \n{}",
+ args,
+ String::from_utf8_lossy(&output.stderr)
+ );
+ }
+}
+
+/// Takes a readable tokio stream (e.g. from a child process `ChildStderr` or `ChildStdout`) and
+/// a `Regex` pattern, and checks each line against the given pattern as it is produced.
+/// The function returns OK(()) as soon as a line matching the pattern is found, or an Err if
+/// the stream ends without any lines matching the pattern.
+///
+/// # Arguments
+///
+/// * `child_stream` - An async tokio stream, e.g. from a child process `ChildStderr` or
+/// `ChildStdout`.
+/// * `re` - A `Regex` pattern to search for in the stream.
+///
+/// # Returns
+///
+/// * `Ok(())` if a line matching the pattern is found.
+/// * `Err(String)` if the stream ends without any lines matching the pattern.
+///
+/// # Examples
+///
+/// ```ignore
+/// use regex::Regex;
+/// use tokio::process::Command;
+/// use tokio::io::AsyncRead;
+///
+/// # async fn run() {
+/// let child = Command::new("some-command").stderr(std::process::Stdio::piped()).spawn().unwrap();
+/// let stderr = child.stderr.unwrap();
+/// let re = Regex::new("error:").unwrap();
+///
+/// match wait_for_pattern_match_in_stream(stderr, re).await {
+/// Ok(()) => println!("Error found in stderr"),
+/// Err(e) => println!("Error: {}", e),
+/// }
+/// # }
+/// ```
+pub async fn wait_for_stream_pattern_match<R>(stream: R, re: Regex) -> Result<(), String>
+where
+ R: AsyncRead + Unpin,
+{
+ let mut stdio_reader = tokio::io::BufReader::new(stream).lines();
+ while let Ok(Some(line)) = stdio_reader.next_line().await {
+ match re.find(line.as_str()) {
+ Some(_) => return Ok(()),
+ None => (),
+ }
+ }
+ Err(String::from("Stream closed without any lines matching the regex."))
+}
 
 /// Run the given `future` and panic if the `timeout` is hit.
 pub async fn run_with_timeout(timeout: Duration, future: impl futures::Future<Output = ()>) {