Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documentation to support modern CMake FetchContent method over ExternalProject. #698

Open
Areustle opened this issue Sep 22, 2022 · 7 comments

Comments

@Areustle
Copy link

Areustle commented Sep 22, 2022

Description

Hello, thank you for creating or contributing to such a useful and vital tool.

The current build_system documentation describes using the CMake ExternalProject mechanism for acquiring the doctest header file and using it in a CMake project. ExternalProject is great for use with Non-CMake externals, but doctest is a modern CMake package with target support, so users can instead use the more efficient and elegant FetchContent mechanism.

FetchContent supports CMake targets. It also moves the download step to project configure time rather build time like ExternalProject.

I suggest updating the documentation to prefer FetchContent.

For example:

  • You can also use the following CMake snippet to automatically fetch the entire doctest repository from github and configure it as an available target:
include(FetchContent)
FetchContent_Declare(
  doctest
  GIT_REPOSITORY https://github.com/doctest/doctest
  GIT_TAG master # or any SHA, Branch or tag like v2.4.9
)
FetchContent_MakeAvailable(doctest)

The target doctest::doctest is now exposed at the global scope

And later you'll be able to use the doctest include directory like this:

# Link doctest to a target
target_link_libraries(my_target PUBLIC doctest::doctest)

# Or link it globally
link_libraries(doctest::doctest)

Access the doctest headers with a scoped #include directive. For example in tests/main.cpp

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include "doctest/doctest.h"

int factorial(int number) { return number <= 1 ? number : factorial(number - 1) * number; }

TEST_CASE("testing the factorial function") {
    CHECK(factorial(1) == 1);
    CHECK(factorial(2) == 2);
    CHECK(factorial(3) == 6);
    CHECK(factorial(10) == 3628800);
}

If this is something you're interested in supporting I can open a PR with the above changes.

Steps to reproduce

Extra information

  • doctest version: v42.42.42
  • Operating System: Joe's discount operating system
  • Compiler+version: Hidden Dragon v1.2.3
@stephenlevitt
Copy link

Thanks for these instructions! I was missing the scoped include directive.

Ectras added a commit to Ectras/t1m that referenced this issue Oct 10, 2023
FetchContent downloads the project at configure time rather than build
time. This should also fix the problem that the initial build of this
project always failed on my system. Solution is taken from
doctest/doctest#698
@PhilipNelson5
Copy link

I'm trying to get this working, however the test discovery function doctest_discover_tests can't be found. I need to include something but I'm not sure what. I think this information would be good to include in the docs.

@PhilipNelson5
Copy link

PhilipNelson5 commented Oct 24, 2023

ok, so if you include("${CMAKE_BINARY_DIR}/_deps/doctest-src/scripts/cmake/doctest.cmake") it works but that doesn't seem like the right way.

@PhilipNelson5
Copy link

slightly better, include("${doctest_SOURCE_DIR}/scripts/cmake/doctest.cmake"). Is there a better way?

@PhilipNelson5
Copy link

I figured out another way

list(APPEND CMAKE_MODULE_PATH "${doctest_SOURCE_DIR}/scripts/cmake")
include(doctest)

@stephenlevitt
Copy link

This is how I do it - not sure if it is the best way:

include(CTest)
enable_testing()
include(${doctest_SOURCE_DIR}/scripts/cmake/doctest.cmake)
# automatically add doctest tests to CTest; specify WORKING_DIRECTORY to ensure that relative paths are correct for CTest
doctest_discover_tests(${TESTS_EXE_NAME} WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})

What is the advantage of list(APPEND ... ?

@PhilipNelson5
Copy link

I don't know what the implications of adding ${doctest_SOURCE_DIR}/scripts/cmake to the CMAKE_MODULE_PATH are. Maybe if you wanted to include multiple things. But if you are just including one thing, maybe it's best to just include the one file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants