Merge bitcoin/bitcoin#29771: test: Run framework unit tests in parallel

f19f0a2 test: Run framework unit tests in parallel (tdb3)

Pull request description:

  Functional test framework unit tests are currently run prior to all other functional tests.

  This PR enables execution of the test framework unit tests in parallel with the functional tests, rather than before the functional tests, saving runtime and more efficiently using available cores.

  This is a follow up to  bitcoin/bitcoin#29470 (comment)

  ### New behavior:
  1) When running all tests, the framework unit tests are run in parallel with the other tests (unless explicitly skipped with `--exclude`).  This parallelization introduces marginal time savings when running all tests, depending on the machine used.  As an example, a 2-3% time savings (9 seconds) was observed on a machine using `--jobs=18` (with 18 available cores).
  2) When running specific functional tests, framework unit tests are now skipped by default.  Framework unit tests can be added by including `feature_framework_unit_tests.py` in the list of specific tests being executed.  The rationale for skipping by default is that if the tester is running specific functional tests, there is a conscious decision to focus testing, and choosing to run all tests (where unit tests are run by default) would be a next step.
  3) The `--skipunit` option is now removed since unit tests are parallelized (they no longer delay other tests).  Unit tests are treated equally as functional tests.

  ### Implementation notes:
  Since `TextTestRunner` can be noisy (even with verbosity=0, and therefore trigger job failure through the presence of non-failure stderr output), the approach taken was to send output to stdout, and forward test result (as determined by `TestResult` returned).  This aligns with the previous check for unit test failure (`if not result.wasSuccessful():`).

  This approach was tested by inserting `self.assertEquals(True, False)` into test_framework/address.py and seeing specifics of the failure reported.

  ```
  135/302 - feature_framework_unit_tests.py failed, Duration: 0 s

  stdout:
  .F
  ======================================================================
  FAIL: test_bech32_decode (test_framework.address.TestFrameworkScript.test_bech32_decode)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/dev/myrepos/bitcoin/test/functional/test_framework/address.py", line 228, in test_bech32_decode
      self.assertEqual(True, False)
  AssertionError: True != False

  ----------------------------------------------------------------------
  Ran 2 tests in 0.003s

  FAILED (failures=1)

  stderr:
  ```

  There was an initial thought to parallelize the execution of the unit tests themselves (i.e. run the 12 unit test files in parallel), however, this is not anticipated to further reduce runtime meaningfully and is anticipated to add unnecessary complexity.

ACKs for top commit:
  maflcko:
    ACK f19f0a2 🌽
  achow101:
    ACK f19f0a2
  kevkevinpal:
    Approach ACK f19f0a2

Tree-SHA512: ab9f82c30371b2242bc7a263ea0e25d35e68e2ddf223d2a55498ad940d1e5b73bba76cce8b264d71e2ed31b753430d8ef8d57efc1e4fd9ced7fb845e27f4f47e

Author: achow101

test/functional/feature_framework_unit_tests.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017-2024 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Framework unit tests
+
+Unit tests for the test framework.
+"""
+
+import sys
+import unittest
+
+from test_framework.test_framework import TEST_EXIT_PASSED, TEST_EXIT_FAILED
+
+# List of framework modules containing unit tests. Should be kept in sync with
+# the output of `git grep unittest.TestCase ./test/functional/test_framework`
+TEST_FRAMEWORK_MODULES = [
+    "address",
+    "crypto.bip324_cipher",
+    "blocktools",
+    "crypto.chacha20",
+    "crypto.ellswift",
+    "key",
+    "messages",
+    "crypto.muhash",
+    "crypto.poly1305",
+    "crypto.ripemd160",
+    "script",
+    "segwit_addr",
+    "wallet_util",
+]
+
+
+def run_unit_tests():
+    test_framework_tests = unittest.TestSuite()
+    for module in TEST_FRAMEWORK_MODULES:
+        test_framework_tests.addTest(
+            unittest.TestLoader().loadTestsFromName(f"test_framework.{module}")
+        )
+    result = unittest.TextTestRunner(stream=sys.stdout, verbosity=1, failfast=True).run(
+        test_framework_tests
+    )
+    if not result.wasSuccessful():
+        sys.exit(TEST_EXIT_FAILED)
+    sys.exit(TEST_EXIT_PASSED)
+
+
+if __name__ == "__main__":
+    run_unit_tests()
+

test/functional/test_runner.py

@@ -26,7 +26,6 @@
 import tempfile
 import re
 import logging
-import unittest
 
 os.environ["REQUIRE_WALLET_TYPE_SET"] = "1"
 
@@ -70,23 +69,7 @@
 TEST_EXIT_PASSED = 0
 TEST_EXIT_SKIPPED = 77
 
-# List of framework modules containing unit tests. Should be kept in sync with
-# the output of `git grep unittest.TestCase ./test/functional/test_framework`
-TEST_FRAMEWORK_MODULES = [
-    "address",
-    "crypto.bip324_cipher",
-    "blocktools",
-    "crypto.chacha20",
-    "crypto.ellswift",
-    "key",
-    "messages",
-    "crypto.muhash",
-    "crypto.poly1305",
-    "crypto.ripemd160",
-    "script",
-    "segwit_addr",
-    "wallet_util",
-]
+TEST_FRAMEWORK_UNIT_TESTS = 'feature_framework_unit_tests.py'
 
 EXTENDED_SCRIPTS = [
     # These tests are not run by default.
@@ -255,6 +238,7 @@
     'wallet_keypool.py --descriptors',
     'wallet_descriptor.py --descriptors',
     'p2p_nobloomfilter_messages.py',
+    TEST_FRAMEWORK_UNIT_TESTS,
     'p2p_filter.py',
     'rpc_setban.py --v1transport',
     'rpc_setban.py --v2transport',
@@ -440,7 +424,6 @@ def main():
     parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs")
     parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure')
     parser.add_argument('--filter', help='filter scripts to run by regular expression')
-    parser.add_argument('--skipunit', '-u', action='store_true', help='skip unit tests for the test framework')
 
 
     args, unknown_args = parser.parse_known_args()
@@ -552,10 +535,9 @@ def main():
         combined_logs_len=args.combinedlogslen,
         failfast=args.failfast,
         use_term_control=args.ansi,
-        skipunit=args.skipunit,
     )
 
-def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control, skipunit=False):
+def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control):
     args = args or []
 
     # Warn if bitcoind is already running
@@ -578,15 +560,6 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=
     # a hard link or a copy on any platform. See https://github.com/bitcoin/bitcoin/pull/27561.
     sys.path.append(tests_dir)
 
-    if not skipunit:
-        print("Running Unit Tests for Test Framework Modules")
-        test_framework_tests = unittest.TestSuite()
-        for module in TEST_FRAMEWORK_MODULES:
-            test_framework_tests.addTest(unittest.TestLoader().loadTestsFromName("test_framework.{}".format(module)))
-        result = unittest.TextTestRunner(verbosity=1, failfast=True).run(test_framework_tests)
-        if not result.wasSuccessful():
-            sys.exit("Early exiting after failure in TestFramework unit tests")
-
     flags = ['--cachedir={}'.format(cache_dir)] + args
 
     if enable_coverage: