qa: Increase includeconf test coverage

MarcoFalke · MarcoFalke · commit fa4760fbb3f1 · 2018-06-01T13:24:50.000-04:00
diff --git a/src/util.cpp b/src/util.cpp
@@ -459,9 +459,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
     if (it != m_override_args.end()) {
         if (it->second.size() > 0) {
             for (const auto& ic : it->second) {
-                fprintf(stderr, "warning: -includeconf cannot be used from commandline; ignoring -includeconf=%s\n", ic.c_str());
+                error += "-includeconf cannot be used from commandline; -includeconf=" + ic + "\n";
             }
-            m_override_args.erase(it);
+            return false;
         }
     }
     return true;
@@ -849,11 +849,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
         // if there is an -includeconf in the override args, but it is empty, that means the user
         // passed '-noincludeconf' on the command line, in which case we should not include anything
         if (m_override_args.count("-includeconf") == 0) {
+            std::string chain_id = GetChainName();
             std::vector<std::string> includeconf(GetArgs("-includeconf"));
             {
                 // We haven't set m_network yet (that happens in SelectParams()), so manually check
                 // for network.includeconf args.
-                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));
                 includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
             }
 
@@ -862,7 +863,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
             {
                 LOCK(cs_args);
                 m_config_args.erase("-includeconf");
-                m_config_args.erase(std::string("-") + GetChainName() + ".includeconf");
+                m_config_args.erase(std::string("-") + chain_id + ".includeconf");
             }
 
             for (const std::string& to_include : includeconf) {
@@ -873,15 +874,22 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
                     }
                     LogPrintf("Included configuration file %s\n", to_include.c_str());
                 } else {
-                    fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
+                    error = "Failed to include configuration file " + to_include;
+                    return false;
                 }
             }
 
             // Warn about recursive -includeconf
             includeconf = GetArgs("-includeconf");
             {
-                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));
                 includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+                std::string chain_id_final = GetChainName();
+                if (chain_id_final != chain_id) {
+                    // Also warn about recursive includeconf for the chain that was specified in one of the includeconfs
+                    includeconf_net = GetArgs(std::string("-") + chain_id_final + ".includeconf");
+                    includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+                }
             }
             for (const std::string& to_include : includeconf) {
                 fprintf(stderr, "warning: -includeconf cannot be used from included files; ignoring -includeconf=%s\n", to_include.c_str());
diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
@@ -41,14 +41,9 @@ def run_test(self):
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
         assert subversion.endswith("main; relative)/")
 
-        self.log.info("-includeconf cannot be used as command-line arg. subversion should still end with 'main; relative)/'")
+        self.log.info("-includeconf cannot be used as command-line arg")
         self.stop_node(0)
-
-        self.start_node(0, extra_args=["-includeconf=relative2.conf"])
-
-        subversion = self.nodes[0].getnetworkinfo()["subversion"]
-        assert subversion.endswith("main; relative)/")
-        self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf")
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-includeconf=relative2.conf"], expected_msg="Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=relative2.conf")
 
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
@@ -59,8 +54,18 @@ def run_test(self):
         assert subversion.endswith("main; relative)/")
         self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from included files; ignoring -includeconf=relative2.conf")
 
+        self.log.info("-includeconf cannot contain invalid arg")
+        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            f.write("foo=bar\n")
+        self.nodes[0].assert_start_raises_init_error(expected_msg="Error reading configuration file: Invalid configuration value foo")
+
+        self.log.info("-includeconf cannot be invalid path")
+        os.remove(os.path.join(self.options.tmpdir, "node0", "relative.conf"))
+        self.nodes[0].assert_start_raises_init_error(expected_msg="Error reading configuration file: Failed to include configuration file relative.conf")
+
         self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            # Restore initial file contents
             f.write("uacomment=relative\n")
 
         with open(os.path.join(self.options.tmpdir, "node0", "bitcoin.conf"), "a", encoding='utf8') as f:

Original file line number	Diff line number	Diff line change
`@@ -459,9 +459,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin`
`459`	`459`	`if (it != m_override_args.end()) {`
`460`	`460`	`if (it->second.size() > 0) {`
`461`	`461`	`for (const auto& ic : it->second) {`
`462`		`- fprintf(stderr, "warning: -includeconf cannot be used from commandline; ignoring -includeconf=%s\n", ic.c_str());`
	`462`	`+ error += "-includeconf cannot be used from commandline; -includeconf=" + ic + "\n";`
`463`	`463`	`}`
`464`		`- m_override_args.erase(it);`
	`464`	`+ return false;`
`465`	`465`	`}`
`466`	`466`	`}`
`467`	`467`	`return true;`
`@@ -849,11 +849,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)`
`849`	`849`	`// if there is an -includeconf in the override args, but it is empty, that means the user`
`850`	`850`	`// passed '-noincludeconf' on the command line, in which case we should not include anything`
`851`	`851`	`if (m_override_args.count("-includeconf") == 0) {`
	`852`	`+ std::string chain_id = GetChainName();`
`852`	`853`	`std::vector<std::string> includeconf(GetArgs("-includeconf"));`
`853`	`854`	`{`
`854`	`855`	`// We haven't set m_network yet (that happens in SelectParams()), so manually check`
`855`	`856`	`// for network.includeconf args.`
`856`		`- std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));`
	`857`	`+ std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));`
`857`	`858`	`includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());`
`858`	`859`	`}`
`859`	`860`
`@@ -862,7 +863,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)`
`862`	`863`	`{`
`863`	`864`	`LOCK(cs_args);`
`864`	`865`	`m_config_args.erase("-includeconf");`
`865`		`- m_config_args.erase(std::string("-") + GetChainName() + ".includeconf");`
	`866`	`+ m_config_args.erase(std::string("-") + chain_id + ".includeconf");`
`866`	`867`	`}`
`867`	`868`
`868`	`869`	`for (const std::string& to_include : includeconf) {`
`@@ -873,15 +874,22 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)`
`873`	`874`	`}`
`874`	`875`	`LogPrintf("Included configuration file %s\n", to_include.c_str());`
`875`	`876`	`} else {`
`876`		`- fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());`
	`877`	`+ error = "Failed to include configuration file " + to_include;`
	`878`	`+ return false;`
`877`	`879`	`}`
`878`	`880`	`}`
`879`	`881`
`880`	`882`	`// Warn about recursive -includeconf`
`881`	`883`	`includeconf = GetArgs("-includeconf");`
`882`	`884`	`{`
`883`		`- std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));`
	`885`	`+ std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));`
`884`	`886`	`includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());`
	`887`	`+ std::string chain_id_final = GetChainName();`
	`888`	`+ if (chain_id_final != chain_id) {`
	`889`	`+ // Also warn about recursive includeconf for the chain that was specified in one of the includeconfs`
	`890`	`+ includeconf_net = GetArgs(std::string("-") + chain_id_final + ".includeconf");`
	`891`	`+ includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());`
	`892`	`+ }`
`885`	`893`	`}`
`886`	`894`	`for (const std::string& to_include : includeconf) {`
`887`	`895`	`fprintf(stderr, "warning: -includeconf cannot be used from included files; ignoring -includeconf=%s\n", to_include.c_str());`