From aca2c2fb51c55eb763736a3355d16876351f1fcb Mon Sep 17 00:00:00 2001
From: Anton Fetisov <a.fetisov@picodata.io>
Date: Fri, 7 Feb 2025 17:54:13 +0300
Subject: [PATCH] test: split config.rs::parameter_source_precedence into
 separate tests

---
 src/config.rs | 278 ++++++++++++++++++++++++++++----------------------
 1 file changed, 154 insertions(+), 124 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index f2866b01bc..584ae60668 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -610,8 +610,8 @@ Using configuration file '{args_path}'.");
         for (name, info) in tiers {
             if let Some(explicit_name) = &info.name {
                 return Err(Error::InvalidConfiguration(format!(
-                "tier '{name}' has an explicit name field '{explicit_name}', which is not allowed. Tier name is always derived from the outer dictionary's key"
-            )));
+                    "tier '{name}' has an explicit name field '{explicit_name}', which is not allowed. Tier name is always derived from the outer dictionary's key"
+                )));
             }
         }
 
@@ -691,8 +691,8 @@ Using configuration file '{args_path}'.");
                     if from_storage != from_config.to_host_port() =>
                 {
                     return Err(Error::InvalidConfiguration(format!(
-                        "instance restarted with a different `iproto_advertise`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
-                    )));
+                            "instance restarted with a different `iproto_advertise`, which is not allowed, was: '{from_storage}' became: '{from_config}'"
+                        )));
                 }
                 _ => {}
             }
@@ -2194,90 +2194,90 @@ instance:
         })
     }
 
