Merge bitcoin/bitcoin#35846: test: Use throwing config parser getters without fallback

fabe100c2b test: Use throwing config parser getters without fallback (MarcoFalke)
fa8acd57cd test: Write true/false values in config.ini (MarcoFalke)

Pull request description:

  Currently, the called `getboolean` member function is *not* the throwing https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.getboolean, but a non-throwing member function on a dict-like proxy object.

  This is confusing and brittle, because tests shouldn't silently skip when a config key is missing. Instead, tests should loudly fail, e.g. when the config key is renamed in one place, but not the other.

ACKs for top commit:
  jeanpablojp:
    tACK fabe100c2b
  willcl-ark:
    ACK fabe100c2b

Tree-SHA512: a970d74ad285372b8adcce8e2a52b01f5a3b563899dfc5262e6ffbf3d8aba43e72f7b03111e8d5188924c7d3d789992407cddb5d182d9be5e42f07896d8ad4a3

test/CMakeLists.txt

@@ -9,9 +9,9 @@ function(create_test_config)
 
   macro(set_configure_variable var conf_var)
     if(${var})
-      set(${conf_var}_TRUE "")
+      set(${conf_var}_VALUE true)
     else()
-      set(${conf_var}_TRUE "#")
+      set(${conf_var}_VALUE false)
     endif()
   endmacro()
 

test/config.ini.in

@@ -14,19 +14,19 @@ EXEEXT=@EXEEXT@
 RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py
 
 [components]
-# Which components are enabled. These are commented out by cmake if they were disabled during configuration.
-@ENABLE_WALLET_TRUE@ENABLE_WALLET=true
-@BUILD_BENCH_TRUE@BUILD_BENCH=true
-@BUILD_BITCOIN_CLI_TRUE@ENABLE_CLI=true
-@BUILD_BITCOIN_TX_TRUE@BUILD_BITCOIN_TX=true
-@BUILD_BITCOIN_UTIL_TRUE@ENABLE_BITCOIN_UTIL=true
-@BUILD_BITCOIN_CHAINSTATE_TRUE@ENABLE_BITCOIN_CHAINSTATE=true
-@BUILD_BITCOIN_WALLET_TRUE@ENABLE_WALLET_TOOL=true
-@BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=true
-@ENABLE_FUZZ_BINARY_TRUE@ENABLE_FUZZ_BINARY=true
-@ENABLE_ZMQ_TRUE@ENABLE_ZMQ=true
-@ENABLE_EMBEDDED_ASMAP_TRUE@ENABLE_EMBEDDED_ASMAP=true
-@ENABLE_EXTERNAL_SIGNER_TRUE@ENABLE_EXTERNAL_SIGNER=true
-@ENABLE_USDT_TRACEPOINTS_TRUE@ENABLE_USDT_TRACEPOINTS=true
-@ENABLE_IPC_TRUE@ENABLE_IPC=true
-@BUILD_GUI_TRUE@BUILD_GUI=true
+# Which components are enabled.
+ENABLE_WALLET=@ENABLE_WALLET_VALUE@
+BUILD_BENCH=@BUILD_BENCH_VALUE@
+ENABLE_CLI=@BUILD_BITCOIN_CLI_VALUE@
+BUILD_BITCOIN_TX=@BUILD_BITCOIN_TX_VALUE@
+ENABLE_BITCOIN_UTIL=@BUILD_BITCOIN_UTIL_VALUE@
+ENABLE_BITCOIN_CHAINSTATE=@BUILD_BITCOIN_CHAINSTATE_VALUE@
+ENABLE_WALLET_TOOL=@BUILD_BITCOIN_WALLET_VALUE@
+ENABLE_BITCOIND=@BUILD_BITCOIND_VALUE@
+ENABLE_FUZZ_BINARY=@ENABLE_FUZZ_BINARY_VALUE@
+ENABLE_ZMQ=@ENABLE_ZMQ_VALUE@
+ENABLE_EMBEDDED_ASMAP=@ENABLE_EMBEDDED_ASMAP_VALUE@
+ENABLE_EXTERNAL_SIGNER=@ENABLE_EXTERNAL_SIGNER_VALUE@
+ENABLE_USDT_TRACEPOINTS=@ENABLE_USDT_TRACEPOINTS_VALUE@
+ENABLE_IPC=@ENABLE_IPC_VALUE@
+BUILD_GUI=@BUILD_GUI_VALUE@

test/functional/test_framework/test_framework.py

@@ -1124,55 +1124,55 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
 
     def is_bench_compiled(self):
         """Checks whether bench_bitcoin was compiled."""
-        return self.config["components"].getboolean("BUILD_BENCH")
+        return self.config.getboolean("components", "BUILD_BENCH")
 
     def is_cli_compiled(self):
         """Checks whether bitcoin-cli was compiled."""
-        return self.config["components"].getboolean("ENABLE_CLI")
+        return self.config.getboolean("components", "ENABLE_CLI")
 
     def is_external_signer_compiled(self):
         """Checks whether external signer support was compiled."""
-        return self.config["components"].getboolean("ENABLE_EXTERNAL_SIGNER")
+        return self.config.getboolean("components", "ENABLE_EXTERNAL_SIGNER")
 
     def is_wallet_compiled(self):
         """Checks whether the wallet module was compiled."""
-        return self.config["components"].getboolean("ENABLE_WALLET")
+        return self.config.getboolean("components", "ENABLE_WALLET")
 
     def is_wallet_tool_compiled(self):
         """Checks whether bitcoin-wallet was compiled."""
-        return self.config["components"].getboolean("ENABLE_WALLET_TOOL")
+        return self.config.getboolean("components", "ENABLE_WALLET_TOOL")
 
     def is_bitcoin_tx_compiled(self):
         """Checks whether bitcoin-tx was compiled."""
-        return self.config["components"].getboolean("BUILD_BITCOIN_TX")
+        return self.config.getboolean("components", "BUILD_BITCOIN_TX")
 
     def is_bitcoin_util_compiled(self):
         """Checks whether bitcoin-util was compiled."""
-        return self.config["components"].getboolean("ENABLE_BITCOIN_UTIL")
+        return self.config.getboolean("components", "ENABLE_BITCOIN_UTIL")
 
     def is_bitcoin_chainstate_compiled(self):
         """Checks whether bitcoin-chainstate was compiled."""
-        return self.config["components"].getboolean("ENABLE_BITCOIN_CHAINSTATE")
+        return self.config.getboolean("components", "ENABLE_BITCOIN_CHAINSTATE")
 
     def is_zmq_compiled(self):
         """Checks whether the zmq module was compiled."""
-        return self.config["components"].getboolean("ENABLE_ZMQ")
+        return self.config.getboolean("components", "ENABLE_ZMQ")
 
     def is_embedded_asmap_compiled(self):
         """Checks whether ASMap data was embedded during compilation."""
-        return self.config["components"].getboolean("ENABLE_EMBEDDED_ASMAP")
+        return self.config.getboolean("components", "ENABLE_EMBEDDED_ASMAP")
 
     def is_usdt_compiled(self):
         """Checks whether the USDT tracepoints were compiled."""
-        return self.config["components"].getboolean("ENABLE_USDT_TRACEPOINTS")
+        return self.config.getboolean("components", "ENABLE_USDT_TRACEPOINTS")
 
     def is_ipc_compiled(self):
         """Checks whether ipc was compiled."""
-        return self.config["components"].getboolean("ENABLE_IPC")
+        return self.config.getboolean("components", "ENABLE_IPC")
 
     def is_gui_compiled(self):
         """Checks whether the GUI was compiled."""
-        return self.config["components"].getboolean("BUILD_GUI", fallback=False)
+        return self.config.getboolean("components", "BUILD_GUI")
 
     def has_blockfile(self, node, filenum: str):
         return (node.blocks_path/ f"blk{filenum}.dat").is_file()

test/functional/test_runner.py

@@ -479,7 +479,7 @@ def main():
         assert results_filepath.parent.exists(), "Results file parent directory does not exist"
         logging.debug("Test results will be written to " + str(results_filepath))
 
-    enable_bitcoind = config["components"].getboolean("ENABLE_BITCOIND")
+    enable_bitcoind = config.getboolean("components", "ENABLE_BITCOIND")
 
     if not enable_bitcoind:
         print("No functional tests to run.")
@@ -548,7 +548,7 @@ def main():
                 # Exclude all variants of a test
                 remove_tests([test for test in test_list if test.split('.py')[0] == exclude_test.split('.py')[0]])
 
-    if config["components"].getboolean("BUILD_BENCH") and TOOL_BENCH_SANITY_CHECK in test_list:
+    if config.getboolean("components", "BUILD_BENCH") and TOOL_BENCH_SANITY_CHECK in test_list:
         # Remove it, and expand it for each bench in the list
         test_list.remove(TOOL_BENCH_SANITY_CHECK)
         bench_cmd = Binaries(get_binary_paths(config), bin_dir=None).bench_argv() + ["-list"]

test/fuzz/test_runner.py

@@ -101,7 +101,7 @@ def main():
     configfile = os.path.abspath(os.path.dirname(__file__)) + "/../config.ini"
     config.read_file(open(configfile))
 
-    if not config["components"].getboolean("ENABLE_FUZZ_BINARY"):
+    if not config.getboolean("components", "ENABLE_FUZZ_BINARY"):
         logging.error("Must have fuzz executable built")
         sys.exit(1)