Tests: support 'substrate-node' too and allow multiple binary paths (#…

…1102)

* Support 'substrate-node' too and allow multiple binary paths

* fmt

* clippy

* fix path

testing/integration-tests/src/utils/node_proc.rs

@@ -76,7 +76,7 @@ impl TestNodeProcessBuilder {
     {
         let mut node_builder = SubstrateNode::builder();
 
-        node_builder.binary_path(self.node_path);
+        node_builder.binary_paths(&[self.node_path]);
 
         if let Some(authority) = &self.authority {
             node_builder.arg(authority.to_lowercase());

testing/substrate-runner/src/lib.rs

@@ -7,15 +7,15 @@ mod error;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::ffi::OsString;
-use std::io::{BufRead, BufReader, Read};
-use std::process::{self, Command};
+use std::io::{self, BufRead, BufReader, Read};
+use std::process::{self, Child, Command};
 
 pub use error::Error;
 
 type CowStr = Cow<'static, str>;
 
 pub struct SubstrateNodeBuilder {
-    binary_path: OsString,
+    binary_paths: Vec<OsString>,
     custom_flags: HashMap<CowStr, Option<CowStr>>,
 }
 
@@ -29,14 +29,19 @@ impl SubstrateNodeBuilder {
     /// Configure a new Substrate node.
     pub fn new() -> Self {
         SubstrateNodeBuilder {
-            binary_path: "substrate".into(),
+            binary_paths: vec!["substrate-node".into(), "substrate".into()],
             custom_flags: Default::default(),
         }
     }
 
-    /// Set the path to the `substrate` binary; defaults to "substrate".
-    pub fn binary_path(&mut self, path: impl Into<OsString>) -> &mut Self {
-        self.binary_path = path.into();
+    /// Set the path to the `substrate` binary; defaults to "substrate-node"
+    /// or "substrate".
+    pub fn binary_paths<Paths, S>(&mut self, paths: Paths) -> &mut Self
+    where
+        Paths: IntoIterator<Item = S>,
+        S: Into<OsString>,
+    {
+        self.binary_paths = paths.into_iter().map(|p| p.into()).collect();
         self
     }
 
@@ -54,23 +59,23 @@ impl SubstrateNodeBuilder {
 
     /// Spawn the node, handing back an object which, when dropped, will stop it.
     pub fn spawn(self) -> Result<SubstrateNode, Error> {
-        let mut cmd = Command::new(self.binary_path);
-
-        cmd.env("RUST_LOG", "info,libp2p_tcp=debug")
-            .stdout(process::Stdio::piped())
-            .stderr(process::Stdio::piped())
-            .arg("--dev")
-            .arg("--port=0");
-
-        for (key, val) in self.custom_flags {
-            let arg = match val {
-                Some(val) => format!("--{key}={val}"),
-                None => format!("--{key}"),
-            };
-            cmd.arg(arg);
+        // Try to spawn the binary at each path, returning the
+        // first "ok" or last error that we encountered.
+        let mut res = Err(io::Error::new(
+            io::ErrorKind::Other,
+            "No binary path provided",
+        ));
+        for binary_path in &self.binary_paths {
+            res = SubstrateNodeBuilder::try_spawn(binary_path, &self.custom_flags);
+            if res.is_ok() {
+                break;
+            }
         }
 
-        let mut proc = cmd.spawn().map_err(Error::Io)?;
+        let mut proc = match res {
+            Ok(proc) => proc,
+            Err(e) => return Err(Error::Io(e)),
+        };
 
         // Wait for RPC port to be logged (it's logged to stderr).
         let stderr = proc.stderr.take().unwrap();
@@ -87,6 +92,30 @@ impl SubstrateNodeBuilder {
             p2p_port,
         })
     }
+
+    // Attempt to spawn a binary with the path/flags given.
+    fn try_spawn(
+        binary_path: &OsString,
+        custom_flags: &HashMap<CowStr, Option<CowStr>>,
+    ) -> Result<Child, std::io::Error> {
+        let mut cmd = Command::new(binary_path);
+
+        cmd.env("RUST_LOG", "info,libp2p_tcp=debug")
+            .stdout(process::Stdio::piped())
+            .stderr(process::Stdio::piped())
+            .arg("--dev")
+            .arg("--port=0");
+
+        for (key, val) in custom_flags {
+            let arg = match val {
+                Some(val) => format!("--{key}={val}"),
+                None => format!("--{key}"),
+            };
+            cmd.arg(arg);
+        }
+
+        cmd.spawn()
+    }
 }
 
 pub struct SubstrateNode {

testing/test-runtime/build.rs

@@ -6,6 +6,7 @@ use codec::{Decode, Encode};
 use std::{env, fs, path::Path};
 use substrate_runner::{Error as SubstrateNodeError, SubstrateNode};
 
+// This variable accepts a single binary name or comma separated list.
 static SUBSTRATE_BIN_ENV_VAR: &str = "SUBSTRATE_NODE_PATH";
 
 #[tokio::main]
@@ -15,10 +16,12 @@ async fn main() {
 
 async fn run() {
     // Select substrate binary to run based on env var.
-    let substrate_bin = env::var(SUBSTRATE_BIN_ENV_VAR).unwrap_or_else(|_| "substrate".to_owned());
+    let substrate_bins: String =
+        env::var(SUBSTRATE_BIN_ENV_VAR).unwrap_or_else(|_| "substrate-node,substrate".to_owned());
+    let substrate_bins_vec: Vec<&str> = substrate_bins.split(',').map(|s| s.trim()).collect();
 
     let mut node_builder = SubstrateNode::builder();
-    node_builder.binary_path(substrate_bin.clone());
+    node_builder.binary_paths(substrate_bins_vec.iter());
 
     let node = match node_builder.spawn() {
         Ok(node) => node,
@@ -29,7 +32,7 @@ async fn run() {
             )
         }
         Err(e) => {
-            panic!("Cannot spawn substrate command '{substrate_bin}': {e}")
+            panic!("Cannot spawn substrate command from any of {substrate_bins_vec:?}: {e}")
         }
     };
 
@@ -81,14 +84,15 @@ async fn run() {
     let runtime_path = Path::new(&out_dir).join("runtime.rs");
     fs::write(runtime_path, runtime_api_contents).expect("Couldn't write runtime rust output");
 
-    let substrate_path =
-        which::which(substrate_bin).expect("Cannot resolve path to substrate binary");
+    for substrate_node_path in substrate_bins_vec {
+        let Ok(full_path) = which::which(substrate_node_path) else {
+            continue
+        };
+
+        // Re-build if the substrate binary we're pointed to changes (mtime):
+        println!("cargo:rerun-if-changed={}", full_path.to_string_lossy());
+    }
 
-    // Re-build if the substrate binary we're pointed to changes (mtime):
-    println!(
-        "cargo:rerun-if-changed={}",
-        substrate_path.to_string_lossy()
-    );
     // Re-build if we point to a different substrate binary:
     println!("cargo:rerun-if-env-changed={SUBSTRATE_BIN_ENV_VAR}");
     // Re-build if this file changes: