Merge bitcoin/bitcoin#32697: test: Turn util/test_runner into functional test

fa21631 test: Use self.log (MarcoFalke)
fa346f7 test: Move error string into exception (MarcoFalke)
fa19861 test: Remove useless catch-throw (MarcoFalke)
fa2f1c5 move-only util data to test/functional/data/util (MarcoFalke)
faa18bf test: Turn util/test_runner into functional test (MarcoFalke)
fa95515 test: Add missing skip_if_no_bitcoin_tx (MarcoFalke)
fac9db6 test: Add missing tx util to Binaries (MarcoFalke)
fa91835 test: Use lowercase env var as attribute name (MarcoFalke)
fac4909 test: Remove duplicate ConfigParser (MarcoFalke)

Pull request description:

  The `test/util/test_runner.py` has many issues:

  * The boilerplate for the test runner is duplicate or inconsistent with the other (functional) tests. For example, logging options, `ConfigParser` handling, `Binaries` handling ...
  * The cmake/ci behavior is brittle and can silently fail, as explained in bitcoin/bitcoin#31476
  * corecheck (and likely other places that manually run the tests) completely forget to run it
  * If the test is manually called, it runs single threaded, when it could just run in parallel with the other functional tests

  Fix all issues by removing the util test_runner and moving the test logic into a new functional test file.

ACKs for top commit:
  janb84:
    re ACK fa21631
  brunoerg:
    re-ACK fa21631
  hebasto:
    re-ACK fa21631, additional feedback has been addressed since my previous [review](bitcoin/bitcoin#32697 (review)).

Tree-SHA512: 694e647887801f002843a74011035d5ed3dfed091d3f0ae18e812a16a4680e04e60e50de0a92af7e047e8ddd6ff5a7834c690f16fd42b74ebc1674bf9989406f

Author: glozow

.github/workflows/ci.yml

@@ -260,6 +260,7 @@ jobs:
         env:
           BITCOIND: '${{ github.workspace }}\build\bin\Release\bitcoind.exe'
           BITCOINCLI: '${{ github.workspace }}\build\bin\Release\bitcoin-cli.exe'
+          BITCOINTX: '${{ github.workspace }}\build\bin\Release\bitcoin-tx.exe'
           BITCOINUTIL: '${{ github.workspace }}\build\bin\Release\bitcoin-util.exe'
           BITCOINWALLET: '${{ github.workspace }}\build\bin\Release\bitcoin-wallet.exe'
           TEST_RUNNER_EXTRA: ${{ github.event_name != 'pull_request' && '--extended' || '' }}
@@ -389,9 +390,6 @@ jobs:
           (Get-Content "test/config.ini") -replace '(?<=^SRCDIR=).*', '${{ github.workspace }}' -replace '(?<=^BUILDDIR=).*', '${{ github.workspace }}' -replace '(?<=^RPCAUTH=).*', '${{ github.workspace }}/share/rpcauth/rpcauth.py' | Set-Content "test/config.ini"
           Get-Content "test/config.ini"
 
-      - name: Run util tests
-        run: py -3 test/util/test_runner.py
-
       - name: Run rpcauth test
         run: py -3 test/util/rpcauth-test.py
 

CMakeLists.txt

@@ -594,7 +594,7 @@ if(Python3_EXECUTABLE)
   set(PYTHON_COMMAND ${Python3_EXECUTABLE})
 else()
   list(APPEND configure_warnings
-    "Minimum required Python not found. Utils and rpcauth tests are disabled."
+    "Minimum required Python not found. Rpcauth tests are disabled."
   )
 endif()
 

cmake/tests.cmake

@@ -2,12 +2,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or https://opensource.org/license/mit/.
 
-if(TARGET bitcoin-util AND TARGET bitcoin-tx AND PYTHON_COMMAND)
-  add_test(NAME util_test_runner
-    COMMAND ${CMAKE_COMMAND} -E env BITCOINUTIL=$<TARGET_FILE:bitcoin-util> BITCOINTX=$<TARGET_FILE:bitcoin-tx> ${PYTHON_COMMAND} ${PROJECT_BINARY_DIR}/test/util/test_runner.py
-  )
-endif()
-
 if(PYTHON_COMMAND)
   add_test(NAME util_rpcauth_test
     COMMAND ${PYTHON_COMMAND} ${PROJECT_BINARY_DIR}/test/util/rpcauth-test.py

test/CMakeLists.txt

@@ -17,6 +17,7 @@ function(create_test_config)
 
   set_configure_variable(ENABLE_WALLET ENABLE_WALLET)
   set_configure_variable(BUILD_CLI BUILD_BITCOIN_CLI)
+  set_configure_variable(BUILD_TX BUILD_BITCOIN_TX)
   set_configure_variable(BUILD_UTIL BUILD_BITCOIN_UTIL)
   set_configure_variable(BUILD_UTIL_CHAINSTATE BUILD_BITCOIN_CHAINSTATE)
   set_configure_variable(BUILD_WALLET_TOOL BUILD_BITCOIN_WALLET)
@@ -36,7 +37,7 @@ file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/fuzz)
 file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/util)
 
 file(GLOB_RECURSE functional_tests RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} functional/*)
-foreach(script ${functional_tests} fuzz/test_runner.py util/rpcauth-test.py util/test_runner.py)
+foreach(script ${functional_tests} fuzz/test_runner.py util/rpcauth-test.py)
   if(CMAKE_HOST_WIN32)
     set(symlink)
   else()

test/README.md

@@ -17,6 +17,7 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py
 # Which components are enabled. These are commented out by cmake if they were disabled during configuration.
 @ENABLE_WALLET_TRUE@ENABLE_WALLET=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