no e2e feature, and don't assume 'cargo run' is a good default

.github/workflows/backend.yml

@@ -21,7 +21,7 @@ jobs:
 
     - name: Run tests
       working-directory: ./backend
-      run: cargo test --verbose --features e2e
+      run: cargo test --verbose
 
     - name: Build, release and call telemetry executable
       working-directory: ./backend

backend/telemetry_core/Cargo.toml

@@ -5,10 +5,6 @@ authors = ["Parity Technologies Ltd. <admin@parity.io>"]
 edition = "2018"
 license = "GPL-3.0"
 
-[features]
-# Enable this feature when running tests to also run the e2e tests:
-e2e = []
-
 [dependencies]
 anyhow = "1.0.41"
 bimap = "0.6.1"

backend/telemetry_core/tests/basic_tests.rs

@@ -1,17 +1,22 @@
-//! These only run when the "e2e" feature is set (eg `cargo test --features e2e`).
-//! The rust IDE plugins may behave better if you comment out this line during development:
-#![cfg(feature = "e2e")]
-
-use test_utils::{feed_message_de::{ FeedMessage, NodeDetails }, server::Server, assert_contains_matches};
+use test_utils::{
+    feed_message_de::{ FeedMessage, NodeDetails },
+    server::{ self, Server },
+    assert_contains_matches
+};
 use serde_json::json;
 use std::time::Duration;
 use common::node_types::{ BlockHash };
 
