tests: speed up coins_tests by parallelizing

ajtowns commented at 8:41 am on July 11, 2025: contributor

Updates the cmake logic to generate a separate test for each BOOST_FIXTURE_TEST_SUITE declaration in a file, and splits coins_tests.cpp into three separate suites so that they can be run in parallel. Also updates the convention enforced by test/lint/lint-tests.py.

DrahtBot added the label Tests on Jul 11, 2025

DrahtBot commented at 8:41 am on July 11, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32945.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, maflcko, achow101
Concept ACK	willcl-ark

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

ajtowns commented at 8:44 am on July 11, 2025: contributor

For me, coins_tests it’s a pretty significant bottleneck to running tests. With this patch and -j20 I go from:

0137/139 Test  [#26](/bitcoin-bitcoin/26/): checkqueue_tests .....................   Passed   50.37 sec
1138/139 Test [#126](/bitcoin-bitcoin/126/): coinselector_tests ...................   Passed   52.53 sec
2139/139 Test  [#28](/bitcoin-bitcoin/28/): coins_tests ..........................   Passed  137.43 sec
3
4100% tests passed, 0 tests failed out of 139
5
6Total Test time (real) = 137.43 sec

to

0 69/141 Test  [#30](/bitcoin-bitcoin/30/): coins_tests ..........................   Passed   24.46 sec
1...
2138/141 Test [#128](/bitcoin-bitcoin/128/): coinselector_tests ...................   Passed   50.16 sec
3139/141 Test  [#28](/bitcoin-bitcoin/28/): coins_base_tests .....................   Passed   53.28 sec
4140/141 Test  [#26](/bitcoin-bitcoin/26/): checkqueue_tests .....................   Passed   54.31 sec
5141/141 Test  [#29](/bitcoin-bitcoin/29/): coins_dbbase_tests ...................   Passed   63.89 sec
6
7100% tests passed, 0 tests failed out of 141
8
9Total Test time (real) =  63.89 sec

ie, less than half the time. (The bench_sanity test is also pretty slow though, if benchmarking is enabled)

ajtowns force-pushed on Jul 11, 2025

DrahtBot added the label CI failed on Jul 11, 2025

DrahtBot commented at 8:55 am on July 11, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45784666394 LLM reason (✨ experimental): The CI failure is caused by errors in the linting step.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

ajtowns force-pushed on Jul 11, 2025

TheCharlatan commented at 9:04 pm on July 11, 2025: contributor

Are you running this on an older machine, or are missing some of the crypto accelerator features? Your reported runtimes are about an order of magnitude longer than mine and even our CI machines report the tests running much faster.

ajtowns commented at 9:31 pm on July 11, 2025: contributor

Are you running this on an older machine, or are missing some of the crypto accelerator features? Your reported runtimes are about an order of magnitude longer than mine and even our CI machines report the tests running much faster.

Hmm, it’s a bit old I guess; 2019-era high-end desktop cpu, and running in a vm under qemu. It’s also a debug build, with the extra checks that implies.

CI failures seem to indicate my cmake code doesn’t work with something other than ninja as the test conductor?

l0rinc commented at 1:26 pm on July 12, 2025: contributor

Given that the ands and alsos in the description, I personally would find it useful to split the PR into smaller commits accordingly. I’d ACK the coins_tests.cpp changes (had problems because of that simulation test being part of the suite numerous times), no opinion about the rest.

ajtowns commented at 4:44 am on July 17, 2025: contributor

Given that the ands and alsos in the description, I personally would find it useful to split the PR into smaller commits accordingly. I’d ACK the coins_tests.cpp changes (had problems because of that simulation test being part of the suite numerous times), no opinion about the rest.

Without the lint changes, having multiple test fixtures in a single cpp file causes CI failures. Without the cmake changes, the additional test fixtures aren’t executed via ctest. So I don’t think it makes sense to do the coins_test changes without the other changes.

willcl-ark commented at 12:24 pm on July 18, 2025: member

On my machine I see this go from master:

0135/139 Test  [#28](/bitcoin-bitcoin/28/): coins_tests ..........................   Passed    7.61 sec
1136/139 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_exhaustive_tests ...........   Passed    8.13 sec
2137/139 Test [#126](/bitcoin-bitcoin/126/): coinselector_tests ...................   Passed   11.85 sec
3138/139 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed   14.59 sec
4139/139 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed   29.11 sec

This branch (with a small patch):

 0diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt
 1index 93db34d5a21..41bac7286b9 100644
 2--- a/src/test/CMakeLists.txt
 3+++ b/src/test/CMakeLists.txt
 4@@ -183,7 +183,7 @@ function(add_boost_test source_file)
 5 
 6   file(READ "${source_file}" source_file_content)
 7   string(REGEX
 8-    MATCHALL "(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\\(([A-Za-z0-9_]+)"
 9+    MATCHALL "(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\\(([A-Za-z0-9_-]+)"
10     test_suite_macro "${source_file_content}"
11   )
12   list(TRANSFORM test_suite_macro
13diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
14index a3dfad3168d..6ce0c7996f8 100644
15--- a/src/test/coins_tests.cpp
16+++ b/src/test/coins_tests.cpp
17@@ -281,7 +281,7 @@ void SimulationTest(CCoinsView* base, bool fake_best_block)
18 }
19 }; // struct CacheTest
20 
21-BOOST_FIXTURE_TEST_SUITE(coins_tests-base, BasicTestingSetup)
22+BOOST_FIXTURE_TEST_SUITE(coins_tests_base, BasicTestingSetup)
23 
24 // Run the above simulation for multiple base types.
25 BOOST_FIXTURE_TEST_CASE(coins_cache_base_simulation_test, CacheTest)
26@@ -292,7 +292,7 @@ BOOST_FIXTURE_TEST_CASE(coins_cache_base_simulation_test, CacheTest)
27 
28 BOOST_AUTO_TEST_SUITE_END()
29 
30-BOOST_FIXTURE_TEST_SUITE(coins_tests-dbbase, BasicTestingSetup)
31+BOOST_FIXTURE_TEST_SUITE(coins_tests_dbbase, BasicTestingSetup)
32 
33 BOOST_FIXTURE_TEST_CASE(coins_cache_dbbase_simulation_test, CacheTest)
34 {

0 62/141 Test  [#30](/bitcoin-bitcoin/30/): coins_tests ..........................   Passed    1.49 sec
1 89/141 Test  [#28](/bitcoin-bitcoin/28/): coins_tests_base .....................   Passed    2.31 sec
2135/141 Test  [#29](/bitcoin-bitcoin/29/): coins_tests_dbbase ...................   Passed    5.70 sec
3138/141 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_exhaustive_tests ...........   Passed    8.03 sec
4139/141 Test [#128](/bitcoin-bitcoin/128/): coinselector_tests ...................   Passed    9.90 sec
5140/141 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed   14.14 sec
6141/141 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed   29.01 sec

On the new Cirrus Runners we are currently testing, I see the MSAN job taking:

0136/140 Test [#127](/bitcoin-bitcoin/127/): coinselector_tests ...................   Passed   96.82 sec
1137/140 Test  [#29](/bitcoin-bitcoin/29/): coins_tests ..........................   Passed  145.81 sec
2138/140 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed  172.30 sec
3139/140 Test   [#6](/bitcoin-bitcoin/6/): bench_sanity_check ...................   Passed  253.65 sec
4140/140 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed  522.70 sec
5
6Total Test time (real) = 522.71 sec

And the MSAN job with this patch:

0128/142 Test  [#31](/bitcoin-bitcoin/31/): coins_tests ..........................   Passed   21.71 sec
1131/142 Test  [#29](/bitcoin-bitcoin/29/): coins_tests_base .....................   Passed   25.68 sec
2134/142 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_exhaustive_tests ...........   Passed   38.39 sec
3138/142 Test  [#30](/bitcoin-bitcoin/30/): coins_tests_dbbase ...................   Passed   51.80 sec
4139/142 Test [#129](/bitcoin-bitcoin/129/): coinselector_tests ...................   Passed   58.30 sec
5140/142 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed  136.68 sec
6141/142 Test   [#6](/bitcoin-bitcoin/6/): bench_sanity_check ...................   Passed  170.47 sec
7142/142 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed  218.99 sec
8
9Total Test time (real) = 219.00 sec

and on the ASAN job:

0 137/141 Test [#128](/bitcoin-bitcoin/128/): coinselector_tests ...................   Passed  124.68 sec
1138/141 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed  159.47 sec
2139/141 Test   [#7](/bitcoin-bitcoin/7/): bench_sanity_check ...................   Passed  185.79 sec
3140/141 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed  271.56 sec
4141/141 Test  [#30](/bitcoin-bitcoin/30/): coins_tests ..........................   Passed  341.78 sec
5
6Total Test time (real) = 341.84 sec

With this patch the same ASAN job:

 0133/143 Test  [#32](/bitcoin-bitcoin/32/): coins_tests ..........................   Passed   59.72 sec
 1134/143 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_exhaustive_tests ...........   Passed   61.56 sec
 2135/143 Test [#139](/bitcoin-bitcoin/139/): wallet_crypto_tests ..................   Passed   53.21 sec
 3138/143 Test [#130](/bitcoin-bitcoin/130/): coinselector_tests ...................   Passed  144.01 sec
 4139/143 Test   [#3](/bitcoin-bitcoin/3/): secp256k1_noverify_tests .............   Passed  184.61 sec
 5140/143 Test  [#30](/bitcoin-bitcoin/30/): coins_tests_base .....................   Passed  202.01 sec
 6141/143 Test  [#31](/bitcoin-bitcoin/31/): coins_tests_dbbase ...................   Passed  211.12 sec
 7142/143 Test   [#7](/bitcoin-bitcoin/7/): bench_sanity_check ...................   Passed  215.51 sec
 8143/143 Test   [#4](/bitcoin-bitcoin/4/): secp256k1_tests ......................   Passed  314.80 sec
 9
10Total Test time (real) = 314.81 sec

So concept ACK from me!

ajtowns force-pushed on Jul 19, 2025

DrahtBot removed the label CI failed on Jul 19, 2025

ajtowns commented at 4:56 am on July 20, 2025: contributor

This branch (with a small patch):

Nice, that seems to have fixed my cmake errors.

maflcko commented at 7:43 am on July 21, 2025: member

lgtm ACK 2a8fdddd4df4b630c0a580f4df6521cb3af01804

DrahtBot requested review from l0rinc on Jul 21, 2025

DrahtBot requested review from willcl-ark on Jul 21, 2025

in test/lint/lint-tests.py:39 in 2a8fdddd4d outdated

37         not_matching = "\n".join(not_matching)
38         error_msg = (
39             "The test suite in file src/test/foo_tests.cpp should be named\n"
40-            '"foo_tests". Please make sure the following test suites follow\n'
41-            "that convention:\n\n"
42+            '"foo_tests", or if there are multiple test suites, `foo_tests_bar`.\n'

maflcko commented at 7:44 am on July 21, 2025:

0            '`foo_tests`, or if there are multiple test suites, `foo_tests_bar`.\n'

style nit: seems odd to use multiple format styles in the same line.

ajtowns commented at 4:22 am on July 22, 2025:

Done

l0rinc commented at 7:47 pm on July 21, 2025: contributor

ACK 2a8fdddd4df4b630c0a580f4df6521cb3af01804

Ran the tests a few times in debug mode. Before:

139/140 Test #29: coins_tests …………………….. Passed 71.43 sec

and

138/140 Test #29: coins_tests …………………….. Passed 60.65 sec

After:

66/142 Test #31: coins_tests …………………….. Passed 10.86 sec 128/142 Test #29: coins_tests_base ………………… Passed 23.53 sec 138/142 Test #30: coins_tests_dbbase ………………. Passed 42.49 sec

and

66/142 Test #31: coins_tests …………………….. Passed 10.12 sec 115/142 Test #29: coins_tests_base ………………… Passed 19.21 sec 137/142 Test #30: coins_tests_dbbase ………………. Passed 34.29 sec

tests: speed up coins_tests by parallelizing

Updates the cmake logic to generate a separate test for each
BOOST_FIXTURE_TEST_SUITE declaration in a file, and splits coins_tests.cpp
into three separate suites so that they can be run in parallel. Also
updates the convention enforced by test/lint/lint-tests.py.

06ab3a394a

ajtowns force-pushed on Jul 22, 2025

l0rinc commented at 2:59 am on July 22, 2025: contributor

reACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

Only difference since last review was applying #32945 (review)

DrahtBot requested review from maflcko on Jul 22, 2025

maflcko commented at 8:11 am on July 22, 2025: member

lgtm ACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

achow101 commented at 6:13 pm on July 22, 2025: member

ACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

The test isn’t that slow for me, but this is still a significant improvement.

achow101 merged this on Jul 22, 2025

achow101 closed this on Jul 22, 2025

willcl-ark commented at 8:37 am on July 25, 2025: member

Post-merge ACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

tests: speed up coins_tests by parallelizing #32945

Code Coverage & Benchmarks

Reviews