cmake: Add dynamic test discovery to improve parallelism #1760

pull hebasto wants to merge 4 commits into bitcoin-core:master from hebasto:251016-ctest-opt changing 3 files +96 −11

hebasto commented at 2:24 PM on October 16, 2025: member

This PR implements the idea suggested in #1734#pullrequestreview-3284918572 and is based on the work from https://github.com/bitcoin/bitcoin/pull/33483.

Here is an example of the ctest output:

$ ctest --test-dir build -j $(nproc)
Test project /home/hebasto/dev/secp256k1/secp256k1/build
        Start   1: secp256k1.noverify_tests.selftest_tests
        Start   2: secp256k1.noverify_tests.all_proper_context_tests
        Start   3: secp256k1.noverify_tests.all_static_context_tests
        Start   4: secp256k1.noverify_tests.deprecated_context_flags_test
<snip>
193/196 Test  [#31](/bitcoin-core-secp256k1/31/): secp256k1.noverify_tests.ecmult_constants .........................   Passed    5.32 sec
194/196 Test [#184](/bitcoin-core-secp256k1/184/): secp256k1.tests.ellswift_xdh_correctness_tests ....................   Passed    5.62 sec
195/196 Test [#191](/bitcoin-core-secp256k1/191/): secp256k1.exhaustive_tests ........................................   Passed    6.97 sec
196/196 Test [#126](/bitcoin-core-secp256k1/126/): secp256k1.tests.ecmult_constants ..................................   Passed    9.60 sec

100% tests passed, 0 tests failed out of 196

Label Time Summary:
secp256k1_example           =   0.02 sec*proc (5 tests)
secp256k1_exhaustive        =   6.97 sec*proc (1 test)
secp256k1_noverify_tests    =  23.77 sec*proc (95 tests)
secp256k1_tests             =  43.67 sec*proc (95 tests)

Total Test time (real) =  10.21 sec

For comparison, here is the output for the master branch on the same machine:

$ ctest --test-dir build -j $(nproc)
Test project /home/hebasto/dev/secp256k1/secp256k1/build
    Start 1: secp256k1_noverify_tests
    Start 2: secp256k1_tests
    Start 3: secp256k1_exhaustive_tests
    Start 4: secp256k1_ecdsa_example
    Start 5: secp256k1_ecdh_example
    Start 6: secp256k1_schnorr_example
    Start 7: secp256k1_ellswift_example
    Start 8: secp256k1_musig_example
1/8 Test [#4](/bitcoin-core-secp256k1/4/): secp256k1_ecdsa_example ..........   Passed    0.00 sec
2/8 Test [#5](/bitcoin-core-secp256k1/5/): secp256k1_ecdh_example ...........   Passed    0.00 sec
3/8 Test [#6](/bitcoin-core-secp256k1/6/): secp256k1_schnorr_example ........   Passed    0.00 sec
4/8 Test [#7](/bitcoin-core-secp256k1/7/): secp256k1_ellswift_example .......   Passed    0.00 sec
5/8 Test [#8](/bitcoin-core-secp256k1/8/): secp256k1_musig_example ..........   Passed    0.00 sec
6/8 Test [#3](/bitcoin-core-secp256k1/3/): secp256k1_exhaustive_tests .......   Passed    6.26 sec
7/8 Test [#1](/bitcoin-core-secp256k1/1/): secp256k1_noverify_tests .........   Passed   14.31 sec
8/8 Test [#2](/bitcoin-core-secp256k1/2/): secp256k1_tests ..................   Passed   31.65 sec

100% tests passed, 0 tests failed out of 8

Total Test time (real) =  31.65 sec

New Feature: As the number of tests has grown, the labels have been introduced to simplify test management. Now, one can run:

$ ctest --test-dir build -j $(nproc) -L example
Test project /home/hebasto/dev/secp256k1/secp256k1/build
    Start 192: secp256k1.example.ecdsa
    Start 193: secp256k1.example.ecdh
    Start 194: secp256k1.example.schnorr
    Start 195: secp256k1.example.ellswift
    Start 196: secp256k1.example.musig
1/5 Test [#192](/bitcoin-core-secp256k1/192/): secp256k1.example.ecdsa ..........   Passed    0.00 sec
2/5 Test [#193](/bitcoin-core-secp256k1/193/): secp256k1.example.ecdh ...........   Passed    0.00 sec
3/5 Test [#194](/bitcoin-core-secp256k1/194/): secp256k1.example.schnorr ........   Passed    0.00 sec
4/5 Test [#195](/bitcoin-core-secp256k1/195/): secp256k1.example.ellswift .......   Passed    0.00 sec
5/5 Test [#196](/bitcoin-core-secp256k1/196/): secp256k1.example.musig ..........   Passed    0.00 sec

100% tests passed, 0 tests failed out of 5

Label Time Summary:
secp256k1_example    =   0.01 sec*proc (5 tests)

Total Test time (real) =   0.01 sec

$ ctest --test-dir build -j $(nproc) -LE tests
Test project /home/hebasto/dev/secp256k1/secp256k1/build
    Start 192: secp256k1.example.ecdsa
    Start 193: secp256k1.example.ecdh
    Start 194: secp256k1.example.schnorr
    Start 195: secp256k1.example.ellswift
    Start 196: secp256k1.example.musig
    Start 191: secp256k1.exhaustive_tests
1/6 Test [#192](/bitcoin-core-secp256k1/192/): secp256k1.example.ecdsa ..........   Passed    0.00 sec
2/6 Test [#193](/bitcoin-core-secp256k1/193/): secp256k1.example.ecdh ...........   Passed    0.00 sec
3/6 Test [#194](/bitcoin-core-secp256k1/194/): secp256k1.example.schnorr ........   Passed    0.00 sec
4/6 Test [#195](/bitcoin-core-secp256k1/195/): secp256k1.example.ellswift .......   Passed    0.00 sec
5/6 Test [#196](/bitcoin-core-secp256k1/196/): secp256k1.example.musig ..........   Passed    0.00 sec
6/6 Test [#191](/bitcoin-core-secp256k1/191/): secp256k1.exhaustive_tests .......   Passed    6.19 sec

100% tests passed, 0 tests failed out of 6

Label Time Summary:
secp256k1_example       =   0.01 sec*proc (5 tests)
secp256k1_exhaustive    =   6.19 sec*proc (1 test)

Total Test time (real) =   6.20 sec

real-or-random commented at 7:02 AM on October 17, 2025: contributor

The output is nice for sure, but wouldn't it be much simpler to just invoke the test binaries with -j?

My concern is about the list of tests here. It will require maintenance work, and it's easy to forget a test.
real-or-random added the label ci on Oct 17, 2025
real-or-random added the label tweak/refactor on Oct 17, 2025
sipa commented at 1:06 PM on October 17, 2025: contributor

The output is nice for sure, but wouldn't it be much simpler to just invoke the test binaries with -j?

I don't think that's desirable, because it lacks the ability to share the granularity with other test targets.

Specifically, I'm thinking in the context of the Bitcoin Core build, which has tons of test cases of its own. The ideal scenario is that you have a single process pool (controlled by the -j argument to cmake), and it is used to run both the Bitcoin Core and libsecp256k1 (and other) tests. This will generally result in all processes making progress throughout the entire run, until cmake runs out of test tasks to schedule.

In contrast, if you push the task scheduling down to libsecp256k1's own test binary, you at best end up with a situation where you first run all Bitcoin Core tests, wait for those to complete (temporarily running out of tasks in the process), and then run libsecp256k1's tests with -j. I don't know if that's actually possible to do in cmake's supported generators, and more likely you'll end up in a situation where the entire libsecp256k1 test run (-j and all) is seen as a single process from the perspective of the build system - meaning it'd simultaneously run N-1 Bitcoin Core tests, plus 1 libsecp256k1 test (which itself runs in N processes), temporarily raising the parallellism to 2N-1.

in src/CMakeLists.txt:163 in 225103b4e1 outdated

 166 |    if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage")
 167 | -    add_executable(tests tests.c)
 168 | -    target_compile_definitions(tests PRIVATE VERIFY ${TEST_DEFINITIONS})
 169 | -    target_link_libraries(tests secp256k1_precomputed secp256k1_asm)
 170 | -    add_test(NAME secp256k1_tests COMMAND tests)
 171 | +    add_executable_and_tests(YES)

furszy commented at 2:04 PM on October 17, 2025:

Could write this shorter as:

function(add_executable_and_tests bin_name verify_definition)
  add_executable(${bin_name} tests.c)
  target_link_libraries(${bin_name} secp256k1_precomputed secp256k1_asm)
  target_compile_definitions(${bin_name} PRIVATE ${verify_definition} ${TEST_DEFINITIONS})
  add_test(NAME secp256k1_${bin_name} COMMAND ${bin_name})
endfunction()

add_executable_and_tests(noverify_tests "")
if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage")
  add_executable_and_tests(tests VERIFY)
endif()

Also, probably the target_link_libraries should be PRIVATE too.

See 00f20f694704079f1f9c05851e7407883763268e (I applied it on top of your first commit - could directly squash it there)

hebasto commented at 7:43 AM on October 21, 2025:

Also, probably the target_link_libraries should be PRIVATE too.

All target_link_libraries commands should use the same signature. I think this change should be made in a separate PR.

hebasto commented at 7:58 AM on October 21, 2025:

Could write this shorter as: ...

Thanks! Reworked.

furszy commented at 3:28 PM on October 17, 2025: member

Regarding the maintenance topic, wouldn't be better to call --list_tests to fetch the test names instead of manually adding them one by one? We could make --list_tests=2 output the tests names separated by commas to make your life easier at the parsing side.

I experimented a bit with this idea and was able to dynamically generate a file containing all the tests names. See https://github.com/furszy/secp256k1/tree/2025_ci_concurrent_tests (just need to run the tests_discover target). The next step would be to connect this to run separate ctest targets in some way, which doesn't seem to be so simple because the binary does not exist during configure time but ctest targets are added there.. (chicken-egg problem). Maybe we could execute a custom script that reads the file and launches per target ctest runs dynamically too.
hebasto commented at 7:36 AM on October 21, 2025: member

The output is nice for sure, but wouldn't it be much simpler to just invoke the test binaries with -j?

I don't think that's desirable, because it lacks the ability to share the granularity with other test targets.

Specifically, I'm thinking in the context of the Bitcoin Core build, which has tons of test cases of its own. The ideal scenario is that you have a single process pool (controlled by the -j argument to cmake), and it is used to run both the Bitcoin Core and libsecp256k1 (and other) tests. This will generally result in all processes making progress throughout the entire run, until cmake runs out of test tasks to schedule.

I share the same perspective.
hebasto commented at 7:39 AM on October 21, 2025: member

Regarding the maintenance topic, wouldn't be better to call --list_tests to fetch the test names instead of manually adding them one by one? We could make --list_tests=2 output the tests names separated by commas to make your life easier at the parsing side.

I experimented a bit with this idea and was able to dynamically generate a file containing all the tests names. See https://github.com/furszy/secp256k1/tree/2025_ci_concurrent_tests (just need to run the tests_discover target). The next step would be to connect this to run separate ctest targets in some way, which doesn't seem to be so simple because the binary does not exist during configure time but ctest targets are added there.. (chicken-egg problem). Maybe we could execute a custom script that reads the file and launches per target ctest runs dynamically too. @purpleKarrot's approach could be applied as a follow-up.
hebasto force-pushed on Oct 21, 2025
hebasto commented at 7:58 AM on October 21, 2025: member

Feedback from @furszy has been addressed.
in src/CMakeLists.txt:157 in 7600f5f5fb outdated
```
 153 | +  list(APPEND test_targets ecmult_constants)
 154 | +  list(APPEND test_names   ecmult_constants)
 155 | +  list(APPEND test_targets "ecmult_pre_g wnaf point_times_order ecmult_near_split_bound ecmult_chain ecmult_gen_blind ecmult_const_tests ecmult_multi_tests ec_combine")
 156 | +  list(APPEND test_names   ecmult)
 157 | +  # Continue with the remaining tests.
 158 | +  list(APPEND test_targets integer scalar field group ec ecdh ecdsa recovery extrakeys schnorrsig musig)
```
real-or-random commented at 9:59 AM on October 22, 2025:

Is there a reason why you have quotation marks in the first line here but not in the last?

hebasto commented at 8:27 AM on October 23, 2025:

The first line has quotes to combine the quoted test cases into a single ctest's test "ecmult".

In the last line, all mentioned test cases and modules are wrapped as its own ctest's tests.

purpleKarrot commented at 1:58 PM on October 30, 2025:

I take @real-or-random's question as a proof that procedural logic in CMakeLists.txt is hard to reason about and should be avoided.

purpleKarrot commented at 2:21 PM on October 30, 2025:

If we switch to automated test discovery in a follow-up, we will no longer have the ability to combine test cases. Maybe we should keep them separated so that the follow-up will not change the number of tests again?

real-or-random commented at 10:11 AM on November 7, 2025:

Maybe we should keep them separated so that the follow-up will not change the number of tests again?

Sounds good, let's keep it simple.

For test ordering, we could perhaps simply make sure that automated discovery runs the tests in the order they are listed by tests -l.

furszy commented at 3:54 PM on November 17, 2025:

For test ordering, we could perhaps simply make sure that automated discovery runs the tests in the order they are listed by tests -l.

We could reorder them internally based on their execution time, so the slow ones run first. It is just matter of moving a few lines in the registry anyway, very simple.

purpleKarrot commented at 8:43 AM on November 18, 2025:

We could reorder them internally based on their execution time, so the slow ones run first. It is just matter of moving a few lines in the registry anyway, very simple.

The order in which they are registered to CTest is not the order they will be executed. If you want slow tests to run first, you need to set the COST property.

real-or-random commented at 9:02 AM on November 18, 2025:

Oh, I see, also fine! I suggest we just run the test suite once on some dev's machine and hard-code the resulting numbers.

furszy commented at 10:23 PM on December 14, 2025:

Created #1788 so we don’t have to worry about the test costs for now. The longest running test is just an all-in-one test that we can split very easily (the gains of it are very noticeable).

furszy commented at 5:36 PM on December 28, 2025:

Can we simplify anything now that #1788 was merged?
cmake, refactor: Deduplicate test-related code
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
4ac651144b

cmake: Add DiscoverTests module

Co-authored-by: Daniel Pfeifer <daniel@pfeifer-mail.de>

f95b263f23

in src/CMakeLists.txt:183 in 2d8da55816

 186 |    if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage")
 187 | -    add_executable(tests tests.c)
 188 | -    target_compile_definitions(tests PRIVATE VERIFY ${TEST_DEFINITIONS})
 189 | -    target_link_libraries(tests secp256k1_precomputed secp256k1_asm)
 190 | -    add_test(NAME secp256k1_tests COMMAND tests)
 191 | +    add_executable_and_tests(tests VERIFY secp256k1_verify)

purpleKarrot commented at 2:19 PM on October 30, 2025:

The target name tests is very generic and likely to cause clashes when the project is used as a subproject. This is orthogonal to the PR, but should be kept in mind for a follow up.

hebasto force-pushed on Jan 13, 2026
hebasto renamed this:
~~cmake: Split test cases to improve parallelism~~
cmake: Add dynamic test discovery to improve parallelism
on Jan 13, 2026
hebasto commented at 4:47 PM on January 13, 2026: member

Regarding the maintenance topic, wouldn't be better to call --list_tests to fetch the test names instead of manually adding them one by one? We could make --list_tests=2 output the tests names separated by commas to make your life easier at the parsing side.

I experimented a bit with this idea and was able to dynamically generate a file containing all the tests names. See https://github.com/furszy/secp256k1/tree/2025_ci_concurrent_tests (just need to run the tests_discover target). The next step would be to connect this to run separate ctest targets in some way, which doesn't seem to be so simple because the binary does not exist during configure time but ctest targets are added there.. (chicken-egg problem). Maybe we could execute a custom script that reads the file and launches per target ctest runs dynamically too.

Reworked. Using the dynamic test discovery now.
hebasto marked this as a draft on Jan 13, 2026
hebasto force-pushed on Jan 13, 2026
hebasto marked this as ready for review on Jan 13, 2026
furszy commented at 5:29 PM on January 13, 2026: member

nice! will review soon.
in examples/CMakeLists.txt:12 in 0c8178e86c outdated
```
   8 | @@ -9,7 +9,7 @@ function(add_example name)
   9 |      $<$<PLATFORM_ID:Windows>:bcrypt>
  10 |    )
  11 |    set(test_name ${name}_example)
  12 | -  add_test(NAME secp256k1_${test_name} COMMAND ${target_name})
  13 | +  add_test(NAME secp256k1.example.${name} COMMAND ${target_name})
```
furszy commented at 8:09 PM on January 19, 2026:

In 0c8178e86cfddc193429ae398c6d5ff6537ef73c: with this change, test_name is no longer used.

Update: I see you are removing it in the next commit: 54ffc2137004ea3085d3c5068c1562ca46a6faeb.

hebasto commented at 4:55 PM on January 20, 2026:

Update: I see you are removing it in the next commit: 54ffc21.

It has been moved to previous commit.
in src/CMakeLists.txt:158 in 54ffc21370 outdated
```
 153 | @@ -154,6 +154,8 @@ if(SECP256K1_BUILD_TESTS)
 154 |        DISCOVERY_MATCH       "^\\t\\\\[ *[0-9]+\\\\] ([^ ].*)$"
 155 |        TEST_NAME_REPLACEMENT "secp256k1.${exe_name}.\\\\1"
 156 |        TEST_ARGS_REPLACEMENT "--target=\\\\1 --log=1"
 157 | +      PROPERTIES
 158 | +        LABELS "secp256k1_${exe_name}"
```
furszy commented at 8:12 PM on January 19, 2026:

it would be nice to explain what LABELS are.

hebasto commented at 5:02 PM on January 20, 2026:

From Professional CMake: A Practical Guide 21st Edition:

Selecting tests individually by name or number can become cumbersome if a large set of related tests needs to be executed. Tests can be assigned an arbitrary list of labels using the LABELS test property, and then tests can be selected by these labels. The -L and -LE options are analogous to the -R and -E options respectively, except they operate on test labels rather than test names. ... Labels not only enable convenient grouping for test execution, they also provide grouping for basic execution time statistics. As seen in the example output above, the ctest command prints a label summary when any test in the set of executed tests has its LABELS property set. This allows the developer to get an idea how each label group is contributing to the overall test time.

in cmake/DiscoverTests.cmake:20 in 54ffc21370 outdated

  15 | +    set(properties_content "      set_tests_properties(\"\${test_name}\" PROPERTIES\n")
  16 | +    math(EXPR num_properties "${properties_len} / 2")
  17 | +    foreach(i RANGE 0 ${num_properties} 2)
  18 | +      math(EXPR value_index "${i} + 1")
  19 | +      list(GET arg_PROPERTIES ${i} name)
  20 | +      list(GET arg_PROPERTIES ${value_index} value)

furszy commented at 8:34 PM on January 19, 2026:

This assumes properties come in pairs, but someone could:

discover_tests(${exe_name}
      DISCOVERY_ARGS        "--list_tests"
      DISCOVERY_MATCH       "^\\t\\\\[ *[0-9]+\\\\] ([^ ].*)$"
      TEST_NAME_REPLACEMENT "secp256k1.${exe_name}.\\\\1"
      TEST_ARGS_REPLACEMENT "--target=\\\\1 --log=1"
      PROPERTIES
        LABELS "secp256k1_${exe_name}"
        NO_TIMEOUT   // <-- property with no value
)

It would be good to check that the value exist before accessing it, and fail with a clear error if not: "Property 'name' has no associated value. Must be specified as name/value pair" or something similar.

hebasto commented at 5:16 PM on January 20, 2026:

Every set property has a value. The syntax in the snippet above is simply incorrect. Compare it with the syntax of the set_tests_properties command.

furszy commented at 8:37 PM on January 19, 2026: member
Aside from the minimal comments just left, this works nicely.

For reviewers, just run:
```
ctest -j <threads_num>
```

cmake: Integrate DiscoverTests and normalize test names

Updates the build system to use the new DiscoverTests module.
This also standardizes test names to use dot-separated parts for
consistency.

29f26ec3cf

cmake: Set `LABELS` property for tests 8354618e02
hebasto force-pushed on Jan 20, 2026
hebasto commented at 5:18 PM on January 20, 2026: member

@furszy

Thank you for the review! Your feedback has been addressed.
furszy commented at 6:39 PM on January 20, 2026: member

Tested ACK 8354618
real-or-random requested review from purpleKarrot on Jan 21, 2026
purpleKarrot commented at 10:46 AM on February 19, 2026: contributor

ACK 8354618e02bcccf3893bcf0ec99639b6e520509f

Reviewed and tested.
real-or-random merged this on Feb 19, 2026
real-or-random closed this on Feb 19, 2026
fanquake referenced this in commit fa3704594c on Feb 19, 2026
hebasto deleted the branch on Feb 19, 2026

Contributors

Review Requested
purpleKarrot

Labels

ci tweak/refactor

Linked (view graph)

#1 Slice bytes of G multiples to avoid cache timings.#2 Move implementations from impl/*.h to *_impl.h #3 Write a big fat disclaimer README.md #4 Add a high-level API #5 More details in README.md #6 Tests don't work with NDEBUG.#7 Fix build on x32 #8 Check signature nonces for validity #31 Introduce CHECK() for tests that works with NDEBUG.#126 Add two ECDSA edge case tests #184 Documentation should provide example callers.#191 Reduce the amount of runtime-initialized constants #192 C89 nits and dead code removal.#193 Convert field code to strict C89 (+ long long, +__int128)#194 Shouldn't this be #ifdef?#195 Covert several more files to C89.#196 Get rid of variable-length hex string conversions #1734 Introduce (mini) unit test framework