-    #[rustfmt::skip]
-    #[test]
-    fn parameter_source_precedence() {
-        let _guard = protect_env();
+    mod parameter_source_precedence {
+        use super::*;
+        use pretty_assertions::assert_eq;
+
+        #[test]
+        fn check_defaults() {
+            let _guard = protect_env();
 
-        //
-        // Defaults
-        //
-        {
             let config = setup_for_tests(None, &["run"]).unwrap();
 
+            assert_eq!(config.instance.peers(), vec![IprotoAddress::default()]);
+            assert_eq!(config.instance.name(), None);
             assert_eq!(
-                config.instance.peers(),
-                vec![IprotoAddress::default()]
+                config.instance.iproto_listen().to_host_port(),
+                IprotoAddress::default_host_port()
+            );
+            assert_eq!(
+                config.instance.iproto_advertise().to_host_port(),
+                IprotoAddress::default_host_port()
             );
-            assert_eq!(config.instance.name(), None);
-            assert_eq!(config.instance.iproto_listen().to_host_port(), IprotoAddress::default_host_port());
-            assert_eq!(config.instance.iproto_advertise().to_host_port(), IprotoAddress::default_host_port());
             assert_eq!(config.instance.log_level(), SayLevel::Info);
             assert!(config.instance.failure_domain().data.is_empty());
         }
 
-        //
-        // Precedence: command line > env > config
-        //
-        {
+        #[test]
+        fn cli_env_config() {
+            let _guard = protect_env();
+            // Precedence: command line > env > config
+
             let yaml = r###"
-instance:
-    name: I-CONFIG
-"###;
+            instance:
+                name: I-CONFIG
+            "###;
 
             // only config
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
             assert_eq!(config.instance.name().unwrap(), "I-CONFIG");
 
             // PICODATA_CONFIG_PARAMETERS > config
-            std::env::set_var("PICODATA_CONFIG_PARAMETERS", "instance.name=I-ENV-CONF-PARAM");
+            std::env::set_var(
+                "PICODATA_CONFIG_PARAMETERS",
+                "instance.name=I-ENV-CONF-PARAM",
+            );
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
             assert_eq!(config.instance.name().unwrap(), "I-ENV-CONF-PARAM");
 
             // other env > PICODATA_CONFIG_PARAMETERS
             std::env::set_var("PICODATA_INSTANCE_NAME", "I-ENVIRON");
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
             assert_eq!(config.instance.name().unwrap(), "I-ENVIRON");
 
             // command line > env
-            let config = setup_for_tests(Some(yaml), &["run", "--instance-name=I-COMMANDLINE"]).unwrap();
-
+            let config =
+                setup_for_tests(Some(yaml), &["run", "--instance-name=I-COMMANDLINE"]).unwrap();
             assert_eq!(config.instance.name().unwrap(), "I-COMMANDLINE");
 
             // -c PARAMETER=VALUE > other command line
-            let config = setup_for_tests(Some(yaml), &["run", "-c", "instance.name=I-CLI-CONF-PARAM", "--instance-name=I-COMMANDLINE"]).unwrap();
-
+            #[rustfmt::skip]
+            let config = setup_for_tests(Some(yaml), &["run",
+                "-c", "instance.name=I-CLI-CONF-PARAM", "--instance-name=I-COMMANDLINE",
+            ]) .unwrap();
             assert_eq!(config.instance.name().unwrap(), "I-CLI-CONF-PARAM");
         }
 
-        //
-        // peer parsing
-        //
-        {
+        #[test]
+        fn peer_parsing() {
+            let _guard = protect_env();
+
             // empty config means default
             let yaml = r###"
-instance:
-    peer:
-"###;
+            instance:
+                peer:
+            "###;
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
-            assert_eq!(
-                config.instance.peers(),
-                vec![IprotoAddress::default()]
-            );
+            assert_eq!(config.instance.peers(), vec![IprotoAddress::default()]);
 
             // only config
             let yaml = r###"
-instance:
-    peer:
-        - bobbert:420
-        - tomathan:69
-"###;
+            instance:
+                peer:
+                    - bobbert:420
+                    - tomathan:69
+            "###;
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
             assert_eq!(
                 config.instance.peers(),
                 vec![
@@ -2295,9 +2295,11 @@ instance:
             );
 
             // env > config
-            std::env::set_var("PICODATA_PEER", "oops there's a space over here -> <-:13,             maybe we should at least strip these:37");
+            std::env::set_var(
+                "PICODATA_PEER",
+                "oops there's a space over here -> <-:13,             maybe we should at least strip these:37",
+            );
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
             assert_eq!(
                 config.instance.peers(),
                 vec![
@@ -2315,11 +2317,11 @@ instance:
             );
 
             // command line > env
+            #[rustfmt::skip]
             let config = setup_for_tests(Some(yaml), &["run",
                 "--peer", "one:1",
-                "--peer", "two:2,    <- same problem here,localhost:3,test4"
+                "--peer", "two:2,    <- same problem here,localhost:3,test4",
             ]).unwrap();
-
             assert_eq!(
                 config.instance.peers(),
                 vec![
@@ -2352,10 +2354,10 @@ instance:
             );
 
             // --config-parameter > --peer
+            #[rustfmt::skip]
             let config = setup_for_tests(Some(yaml), &["run",
                 "-c", "instance.peer=[  host:123  , ghost :321, гост:666]",
             ]).unwrap();
-
             assert_eq!(
                 config.instance.peers(),
                 vec![
@@ -2378,44 +2380,61 @@ instance:
             );
         }
 
-        //
-        // Advertise = listen unless specified explicitly
-        //
-            {
+        #[test]
+        fn check_advertise() {
+            let _guard = protect_env();
+
+            // Advertise = listen unless specified explicitly
             std::env::set_var("PICODATA_LISTEN", "L-ENVIRON");
             let config = setup_for_tests(Some(""), &["run"]).unwrap();
-
-            assert_eq!(config.instance.iproto_listen().to_host_port(), "L-ENVIRON:3301");
-            assert_eq!(config.instance.iproto_advertise().to_host_port(), "L-ENVIRON:3301");
+            assert_eq!(
+                config.instance.iproto_listen().to_host_port(),
+                "L-ENVIRON:3301"
+            );
+            assert_eq!(
+                config.instance.iproto_advertise().to_host_port(),
+                "L-ENVIRON:3301"
+            );
 
             let yaml = r###"
-instance:
-    iproto_advertise: A-CONFIG
-"###;
+            instance:
+                iproto_advertise: A-CONFIG
+            "###;
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
-
-            assert_eq!(config.instance.iproto_listen().to_host_port(), "L-ENVIRON:3301");
-            assert_eq!(config.instance.iproto_advertise().to_host_port(), "A-CONFIG:3301");
+            assert_eq!(
+                config.instance.iproto_listen().to_host_port(),
+                "L-ENVIRON:3301"
+            );
+            assert_eq!(
+                config.instance.iproto_advertise().to_host_port(),
+                "A-CONFIG:3301"
+            );
 
             let config = setup_for_tests(Some(yaml), &["run", "-l", "L-COMMANDLINE"]).unwrap();
-
-            assert_eq!(config.instance.iproto_listen().to_host_port(), "L-COMMANDLINE:3301");
-            assert_eq!(config.instance.iproto_advertise().to_host_port(), "A-CONFIG:3301");
+            assert_eq!(
+                config.instance.iproto_listen().to_host_port(),
+                "L-COMMANDLINE:3301"
+            );
+            assert_eq!(
+                config.instance.iproto_advertise().to_host_port(),
+                "A-CONFIG:3301"
+            );
         }
 
-        //
-        // Failure domain is parsed correctly
-        //
-        {
+        #[test]
+        fn failure_domain() {
+            let _guard = protect_env();
+            // Failure domain is parsed correctly
+
             // FIXME: duplicate keys in failure-domain should be a hard error
             // config
             let yaml = r###"
-instance:
-    failure_domain:
-        kconf1: vconf1
-        kconf2: vconf2
-        kconf2: vconf2-replaced
-"###;
+            instance:
+                failure_domain:
+                    kconf1: vconf1
+                    kconf2: vconf2
+                    kconf2: vconf2-replaced
+            "###;
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
             assert_eq!(
                 config.instance.failure_domain(),
@@ -2423,7 +2442,10 @@ instance:
             );
 
             // environment
-            std::env::set_var("PICODATA_FAILURE_DOMAIN", "kenv1=venv1,kenv2=venv2,kenv2=venv2-replaced");
+            std::env::set_var(
+                "PICODATA_FAILURE_DOMAIN",
+                "kenv1=venv1,kenv2=venv2,kenv2=venv2-replaced",
+            );
             let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
             assert_eq!(
                 config.instance.failure_domain(),
@@ -2431,59 +2453,62 @@ instance:
             );
 
             // command line
-            let config = setup_for_tests(Some(yaml), &["run", "--failure-domain", "karg1=varg1,karg1=varg1-replaced"]).unwrap();
+            #[rustfmt::skip]
+            let config = setup_for_tests(Some(yaml), &["run",
+                "--failure-domain", "karg1=varg1,karg1=varg1-replaced",
+            ]).unwrap();
             assert_eq!(
                 config.instance.failure_domain(),
                 FailureDomain::from([("KARG1", "VARG1-REPLACED")])
             );
 
             // command line more complex
+            #[rustfmt::skip]
             let config = setup_for_tests(Some(yaml), &["run",
-                "--failure-domain", "foo=1",
-                "--failure-domain", "bar=2,baz=3"
+                "--failure-domain", "foo=1", "--failure-domain", "bar=2,baz=3",
             ]).unwrap();
             assert_eq!(
                 config.instance.failure_domain(),
-                FailureDomain::from([
-                    ("FOO", "1"),
-                    ("BAR", "2"),
-                    ("BAZ", "3"),
-                ])
+                FailureDomain::from([("FOO", "1"), ("BAR", "2"), ("BAZ", "3"),])
             );
 
             // --config-parameter
+            #[rustfmt::skip]
             let config = setup_for_tests(Some(yaml), &["run",
-                "-c", "instance.failure_domain={foo: '11', bar: '22'}"
-            ]).unwrap();
+                "-c", "instance.failure_domain={foo: '11', bar: '22'}",
+            ]) .unwrap();
             assert_eq!(
                 config.instance.failure_domain(),
-                FailureDomain::from([
-                    ("FOO", "11"),
-                    ("BAR", "22"),
-                ])
+                FailureDomain::from([("FOO", "11"), ("BAR", "22"),])
             );
         }
 
-        // --config-parameter + PICODATA_CONFIG_PARAMETERS
-        {
+        #[test]
+        fn config_parameter_and_env() {
+            let _guard = protect_env();
+            // --config-parameter + PICODATA_CONFIG_PARAMETERS
+
             let yaml = r###"
-instance:
-    tier: this-will-be-overloaded
-"###;
+            instance:
+                tier: this-will-be-overloaded
+            "###;
             std::env::set_var(
                 "PICODATA_CONFIG_PARAMETERS",
                 "  instance.tier = ABC;cluster.name=DEF  ;
                 instance.audit=audit.txt ;;
-                ; instance.instance_dir=. ;"
+                ; instance.instance_dir=. ;",
             );
+            #[rustfmt::skip]
             let config = setup_for_tests(Some(yaml), &["run",
-                "-c", "  instance.log .level =debug  ",
-                "--config-parameter", "instance. memtx . memory=  999",
+                "-c", "  instance.log .level =debug  ", "--config-parameter", "instance. memtx . memory=  999",
             ]).unwrap();
             assert_eq!(config.instance.tier.unwrap(), "ABC");
             assert_eq!(config.cluster.name.unwrap(), "DEF");
             assert_eq!(config.instance.log.level.unwrap(), args::LogLevel::Debug);
-            assert_eq!(config.instance.memtx.memory.unwrap().to_string(), String::from("999B"));
+            assert_eq!(
+                config.instance.memtx.memory.unwrap().to_string(),
+                String::from("999B")
+            );
             assert_eq!(config.instance.audit.unwrap(), "audit.txt");
             assert_eq!(config.instance.instance_dir.unwrap(), PathBuf::from("."));
 
@@ -2491,18 +2516,21 @@ instance:
             // Errors
             //
             let yaml = r###"
-"###;
-            let e = setup_for_tests(Some(yaml), &["run",
-                "-c", "  instance.hoobabooba = malabar  ",
+            "###;
+            #[rustfmt::skip]
+            let e = setup_for_tests(Some(yaml), &["run", 
+                "-c", "  instance.hoobabooba = malabar  "
             ]).unwrap_err();
-            assert!(dbg!(e.to_string()).starts_with("invalid configuration: instance: unknown field `hoobabooba`, expected one of"));
+            assert!(dbg!(e.to_string()).starts_with(
+                "invalid configuration: instance: unknown field `hoobabooba`, expected one of"
+            ));
 
             let yaml = r###"
-"###;
+            "###;
             std::env::set_var(
                 "PICODATA_CONFIG_PARAMETERS",
                 "  cluster.name=DEF  ;
-                  cluster.asdfasdfbasdfbasd = "
+                  cluster.asdfasdfbasdfbasd = ",
             );
             let e = setup_for_tests(Some(yaml), &["run"]).unwrap_err();
             assert!(dbg!(e.to_string()).starts_with("invalid configuration: cluster: unknown field `asdfasdfbasdfbasd`, expected one of"));
@@ -2514,8 +2542,8 @@ instance:
         let _guard = protect_env();
 
         let yaml = r###"
-instance:
-"###;
+        instance:
+        "###;
         let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
         let pg = config.instance.pg;
         // pg section wasn't specified, but it should be enabled by default
@@ -2525,11 +2553,11 @@ instance:
         );
 
         let yaml = r###"
-instance:
-    pg:
-        listen: "127.0.0.1:5432"
-        ssl: true
-"###;
+        instance:
+            pg:
+                listen: "127.0.0.1:5432"
+                ssl: true
+        "###;
         let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
         let pg = config.instance.pg;
         assert_eq!(&pg.listen().to_host_port(), "127.0.0.1:5432");
@@ -2537,10 +2565,10 @@ instance:
 
         // test defaults
         let yaml = r###"
-instance:
-    pg:
-        listen: "127.0.0.1:5432"
-"###;
+        instance:
+            pg:
+                listen: "127.0.0.1:5432"
+        "###;
         let config = setup_for_tests(Some(yaml), &["run"]).unwrap();
         let pg = config.instance.pg;
         assert_eq!(
@@ -2550,8 +2578,10 @@ instance:
         assert!(!pg.ssl());
 
         // test config with -c option
-        let config =
-            setup_for_tests(None, &["run", "-c", "instance.pg.listen=127.0.0.1:5432"]).unwrap();
+        #[rustfmt::skip]
+        let config = setup_for_tests(None, &["run",
+            "-c", "instance.pg.listen=127.0.0.1:5432"
+        ]).unwrap();
         let pg = config.instance.pg;
         assert_eq!(
             pg.listen(),
-- 
GitLab