+async fn cargo_run_server() -> Server {
+    Server::start(server::StartOpts {
+        shard_command: server::cargo_run_commands::telemetry_shard().expect("valid shard command"),
+        core_command: server::cargo_run_commands::telemetry_core().expect("valid core command")
+    }).await.unwrap()
+}
+
 #[tokio::test]
 async fn feed_sent_version_on_connect() {
-    let server = Server::start_default()
-        .await
-        .expect("server could start");
+    let server = cargo_run_server().await;
 
     // Connect a feed:
     let (_feed_tx, mut feed_rx) = server.get_core().connect().await.unwrap();
@@ -26,9 +31,7 @@ async fn feed_sent_version_on_connect() {
 
 #[tokio::test]
 async fn feed_ping_responded_to_with_pong() {
-    let server = Server::start_default()
-        .await
-        .expect("server could start");
+    let server = cargo_run_server().await;
 
     // Connect a feed:
     let (mut feed_tx, mut feed_rx) = server.get_core().connect().await.unwrap();
@@ -47,7 +50,7 @@ async fn feed_ping_responded_to_with_pong() {
 #[tokio::test]
 async fn feed_add_and_remove_node() {
     // Connect server and add shard
-    let mut server = Server::start_default().await.unwrap();
+    let mut server = cargo_run_server().await;
     let shard_id = server.add_shard().await.unwrap();
 
     // Connect a node to the shard:
@@ -109,9 +112,7 @@ async fn feed_add_and_remove_node() {
 
 #[tokio::test]
 async fn feed_add_and_remove_shard() {
-    let mut server = Server::start_default()
-    .await
-    .expect("server could start");
+    let mut server = cargo_run_server().await;
 
     let mut shards = vec![];
     for id in 1 ..= 2 {
@@ -192,7 +193,7 @@ async fn feed_can_subscribe_and_unsubscribe_from_chain() {
     use FeedMessage::*;
 
     // Start server, add shard, connect node:
-    let mut server = Server::start_default().await.unwrap();
+    let mut server = cargo_run_server().await;
     let shard_id = server.add_shard().await.unwrap();
     let (mut node_tx, _node_rx) = server.get_shard(shard_id).unwrap().connect().await.unwrap();
 

backend/test_utils/src/server/cargo_run_commands.rs

@@ -0,0 +1,36 @@
+//! A pair of commands we can use when running `cargo test` style tests in this workspace
+//! that want to test the current code. For more external tests, we may want to ask for the
+//! commands, or connect to a running instance instead.
+
+use super::Command;
+use std::path::PathBuf;
+
+/// Runs `cargo run` in the current workspace to start up a telemetry shard process
+pub fn telemetry_shard() -> Result<Command, std::io::Error> {
+    telemetry_command("telemetry_shard")
+}
+
+/// Runs `cargo run` in the current workspace to start up a telemetry core process
+pub fn telemetry_core() -> Result<Command, std::io::Error> {
+    telemetry_command("telemetry_core")
+}
+
+fn telemetry_command(bin: &'static str) -> Result<Command, std::io::Error> {
+    let mut workspace_dir = try_find_workspace_dir()?;
+    workspace_dir.push("Cargo.toml");
+    Ok(Command::new("cargo")
+        .arg("run")
+        .arg("--bin")
+        .arg(bin)
+        .arg("--manifest-path")
+        .arg(workspace_dir)
+        .arg("--"))
+}
+
+/// A _very_ naive way to find the workspace ("backend") directory
+/// from the current path (which is assumed to be inside it).
+fn try_find_workspace_dir() -> Result<PathBuf, std::io::Error> {
+    let mut dir = std::env::current_dir()?;
+    while !dir.ends_with("backend") && dir.pop() {}
+    Ok(dir)
+}

backend/test_utils/src/server/default_commands.rs

@@ -1,30 +0,0 @@
-use super::Command;
-use std::path::PathBuf;
-
-pub fn default_telemetry_shard_command() -> Result<Command, std::io::Error> {
-    default_telemetry_command("telemetry_shard")
-}
-
-pub fn default_telemetry_core_command() -> Result<Command, std::io::Error> {
-    default_telemetry_command("telemetry_core")
-}
-
-fn default_telemetry_command(bin: &'static str) -> Result<Command, std::io::Error> {
-    let mut workspace_dir = try_find_workspace_dir()?;
-    workspace_dir.push("Cargo.toml");
-    Ok(Command::new("cargo")
-        .arg("run")
-        .arg("--bin")
-        .arg(bin)
-        .arg("--manifest-path")
-        .arg(workspace_dir)
-        .arg("--"))
-}
-
-/// A _very_ naive way to find the workspace ("backend") directory
-/// from the current path (which is assumed to be inside it).
-fn try_find_workspace_dir() -> Result<PathBuf, std::io::Error> {
-    let mut dir = std::env::current_dir()?;
-    while !dir.ends_with("backend") && dir.pop() {}
-    Ok(dir)
-}

backend/test_utils/src/server/mod.rs

@@ -1,6 +1,6 @@
 mod utils;
 mod server;
-mod default_commands;
 
+pub mod cargo_run_commands;
 pub mod channels;
 pub use server::*;

backend/test_utils/src/server/server.rs

@@ -11,21 +11,12 @@ id_type! {
 }
 
 pub struct StartOpts {
-    /// Optional command to run to start a shard (instead of `telemetry_shard`).
+    /// Command to run to start a shard.
     /// The `--listen` and `--log` arguments will be appended within and shouldn't be provided.
-    pub shard_command: Option<Command>,
-    /// Optional command to run to start a telemetry core process (instead of `telemetry_core`).
+    pub shard_command: Command,
+    /// Command to run to start a telemetry core process.
     /// The `--listen` and `--log` arguments will be appended within and shouldn't be provided.
-    pub core_command: Option<Command>
-}
-
-impl Default for StartOpts {
-    fn default() -> Self {
-        StartOpts {
-            shard_command: None,
-            core_command: None
-        }
-    }
+    pub core_command: Command
 }
 
 pub struct ConnectToExistingOpts {
@@ -57,7 +48,9 @@ pub enum Error {
 pub struct Server {
     /// URI to connect a shard to core:
     core_shard_submit_uri: Option<http::Uri>,
-    /// Command to run to start a new shard:
+    /// Command to run to start a new shard. Optional
+    /// because if we connect to running instances it'll
+    /// be unset.
     shard_command: Option<Command>,
     /// Shard processes that we can connect to
     shards: DenseMap<ProcessId, ShardProcess>,
@@ -119,10 +112,10 @@ impl Server {
             None => return Err(Error::CannotAddShardNoHandle)
         };
 
-        let mut shard_cmd: TokioCommand = match &self.shard_command {
-            Some(cmd) => cmd.clone(),
-            None => super::default_commands::default_telemetry_shard_command()?
-        }.into();
+        let mut shard_cmd: TokioCommand = self.shard_command
+            .clone()
+            .ok_or_else(|| Error::CannotAddShardNoHandle)?
+            .into();
 
         shard_cmd
             .arg("--listen")
@@ -169,18 +162,10 @@ impl Server {
         Ok(pid)
     }
 
-    /// Start a telemetry_core process with default opts. From here, we can add/remove shards as needed.
-    pub async fn start_default() -> Result<Server, Error> {
-        Server::start(StartOpts::default()).await
-    }
-
     /// Start a telemetry_core process. From here, we can add/remove shards as needed.
     pub async fn start(opts: StartOpts) -> Result<Server, Error> {
 
-        let mut core_cmd: TokioCommand = match opts.core_command {
-            Some(cmd) => cmd,
-            None => super::default_commands::default_telemetry_core_command()?
-        }.into();
+        let mut core_cmd: TokioCommand = opts.core_command.into();
 
         let mut child = core_cmd
             .arg("--listen")
@@ -208,7 +193,7 @@ impl Server {
             .expect("valid feed URI");
 
         Ok(Server {
-            shard_command: opts.shard_command,
+            shard_command: Some(opts.shard_command),
             core_shard_submit_uri: Some(format!("http://127.0.0.1:{}/shard_submit", core_port)
                 .parse()
                 .expect("valid shard_submit URI")),