Page MenuHomePhabricator

[SECP256K1] Enable the OpenSSL tests (and benchmark)
ClosedPublic

Authored by Fabien on Mar 4 2020, 13:21.

Details

Summary

This diff enables building the OpenSSL tests by default with CMake. In
order to get rid of the dependency, the SECP256K1_BUILD_OPENSSL_TESTS
variable can be set to OFF. It will also be disabled if the tests are
disabled.

Test Plan
cmake -GNinja .. -DSECP256K1_BUILD_TEST=OFF
grep SECP256K1_BUILD_OPENSSL_TESTS CMakeCache.txt

Check that SECP256K1_BUILD_OPENSSL_TESTS is not defined in the cache.

cmake -GNinja .. -DSECP256K1_BUILD_OPENSSL_TESTS=OFF
grep SECP256K1_BUILD_OPENSSL_TESTS CMakeCache.txt

Check that SECP256K1_BUILD_OPENSSL_TESTS is disabled in the cache.

cmake -GNinja ..
grep SECP256K1_BUILD_OPENSSL_TESTS CMakeCache.txt

Check that SECP256K1_BUILD_OPENSSL_TESTS is enabled in the cache.

ninja check-secp256k1
ninja bench-secp256k1

Check in the output that the benchmark ecdsa_verify_openssl is run.

Run the Travis build (see
https://travis-ci.org/Fabcien/secp256k1/builds/657808023).

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Mar 5 2020, 16:54
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/secp256k1/CMakeLists.txt
151 ↗(On Diff #16766)

Can you add a message when this fails to explain this is only required for testing and that people can use -DSECP256K1_BUILD_OPENSSL_TESTS=Off to disable it? The people who just want to use the lib will not want extra dependencies dumped on them.

This revision now requires changes to proceed.Mar 5 2020, 16:54

Add a message to explain why OpenSSL is needed and how to disable the tests.

This revision is now accepted and ready to land.Mar 6 2020, 15:24