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
  1. 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:

     0$ ctest --test-dir build -j $(nproc)
 1Test project /home/hebasto/dev/secp256k1/secp256k1/build
 2        Start   1: secp256k1.noverify_tests.selftest_tests
 3        Start   2: secp256k1.noverify_tests.all_proper_context_tests
 4        Start   3: secp256k1.noverify_tests.all_static_context_tests
 5        Start   4: secp256k1.noverify_tests.deprecated_context_flags_test
 6<snip>
 7193/196 Test  [#31](/bitcoin-core-secp256k1/31/): secp256k1.noverify_tests.ecmult_constants .........................   Passed    5.32 sec
 8194/196 Test [#184](/bitcoin-core-secp256k1/184/): secp256k1.tests.ellswift_xdh_correctness_tests ....................   Passed    5.62 sec
 9195/196 Test [#191](/bitcoin-core-secp256k1/191/): secp256k1.exhaustive_tests ........................................   Passed    6.97 sec
10196/196 Test [#126](/bitcoin-core-secp256k1/126/): secp256k1.tests.ecmult_constants ..................................   Passed    9.60 sec
11
12100% tests passed, 0 tests failed out of 196
13
14Label Time Summary:
15secp256k1_example           =   0.02 sec*proc (5 tests)
16secp256k1_exhaustive        =   6.97 sec*proc (1 test)
17secp256k1_noverify_tests    =  23.77 sec*proc (95 tests)
18secp256k1_tests             =  43.67 sec*proc (95 tests)
19
20Total Test time (real) =  10.21 sec

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

     0$ ctest --test-dir build -j $(nproc)
 1Test project /home/hebasto/dev/secp256k1/secp256k1/build
 2    Start 1: secp256k1_noverify_tests
 3    Start 2: secp256k1_tests
 4    Start 3: secp256k1_exhaustive_tests
 5    Start 4: secp256k1_ecdsa_example
 6    Start 5: secp256k1_ecdh_example
 7    Start 6: secp256k1_schnorr_example
 8    Start 7: secp256k1_ellswift_example
 9    Start 8: secp256k1_musig_example
101/8 Test [#4](/bitcoin-core-secp256k1/4/): secp256k1_ecdsa_example ..........   Passed    0.00 sec
112/8 Test [#5](/bitcoin-core-secp256k1/5/): secp256k1_ecdh_example ...........   Passed    0.00 sec
123/8 Test [#6](/bitcoin-core-secp256k1/6/): secp256k1_schnorr_example ........   Passed    0.00 sec
134/8 Test [#7](/bitcoin-core-secp256k1/7/): secp256k1_ellswift_example .......   Passed    0.00 sec
145/8 Test [#8](/bitcoin-core-secp256k1/8/): secp256k1_musig_example ..........   Passed    0.00 sec
156/8 Test [#3](/bitcoin-core-secp256k1/3/): secp256k1_exhaustive_tests .......   Passed    6.26 sec
167/8 Test [#1](/bitcoin-core-secp256k1/1/): secp256k1_noverify_tests .........   Passed   14.31 sec
178/8 Test [#2](/bitcoin-core-secp256k1/2/): secp256k1_tests ..................   Passed   31.65 sec
18
19100% tests passed, 0 tests failed out of 8
20
21Total 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:

     0$ ctest --test-dir build -j $(nproc) -L example
 1Test project /home/hebasto/dev/secp256k1/secp256k1/build
 2    Start 192: secp256k1.example.ecdsa
 3    Start 193: secp256k1.example.ecdh
 4    Start 194: secp256k1.example.schnorr
 5    Start 195: secp256k1.example.ellswift
 6    Start 196: secp256k1.example.musig
 71/5 Test [#192](/bitcoin-core-secp256k1/192/): secp256k1.example.ecdsa ..........   Passed    0.00 sec
 82/5 Test [#193](/bitcoin-core-secp256k1/193/): secp256k1.example.ecdh ...........   Passed    0.00 sec
 93/5 Test [#194](/bitcoin-core-secp256k1/194/): secp256k1.example.schnorr ........   Passed    0.00 sec
104/5 Test [#195](/bitcoin-core-secp256k1/195/): secp256k1.example.ellswift .......   Passed    0.00 sec
115/5 Test [#196](/bitcoin-core-secp256k1/196/): secp256k1.example.musig ..........   Passed    0.00 sec
12
13100% tests passed, 0 tests failed out of 5
14
15Label Time Summary:
16secp256k1_example    =   0.01 sec*proc (5 tests)
17
18Total Test time (real) =   0.01 sec

    or

     0$ ctest --test-dir build -j $(nproc) -LE tests
 1Test project /home/hebasto/dev/secp256k1/secp256k1/build
 2    Start 192: secp256k1.example.ecdsa
 3    Start 193: secp256k1.example.ecdh
 4    Start 194: secp256k1.example.schnorr
 5    Start 195: secp256k1.example.ellswift
 6    Start 196: secp256k1.example.musig
 7    Start 191: secp256k1.exhaustive_tests
 81/6 Test [#192](/bitcoin-core-secp256k1/192/): secp256k1.example.ecdsa ..........   Passed    0.00 sec
 92/6 Test [#193](/bitcoin-core-secp256k1/193/): secp256k1.example.ecdh ...........   Passed    0.00 sec
103/6 Test [#194](/bitcoin-core-secp256k1/194/): secp256k1.example.schnorr ........   Passed    0.00 sec
114/6 Test [#195](/bitcoin-core-secp256k1/195/): secp256k1.example.ellswift .......   Passed    0.00 sec
125/6 Test [#196](/bitcoin-core-secp256k1/196/): secp256k1.example.musig ..........   Passed    0.00 sec
136/6 Test [#191](/bitcoin-core-secp256k1/191/): secp256k1.exhaustive_tests .......   Passed    6.19 sec
14
15100% tests passed, 0 tests failed out of 6
16
17Label Time Summary:
18secp256k1_example       =   0.01 sec*proc (5 tests)
19secp256k1_exhaustive    =   6.19 sec*proc (1 test)
20
21Total Test time (real) =   6.20 sec
  2. 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.

  3. real-or-random added the label ci on Oct 17, 2025
  4. real-or-random added the label tweak/refactor on Oct 17, 2025
  5. 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.

  6. 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:

     0function(add_executable_and_tests bin_name verify_definition)
 1  add_executable(${bin_name} tests.c)
 2  target_link_libraries(${bin_name} secp256k1_precomputed secp256k1_asm)
 3  target_compile_definitions(${bin_name} PRIVATE ${verify_definition} ${TEST_DEFINITIONS})
 4  add_test(NAME secp256k1_${bin_name} COMMAND ${bin_name})
 5endfunction()
 6
 7add_executable_and_tests(noverify_tests "")
 8if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage")
 9  add_executable_and_tests(tests VERIFY)
10endif()

    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.

  7. 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.

  8. 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.

  9. 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.

  10. hebasto force-pushed on Oct 21, 2025
  11. hebasto commented at 7:58 am on October 21, 2025: member
    Feedback from @furszy has been addressed.
  12. 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?
  13. cmake, refactor: Deduplicate test-related code 
    Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
    4ac651144b
  14. cmake: Add DiscoverTests module 
    Co-authored-by: Daniel Pfeifer <daniel@pfeifer-mail.de>
    f95b263f23
  15. 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.
  16. hebasto force-pushed on Jan 13, 2026
  17. hebasto renamed this:
    cmake: Split test cases to improve parallelism
    cmake: Add dynamic test discovery to improve parallelism
    on Jan 13, 2026
  18. 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.

  19. hebasto marked this as a draft on Jan 13, 2026
  20. hebasto force-pushed on Jan 13, 2026
  21. hebasto marked this as ready for review on Jan 13, 2026
  22. furszy commented at 5:29 pm on January 13, 2026: member
    nice! will review soon.
  23. in examples/CMakeLists.txt:11 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.

  24. 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.

  25. 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:

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

    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.
  26. 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:

    0ctest -j <threads_num>
  27. 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
  28. cmake: Set `LABELS` property for tests 8354618e02
  29. hebasto force-pushed on Jan 20, 2026
  30. hebasto commented at 5:18 pm on January 20, 2026: member

    @furszy

    Thank you for the review! Your feedback has been addressed.

  31. furszy commented at 6:39 pm on January 20, 2026: member
    Tested ACK 8354618
  32. real-or-random requested review from purpleKarrot on Jan 21, 2026


hebasto real-or-random sipa furszy purpleKarrot


purpleKarrot

Labels
ci tweak/refactor

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-01-29 08:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me