search

bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.02k stars 977 forks source link

cmake: Clean up testing code #1554

Open hebasto opened 2 weeks ago

hebasto commented 2 weeks ago
  1. Delete CTest module.

The CTest module handles CDash integration, which we do not use. It is not required for testing functionality.

  1. Clean up cases when to invoke enable_testing()

The enable_testing() command invocation is required for add_test() commands, which are used only for {noverify_}tests, exhaustive_tests and examples.

real-or-random commented 2 weeks ago

The enable_testing() command invocation is required for add_test() commands, which are used only for {noverify_}tests, exhaustive_tests and examples.

I don't think it's required. It's just that add_test() has no effect without enable_testing().

enable_testing() seems cheap. Couldn't we just run it always? Or do you think this has drawbacks?

hebasto commented 2 weeks ago

The enable_testing() command invocation is required for add_test() commands, which are used only for {noverify_}tests, exhaustive_tests and examples.

I don't think it's required. It's just that add_test() has no effect without enable_testing().

enable_testing() seems cheap.

I agree.

Couldn't we just run it always?

Sure. The code has been adjusted.

Or do you think this has drawbacks?

I did some research and I haven't found any drawbacks. When enable_testing() is invoked and no tests being added, the created CTestTestfile.cmake files in the binary tree are noop/empty.

It is recommended:

to call enable_testing() somewhere in the top level CMakeLists.txt file. This would typically be done early, soon after the first project() call.