[refactor]: fix pytests

- do not mutate client config from tests,
  override via env instead
- add `TORII_URL` env var to client config

Signed-off-by: Dmitry Balashov <[email protected]>

client/src/config/user_layer.rs

@@ -1,6 +1,6 @@
 mod boilerplate;
 
-use std::{fs::File, io::Read, path::Path, time::Duration};
+use std::{fs::File, io::Read, path::Path, str::FromStr, time::Duration};
 
 pub use boilerplate::*;
 use eyre::{eyre, Context, Report};
@@ -104,16 +104,54 @@ pub struct Transaction {
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct OnlyHttpUrl(Url);
 
-impl<'de> Deserialize<'de> for OnlyHttpUrl {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let url = Url::deserialize(deserializer)?;
+impl FromStr for OnlyHttpUrl {
+    type Err = ParseHttpUrlError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let url = Url::from_str(s)?;
         if url.scheme() == "http" {
             Ok(Self(url))
         } else {
-            Err(serde::de::Error::custom("only HTTP scheme is supported"))
+            Err(ParseHttpUrlError::NotHttp {
+                found: url.scheme().to_owned(),
+            })
         }
     }
 }
+
+#[derive(Debug, thiserror::Error)]
+pub enum ParseHttpUrlError {
+    #[error(transparent)]
+    Parse(#[from] url::ParseError),
+    #[error("expected `http` scheme, found: `{found}`")]
+    NotHttp { found: String },
+}
+
+iroha_config::base::impl_deserialize_from_str!(OnlyHttpUrl);
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashSet;
+
+    use iroha_config::base::{FromEnv as _, TestEnv};
+
+    use super::*;
+
+    #[test]
+    fn parses_all_envs() {
+        let env = TestEnv::new().set("TORII_URL", "http://localhost:8080");
+
+        let layer = RootPartial::from_env(&env).expect("should not fail since env is valid");
+
+        assert_eq!(env.unvisited(), HashSet::new())
+    }
+
+    #[test]
+    fn non_http_url_error() {
+        let error = "https://localhost:1123"
+            .parse::<OnlyHttpUrl>()
+            .expect_err("should not allow https");
+
+        assert_eq!(format!("{error}"), "expected `http` scheme, found: `https`");
+    }
+}

client/src/config/user_layer/boilerplate.rs

@@ -1,17 +1,18 @@
 //! Code to be generated by a proc macro in future
 
-use std::{fs::File, io::Read, path::Path};
+use std::{error::Error, fs::File, io::Read, path::Path};
 
 use eyre::{eyre, Context};
 use iroha_config::base::{
-    Emitter, FromEnvDefaultFallback, Merge, MissingFieldError, UnwrapPartial, UnwrapPartialResult,
+    Emitter, FromEnv, Merge, MissingFieldError, ParseEnvResult, UnwrapPartial, UnwrapPartialResult,
     UserDuration, UserField,
 };
 use iroha_crypto::{PrivateKey, PublicKey};
 use iroha_data_model::{account::AccountId, ChainId};
 use serde::Deserialize;
 
 use crate::config::{
+    base::{FromEnvResult, ReadEnv},
     user_layer::{Account, Api, OnlyHttpUrl, Root, Transaction},
     BasicAuth, DEFAULT_ADD_TRANSACTION_NONCE, DEFAULT_TRANSACTION_STATUS_TIMEOUT,
     DEFAULT_TRANSACTION_TIME_TO_LIVE,
@@ -53,7 +54,31 @@ impl RootPartial {
 }
 
 // FIXME: should config be read from ENV?
-impl FromEnvDefaultFallback for RootPartial {}
+impl FromEnv for RootPartial {
+    fn from_env<E: Error, R: ReadEnv<E>>(env: &R) -> FromEnvResult<Self>
+    where
+        Self: Sized,
+    {
+        let mut emitter = Emitter::new();
+
+        let api = ApiPartial::from_env(env).map_or_else(
+            |err| {
+                emitter.emit_collection(err);
+                None
+            },
+            Some,
+        );
+
+        emitter.finish()?;
+
+        Ok(Self {
+            chain_id: None.into(),
+            api: api.unwrap(),
+            account: AccountPartial::default(),
+            transaction: TransactionPartial::default(),
+        })
+    }
+}
 
 impl UnwrapPartial for RootPartial {
     type Output = Root;
@@ -100,6 +125,25 @@ impl UnwrapPartial for ApiPartial {
     }
 }
 
+impl FromEnv for ApiPartial {
+    fn from_env<E: Error, R: ReadEnv<E>>(env: &R) -> FromEnvResult<Self>
+    where
+        Self: Sized,
+    {
+        let mut emitter = Emitter::new();
+
+        let torii_url =
+            ParseEnvResult::parse_simple(&mut emitter, env, "TORII_URL", "api.torii_url").into();
+
+        emitter.finish()?;
+
+        Ok(Self {
+            torii_url,
+            basic_auth: None.into(),
+        })
+    }
+}
+
 #[derive(Debug, Clone, Deserialize, Eq, PartialEq, Default, Merge)]
 #[serde(deny_unknown_fields, default)]
 pub struct AccountPartial {

client_cli/pytests/pyproject.toml

@@ -11,6 +11,7 @@ faker = "*"
 allure-python-commons = "*"
 cryptography = "*"
 python-dotenv = "*"
+tomlkit = "^0.12.3"
 
 [tool.poetry.dev-dependencies]
 pytest = "*"

client_cli/pytests/src/client_cli/client_cli.py

@@ -254,19 +254,21 @@ def execute(self, command=None):
         :return: The current ClientCli object.
         :rtype: ClientCli
         """
+        self.config.randomise_torii_url()
         if command is None:
             command = self.command
         else:
             command = [self.BASE_PATH] + self.BASE_FLAGS + command.split()
         allure_command = ' '.join(map(str, command[3:]))
         print(allure_command)
-        with allure.step(f'{allure_command} on the {str(self.config.torii_api_port)} peer'):
+        with allure.step(f'{allure_command} on the {str(self.config.torii_url)} peer'):
             try:
                 with subprocess.Popen(
                         command,
                         stdout=subprocess.PIPE,
                         stderr=subprocess.PIPE,
-                        text=True
+                        text=True,
+                        env=self.config.env
                 ) as process:
                     self.stdout, self.stderr = process.communicate()
                     allure.attach(

client_cli/pytests/src/client_cli/configuration.py

@@ -2,7 +2,7 @@
 This module provides a Config class to manage Iroha network configuration.
 """
 
-import json
+import tomlkit
 import os
 import random
 from urllib.parse import urlparse
@@ -11,8 +11,8 @@
 class Config:
     """
     Configuration class to handle Iroha network configuration. The class provides methods for loading
-    the configuration from a file, updating the TORII_API_URL with a random port number from the specified
-    range, and accessing the configuration values.
+    the configuration from a file, accessing the configuration values, and randomising Torii URL
+    to access different peers.
 
     :param port_min: The minimum port number for the TORII_API_URL.
     :type port_min: int
@@ -24,6 +24,7 @@ def __init__(self, port_min, port_max):
         self.file = None
         self.port_min = port_min
         self.port_max = port_max
+        self._envs = dict()
 
     def load(self, path_config_client_cli):
         """
@@ -35,35 +36,42 @@ def load(self, path_config_client_cli):
         """
         if not os.path.exists(path_config_client_cli):
             raise IOError(f"No config file found at {path_config_client_cli}")
+        # TODO use toml
         with open(path_config_client_cli, 'r', encoding='utf-8') as config_file:
-            self._config = json.load(config_file)
+            self._config = tomlkit.load(config_file)
         self.file = path_config_client_cli
 
-    def update_torii_api_port(self):
+    def randomise_torii_url(self):
         """
-        Update the TORII_API_URL configuration value
-        with a random port number from the specified range.
+        Update Torii URL.
+        Note that in order for update to take effect,
+        `self.env` should be used when executing the client cli.
 
         :return: None
         """
-        if self._config is None:
-            raise ValueError("No configuration loaded. Use load_config(path_config_client_cli) to load the configuration.")
-        parsed_url = urlparse(self._config['TORII_API_URL'])
-        new_netloc = parsed_url.hostname + ':' + str(random.randint(self.port_min, self.port_max))
-        self._config['TORII_API_URL'] = parsed_url._replace(netloc=new_netloc).geturl()
-        with open(self.file, 'w', encoding='utf-8') as config_file:
-            json.dump(self._config, config_file)
+        parsed_url = urlparse(self._config['api']["torii_url"])
+        random_port = random.randint(self.port_min, self.port_max)
+        self._envs["TORII_URL"] = parsed_url._replace(netloc=f"{parsed_url.hostname}:{random_port}").geturl()
 
     @property
-    def torii_api_port(self):
+    def torii_url(self):
         """
-        Get the TORII_API_URL configuration value after updating the port number.
+        Get the Torii URL set in ENV vars.
 
-        :return: The updated TORII_API_URL.
+        :return: Torii URL
         :rtype: str
         """
-        self.update_torii_api_port()
-        return self._config['TORII_API_URL']
+        return self._envs["TORII_URL"]
+
+    @property
+    def env(self):
+        """
+        Get the environment variables set to execute the client cli with.
+
+        :return: Dictionary with env vars (mixed with existing OS vars)
+        :rtype: dict
+        """
+        return {**os.environ, **self._envs}
 
     @property
     def account_id(self):
@@ -73,7 +81,7 @@ def account_id(self):
         :return: The ACCOUNT_ID.
         :rtype: str
         """
-        return self._config['ACCOUNT_ID']
+        return self._config['account']["id"]
 
     @property
     def account_name(self):
@@ -103,4 +111,4 @@ def public_key(self):
         :return: The public key.
         :rtype: str
         """
-        return self._config['PUBLIC_KEY'].split('ed0120')[1]
+        return self._config["account"]['public_key'].split('ed0120')[1]

config_samples/examples/client.example.toml

@@ -7,6 +7,7 @@ private_key.digest_function = "ed25519"
 private_key.payload = "9ac47abf59b356e0bd7dcbbbb4dec080e302156a48ca907e47cb6aea1d32719e7233bfc89dcbd68c19fde6ce6158225298ec1131b6a130d1aeb454c1ab5183c0"
 
 [api]
+# Might be set via `TORII_URL` env var
 torii_url = "http://127.0.0.1:8080/"
 basic_auth.login = "mad_hatter"
 basic_auth.password = "ilovetea"