secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

cmake, test: Add `secp256k1_` prefix to test names

Open hebasto opened this issue 1 year ago • 3 comments

This PR improves regex matching options when using ctest in downstream projects, such as Bitcoin Core.

For instance, a downstream project users can filter their tests like that:

ctest --tests-regex "secp256k1"

or

ctest --exclude-regex "secp256k1"

A ctest log with this PR:

$ ctest --test-dir build -j 16
Internal ctest changing into directory: /home/hebasto/git/secp256k1/secp256k1/build
Test project /home/hebasto/git/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: secp256k1_ecdsa_example ..........   Passed    0.00 sec
2/8 Test #5: secp256k1_ecdh_example ...........   Passed    0.00 sec
3/8 Test #6: secp256k1_schnorr_example ........   Passed    0.00 sec
4/8 Test #7: secp256k1_ellswift_example .......   Passed    0.00 sec
5/8 Test #8: secp256k1_musig_example ..........   Passed    0.00 sec
6/8 Test #3: secp256k1_exhaustive_tests .......   Passed    6.19 sec
7/8 Test #1: secp256k1_noverify_tests .........   Passed   38.83 sec
8/8 Test #2: secp256k1_tests ..................   Passed   91.66 sec

100% tests passed, 0 tests failed out of 8

Total Test time (real) =  91.67 sec

hebasto avatar Aug 12 '24 11:08 hebasto

cc @fanquake @theuni

hebasto avatar Aug 12 '24 11:08 hebasto

Alternatively, could we just add a secp256k1_ prefix to our test names?

sipa avatar Oct 17 '24 08:10 sipa

Rebased. @sipa's suggestion has been accepted.

hebasto avatar Oct 17 '24 09:10 hebasto

Shouldn't autotools get the same treatment though?

This PR modifies only test names, while leaving executable names untouched. The concept of "named tests" is not applicable to Autotools. Changing the executable names seems quite invasive though.

hebasto avatar Oct 28 '24 15:10 hebasto

Thanks, I misunderstood. Based on sipa's comment I thought this was actually renaming, which would've been fine with me too. That explains the simplicity :)

utACK 87384f5c0f2bdf2b500858a18f5397e8dcffa8ec

theuni avatar Oct 28 '24 16:10 theuni

Thanks, I misunderstood. Based on sipa's comment I thought this was actually renaming, which would've been fine with me too. That explains the simplicity :)

Apparently, the actual build target renaming might make sense in the context of the approach in https://github.com/bitcoin/bitcoin/pull/32782:

      - name: Build with secp256k1 tests enabled
        run: |
          cmake -B build \
            -DENABLE_WALLET=OFF \
            -DSECP256K1_BUILD_TESTS=ON \
            -DSECP256K1_BUILD_EXHAUSTIVE_TESTS=ON
          cmake --build build -j14 --target noverify_tests tests exhaustive_tests -j $(nproc)

hebasto avatar Jun 24 '25 10:06 hebasto