[ink_e2e] spawn a separate contracts node instance per test (#1642)

* Spawn a contracts node instance per test * Allow user to specify contracts node executable * Improve CONTRACTS_NODE panic message * Update docs * Remove starting node from examples-test CI * Remove create_and_fund_account retries * Fmt * clippy * Remove tests * Fmt * spellcheck * spellcheck * Add a test for TestNodeProcess * Update CHANGELOG.md * Remove move
use-ink · Feb 7, 2023 · 8bc8593 · 8bc8593
1 parent 9b6f372
commit 8bc8593
Show file tree

Hide file tree

Showing 12 changed files with 307 additions and 154 deletions.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
@@ -87,9 +87,6 @@ workflow:
   tags:
     - kubernetes-parity-build
 
-.start-substrate-contracts-node:                     &start-substrate-contracts-node
-    - substrate-contracts-node -linfo,runtime::contracts=debug  2>&1 | tee /tmp/contracts-node.log &
-
 #### stage:                        lint
 #
 # Note: For all of these lints we `allow_failure` so that the rest of the build can
@@ -375,7 +372,6 @@ examples-test:
     - job:                         clippy-std
       artifacts:                   false
   script:
-    - *start-substrate-contracts-node
     - for example in examples/*/; do
         if [ "$example" = "examples/upgradeable-contracts/" ]; then continue; fi;
         if [ "$example" = "examples/lang-err-integration-tests/" ]; then continue; fi;

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+### Changed
+- E2E: spawn a separate contracts node instance per test ‒ [#1642](https://github.com/paritytech/ink/pull/1642)
+
 ## Version 4.0.0-rc
 
 The first release candidate is here! This is the first release which could become the final

diff --git a/crates/e2e/macro/Cargo.toml b/crates/e2e/macro/Cargo.toml
@@ -28,3 +28,4 @@ serde_json = "1.0.89"
 syn = "1"
 proc-macro2 = "1"
 quote = "1"
+which = "4.4.0"
diff --git a/crates/e2e/macro/src/codegen.rs b/crates/e2e/macro/src/codegen.rs
@@ -72,8 +72,6 @@ impl InkE2ETest {
             syn::ReturnType::Type(rarrow, ret_type) => quote! { #rarrow #ret_type },
         };
 
-        let ws_url = &self.test.config.ws_url();
-
         let mut additional_contracts: Vec<String> =
             self.test.config.additional_contracts();
         let default_main_contract_manifest_path = String::from("Cargo.toml");
@@ -113,6 +111,24 @@ impl InkE2ETest {
             quote! { #bundle_path }
         });
 
+        const DEFAULT_CONTRACTS_NODE: &str = "substrate-contracts-node";
+
+        // use the user supplied `CONTRACTS_NODE` or default to `substrate-contracts-node`
+        let contracts_node: &'static str =
+            option_env!("CONTRACTS_NODE").unwrap_or(DEFAULT_CONTRACTS_NODE);
+
+        // check the specified contracts node.
+        if which::which(contracts_node).is_err() {
+            if contracts_node == DEFAULT_CONTRACTS_NODE {
+                panic!(
+                    "The '{DEFAULT_CONTRACTS_NODE}' executable was not found. Install '{DEFAULT_CONTRACTS_NODE}' on the PATH, \
+                    or specify the `CONTRACTS_NODE` environment variable.",
+                )
+            } else {
+                panic!("The contracts node executable '{contracts_node}' was not found.")
+            }
+        }
+
         quote! {
             #( #attrs )*
             #[test]
@@ -126,28 +142,25 @@ impl InkE2ETest {
 
                 ::ink_e2e::INIT.call_once(|| {
                     ::ink_e2e::env_logger::init();
-                    let check_async = ::ink_e2e::Client::<
-                        ::ink_e2e::PolkadotConfig,
-                        ink::env::DefaultEnvironment
-                    >::new(&#ws_url, []);
-
-                    ::ink_e2e::tokio::runtime::Builder::new_current_thread()
-                        .enable_all()
-                        .build()
-                        .unwrap_or_else(|err|
-                            panic!("Failed building the Runtime during initialization: {}", err))
-                        .block_on(check_async);
                 });
 
                 log_info("creating new client");
 
                 let run = async {
-                    // TODO(#xxx) Make those two generic environments customizable.
+                    // spawn a contracts node process just for this test
+                    let node_proc = ::ink_e2e::TestNodeProcess::<::ink_e2e::PolkadotConfig>
+                        ::build(#contracts_node)
+                        .spawn()
+                        .await
+                        .unwrap_or_else(|err|
+                            ::core::panic!("Error spawning substrate-contracts-node: {:?}", err)
+                        );
+
                     let mut client = ::ink_e2e::Client::<
                         ::ink_e2e::PolkadotConfig,
                         ink::env::DefaultEnvironment
                     >::new(
-                        &#ws_url,
+                        node_proc.client(),
                         [ #( #contracts ),* ]
                     ).await;
 

diff --git a/crates/e2e/macro/src/config.rs b/crates/e2e/macro/src/config.rs
@@ -24,8 +24,6 @@ use ink_ir::{
 /// The End-to-End test configuration.
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct E2EConfig {
-    /// The WebSocket URL where to connect with the node.
-    ws_url: Option<syn::LitStr>,
     /// The set of attributes that can be passed to call builder in the codegen.
     whitelisted_attributes: WhitelistedAttributes,
     /// Additional contracts that have to be built before executing the test.
@@ -36,24 +34,11 @@ impl TryFrom<ast::AttributeArgs> for E2EConfig {
     type Error = syn::Error;
 
     fn try_from(args: ast::AttributeArgs) -> Result<Self, Self::Error> {
-        let mut ws_url: Option<(syn::LitStr, ast::MetaNameValue)> = None;
         let mut whitelisted_attributes = WhitelistedAttributes::default();
         let mut additional_contracts: Option<(syn::LitStr, ast::MetaNameValue)> = None;
 
         for arg in args.into_iter() {
-            if arg.name.is_ident("ws_url") {
-                if let Some((_, ast)) = ws_url {
-                    return Err(duplicate_config_err(ast, arg, "ws_url", "e2e test"))
-                }
-                if let ast::PathOrLit::Lit(syn::Lit::Str(lit_str)) = &arg.value {
-                    ws_url = Some((lit_str.clone(), arg))
-                } else {
-                    return Err(format_err_spanned!(
-                        arg,
-                        "expected a string literal for `ws_url` ink! e2e test configuration argument",
-                    ))
-                }
-            } else if arg.name.is_ident("keep_attr") {
+            if arg.name.is_ident("keep_attr") {
                 whitelisted_attributes.parse_arg_value(&arg)?;
             } else if arg.name.is_ident("additional_contracts") {
                 if let Some((_, ast)) = additional_contracts {
@@ -83,23 +68,13 @@ impl TryFrom<ast::AttributeArgs> for E2EConfig {
             .map(|(value, _)| value.value().split(' ').map(String::from).collect())
             .unwrap_or_else(Vec::new);
         Ok(E2EConfig {
-            ws_url: ws_url.map(|(value, _)| value),
             additional_contracts,
             whitelisted_attributes,
         })
     }
 }
 
 impl E2EConfig {
-    /// Returns the WebSocket URL where to connect to the RPC endpoint
-    /// of the node, if specified. Otherwise returns the default URL
-    /// `ws://localhost:9944`.
-    pub fn ws_url(&self) -> syn::LitStr {
-        let default_ws_url =
-            syn::LitStr::new("ws://0.0.0.0:9944", proc_macro2::Span::call_site());
-        self.ws_url.clone().unwrap_or(default_ws_url)
-    }
-
     /// Returns a vector of additional contracts that have to be built
     /// and imported before executing the test.
     pub fn additional_contracts(&self) -> Vec<String> {
@@ -175,7 +150,6 @@ mod tests {
                 keep_attr = "foo, bar"
             },
             Ok(E2EConfig {
-                ws_url: None,
                 whitelisted_attributes: attrs,
                 additional_contracts: Vec::new(),
             }),

diff --git a/crates/e2e/macro/src/lib.rs b/crates/e2e/macro/src/lib.rs
@@ -29,11 +29,10 @@ use syn::Result;
 ///
 /// The system requirements are:
 ///
-/// - A Substrate node with `pallet-contracts` running in the background.
+/// - A Substrate node with `pallet-contracts` installed on the local system.
 ///   You can e.g. use [`substrate-contracts-node`](https://github.com/paritytech/substrate-contracts-node)
-///   and launch it with
-///   `substrate-contracts-node -lerror,runtime::contracts=debug > /tmp/contracts-node.log 2>&1`.
-/// - A `cargo-contract` installation that can build the contract.
+///   and install it on your PATH, or provide a path to an executable using the `CONTRACTS_NODE`
+///   environment variable.
 ///
 /// Before the test function is invoked the contract will have been build. Any errors
 /// that occur during the contract build will prevent the test function from being
@@ -44,25 +43,6 @@ use syn::Result;
 /// The `#[ink::e2e_test]` macro can be provided with some additional comma-separated
 /// header arguments:
 ///
-/// - `ws_url: String`
-///
-///     The `ws_url` denotes the WebSocket URL where to connect to the RPC
-///     endpoint of the node.
-///
-///     **Usage Example:**
-///     ```no_compile
-///     # // TODO(#xxx) Remove the `no_compile`.
-///     type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
-///     #[ink::e2e_test(ws_url = "ws://localhost:9944")]
-///     async fn e2e_contract_must_transfer_value_to_sender(
-///         mut client: ::ink_e2e::Client<C, E>,
-///     ) -> E2EResult<()> {
-///         Ok(())
-///     }
-///     ```
-///
-///     **Default value:** `"ws://localhost:9944"`.
-///
 /// # Example
 ///
 /// ```no_compile

diff --git a/crates/e2e/src/client.rs b/crates/e2e/src/client.rs
@@ -374,21 +374,11 @@ where
     E::Balance: Debug + scale::HasCompact + serde::Serialize,
     E::Hash: Debug + scale::Encode,
 {
-    /// Creates a new [`Client`] instance.
-    pub async fn new(url: &str, contracts: impl IntoIterator<Item = &str>) -> Self {
-        let client = subxt::OnlineClient::from_url(url)
-            .await
-            .unwrap_or_else(|err| {
-                if let subxt::Error::Rpc(subxt::error::RpcError::ClientError(_)) = err {
-                    let error_msg = format!("Error establishing connection to a node at {url}. Make sure you run a node behind the given url!");
-                    log_error(&error_msg);
-                    panic!("{}", error_msg);
-                }
-                log_error(
-                    "Unable to create client! Please check that your node is running.",
-                );
-                panic!("Unable to create client: {err:?}");
-            });
+    /// Creates a new [`Client`] instance using a `subxt` client.
+    pub async fn new(
+        client: subxt::OnlineClient<C>,
+        contracts: impl IntoIterator<Item = &str>,
+    ) -> Self {
         let contracts = contracts
             .into_iter()
             .map(|path| {
@@ -405,7 +395,7 @@ where
             .collect();
 
         Self {
-            api: ContractsApi::new(client, url).await,
+            api: ContractsApi::new(client).await,
             contracts,
         }
     }
@@ -421,38 +411,30 @@ where
     ) -> Signer<C>
     where
         E::Balance: Clone,
-        C::AccountId: Clone + core::fmt::Display + core::fmt::Debug,
+        C::AccountId: Clone + core::fmt::Display + Debug,
         C::AccountId: From<sp_core::crypto::AccountId32>,
     {
         let (pair, _, _) = <sr25519::Pair as Pair>::generate_with_phrase(None);
         let pair_signer = PairSigner::<C, _>::new(pair);
         let account_id = pair_signer.account_id().to_owned();
 
-        for _ in 0..6 {
-            let transfer_result = self
-                .api
-                .try_transfer_balance(origin, account_id.clone(), amount)
-                .await;
-            match transfer_result {
-                Ok(_) => {
-                    log_info(&format!(
-                        "transfer from {} to {} succeeded",
-                        origin.account_id(),
-                        account_id,
-                    ));
-                    break
-                }
-                Err(err) => {
-                    log_info(&format!(
-                        "transfer from {} to {} failed with {:?}",
-                        origin.account_id(),
-                        account_id,
-                        err
-                    ));
-                    tokio::time::sleep(std::time::Duration::from_secs(1)).await;
-                }
-            }
-        }
+        self.api
+            .try_transfer_balance(origin, account_id.clone(), amount)
+            .await
+            .unwrap_or_else(|err| {
+                panic!(
+                    "transfer from {} to {} failed with {:?}",
+                    origin.account_id(),
+                    account_id,
+                    err
+                )
+            });
+
+        log_info(&format!(
+            "transfer from {} to {} succeeded",
+            origin.account_id(),
+            account_id,
+        ));
 
         pair_signer
     }

diff --git a/crates/e2e/src/lib.rs b/crates/e2e/src/lib.rs
@@ -22,8 +22,7 @@
 mod builders;
 mod client;
 mod default_accounts;
-#[cfg(test)]
-mod tests;
+mod node_proc;
 pub mod utils;
 mod xts;
 
@@ -42,6 +41,10 @@ pub use client::{
 pub use default_accounts::*;
 pub use env_logger;
 pub use ink_e2e_macro::test;
+pub use node_proc::{
+    TestNodeProcess,
+    TestNodeProcessBuilder,
+};
 pub use sp_core::H256;
 pub use sp_keyring::AccountKeyring;
 pub use subxt::{