refactor: move Configuration unit test to inner mods

Author: josecelano

packages/configuration/src/lib.rs

@@ -157,136 +157,3 @@ impl From<figment::Error> for Error {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::Configuration;
-
-    #[cfg(test)]
-    fn default_config_toml() -> String {
-        let config = r#"log_level = "info"
-                                mode = "public"
-                                db_driver = "Sqlite3"
-                                db_path = "./storage/tracker/lib/database/sqlite3.db"
-                                announce_interval = 120
-                                min_announce_interval = 120
-                                on_reverse_proxy = false
-                                external_ip = "0.0.0.0"
-                                tracker_usage_statistics = true
-                                persistent_torrent_completed_stat = false
-                                max_peer_timeout = 900
-                                inactive_peer_cleanup_interval = 600
-                                remove_peerless_torrents = true
-
-                                [[udp_trackers]]
-                                enabled = false
-                                bind_address = "0.0.0.0:6969"
-
-                                [[http_trackers]]
-                                enabled = false
-                                bind_address = "0.0.0.0:7070"
-                                ssl_enabled = false
-                                ssl_cert_path = ""
-                                ssl_key_path = ""
-
-                                [http_api]
-                                enabled = true
-                                bind_address = "127.0.0.1:1212"
-                                ssl_enabled = false
-                                ssl_cert_path = ""
-                                ssl_key_path = ""
-
-                                [http_api.access_tokens]
-                                admin = "MyAccessToken"
-
-                                [health_check_api]
-                                bind_address = "127.0.0.1:1313"
-        "#
-        .lines()
-        .map(str::trim_start)
-        .collect::<Vec<&str>>()
-        .join("\n");
-        config
-    }
-
-    #[test]
-    fn configuration_should_have_default_values() {
-        let configuration = Configuration::default();
-
-        let toml = toml::to_string(&configuration).expect("Could not encode TOML value");
-
-        assert_eq!(toml, default_config_toml());
-    }
-
-    #[test]
-    fn configuration_should_contain_the_external_ip() {
-        let configuration = Configuration::default();
-
-        assert_eq!(configuration.external_ip, Some(String::from("0.0.0.0")));
-    }
-
-    #[test]
-    fn configuration_should_be_saved_in_a_toml_config_file() {
-        use std::{env, fs};
-
-        use uuid::Uuid;
-
-        // Build temp config file path
-        let temp_directory = env::temp_dir();
-        let temp_file = temp_directory.join(format!("test_config_{}.toml", Uuid::new_v4()));
-
-        // Convert to argument type for Configuration::save_to_file
-        let config_file_path = temp_file;
-        let path = config_file_path.to_string_lossy().to_string();
-
-        let default_configuration = Configuration::default();
-
-        default_configuration
-            .save_to_file(&path)
-            .expect("Could not save configuration to file");
-
-        let contents = fs::read_to_string(&path).expect("Something went wrong reading the file");
-
-        assert_eq!(contents, default_config_toml());
-    }
-
-    #[cfg(test)]
-    fn create_temp_config_file_with_default_config() -> String {
-        use std::env;
-        use std::fs::File;
-        use std::io::Write;
-
-        use uuid::Uuid;
-
-        // Build temp config file path
-        let temp_directory = env::temp_dir();
-        let temp_file = temp_directory.join(format!("test_config_{}.toml", Uuid::new_v4()));
-
-        // Convert to argument type for Configuration::load_from_file
-        let config_file_path = temp_file.clone();
-        let path = config_file_path.to_string_lossy().to_string();
-
-        // Write file contents
-        let mut file = File::create(temp_file).unwrap();
-        writeln!(&mut file, "{}", default_config_toml()).unwrap();
-
-        path
-    }
-
-    #[test]
-    fn configuration_should_be_loaded_from_a_toml_config_file() {
-        let config_file_path = create_temp_config_file_with_default_config();
-
-        let configuration = Configuration::load_from_file(&config_file_path).expect("Could not load configuration from file");
-
-        assert_eq!(configuration, Configuration::default());
-    }
-
-    #[test]
-    fn http_api_configuration_should_check_if_it_contains_a_token() {
-        let configuration = Configuration::default();
-
-        assert!(configuration.http_api.access_tokens.values().any(|t| t == "MyAccessToken"));
-        assert!(!configuration.http_api.access_tokens.values().any(|t| t == "NonExistingToken"));
-    }
-}

packages/configuration/src/v1/mod.rs

