cmake: Clean up testing code #1554

1. 
   hebasto commented at 10:42 pm on June 26, 2024: member

   1. Delete CTest module.

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

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

2. cmake: Delete `CTest` module

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

   6aa576515e
3. real-or-random added the label assurance on Jun 27, 2024
4. real-or-random added the label build on Jun 27, 2024
5. 
   real-or-random commented at 9:12 am on June 27, 2024: contributor

   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?

6. cmake: Call `enable_testing()` unconditionally

   This change simplifies the code.
   Also comments has been added to highlight the code structure.

   7c987ec89e
7. hebasto force-pushed on Jun 27, 2024
8. 
   hebasto commented at 10:35 am on June 27, 2024: member

   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.

9. real-or-random approved
10. 
    real-or-random commented at 10:47 am on June 27, 2024: contributor

    utACK 7c987ec89e6cbee5d087683ba29b97012e679484
11. theStack approved
12. 
    theStack commented at 10:40 am on September 18, 2024: contributor

    ACK 7c987ec89e6cbee5d087683ba29b97012e679484

    I don’t have too much knowledge about CMake, but following the discussion here and reading some CMake forum discussions (e.g. https://discourse.cmake.org/t/is-there-any-reason-to-prefer-include-ctest-or-enable-testing-over-the-other/1905/2), the simplification looks correct.

    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.

    Verified this by creating a fresh build directory via

    0$ rm -rf ./build && cmake -B build -DSECP256K1_BUILD_BENCHMARK=OFF -DSECP256K1_BUILD_TESTS=OFF -DSECP256K1_BUILD_EXHAUSTIVE_TESTS=OFF -DSECP256K1_BUILD_CTIME_TESTS=OFF
    1$ cat ./build/src/CTestTestfile.cmake
    

    and checking that the file only contains comment. If one of these options is ON, the file contains add_test(...) calls.

13. real-or-random merged this on Sep 18, 2024
14. real-or-random closed this on Sep 18, 2024

    ---

15. hebasto deleted the branch on Sep 18, 2024

Contributors
hebasto real-or-random theStack

Labels
assurance build