Merge bitcoin/bitcoin#32881: test: Turn rpcauth.py test into functional test

fa4d68c Turn rpcauth.py test into functional test (MarcoFalke)

Pull request description:

  Currently the `rpcauth-test.py` is problematic, because:

  * The boilerplate for the test runner is duplicate or inconsistent with the other (functional) tests. Specifically `ConfigParser`.
  * The cmake/ci behavior is brittle and can silently fail, as explained in bitcoin/bitcoin#31476.
  * Outside of ctest, this single test has to be run manually and separately, which is easy to forget.
  * If the test is manually called, it runs single threaded, when it could just run in parallel with the other functional tests.
  * It is also the only "unit" test written in Python, but not called by the functional test runner.

  Fix all issues by turning it into a functional test.

ACKs for top commit:
  l0rinc:
    ACK fa4d68c
  janb84:
    LGTM ACK fa4d68c
  w0xlt:
    ACK bitcoin/bitcoin@fa4d68c

Tree-SHA512: a3b2b03be31c33288dee23c544b33ec43295e796c2047777597ceb86acce9f697478e32d891aa986c1d7d5749d62eded65eeb858e9d7479bda7a400eb1167040

Author: fanquake

.github/workflows/ci.yml

@@ -390,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 rpcauth test
-        run: py -3 test/util/rpcauth-test.py
-
       - name: Set previous release directory
         run: |
           echo "PREVIOUS_RELEASES_DIR=${{ runner.temp }}/previous_releases" >> "$GITHUB_ENV"

CMakeLists.txt

@@ -592,7 +592,7 @@ mark_as_advanced(Python3_FIND_FRAMEWORK Python3_FIND_UNVERSIONED_NAMES)
 find_package(Python3 3.10 COMPONENTS Interpreter)
 if(NOT TARGET Python3::Interpreter)
   list(APPEND configure_warnings
-    "Minimum required Python not found. Rpcauth tests are disabled."
+    "Minimum required Python not found."
   )
 endif()
 
@@ -636,8 +636,6 @@ add_subdirectory(doc)
 
 add_subdirectory(src)
 
-include(cmake/tests.cmake)
-
 include(Maintenance)
 setup_split_debug_script()
 add_maintenance_targets()

cmake/tests.cmake

@@ -37,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)
+foreach(script ${functional_tests} fuzz/test_runner.py)
   if(CMAKE_HOST_WIN32)
     set(symlink)
   else()

test/CMakeLists.txt

@@ -3,7 +3,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 # These environment variables are set by the build process and read by
-# test/*/test_runner.py and test/util/rpcauth-test.py
+# test/*/test_runner.py
 
 [environment]
 CLIENT_NAME=@CLIENT_NAME@

test/config.ini.in

@@ -353,6 +353,7 @@
     'rpc_getdescriptorinfo.py',
     'rpc_mempool_info.py',
     'rpc_help.py',
+    'tool_rpcauth.py',
     'p2p_handshake.py',
     'p2p_handshake.py --v2transport',
     'feature_dirsymlinks.py',

test/functional/test_runner.py

@@ -1,37 +1,46 @@
 #!/usr/bin/env python3
-# Copyright (c) 2015-2018 The Bitcoin Core developers
+# Copyright (c) 2015-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test share/rpcauth/rpcauth.py
 """
-import re
-import configparser
 import hmac
 import importlib
 import os
+import re
 import sys
-import unittest
 
-class TestRPCAuth(unittest.TestCase):
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
+
+class RpcAuthTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 0  # No node/datadir needed
+
+    def setup_network(self):
+        pass
+
     def setUp(self):
-        config = configparser.ConfigParser()
-        config_path = os.path.abspath(
-            os.path.join(os.sep, os.path.abspath(os.path.dirname(__file__)),
-            "../config.ini"))
-        with open(config_path, encoding="utf8") as config_file:
-            config.read_file(config_file)
-        sys.path.insert(0, os.path.dirname(config['environment']['RPCAUTH']))
+        sys.path.insert(0, os.path.dirname(self.config["environment"]["RPCAUTH"]))
         self.rpcauth = importlib.import_module('rpcauth')
 
+    def run_test(self):
+        self.setUp()
+
+        self.test_generate_salt()
+        self.test_generate_password()
+        self.test_check_password_hmac()
+
     def test_generate_salt(self):
         for i in range(16, 32 + 1):
-            self.assertEqual(len(self.rpcauth.generate_salt(i)), i * 2)
+            assert_equal(len(self.rpcauth.generate_salt(i)), i * 2)
 
     def test_generate_password(self):
         """Test that generated passwords only consist of urlsafe characters."""
         r = re.compile(r"[0-9a-zA-Z_-]*")
         password = self.rpcauth.generate_password()
-        self.assertTrue(r.fullmatch(password))
+        assert r.fullmatch(password)
 
     def test_check_password_hmac(self):
         salt = self.rpcauth.generate_salt(16)
@@ -41,7 +50,8 @@ def test_check_password_hmac(self):
         m = hmac.new(salt.encode('utf-8'), password.encode('utf-8'), 'SHA256')
         expected_password_hmac = m.hexdigest()
 
-        self.assertEqual(expected_password_hmac, password_hmac)
+        assert_equal(expected_password_hmac, password_hmac)
+
 
 if __name__ == '__main__':
-    unittest.main()
+    RpcAuthTest(__file__).main()