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

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:240812-testname changing 2 files +4 −4

hebasto commented at 11:15 AM on August 12, 2024: member

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"

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](/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.19 sec
7/8 Test [#1](/bitcoin-core-secp256k1/1/): secp256k1_noverify_tests .........   Passed   38.83 sec
8/8 Test [#2](/bitcoin-core-secp256k1/2/): secp256k1_tests ..................   Passed   91.66 sec

100% tests passed, 0 tests failed out of 8

Total Test time (real) =  91.67 sec

hebasto commented at 11:15 AM on August 12, 2024: member

cc @fanquake @theuni
real-or-random added the label feature on Aug 17, 2024
real-or-random added the label build on Aug 17, 2024
real-or-random commented at 2:17 PM on August 17, 2024: contributor

Concept ACK if this helps Core
sipa commented at 8:23 AM on October 17, 2024: contributor

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

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

This change improves regex matching options when using `ctest` in
downstream projects.

87384f5c0f

hebasto force-pushed on Oct 17, 2024
hebasto commented at 9:01 AM on October 17, 2024: member

Rebased. @sipa's suggestion has been accepted.
hebasto renamed this:
~~cmake, test: Introduce `SECP256K1_TESTNAME_PREFIX` variable~~
cmake, test: Add `secp256k1_` prefix to test names
on Oct 17, 2024
real-or-random removed the label feature on Oct 17, 2024
real-or-random added the label refactor/smell on Oct 17, 2024
real-or-random approved
real-or-random commented at 2:19 PM on October 17, 2024: contributor

utACK 87384f5c0f2bdf2b500858a18f5397e8dcffa8ec
theuni commented at 3:35 PM on October 28, 2024: contributor

Concept ACK.

Shouldn't autotools get the same treatment though?
hebasto commented at 3:56 PM on October 28, 2024: member

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.
theuni commented at 4:35 PM on October 28, 2024: contributor

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
real-or-random merged this on Oct 29, 2024
real-or-random closed this on Oct 29, 2024
hebasto deleted the branch on Oct 30, 2024
achow101 referenced this in commit 378ca17fd1 on Nov 1, 2024
achow101 referenced this in commit 2d46a89386 on Nov 4, 2024
vmta referenced this in commit b5ae194ce5 on Nov 6, 2024
vmta referenced this in commit 8999068421 on Nov 6, 2024
Eunovo referenced this in commit 55a2f7a840 on Nov 12, 2024
vmta referenced this in commit f40affbf6c on Nov 21, 2024
vmta referenced this in commit cc8d145633 on Nov 22, 2024
janus referenced this in commit a4b4239cb4 on Jan 19, 2025
div72 referenced this in commit af627d47c3 on Apr 12, 2025
hebasto commented at 10:45 AM on June 24, 2025: member
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)
```

Contributors

Labels

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