@@ -345,17 +345,7 @@ impl Default for Configuration {
             remove_peerless_torrents: true,
             udp_trackers: Vec::new(),
             http_trackers: Vec::new(),
-            http_api: HttpApi {
-                enabled: true,
-                bind_address: String::from("127.0.0.1:1212"),
-                ssl_enabled: false,
-                ssl_cert_path: None,
-                ssl_key_path: None,
-                access_tokens: [(String::from("admin"), String::from("MyAccessToken"))]
-                    .iter()
-                    .cloned()
-                    .collect(),
-            },
+            http_api: HttpApi::default(),
             health_check_api: HealthCheckApi {
                 bind_address: String::from("127.0.0.1:1313"),
             },
@@ -399,17 +389,9 @@ impl Configuration {
     ///
     /// Will return `Err` if `path` does not exist or has a bad configuration.    
     pub fn load_from_file(path: &str) -> Result<Configuration, Error> {
-        let figment = Figment::new().merge(Toml::file(path));
-        //.merge(Env::prefixed("TORRUST_TRACKER_"));
-
-        // code-review: merging values from env vars makes the test
-        // "configuration_should_be_loaded_from_a_toml_config_file" fail.
-        //
-        // It's because this line in a new test:
-        //
-        // jail.set_env("TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN", "NewToken");
-        //
-        // It seems env vars are shared between tests.
+        let figment = Figment::new()
+            .merge(Toml::file(path))
+            .merge(Env::prefixed("TORRUST_TRACKER_"));
 
         let config: Configuration = figment.extract()?;
 
@@ -475,8 +457,6 @@ impl Configuration {
 
 #[cfg(test)]
 mod tests {
-    use figment::providers::{Env, Format, Toml};
-    use figment::Figment;
 
     use crate::v1::Configuration;
 
@@ -527,19 +507,55 @@ mod tests {
         config
     }
 
+    #[test]
+    fn configuration_should_have_default_values() {
+        let configuration = Configuration::default();
+
+        let toml = toml::to_string(&configuration).expect("Could not encode TOML value");
+
+        assert_eq!(toml, default_config_toml());
+    }
+
+    #[test]
+    fn configuration_should_contain_the_external_ip() {
+        let configuration = Configuration::default();
+
+        assert_eq!(configuration.external_ip, Some(String::from("0.0.0.0")));
+    }
+
+    #[test]
+    fn configuration_should_be_saved_in_a_toml_config_file() {
+        use std::{env, fs};
+
+        use uuid::Uuid;
+
+        // Build temp config file path
+        let temp_directory = env::temp_dir();
+        let temp_file = temp_directory.join(format!("test_config_{}.toml", Uuid::new_v4()));
+
+        // Convert to argument type for Configuration::save_to_file
+        let config_file_path = temp_file;
+        let path = config_file_path.to_string_lossy().to_string();
+
+        let default_configuration = Configuration::default();
+
+        default_configuration
+            .save_to_file(&path)
+            .expect("Could not save configuration to file");
+
+        let contents = fs::read_to_string(&path).expect("Something went wrong reading the file");
+
+        assert_eq!(contents, default_config_toml());
+    }
+
     #[test]
     fn configuration_should_be_loaded_from_a_toml_config_file() {
         figment::Jail::expect_with(|jail| {
-            jail.create_file("Config.toml", &default_config_toml())?;
-
-            // todo: replace with Configuration method
-            let figment = Figment::new()
-                .merge(Toml::file("Config.toml"))
-                .merge(Env::prefixed("TORRUST_TRACKER_"));
+            jail.create_file("tracker.toml", &default_config_toml())?;
 
-            let config: Configuration = figment.extract()?;
+            let configuration = Configuration::load_from_file("tracker.toml").expect("Could not load configuration from file");
 
-            assert_eq!(config, Configuration::default());
+            assert_eq!(configuration, Configuration::default());
 
             Ok(())
         });
@@ -548,19 +564,14 @@ mod tests {
     #[test]
     fn configuration_should_allow_to_overwrite_the_default_tracker_api_token_for_admin() {
         figment::Jail::expect_with(|jail| {
-            jail.create_file("Config.toml", &default_config_toml())?;
+            jail.create_file("tracker.toml", &default_config_toml())?;
 
             jail.set_env("TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN", "NewToken");
 
-            // todo: replace with Configuration method
-            let figment = Figment::new()
-                .merge(Toml::file("Config.toml"))
-                .merge(Env::prefixed("TORRUST_TRACKER_"));
-
-            let config: Configuration = figment.extract()?;
+            let configuration = Configuration::load_from_file("tracker.toml").expect("Could not load configuration from file");
 
             assert_eq!(
-                config.http_api.access_tokens.get("admin"),
+                configuration.http_api.access_tokens.get("admin"),
                 Some("NewToken".to_owned()).as_ref()
             );
 

packages/configuration/src/v1/tracker_api.rs

@@ -31,8 +31,37 @@ pub struct HttpApi {
     pub access_tokens: AccessTokens,
 }
 
+impl Default for HttpApi {
+    fn default() -> Self {
+        Self {
+            enabled: true,
+            bind_address: String::from("127.0.0.1:1212"),
+            ssl_enabled: false,
+            ssl_cert_path: None,
+            ssl_key_path: None,
+            access_tokens: [(String::from("admin"), String::from("MyAccessToken"))]
+                .iter()
+                .cloned()
+                .collect(),
+        }
+    }
+}
+
 impl HttpApi {
     pub fn override_admin_token(&mut self, api_admin_token: &str) {
         self.access_tokens.insert("admin".to_string(), api_admin_token.to_string());
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::v1::tracker_api::HttpApi;
+
+    #[test]
+    fn http_api_configuration_should_check_if_it_contains_a_token() {
+        let configuration = HttpApi::default();
+
+        assert!(configuration.access_tokens.values().any(|t| t == "MyAccessToken"));
+        assert!(!configuration.access_tokens.values().any(|t| t == "NonExistingToken"));
+    }
+}